C opendir() not opening a directory?

Question

I'm not all that experienced in C, but have an assignment in one of my classes, and I'm having a few issues opening a directory.

The purpose of the program is to (paraphrased):

If no flag or directory is provided, list names of files in current directory.
If a flag (-l) is provided, list more information about each file.
If a flag and a directory are provided, go to that directory and list information about each file (or directory) in that directory. It's not necessary to recurse through subdirectories.

It doesn't need to handle getting only a directory but no flag, or an invalid directory. It's more proof-of-concept.

I've managed to satisfy parts 1 and 2, but part 3 keeps breaking, giving a segmentation fault.

My code is as follows:

void recursedirect(char* flag, char* directory)
{
    DIR* dir;
    struct dirent *entry;

    printf("Flag: %s\nDirectory: %s\n\n", flag, directory);

    if (flag == NULL)
    // No flag provided, therefore only do file names.
    {
        dir = opendir(".");
        entry = readdir(dir);

        while (entry != NULL) 
        {
            printf ("%s\n", entry->d_name);
            entry = readdir(dir);
        }
    }
    else if (strcmp(flag, "-l") == 0 && directory == NULL)
    // No directory provided, but flag provided
    {
        dir = opendir(".");
        entry = readdir(dir);

        while (entry != NULL) 
        {
            fileinfo(directory);
            entry = readdir(dir);
        }   
    }
    else if (strcmp(flag, "-l") != 0)
    {
        printf("Invalid flag.\n");
    }
    else
    // A directory is provided
    {
        dir = opendir(directory);

        if (dir != NULL)
        {
            entry = readdir(dir);

            while (entry != NULL) 
            {
                fileinfo(directory);
                entry = readdir(dir);
            }   
        }
        else
            printf("Something went wrong. It might be an invalid filename.\n");
    }
}

Specifically, the else statement at the end is giving me trouble. I've tried giving it any filepaths I can think of, hardcoded and otherwise, and it's still giving me issues. I'm guessing it's something simple, but I don't have enough experience to know what it is.

fileinfo is a function that prints out the file information given a DIR and dirent as arguments. However, the error definitely isn't in that code best I can tell.

EDIT: I made the mistake of not sharing everything. My main method is pretty simple.

int main (int argc, char** argv)
{
    char* flag = argv[1];
    char* directory = argv[2];

    recursedirect(flag, directory);
}//main

It includes the following:

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

And the fileinfo function is as follows (edited slightly, as I realised I no longer needed to give it an argument of DIR *dir):

void fileinfo(struct dirent *entry)
{
    struct stat fileStats;
    stat(entry->d_name, &fileStats);

    printf("File Name: %s\n", entry->d_name);
    printf("File Size: %d bytes\n", (int) fileStats.st_size);
    printf("Number of Blocks: %d\n", (int) fileStats.st_blocks);
    printf("Number of Links/References: %d\n", (int) fileStats.st_nlink);
    printf("I-Node: %d\n", (int) fileStats.st_ino);
    printf("Device ID: %d\n", (int) fileStats.st_dev);
    printf("User ID: %d\n", (int) fileStats.st_uid);
    printf("Group ID: %d\n", (int) fileStats.st_gid);

    stat("alphabet",&fileStats);
    printf("Time of Last Access: %s", ctime(&fileStats.st_atime));
    printf("Time of Last Modification: %s", ctime(&fileStats.st_mtime));
    printf("Time of Last Status Change: %s", ctime(&fileStats.st_ctime));

    stat(entry->d_name, &fileStats);
    printf("File Permissions: ");
    printf((int) (S_ISDIR(fileStats.st_mode)) ? "d" : "-");
    printf((int) (fileStats.st_mode & S_IRUSR) ? "r" : "-");
    printf((int) (fileStats.st_mode & S_IWUSR) ? "w" : "-");
    printf((int) (fileStats.st_mode & S_IXUSR) ? "x" : "-");
    printf((int) (fileStats.st_mode & S_IRGRP) ? "r" : "-");
    printf((int) (fileStats.st_mode & S_IWGRP) ? "w" : "-");
    printf((int) (fileStats.st_mode & S_IXGRP) ? "x" : "-");
    printf((int) (fileStats.st_mode & S_IROTH) ? "r" : "-");
    printf((int) (fileStats.st_mode & S_IWOTH) ? "w" : "-");
    printf((int) (fileStats.st_mode & S_IXOTH) ? "x\n\n" : "-\n\n");
}

As for more clarification on failed inputs, ./files and ./files -l both work. ./files with any invalid flag is caught as intended. However, ./files -l *any valid filepath* consistently fails.

Everything is compiled using gcc in Ubuntu 14.04.5 in the c9.io IDE.

Thank you!

EDIT 2: I've tried deleting and putting back the file, and now strangely I'm not getting the error. However, now it won't print full file info for anything. I'll troubleshoot some more.

EDIT 3: I've fixed it with one exception. The issue was an incorrect if statement checking the flag. Apparently something was going wrong in the IDE. I'm now getting this error: Couldn't open MANPATH=/home/ubuntu/.nvm/versions/node/v6.11.2/share/man:/usr/local/rvm/rubies/ruby-2.4.0/share/man:/usr/local/man:/usr/local/share/man:/usr/share/man:/usr/local/rvm/man

It looks like this isn't the code and is a problem with the c9 server in some way. I've tried giving both the source code and compiled code files full permissions, restarting the workspace, and restarting Apache2. Thank you all for everything!

