patterncMinor
Memory-safe file-reading in C
Viewed 0 times
safereadingfilememory
Problem
I am trying to write a single function that will read a certain number of bytes from a C file, or the entire file, based on the arguments. I want it to be memory-safe of course, and deal with
Is this efficient enough for large files and fringe cases?
NULL terminators and the like. Not only that, but it should be efficient.#include
#include
#include
char *read(FILE *file, long bytes) {
/* This function takes a FILE pointer and reads bytes from it. These
* bytes are returned as a char* back to the calling function and should be freed
* since they were malloc'd. This function was an attempt to recreate the
* .read( [bytes] ) method on python file objects */
/**********************************
Set bytes to 0 to read entire file
**********************************/
long fileSize = 0; // File size to zero before byte check
if (bytes == 0) {
fseek(file, 0L, SEEK_END); // seek to end
fileSize = ftell(file); // find size of file by checking end
rewind(file); // back to the beginning for reading
} else {
fileSize = ++bytes; // extra byte for NULL
}
char buffer[16]; // We will read the file 16 bytes at a time
char *file_out = malloc(fileSize + 1);
if (file_out == NULL) {
fputs("Could not allocate enough memory", stderr);
exit(EXIT_FAILURE);
}
while (fgets(buffer, 16, file) != NULL) {
strncat(file_out, buffer, 16);
}
return file_out;
}Is this efficient enough for large files and fringe cases?
Solution
Instead of
This avoids the \$O(n^2)\$ complexity of doing
Doing an exit from within a helper function is not recommended because the calling code cannot recover from it. Instead, return a special value that specifies something went wrong.
Also, make up your mind about the naming convention of camelCase vs. snake_case.
fgets, you can use fread to read directly into the file_out:int totalRead = 0;
int size;
while( fileSize > total_read && (size = fread(file_out + totalRead, 1, fileSize - totalRead, file) != 0) {
totalRead+=size;
}
if(totalRead < fileSize){
//couldn't read entire file
}
file_out[total_read] = '\0';
return file_out;This avoids the \$O(n^2)\$ complexity of doing
strncat on a growing string. Doing an exit from within a helper function is not recommended because the calling code cannot recover from it. Instead, return a special value that specifies something went wrong.
Also, make up your mind about the naming convention of camelCase vs. snake_case.
Code Snippets
int totalRead = 0;
int size;
while( fileSize > total_read && (size = fread(file_out + totalRead, 1, fileSize - totalRead, file) != 0) {
totalRead+=size;
}
if(totalRead < fileSize){
//couldn't read entire file
}
file_out[total_read] = '\0';
return file_out;Context
StackExchange Code Review Q#112613, answer score: 9
Revisions (0)
No revisions yet.