fprintf not working properly in a while loop

Question

I've been learning about dynamic memory allocation in C and practicing it in code. While I think I've allocated the memory properly, I could be wrong about that. So any useful comments about that are fine (but please explain any suggestions so I can understand).

I've written a C program to first get a list of files in the current working directory, and write to each one. But the loop is seeing NULL before it's finished getting though all of the files. It was segfaulting before that, but I managed to fix that. Where it was not pointing to the correct part of memory, which was causing functions like fopen() to not get any filenames.

But while I can print out a list of files, the loop in writeToFiles() is just not completing the list of filenames that I've dynamically allocated whenever I use pretty much any of f* (file related) C library functions.

I've managed to fix all compilers errors and warning messages I was getting, but I really have no idea what's going on with it now. What I think is happening is that the pointer pointing to the struct member called filename isn't at the start of the pointer array. I'm pretty sure my buildFileList() function is fine and it IS working correctly. So do I need to "rewind" the pointer or something ?

If I could politely ask those that wish to respond to be as clear and complete as possible in any answers given. As I'm clearly missing something here, but I have no idea what.

WARNING: The following code if run will OVERWRITE the contents of the current working directory.

I've posted my code below:

___dyn_char_mem_alloc.c

char *___dyn_char_mem_alloc(size_t buffSize) {    
    
    // allocate char memory with malloc() for char *        
   
    char *ptr = malloc(buffSize+1);
   
    //printf("buffSize (char): %d\n", buffSize);
    
    // if malloc returns a pointer that is not NULL, we return it to the caller
    if (ptr != NULL) {
        // puts("malloc() pointer returned (char *)\n");
        // printf("ptr in ___dyn_char_mem_alloc() function = %p\n");
        return ptr;
    } else {       
        // puts("malloc() call failed!\n");
        return NULL; 
   }
}

___dyn_char_mem_alloc.h

#include <stdio.h>
#include <stdlib.h> // for malloc()

char *___dyn_char_mem_alloc(size_t buffSize);

___dyn_struct_mem_alloc.c

#include "___dyn_struct_mem_alloc.h"

struct filepath *___dyn_struct_mem_alloc(size_t buffSize) {
        
    // allocate char memory with malloc() for struct
    
    struct filepath *t_filepath = malloc(buffSize);   
    
    // if malloc returns a pointer that is not NULL, we return it to the caller
    if (t_filepath != NULL) {      
        return t_filepath;
    } else {       
        // puts("malloc() call failed!\n");
        return NULL;
    }
}

___dyn_struct_mem_alloc.h

#include <stdio.h>
#include <stdlib.h> // for malloc()
#include <linux/limits.h>

//struct filepath *___dyn_struct_mem_alloc(size_t buffSize);

struct filepath {
    char *path[PATH_MAX];
    char *filename[FILENAME_MAX];
};

build_file_list.c

struct filepath *buildFileList(void) {
    
    DIR *d = opendir(".");
    struct dirent *dir;
    struct stat stats;
    //struct filepath *t_filepath = malloc(sizeof(struct filepath));
    struct filepath *t_filepath = ___dyn_struct_mem_alloc(sizeof(struct filepath)); 
    int i = 0, strLength = 0, totalNumFiles = 0;        
    const char *pDirFsPtr = "..", *curWorkDirFsPtr = ".";        
    
    while (((dir = readdir(d)) != NULL)) {
        
        if ( (!strcmp(dir->d_name, pDirFsPtr)) || (!strcmp(dir->d_name, curWorkDirFsPtr)) )  {              
            continue;
        } else {              
            // check if it's a directory and if so skip it              
            stat(dir->d_name, &stats);
            if (!S_ISDIR(stats.st_mode)) {
                // get length of filename
                strLength = strlen(dir->d_name);
                  
                // allocate some memory for the filename               
                //t_filepath->filename[i] = malloc(sizeof(char) * (strLength+1));
                t_filepath->filename[i] = ___dyn_char_mem_alloc(strLength+1);                
                  
                // let's copy each filename from the dirent structure's d_name member to our newly dynamically allocated memory                 
                //  t_filepath->filename[i]=(dir->d_name); // doesn't matter if i use strcpy() below instead or not - same problem
                strcpy(t_filepath->filename[i], dir->d_name);                  
                //  printf("filepath->filename[%d] name: %s | ptr: %p \n",i, t_filepath->filename[i], t_filepath->filename[i]); // works no problem
                  
                totalNumFiles++;                  
            }                                         
        }
        ++i;
    }    
    closedir(d);
    
    printf("t_filepath struct ptr from buildFileList(): %p\n",  t_filepath);
    printf("t_filepath->filename struct ptr from buildFileList(): %p\n",  t_filepath->filename);
        
    #ifdef DEBUG    
    printf("\nTotal number of files in dir: %d\n\n", totalNumFiles);
    #endif
    
    #ifdef DEBUG
    int k = 0;
    while (t_filepath->filename[k] != NULL) {
        printf("t_filepath->filename[%d] name: %s | ptr addr: %p \n",k, t_filepath->filename[k], t_filepath->filename[k]);
        ++k;
    }
    #endif
    
    return t_filepath;
}

build_file_list.h

#include <stdio.h>
#include <dirent.h>
#include <sys/stat.h>
#include <string.h>
#include <unistd.h>
#include <libgen.h>
#include <linux/limits.h>

struct filepath *buildFileList(void);
char *___dyn_char_mem_alloc(size_t buffSize);
struct filepath *___dyn_struct_mem_alloc(size_t buffSize);

struct filepath {
    char *path[PATH_MAX];
    char *filename[FILENAME_MAX];
};

main.c

#include "main.h"

