trjhtr

I wrote a function to read the entire content of a file and return a pointer to an allocated bufer containing all the data.

Please could you tell me if there is a better way of doing this ? One solution could be to use stat() to pre-allocate a buffer and do only one read() call but I don't know if it's better, any though ?

#include <stdio.h>
#include <memory.h>

char *file_read(const char *path, size_t *file_size)

 FILE *file = fopen(path, "r");
 char buffer[FILE_READ_BUFFER_SIZE];
 char *tmp = NULL;
 char *data = NULL; // The buffer containing the data of the file
 char *data_cursor = NULL; // The current position in the data buffer
 size_t data_size = 0; // The size of the data buffer
 size_t n = 0; // The number of bytes read by fread()

 if (file == NULL)
 goto error;

 while ((n = fread(buffer, sizeof(char), sizeof(buffer), file)) > 0) 
 if (data == NULL) 
 data = malloc(sizeof(char) * n);
 data_cursor = data;
 data_size = n;
 else 
 // We compute the position in the buffer as an integer
 // to be able to re-apply after realloc(). Because realloc()
 // can move the pointer we need this position as an integer.
 size_t cursor_pos = data_cursor - data;
 tmp = realloc(data, sizeof(char) * data_size + n);
 if (tmp == NULL)
 goto error;
 data = tmp;
 data_cursor = data + cursor_pos;
 data_size += n;
 
 memcpy(data_cursor, buffer, n);
 

 if (ferror(file))
 goto error;

 fclose(file);
 if (file_size != NULL)
 *file_size = data_size;
 return data;

 error:
 if (file != NULL)
 fclose(file);
 if (data != NULL)
 free(data);
 return NULL;

• I recommend to pass a file pointer rather than a path. This will enable the code to work with streams as well. It would also simplify the awkward error handling.




• Modern c allows, and recommends, to declare variables closer to their use.




• It is OK to realloc even when data is NULL.




• sizeof(char) is guaranteed to be 1.



• 
  

  Incrementing data_size after memcpy lets you drop data_cursor and cursor_pos, which IMHO obfuscate the code. You'd need to

  
  
  

   memcpy(data + data_size, buffer, n);
   data_size += n;
  

  
  



• Allocating an array on the stack is usually not recommended. Consider reading directly into data + data_size, and get away without buffer and memcpy.