Return array from function C++

Question

I'm new to C++, I have experience with C#, Objective-C and JavaScript.

At the moment I'm trying to write a function that takes a path and returns a directory listing (all files and folders at that path). I'm doing this on Ubuntu.

Here's my code so far, to be honest I'm struggling to understand the double pointer syntax and what it's achieving but this is where my googling has lead me...

int FileManager::GetDirectoryListing(char *path, dirent **directoryEntries)
{
    // Debug output...
    printf("Listing directory at %s\n", path);

    // Allocate memory for the directory entries
    *directoryEntries = new dirent[MAX_FILES];

    // Open the path we were provided
    DIR *directory = opendir(path);

    // A counter of how many entries we have read
    int entryCount = 0;

    // Make sure we were able to open the directory
    if(directory) {

        printf("Successfully opened directory\n");

        // Read the first entry in the directory
        struct dirent *directoryEntry = readdir(directory);

        // While we have a directory entry
        while(directoryEntry) {

            // Debug output...
            printf("%s\n", directoryEntry->d_name);

            // Copy the directory entry to the array of directory entries we will return
            memcpy(&directoryEntries[entryCount], directoryEntry, sizeof(struct dirent));

            // Increase our counter
            ++entryCount;

            // Read the next directory
            directoryEntry = readdir(directory);
        }

        // Close the directory
        closedir(directory);
    }

    return entryCount;
}

And then I call this function by:

    dirent *directoryEntries = NULL;

    int numberOfEntries = FileManager::GetDirectoryListing(deviceRootPath, &directoryEntries);

    printf("File Manager returned directory listing.\n");

    for(int i = 0; i < numberOfEntries; ++i) {

        printf("Looping through directory entries, at index: %i\n", i);

        printf("%s\n", directoryEntries[i].d_name);
    }

It's locking up when it tries to access the first element in directoryEntries i.e. The first time around the loop.

I know I'm not understanding what the double pointer is doing and I don't have a clear picture in my head about the structure of directoryEntries after the call to GetDirectoryListing.

What is happening and what is the correct way to loop through directoryEntries?

Answer 1

*directoryEntries = new dirent[MAX_FILES];

What if the number of directories is greater than MAX_FILES? How do you know that it cannot be greater than MAX_FILES?

I think you should use std::vector<dirent> instead of dirent*. Many of the problems will be solved.

I would implement the function as:

std::vector<dirent> FileManager::GetDirectoryListing(char *path)
{
    std::vector<dirent> dirs;
    DIR *directory = opendir(path);
    if(directory) {
        struct dirent *directoryEntry = readdir(directory);
        while(directoryEntry) {
            dirs.push_back(*directoryEntry); //push a copy of the original!
            directoryEntry = readdir(directory);
        }
        closedir(directory);
    }
    return dirs;
}

Modern compilers will most probably optimize this code, to avoiding copy of the return value. This optimization is called:

• Return Value Optimization (RVO) (or Named RVO)

Also note that directories.size() will tell you the number of entries. So at call site, you can simply do this:

std::vector<dirent> dirs = FileManager::GetDirectoryListing(deviceRootPath);
for(size_t i = 0; i < dirs.size() ; ++i)
{
  std::cout << dirs[i].d_name << std:endl;
}

In general, prefer std::cout over printf, as the latter is not safe!

Answer 2

This line

memcpy(&directoryEntries[entryCount], directoryEntry, sizeof(struct dirent));

should instead be this:

memcpy(&(*directoryEntries)[entryCount], directoryEntry, sizeof(struct dirent));

or equivalently:

memcpy(*directoryEntries + entryCount, directoryEntry, sizeof(struct dirent));

The reason is that directoryEntries is a pointer to a pointer to an array. In memory, it looks like this:

                                    +------------+
directoryEntries --> array_head --> | dirents[0] |
                                    +------------+
                                    | dirents[1] |
                                    +------------+
                                    | dirents[2] |
                                    +------------+
                                    |    ...     |

But you're treating it like directoryEntries is a pointer to an array, which it is not:

WRONG!               +------------+
directoryEntries --> | dirents[0] |
                     +------------+
                     | dirents[1] |
                     +------------+
                     |    ...     |

So you're writing out-of-bounds into memory you don't own, resulting in Undefined Behavior.

The reason you need an extra level of indirection is because in C, function parameters are always passed by value. In order to modify a parameter, you need to pass in a pointer to the original value, which is what you're doing. You just have to remember that when dealing with that pointer, you have an extra level of indirection.

If you're using C++ and not C, you would be much better off using a reference parameter instead of a pointer, and you should also use a std::vector<struct dirent>. You don't have the extra level of indirection to worry about, and the memory management is handled for you automatically.

Answer 3

Your mistake is in this line:

        memcpy(&directoryEntries[entryCount], directoryEntry, sizeof(struct dirent));

Your directoryEntries is pointer to pointers to struct dirent. Each entry in it is a pointer to a struct dirent. Your '&' causes you to copy into the address of a pointer, which is not what you wanted. You want:

        memcpy(directoryEntries[entryCount], directoryEntry, sizeof(struct dirent));