int main(int argc, char *argv[]) {
        
    //TODO: add a check for command-line args    
       
    someOtherFunction();
        
    return 0;
}

main.h

#include <stdio.h>

int someOtherFunction(void);

some_other_function.c

#include "some_other_function.h"

int someOtherFunction(void) {
    
    struct filepath *t_filenames = NULL;
    int i = 0;
    
    t_filenames = buildFileList();
    
    puts("\n");
    printf("t_filenames struct ptr from someOtherFunction(): %p\n",  t_filenames);
    printf("t_filenames->filename struct ptr from someOtherFunction(): %p\n",  t_filenames->filename);
    puts("\n");
    
    /* while (t_filenames->filename[i] != NULL) {
        printf("t_filenames->filename[%d] name (from someOtherFunction()): %s | ptr addr: %p \n",i, t_filenames->filename[i], t_filenames->filename[i]);
        ++i;
    } */ // works
    
    writeToFiles(t_filenames);
    
    return 0;
}

some_other_function.h

#include <stdio.h>
#include <linux/limits.h>

int someOtherFunction(void);
struct filepath *buildFileList(void);
void writeToFiles(struct filepath *t_filenames);

struct filepath {
    char *path[PATH_MAX];
    char *filename[FILENAME_MAX];
};

write_to_files.c

#include "write_to_files.h"

void writeToFiles(struct filepath *t_filenames) {
    
    FILE *curFile;
    int i = 0;  
    char msg[100] = "giggle, giggle, giggle....";  
    
    puts("\n");
    printf("t_filenames struct ptr from writeToFiles(): %p\n",  t_filenames);
    printf("t_filenames->filename struct ptr from writeToFiles(): %p\n",  t_filenames->filename);
    puts("\n");
    
    while (t_filenames->filename[i] != NULL) { 
        printf("filename name (from writeToFiles()): %s ptr addr: %p\n", t_filenames->filename[i], t_filenames->filename[i]);
        curFile = fopen(t_filenames->filename[i], "wb"); // works, but calling anymore functions were causing segfaults
        if (curFile != NULL) {
            puts("fprintf(curFile, %s, msg);");
            printf("current filename name: %s\n", t_filenames->filename[i]);
            // print childish message to file :-P
            fprintf(curFile, "%s", msg);
            puts("fclose(curFile);"); 
            fclose(curFile);
        } else {
            puts("loop broken!\n"); // always gets here way too early
            break;
        }         
        ++i;        
    }
    
    return;
}

write_to_files.h

#include <stdio.h>
#include <string.h>
#include <stdlib.h> // needed for exit() call -- remove?
#include <linux/limits.h>

struct filepath {
    char *path[PATH_MAX];
    char *filename[FILENAME_MAX];
};

Thanks for any help with this!

Answer 1

Several problems are immediately apparent:

1. You're not initializing variables but relying on their value anyway
2. You're not ensuring the files added to your list actually fit in the memory you allocated
3. You're not ensuring that once you do create a list, there's a guaranteed way to know when to stop iterating over it when you read it.

This is not a complete list of all possible problems with your code. There may very well be other issues.

Your ___dyn_struct_mem_alloc() function fails to initialize the contents of the structure:

   struct filepath * ___dyn_struct_mem_alloc(size_t buffSize) {
            
       // allocate char memory with malloc() for struct
        
        struct filepath *t_filepath = malloc(buffSize);   
        
       // if malloc returns a pointer that is not NULL, we return it to the caller
       if (t_filepath != NULL) {      
           return t_filepath;
       }
       else {       
           // puts("malloc() call failed!\n");
            return NULL;
       }
    }

Yet this code:

while(t_filenames->filename[i] != NULL) { 
    .
    .
    .       
}

assumes unset values are NULL.

You also never ensure you don't try to add more than FILENAME_MAX files in any struct filepath. This look will keep adding filenames to the field no matter how many there are:

while (((dir = readdir(d)) != NULL)) {
      if ( (!strcmp(dir->d_name, pDirFsPtr)) || (!strcmp(dir->d_name, curWorkDirFsPtr)) )  {              
          continue;
      }          
      else {              
          // check if it's a directory and if so skip it              
          stat(dir->d_name, &stats);
          if (!S_ISDIR(stats.st_mode)) {
              // get length of filename
              strLength = strlen(dir->d_name);
              
              // allocate some memory for the filename               
              //t_filepath->filename[i] = malloc(sizeof(char) * (strLength+1));
              t_filepath->filename[i] = ___dyn_char_mem_alloc(strLength+1);                
              
              // let's copy each filename from the dirent structure's d_name member to our newly dynamically allocated memory                 
            //  t_filepath->filename[i]=(dir->d_name); // doesn't matter if i use strcpy() below instead or not - same problem
              strcpy(t_filepath->filename[i], dir->d_name);                  
            //  printf("filepath->filename[%d] name: %s | ptr: %p \n",i, t_filepath->filename[i], t_filepath->filename[i]); // works no problem
              
              totalNumFiles++;                  
          }                                         
      }
      ++i;
}    
closedir(d);

If there are more than FILENAME_MAX files in any directory, you will corrupt memory as your code overflows the buffer.

And if there are exactly FILENAME_MAX files in the directory, your code will have no way of knowing when it hits the end of the list because there can't be a NULL sentinal value even if you fix your code and initialize the filename array in the structure to all NULL pointer values.

Also, names such as ___dyn_char_mem_alloc are reserved identifiers:

All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use.

And since you appear to be using Linux, just delete ___dyn_char_mem_alloc() and the manual copy of the string you're doing with a single call to strdup(). Removing extraneous code such as that will make the remaining code easier to follow and understand.