Not able to visit subfolders recursively

Question

I am trying to build a program that recursively lists all folders and files in a directory, along with their file sizes. I'm still working on the first part because the program only seems to go one level of sub-folder in depth.

Can anyone spot the issue here? I'm stuck.

#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <string.h>
#include <strings.h>
#include <dirent.h>
#include <unistd.h>

void listdir(const char *name) {
    DIR *dir;
    struct dirent *entry;
    int file_size;

    if (!(dir = opendir(name)))
        return;
    if (!(entry = readdir(dir)))
        return;

    do {
        if (entry->d_type == DT_DIR) {
            char path[1024];
            if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0)
                continue;
            printf("./%s\n", entry->d_name);
            listdir(entry->d_name);
        }
        else
            printf("./%s\n", entry->d_name);
    } while (readdir(dir) != NULL);
    closedir(dir);
}

int main(void)
{
    listdir(".");
    return 0;
}

Compile with all warnings and debug info (`gcc -Wall -g` if using [GCC](http://gcc.gnu.org/)...). **Use the debugger** (`gdb`). Your fix-my-code question is off-topic, and lacks some other tag (like *linux* or *POSIX*) since standard C does not know about directories (but POSIX does). — Basile Starynkevitch, Mar 27 '17 at 04:59
Reporting an error instead of simply returning might be a good idea ... — too honest for this site, Mar 27 '17 at 05:01
`char *path[1024]` allocates an array of `char` pointers, which is probably not what you want... — bejado, Mar 27 '17 at 05:02
BTW, the GNU `find` program is free software (so you should study its source code, e.g. of [coreutils](https://www.gnu.org/software/coreutils/coreutils.html)...). And Linux has [nftw(3)](http://man7.org/linux/man-pages/man3/nftw.3.html) — Basile Starynkevitch, Mar 27 '17 at 05:02
The `do { … } while` loop is completely inappropriate. It should be `while ((entry = readdir(dir)) != NULL)`. — Jonathan Leffler, Mar 27 '17 at 05:37
One aspect of your problem is described in [Why is `stat()` not working after `readdir()`](http://stackoverflow.com/questions/34168266/why-is-stat-not-working-after-readdir). I'm tempted to close as a duplicate, but there are other issues here too. — Jonathan Leffler, Mar 27 '17 at 06:06
I don't really agree with the closure as there is only one clear error that causes the subdirectories not being traversed, and it is obvious with bare sight. + that the `entry` is not reassigned as it should be. (it works because it has been statically allocated). The question includes a) desired behaviour, b) a specific problem, c) shortest code to reproduce, d) in the question itself. — Antti Haapala -- Слава Україні, Mar 27 '17 at 07:58

fluter · Accepted Answer · 2017-03-27T05:58:56.503

The first problem is in the while condition, you are abandoning the return value of readdir, it should be assigned to entry.

Also, when calling listdir recursively, you should prepend the parent name before the path, other wise it will always search from current working directory. Try this version:

void listdir(const char *name) {
    DIR *dir;
    struct dirent *entry;
    int file_size;

    if (!(dir = opendir(name)))
            return;

    while ((entry = readdir(dir)) != NULL) {   // <--- setting entry
            printf("%s/%s\n", name, entry->d_name);
            if (entry->d_type == DT_DIR) {
                    char path[1024];
                    if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0)
                            continue;
                    sprintf(path, "%s/%s", name, entry->d_name);  // <--- update dir name properly by prepend the parent folder.
                    listdir(path);
            }
    }
    closedir(dir);
}

int main(void)
{
    listdir(".");
    return 0;
}

Your early return leaks an open directory descriptor. That's not good. The `do { … } while` loop is wholly inappropriate — both in the question and in the answer. — Jonathan Leffler, Mar 27 '17 at 05:37
Right, dir should be closed before return.The loop could be better also. — fluter, Mar 27 '17 at 06:00

Antti Haapala -- Слава Україні · Answer 2 · 2017-03-27T06:57:31.693

The following is a minimal fix to your code. I took the liberty of using the non-standard asprintf here; if you're not using glibc, you should use snprintf or alike instead.

Most notably, the path given to listdir must be a complete relative path from the current working directory; or an absolute path. However the one in entry->d_name is just the basename of the file. Thus it has to be concatenated with the path passed in to the listdir. I've also changed the inappropriate do...while into a while loop here.

#define _GNU_SOURCE
#include <sys/types.h>
#include <sys/stat.h>
#include <string.h>
#include <strings.h>
#include <dirent.h>
#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>

void listdir(const char *path) {
    DIR *dir;
    struct dirent *entry;

    if (!(dir = opendir(path)))
        return;

    while ((entry = readdir(dir))) {
        if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0)
            continue;

        char *current;
        if (asprintf(&current, "%s/%s", path, entry->d_name) < 0) {
            // asprintf failed
            fprintf(stderr, "asprintf failed, exiting");
            goto exit;
        }

        puts(current);
        if (entry->d_type == DT_DIR) {
            listdir(current);
        }

        free(current);
    }

exit:
    closedir(dir);
}

int main(void)
{
    listdir(".");
}

Not able to visit subfolders recursively

2 Answers2