Free the buffer inside a function or inside main

Question

Today i was trying to write a program and i realized that i need one file to be read in more places inside my program, but before i got there i had the following:

#include<stdio.h>
#include<string.h>
#include<stdlib.h>

char *printFile(char *fileName){
    long int length;
    char *buffer;
    size_t size;
    FILE *file;

    file = fopen (fileName , "r" );


    fseek (file , 0 , SEEK_END);
    length = ftell (file);
    fseek (file , 0 , SEEK_SET);

    buffer = (char *) malloc (sizeof(char)*(size_t)length);
    if (buffer == NULL){
        fputs ("Memory error",stderr);
        exit (2);
    }


    size = fread (buffer,1,(size_t) length,file);
    if (size != (size_t)length){
        fputs ("Reading error",stderr);
        exit(3);
    }

    fclose (file);
    return buffer;
}

int main (void) {
    char *fileName = "test.txt";
    char *fileContent = printFile(fileName);

    printf("%s", fileContent);

    free(fileContent);

    return 0;
}

As you can see i have used free in the main function, and after i realized that that's not ok for my program I decided to free that buffer inside the printFile function, and I now have this:

#include<stdio.h>
#include<string.h>
#include<stdlib.h>

char *printFile(char *fileName){
    long int length;
    char *buffer,*buffer2;
    size_t size;
    FILE *file;

    file = fopen (fileName , "r" );
    if (file == NULL){
        fputs ("File error",stderr);
        fclose (file);
        exit (1);
    }

    fseek (file , 0 , SEEK_END);
    length = ftell (file);
    fseek (file , 0 , SEEK_SET);

    buffer = (char *) malloc (sizeof(char)*(size_t)length);
    if (buffer == NULL){
        fputs ("Memory error",stderr);
        exit (2);
    }

    buffer2 = (char *) malloc (sizeof(char)*(size_t)length);
    if (buffer2 == NULL){
        fputs ("Memory error",stderr);
        exit (2);
    }

    size = fread (buffer,1,(size_t) length,file);
    if (size != (size_t)length){
        fputs ("Reading error",stderr);
        exit(3);
    }

    strcpy (buffer2, buffer);
    fclose (file);
    free(buffer);

    return buffer2;
}

int main (void) {
    char *fileName = "test.txt";
    char *fileContent = printFile(fileName);

    printf("%s", fileContent);

    return 0;
}

As you probably noticed i used a second pointer (*buffer2) to copy the content of the first buffer inside of it before I free it the first one.

My question is: is my approach right or wrong?

Answer 1

Welcome to memory management!

The question you should be asking is: why would your second approach be superior to the former? You free buffer, but then you return buffer2, and someone has to free buffer2, so you are in exactly the same position as before, except that you copied the file contents twice.

If you don't want to allocate memory inside printFile(), then force the caller to pass in a buffer. In other words, shift the responsibility of dealing with allocation to whatever code uses printFile(). Of course, now the caller has to worry about allocating a buffer that is large enough (although it is easy to get the file size with stat(2), or with stdio wrappers like you did).

Whatever method you end up using, you just can't run away from memory management when you start using dynamic allocation: someone, somewhere, will have to be responsible for freeing memory.

Answer 2

Your approach can be improved.

Instead of

char *printFile(char *fileName);

Use two functions,

char *getFileContents(char *fileName);
void printFile(char *fileName);

With the understanding that getFileContents returns memory that needs to be freed by users of the function. printFile(), and other functions can call getFileContents(), can do with the contents as they please and then call free() on the memory.

Also, when there is an error in reading the contents of the file, return NULL instead of calling exit().

char *getFileContents(char *fileName)
{
   long int length;
   char *buffer;
   size_t size;
   FILE *file;

   file = fopen (fileName , "r" );
   if (file == NULL){
      fputs ("File error",stderr);
      return NULL;
   }

   fseek (file , 0 , SEEK_END);
   length = ftell (file);
   fseek (file , 0 , SEEK_SET);

   buffer = (char *) malloc (sizeof(char)*(size_t)length);
   if (buffer == NULL){
      fclose (file);
      fputs ("Memory error",stderr);
      return NULL;
   }

   size = fread (buffer,1,(size_t) length,file);
   if (size != (size_t)length){
      fputs ("Reading error",stderr);
      fclose (file);
      return NULL;
   }

   fclose (file);

   return buffer;
}

char *printFile(char *fileName)
{
   char* fileContents = getFileContents(fileName);
   if ( NULL != fileContents )
   {
      printf("%s", fileContents);
      free(fileContents);
   }
}