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:
readdir()
(no exception), however they aren't always properly copied into memory after using SDL_strdup()
(code is below).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 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.