Reading a file into memory

Can you review below code for performance and stability considerations? It is supposed to take a filename or NULL. If it is a filename, read given file into memory. If it is NULL, read from stdin into memory.

struct file_data {
    char *data;
    size_t numChars;
};

/* Read whole file into memory */
struct file_data read_file(char *filename) {
    FILE *f;
    char buffer[BUFFER_SIZE];
    char *data = NULL;
    size_t current_size = 0;
    size_t totalChars = 0;
    size_t nchars;

    if(filename) {
        f = fopen(filename, "r");
    } else {
        f = stdin;
    }

    do {
        data = (char *)realloc(data, current_size + BUFFER_SIZE);
        assert(data);
        current_size+=BUFFER_SIZE;
        nchars = fread(buffer, 1, BUFFER_SIZE, f);
        memcpy(&data[totalChars], buffer, nchars);
        totalChars+=nchars;
    } while(nchars == BUFFER_SIZE);

    struct file_data fd = {data, totalChars};
    return fd;
}

Check return values

You're not making sure fopen isn't failing when opening a file:

f = fopen(filename, "r");

Also check the return value of fread for failure:


The total number of elements successfully read is returned. If this
number differs from the count parameter, either a reading error
occurred or the end-of-file was reached while reading. In both cases,
the proper indicator is set, which can be checked with ferror and
feof, respectively. If either size or count is zero, the function
returns zero and both the stream state and the content pointed by ptr
remain unchanged. size_t is an unsigned integral type.

const parameters

The function should never modify the contents of the string filename, so declaring the parameter const char *filename is good practice.

How does read_file signal a failure

You're asserting on realloc failure. I usually prefer returning gracefully even on alloc failures and leave it open to the caller to be able to cope and at least shutdown nicely. But that is not necessary in all cases.

However, touching the same area is how the read_file function signals a failure to it's caller? One way would be:

struct file_data fd = {NULL, 0};
return fd;

Or changing the function to:

int read_file(const char *filename, struct file_data *fd)
{
     assert(fd);
     ...
     fd->data = data;
     fd->numChars = totalChars;
     return 0; // On success.
     ...
     return -1; // On failure.
}

Read directly into the data buffer

You're doing unnecessary copying by using memcpy and a separate temporary buffer to read the file contents into.

How you could read it into the data buffer directly instead (getting rid of buffer):

size_t offset = 0;
nchars = fread(&data[offset], 1, BUFFER_SIZE, f);
// TODO: Remember to check nchars for error condition.
offset += nchars;

StackExchange Code Review Q#82268, answer score: 6