Proper way to build up a path using cstrings in C++

Question

I need to build up a path to a file. I have the following class method:

void Directory::scanDirectory(char *directory) {
    DIR *dirp;
    struct dirent *entry;
    char path[1];

    if(dirp = opendir(directory)) {
        while(entry = readdir(dirp)) {
            if (entry->d_name[0] != '.') {
                strcpy(path, directory);
                strcat(path, "/");
                strcat(path, entry->d_name);
                if (entry->d_type == 8) {
                    // Files
                } else if (entry->d_type == 4) {
                    //scanDirectory(path);
                }
                printf("Name: %s, Type: %d\n", entry->d_name, entry->d_type);
            }
        }
        closedir(dirp);
    }
}

I need to build up the path to files by concatenating the directory and entry->d_name. When I try running this code it segfaults. From what I can tell it's segfaulting at the point where I build the path. Is there a better way of doing this?

score 3 · Answer 1 · answered Oct 09 '10 at 04:57

You're only allocating one byte for the path (char path[1]). You need to allocate enough space to actually hold the whole path you're creating. Given the C++ tag, the obvious possibility would be to use an std::string, and after you've put all the pieces together into a complete path, use its c_str() member function to get access to the contents as a C-style string.

score 2 · Answer 2 · answered Oct 09 '10 at 05:07

2

Why not use Boost.Filesystem ?

answered Oct 09 '10 at 05:07

usta

6,699
3
22
39

score 1 · Accepted Answer · answered Oct 09 '10 at 04:55

The buffer path needs to have enough space to hold the whole path. Right now it only has space for a single character. Try making it bigger. strcat doesn't allocate space itself. You have to manually manage that memory.

As for a better way, you may want to look into using a string. You won't need to worry about memory, and you can concatenate with the + operator.

score 1 · Answer 4 · answered Oct 09 '10 at 04:59

Change char path[1]; to:

char path[512]; //or whatever value you like.

On your code, path only allocated space for 1 character and \0. You need a bigger one obviously, and as far as I know, in unix, the directory name can be upto 255 characters, so 512 would be enough in my opinion.

score 1 · Answer 5 · edited May 23 '17 at 10:33

Be careful using strcpy. It doesn't do bounds checking so even though path is only char[1], it's going to try to copy all of directory into it. That's probably your seg fault.

You have a lot of choices in terms of how to build up a string. Here's a long SO post on C++ string concatenation efficiency:

Efficient string concatenation in C++

If you're using C++, is there any reason you can't just use the built-in string library with the + operator? E.g.:

string path;
//...
path += directory;
path += "/";
path += entry->d_name;
//etc.

Using the string class might be slightly less efficient, but it will have the added benefit of helping you avoid buffer overflow issues and memory exceptions like the segmentation fault you're getting (I'm not saying string will avoid all of those, but it would make your life easier).

There's also been another previous SO post on how to build up a directory string in C++:

c++ how to create a directory from a path

Proper way to build up a path using cstrings in C++

5 Answers5