Allocating more than needed

Question

I have written a code that allocated a buffer that get assigned the file's contents. Running the code through valgrind gives me this

total heap usage: 9 allocs, 7 frees, 10,905 bytes allocated

If I am not mistaken the code allocates only 3 times, and I am deal locating when needed.

void read_from_file (const char* filename, size_t length, char* buffer)
{
  /* put your code here */
  FILE* fOpen;

  buffer = (char*)malloc(length+1);
  fOpen = fopen(filename, "r");

  if(!buffer) // --- Failed to allocate
    printf("Failed to allocate...\n");

  else if(!fOpen){ // --- Failed to open file
    free(buffer);
    fprintf(stdout, "Failed to read %s\n", filename);
    buffer = NULL;
  }

  else if(fgets(buffer, length+1, fOpen)!=NULL){ // Buffer has been copied
    fprintf(stdout, "buff is: %s\n", buffer);
    free(buffer);
    buffer = NULL;
  }
  else{ // --- Failed to copy correctly
    fprintf(stdout, "Failed to copy %s file...\n", filename);
    free(buffer);
    buffer = NULL;
  }
}

void main (int argc, char **argv)
{
  char* buff = NULL;

  read_from_file("test1.txt",10,buff);
  read_from_file("test2.txt",10,buff);
  read_from_file("test3.txt",10,buff);

}

@kaylum is there any way I can ensure that I deallocate the allocated buffers? — AyeJay, Jan 19 '17 at 01:42
@Rob My question is why are there allocated variables that are not being deallocated? and is my deallocation done correctly? — AyeJay, Jan 19 '17 at 01:43
@DavidBowling LEAK SUMMARY: ==8426== definitely lost: 0 bytes in 0 blocks ==8426== indirectly lost: 0 bytes in 0 blocks ==8426== possibly lost: 0 bytes in 0 blocks ==8426== still reachable: 1,104 bytes in 2 blocks ==8426== suppressed: 0 bytes in 0 blocks — AyeJay, Jan 19 '17 at 01:49
You should remove the needless `buffer = NULL;` statements. They make the code verbose and much harder to follow. Every time I see them, I look for where that matters later in the code, find that it doesn't, and wonder why you sent me on a wild goose chase. — David Schwartz, Jan 19 '17 at 01:51
http://stackoverflow.com/questions/1025589/setting-variable-to-null-after-free, 2009 but I think it's still usable, David — Gabriel Pellegrino, Jan 19 '17 at 02:07
@DavidSchwartz I think that setting pointers to NULL after you are done with them is good practice. (In fact it's required coding style at my current employer.) — Crashworks, Jan 19 '17 at 02:37
@GabrielPellegrino Yes, that shows some of the reasons it's awful practice. [This answer](http://stackoverflow.com/a/1879469/721269) is particularly good. — David Schwartz, Jan 19 '17 at 02:53

ad absurdum · Accepted Answer · 2017-01-19T02:35:01.917

It looks you have no memory leaks, but you have not closed the files that you opened. This is the source of the "still reachable" blocks in the Valgrind report. Just fclose(fOpen) before returning from the read_from_file() function.

Also, the return type of main() is int, not void. Here is your updated code:

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

void read_from_file (const char* filename, size_t length, char* buffer)
{
  /* put your code here */
  FILE* fOpen;

  buffer = malloc(length+1);
  fOpen = fopen(filename, "r");

  if(!buffer) // --- Failed to allocate
    printf("Failed to allocate...\n");

  else if(!fOpen){ // --- Failed to open file
    free(buffer);
    fprintf(stdout, "Failed to read %s\n", filename);
    buffer = NULL;
  }

  else if(fgets(buffer, length+1, fOpen)!=NULL){ // Buffer has been copied
    fprintf(stdout, "buff is: %s\n", buffer);
    free(buffer);
    buffer = NULL;
  }
  else{ // --- Failed to copy correctly
    fprintf(stdout, "Failed to copy %s file...\n", filename);
    free(buffer);
    buffer = NULL;
  }

  if (fclose(fOpen) != 0) {
      fprintf(stderr, "Unable to close file");
  }
}

int main (void)
{
  char* buff = NULL;

  read_from_file("test1.txt",10,buff);
  read_from_file("test2.txt",10,buff);
  read_from_file("test3.txt",10,buff);

  return 0;
}

And the output from Valgrind:

==3564== HEAP SUMMARY:
==3564==     in use at exit: 0 bytes in 0 blocks
==3564==   total heap usage: 6 allocs, 6 frees, 1,737 bytes allocated
==3564== 
==3564== All heap blocks were freed -- no leaks are possible
==3564== 
==3564== For counts of detected and suppressed errors, rerun with: -v
==3564== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

~~I added fclose(fOpen) before leaving read_from_file(), now I am getting seg fault.~~ Edit: I forgot the if-statement — AyeJay, Jan 19 '17 at 02:04

Allocating more than needed

1 Answers1