Problems grabbing file names using SDL_strdup and similar

Question

I'm trying to create a program with SDL2.
In a certain part of the code, I'm writing functions to grab names of all present files in a given directory path (and keep them in memory) so that, in another function, I can check if a specified file was present the last moment the directory was checked. I'm using dirent.h to suit my needs but I'm running into a few problems:

All the files are properly captured by readdir() (no exception), however they aren't always properly copied into memory after using SDL_strdup() (code is below).
I'm using SDL_malloc()/SDL_realloc()/SDL_strdup() to be as cross-platform as possible to avoid having problems when porting code (as I've read that strdup isn't C standard).

Here's my code:

typedef struct FileList {
    char **files;
    size_t num;
} FileList;

FileList *GetFileList(const char *path){
struct dirent *dp = NULL;
DIR *dir = NULL;
size_t i = 0;
FileList *filelist = SDL_malloc(sizeof(FileList)); /* changing this to a calloc doesn't help */

/* Check if filelist == NULL */
filelist->files = NULL;

dir = opendir(path);
/* Check if dir == NULL */
while ((dp = readdir(dir))){
    if (dp->d_name[0] == '.'){
        continue;    /* skip self, parent and all files starting with . */
    }

    printf("Copying: %s\n", dp->d_name); /* Always show the name of each file */
    filelist->files = SDL_realloc(filelist->files, ++i);
    filelist->files[i-1] = SDL_strdup(dp->d_name);
    printf("Copied: %s\n\n", filelist->files[i-1]); /* Varies: either shows the file's name, either gives me plain gibberish or just nothing */
}
filelist->num = i;

closedir(dir);

return filelist;

}

Output varies. When it doesn't crash, I either get all filenames correctly copied, or I get most of them copied and some contain nothing or plain gibberish (as commented); if it does crash, sometimes I get a Segfault while using SDL_strdup(), other times I get a Segfault when using closedir().

I've even considered exchanging the SDL_realloc() scenario with an initial memory allocation of filelist->files by giving it the number of files (thanks to another function) but I get the same problem.

Any suggestion to change my coding style to a more defensive one (since I do believe this one is rather dangerous) will be appreciated, although I've tried all I could for this case. I'm currently working on a Mac OS X using built-in gcc Apple LLVM 6.0 (clang-600.0.56).

You don't need to cast from `void *` to any pointer type, the cast is unecessary. — Iharob Al Asimi, Feb 08 '15 at 17:59
I was taught this in college as they claimed it was an old but defensive way to program. I also believed it was unnecessary, so I'll take your suggestion on it. — , Feb 08 '15 at 18:02
[It's considered bad style](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) — Iharob Al Asimi, Feb 08 '15 at 18:10

score 1 · Accepted Answer · answered Feb 08 '15 at 18:00

1

You need space for pointers, and sizeof(char *) != 1 so

filelist->files = (char**) SDL_realloc(filelist->files, ++i);

needs to be

filelist->files = SDL_realloc(filelist->files, ++i * sizeof(char *));

but that's actually a bad idea, because SDL_realloc could return NULL in which case you will loose reference to the original pointer, so a good way of doing it is

void *ptr;

ptr = SDL_realloc(filelist->files, ++i * sizeof(char *));
if (ptr == NULL)
    handleThisErrorAndDoNotContinue();
filelist->files = ptr;

and always check for allocator functions if they returned NULL, because you have no control over the size of the data you are trying to read and you can run out of memory at least in theory, so you should make your code safe by checking the success of these functions.

answered Feb 08 '15 at 18:00

Iharob Al Asimi

52,653
6
59
97

Ah, I see. The size of a pointer is important when reallocating. I thought that I didn't need to specify as I was using char (1 byte), but now I understand. – Feb 08 '15 at 18:07
`char` is always `1` byte, but you are allocating pointers, and not `char`'s. – Iharob Al Asimi Feb 08 '15 at 18:09
Yes, I understood that was my mistake. I confused the two in this case, and I sincerely don't know why. I apologize for that. After some testing, I can say that it works, or I can say that I haven't gotten the same problem ever since I used your solution. – Feb 08 '15 at 18:25

Problems grabbing file names using SDL_strdup and similar

1 Answers1