Valgrind Memory Leak Fix

Question

Trying to figure out what to do to fix my memory leaks. it says that I have 726160 bytes in 382 blocks of definitely loss. I have tried to go through my program and found it to be at the line where my malloc memory but I can't figure out why. The line is:

   int ** pixels = (int **) malloc( *numCols * sizeof(int));

Here is my valgrind report:

doe-MacBook:hw34 doe$ valgrind ./a.out -c 450 228 40 ./balloons.ascii.pgm balloon.pgm
==601== Memcheck, a memory error detector
==601== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==601== Using Valgrind-3.11.0.SVN and LibVEX; rerun with -h for copyright info
==601== Command: ./a.out -c 450 228 40 ./balloons.ascii.pgm balloon.pgm
==601== 
==601== Invalid write of size 8
==601==    at 0x100000989: pgmRead (pgmUtility.c:28)
==601==    by 0x100001A79: main (main.c:112)
==601==  Address 0x100820e30 is 0 bytes after a block of size 2,560 alloc'd
==601==    at 0x1000076C1: malloc (vg_replace_malloc.c:303)
==601==    by 0x100000941: pgmRead (pgmUtility.c:26)
==601==    by 0x100001A79: main (main.c:112)
==601== 
==601== Invalid read of size 8
==601==    at 0x100000A01: pgmRead (pgmUtility.c:32)
==601==    by 0x100001A79: main (main.c:112)
==601==  Address 0x100820e30 is 0 bytes after a block of size 2,560 alloc'd
==601==    at 0x1000076C1: malloc (vg_replace_malloc.c:303)
==601==    by 0x100000941: pgmRead (pgmUtility.c:26)
==601==    by 0x100001A79: main (main.c:112)
==601== 
==601== Invalid read of size 8
==601==    at 0x100000B03: pgmDrawCircle (pgmUtility.c:43)
==601==    by 0x100001AB7: main (main.c:114)
==601==  Address 0x100820e30 is 0 bytes after a block of size 2,560 alloc'd
==601==    at 0x1000076C1: malloc (vg_replace_malloc.c:303)
==601==    by 0x100000941: pgmRead (pgmUtility.c:26)
==601==    by 0x100001A79: main (main.c:112)
==601== 
==601== Invalid read of size 8
==601==    at 0x100000C20: pgmDrawCircle (pgmUtility.c:57)
==601==    by 0x100001AB7: main (main.c:114)
==601==  Address 0x100820e30 is 0 bytes after a block of size 2,560 alloc'd
==601==    at 0x1000076C1: malloc (vg_replace_malloc.c:303)
==601==    by 0x100000941: pgmRead (pgmUtility.c:26)
==601==    by 0x100001A79: main (main.c:112)
==601== 

Successfully wrote image to new file

==601== Invalid read of size 8
==601==    at 0x1000012BE: pgmWrite (pgmUtility.c:123)
==601==    by 0x100001AE3: main (main.c:115)
==601==  Address 0x100820e30 is 0 bytes after a block of size 2,560 alloc'd
==601==    at 0x1000076C1: malloc (vg_replace_malloc.c:303)
==601==    by 0x100000941: pgmRead (pgmUtility.c:26)
==601==    by 0x100001A79: main (main.c:112)
==601== 
==601== 
==601== HEAP SUMMARY:
==601==     in use at exit: 1,267,658 bytes in 1,065 blocks
==601==   total heap usage: 1,149 allocs, 84 frees, 1,284,570 bytes allocated
==601== 
==601== LEAK SUMMARY:
==601==    definitely lost: 726,160 bytes in 382 blocks
==601==    indirectly lost: 0 bytes in 0 blocks
==601==      possibly lost: 0 bytes in 0 blocks
==601==    still reachable: 507,136 bytes in 263 blocks
==601==         suppressed: 34,362 bytes in 420 blocks
==601== Rerun with --leak-check=full to see details of leaked memory
==601== 
==601== For counts of detected and suppressed errors, rerun with: -v
==601== ERROR SUMMARY: 111418 errors from 5 contexts (suppressed: 0 from 0)

If more information is needed, let me know.

Here's the method that's getting the error:

int ** pgmRead( char **header, int *numRows, int *numCols, FILE *in  ){

   int i, j;
   for(i = 0; i < 4; i++)
      fgets(header[i], 100, in);

   rewind(in);
   char x[100];
   fgets(x,100, in);
   fgets(x, 100, in);
   int A=0;
   fscanf(in, "%d %d", numCols, numRows);
   fscanf(in, "%d",&A);
   int ** pixels = malloc( *numCols * sizeof(int*));
   for(i = 0; i < *numCols; i++){
      pixels[i] = malloc(sizeof(int) * (*numRows));
   }
   for(j = 0; j < *numRows; j++){
      for(i = 0; i < *numCols; i++){
         fscanf(in, "%d", &pixels[i][j]);
      }
   }
   return pixels;
}

I do free it in main because how i call this is by

pixels = pgmRead(header, &numRows, &numCols, fp);

then I free(pixels) in main

Answer 1

the code cannot 'just' free(pixels);

which is what your question states.

Rather each pointer that was malloc'd into pixels[i] must also be passed to free. I.E.

for( i = 0; i < numcols; i++ )  free( pixels[i];

then finally:

free( pixels );

please note that the format for .pgm image file is NOT what the posted code is expecting.

Suggest reading: http://netpbm.sourceforge.net/doc/pgm.html

or http://en.wikipedia.org/wiki/Netpbm_format

Also note that the image is laid out in rows of pixels (the columns) where rows follow each other.

The posted code is treating the image as if the first column of pixels is listed, then the next column, etc. Which is not correct.

Answer 2

BTW, in the line

int ** pixels = (int **) malloc( *numCols * sizeof(int));

the argument to malloc does not look right to me.

int ** pixels = malloc( numCols * sizeof(int*));

seems like what you should be using.

Update

To deallocate the data, you'll have to make the exact number of calls to free as you made to malloc.

for(i = 0; i < numCols; i++){
   free(pixels[i];
}
free(pixels);