HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

List files in dir

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
listfilesdir

Problem

I'm making a program where I need to check if files in the directory are correct (46 files and all 11 chars).

I've got two methods to do it but I don't know which one is better.

Using opendir/readdir:

int checkfiles()
{
    DIR             *dir;
    struct dirent   *dp;
    int             count = 0;

    if ((dir = opendir("img/")) != NULL)
    {
        while ((dp = readdir(dir)) != NULL)
            if (dp->d_name[0] != '.')
            {
                count++;
                if (strlen(dp->d_name) != 11)
                    error(4);
            }
            if (count != 46)
                error(3);
        if (closedir(dir) == -1)
            error(2);
    }
    else
        error(1);

    return (0);
}


or using scandir:

int checkfiles()
{
    struct dirent   **namelist;
    int             i = 0;
    int             n;

    n = scandir("img/", &namelist, 0, alphasort);
    if (n d_name[0] == '.')
                continue;
            if (strlen(namelist[i]->d_name) != 11)
                error(4);
            free(namelist[i]);
        }
        free(namelist);
    }

    return (0);
}


The second is more readable with less indentation, but uses more resources by sorting all files.

Solution

...which one is better (?)

6.001 of one, 1/2 dozen of the other - not much difference.

-
Weakness: method 1: Fix indent. if (count != 46) should be indented left. This excessive indentation gives the false impression of when the the test occurs - as part of the while loop or after it? Better to use {}.

while ((dp = readdir(dir)) != NULL)
    if (dp->d_name[0] != '.')
    {
        count++;
        if (strlen(dp->d_name) != 11)
            error(4);
    }
    //if (count != 46)
    //    error(3);
if (count != 46)
    error(3);


-
Weakness: method 2: else if (n != 48) error(3); make the usually good assumption the directory contains 2 entries . and ... This is not always correct given a root may not have a .. entry.

-
Weakness: method 2: Reliance on successful malloc(). namelist[i] may have the value of NULL and if (namelist[i]->d_name[0] == '.') is UB.

-
Weakness: method 2: Code does not free namelist in (n != 48) case.

Code Snippets

while ((dp = readdir(dir)) != NULL)
    if (dp->d_name[0] != '.')
    {
        count++;
        if (strlen(dp->d_name) != 11)
            error(4);
    }
    //if (count != 46)
    //    error(3);
if (count != 46)
    error(3);

Context

StackExchange Code Review Q#140075, answer score: 5

Revisions (0)

No revisions yet.