(Indent code by four spaces. As indicated in the Help popups, there is a button for that.) — Jongware, Apr 27 '18 at 21:09
Aside: I'd expect a `closedir()` to go with each successful `opendir()`. I see none. — chux - Reinstate Monica, Apr 27 '18 at 21:24
" it's still giving me issues." --> What are the issues? The `fileinfo(directory);` code is suspicious. Why is that in a loop? Should it have been `fileinfo(dir);` or `fileinfo(entry)` or perhaps `fileinfo(entry->d_name)`; — chux - Reinstate Monica, Apr 27 '18 at 21:29
"fileinfo is a function that prints out the file information given a DIR and dirent as arguments. " is a description - for people. Posting its function signature would be better. — chux - Reinstate Monica, Apr 27 '18 at 21:34
You are calling `fileinfo()` with a `char *`, not with the `DIR` and `struct dirent` that you say. I'd guess that your real code calls it properly (otherwise the compiler should complain and part 2 would obviously fail)... which lead one to wonder whether the code you've posted is the same code that you are testing with. — mhawke, Apr 28 '18 at 02:15
Also, your code, including part 3, works for me. I added my own `fileinfo(struct dirent *entry)` function that just prints `entry->d_name`. Please double check that we are looking at the same code as you, and provide your `fileinfo()` function. — mhawke, Apr 28 '18 at 02:25
I’ll provide the function as soon as I get the opportunity! Thank you! And I suppose I should’ve been more specific. When I get to that third portion, given a relative path ‘string,’ it breaks. I’ll elaborate more soon. Apologies for that. I’d do all that now but I don’t have access to my code at the moment. — TheAshenKnight, Apr 28 '18 at 03:52
@TheAshenKnight: relative paths work for me on Linux. What OS are you running on? — mhawke, Apr 28 '18 at 04:21
@mhawke I'm using the c9.io IDE, it's running Ubuntu 14.04.5. Finally got back to a computer that can use it, I'll try a few of these things and edit the main post. — TheAshenKnight, Apr 28 '18 at 15:59

zekei · Accepted Answer · 2018-04-28T07:25:18.790

In addition to comments left by chux, you may benefit from restructuring your code to reuse similar blocks. You mention that you've solved parts 1 & 2 (represented by the first two branches in your if statements). So, in theory the following if blocks produce the correct output. If you simplify it so code is rewritten less, you may benefit.

if (flag == NULL)
// No flag provided, therefore only do file names.
{
    dir = opendir(".");
    entry = readdir(dir);

    while (entry != NULL) 
    {
        printf ("%s\n", entry->d_name);
        entry = readdir(dir);
    }
}
else if (strcmp(flag, "-l") == 0 && directory == NULL)
// No directory provided, but flag provided
{
    dir = opendir(".");
    entry = readdir(dir);

    while (entry != NULL) 
    {
        fileinfo(directory);
        entry = readdir(dir);
    }   
}

Instead of writing it this way, you could check the flag beforehand, and set a variable (in this case more_file_info != 0 tells us to print, and we return if the flag is invalid).

int more_file_info = 0;
if (flag != NULL) {
    if(strcmp(flag, "-l") == 0) {
        more_file_info = 1;
    } else {
        printf("Invalid flag.\n");
        return;
    }
}

Then, you can simplify/refactor the above (presumably working) code as follows:

void recursedirect(char* flag, char* directory)
{
    DIR* dir;
    struct dirent *entry;
    int more_file_info = 0;

    printf("Flag: %s\nDirectory: %s\n\n", flag, directory);
    // Validate argument up front
    if (flag != NULL) {
        if(strcmp(flag, "-l") == 0) {
            more_file_info = 1;
        } else {
            printf("Invalid flag.\n");
            return;
        }
    }

    // The 'directory' parameter only changes opendir, so we reflect that here
    if(directory != NULL) {
        dir = opendir(directory);
    } else {
        dir = opendir(".");
    }
    // Adding some error checking, per comment
    if(dir == NULL) {
        printf("Couldn't open %s\n", directory);
        return;
    }
    // The readdir() loop is basically the same
    entry = readdir(dir);
    while (entry != NULL)
    {
        // Although we check a flag every time this loop runs,
        // it makes the code easier to follow
        if(more_file_info != 0) {
            printf ("%s\n", entry->d_name);
        } else {
            fileinfo(directory);
            // Or should this be:
            // fileinfo(entry);
        }
        entry = readdir(dir); 
    }
}

This eliminates a lot of duplicated code, and should hopefully make troubleshooting easier. Also note that this handles the case where a directory is specified but flag is not, which you could explicitly check for if different behavior is desired. Hopefully with more information we can provide a more suitable answer.

After your call to `opendir` you should check that `dir` is not `NULL` before calling `readdir (dir)`. While `readdir` will return `EBADF` if `dir` is invalid, that isn't being checked for either. Otherwise, good effort on the answer. — David C. Rankin, Apr 28 '18 at 07:11
I definitely agree, and added your suggestion to my answer. I will note that there are more improvements that could be made, but the intent of my answer was not to write a perfect solution, but merely to restructure the given code in a way that might make it easier for the asker to troubleshoot. There are definitely more improvements that could be made (e.g. using `strncmp` instead of `strcmp` in case `flag` is not `\0` terminated), and I think those are definitely valuable lessons to learn, but that may detract from solving the original problem (which occurs on good input). — zekei, Apr 28 '18 at 07:37
Thank you for this! The code is significantly cleaner now. I did a bit more troubleshooting, and I'm still getting the same error. I tried just printing the file name inside the else block while reading the files instead of calling `fileinfo` and am still getting a segmentation fault. My best guess is it isn't wanting to properly open the directory, which seems strange to me... I've even tried passing in `.` as the directory. This occurs while hardcoding it in as well. — TheAshenKnight, Apr 28 '18 at 16:30

C opendir() not opening a directory?

1 Answers1