Thread: Pls correct me if I am wrong, but is this a bit of bad code?

THIS IS NOT MY CODE, it is open source so I am just using it.

I got this open file, read dir, using a char *path, then freeing path without malloc to create it. as my knowledge is limited, would that not cause problems in a long run?

Code:

int len = strlen(origpath);
char *path = (char*) malloc(len+1);

should not the above be added to this code? or something like that.
as I just wrote that off the top of my head. not really sure if that
is even right even though it looks logical.

Code:

void add_file_to_filelist_recursively(char *origpath, unsigned char level)
{
    struct stat st;
    char *path;
    int recursive = 1;


    if (!origpath)
        return;

    path = strdup(origpath);
 

    if (level == FILELIST_FIRST)
    {
        /* First time through, sort out pathname */
        int len = 0;

        len = strlen(path);
        if (path[len - 1] == '/')
            path[len - 1] = '\0';

    char *newpath = get_absolute_path(path);

            free(path); <------ here
            path = newpath;

    }

    errno = 0;
    if (stat(path, &st))

    {
        printf("bad path %s\n", path);
        free(path); <--- here
        exit(1);
    }

        if ((S_ISDIR(st.st_mode)) && (level != FILELIST_LAST)) {
        struct dirent **de;
        DIR *dir;
        int cnt, n;

        if ((dir = opendir(path)) == NULL)
        {
            printf("couldn't open directory %s:", path);
            free(path); < ---- here
            return;
        }

        n = scandir(path, &de, NULL, NULL);
        if (n < 0) {
            switch (errno) {
            case ENOMEM:
                printf("Insufficient memory to scan directory %s:", path);
                break;
            default:
                printf("Failed to scan directory %s:", path);
            }
        } else
        {
            for (cnt = 0; cnt < n; cnt++) {
                if (strcmp(de[cnt]->d_name, ".")
                        && strcmp(de[cnt]->d_name, ".."))
                        {
                            char *newfile;

                            newfile = strjoin("", path, "/", de[cnt]->d_name, NULL);
                      //      printf("add_2_front %s newfile %s\n", path, newfile);

                    //gets recursive always
                    if (recursive)
                    {

                        add_file_to_filelist_recursively(newfile, FILELIST_CONTINUE);
                    }
                    else

                        add_file_to_filelist_recursively(newfile, FILELIST_LAST);
                        free(newfile);
                }
                free(de[cnt]);
            }
            free(de);
        }
        closedir(dir);
    }
    else if (S_ISREG(st.st_mode))
    {
        add_2_front(&filelist, path);
    }
    free(path); <-- here
    return;
}

MOD:
I just added

Code:

int len = strlen(origpath);
char *path = (char*) malloc(len+1);

and compiled, and that little bit of code I asked about is working, but it is proper?

Well the best thing to do is compile it with the "-g" flag to get debug information, then run the program in the debugger.

When it locks up (or seems to), you can interrupt it and get a stack trace.

From there, you can examine variables, set breakpoints, single step the code (and so on).
Determine whether it's actually "stuck", or just stuck in some loop of yours which never ends.

Maybe your list wound up pointing to itself.

A resource you're likely to run out of early is open file handles. Check that you're closing everything you open.