Free all the memory allocated by malloc(), realloc() in C

Question

I'm trying to free() all the allocated memory by malloc(), realloc() but valgrind says that the is a memory leak.

The code:

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


int main(int argc, char *argv[]) {

    int lines_allocated = 128;
    int max_line_len = 50;

    FILE *fp;

    /* File allocate lines of text */
    char **array = (char **)malloc(sizeof(char*)*lines_allocated);
    if (array==NULL) {
        fprintf(stderr,"Out of memory (1).\n");
        exit(1);
    }

    FILE *file = fopen("file.txt", "r");
    if (file == NULL) {
        fprintf(stderr,"Error opening file.\n");
        exit(2);
    }

    int il;
    for (il = 0; 1; il++) {
        int j;

        /* Have we gone over our line allocation? */
        if (il >= lines_allocated) {
            int new_size;

            /* Double our allocation and re-allocate */
            new_size = lines_allocated*2;
            array = (char **)realloc(array,sizeof(char*)*new_size);

            if (array==NULL) {
                fprintf(stderr,"Out of memory.\n");
                exit(3);
            }

            lines_allocated = new_size;
        }

        /* Allocate space for the next line */
        array[il] = malloc(max_line_len);
        if (array[il]==NULL) {
                fprintf(stderr,"Out of memory (3).\n");
                exit(4);
            }
        if (fgets(array[il], max_line_len-1, file)==NULL)
            break;

        /* Get rid of CR or LF at end of line */
        for (j=strlen(array[il])-1;j>=0 && (array[il][j]=='\n' || array[il][j]=='\r');j--)
            ;

        array[il][j+1]='\0';
    }

    /* Close file */
    fclose(file);

    /* Print and free the every element of the array */
    int cc;
    for (cc = 0; cc < il; cc++) {
        printf("%s\n", array[cc]);
        
        /* Free the every element of the array */
        free(array[cc]);
    }

    /* Free hole array */
    free(array);

    return 0;
}

valgrind ./main

valgrind --leak-check=full --show-reachable=yes ./main
==4806== Memcheck, a memory error detector
==4806== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==4806== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==4806== Command: ./main
==4806== 
1
2
3
4
5
6
7
8
9
10
11
==4806== 
==4806== HEAP SUMMARY:
==4806==     in use at exit: 50 bytes in 1 blocks
==4806==   total heap usage: 14 allocs, 13 frees, 2,192 bytes allocated
==4806== 
==4806== 50 bytes in 1 blocks are definitely lost in loss record 1 of 1
==4806==    at 0x4C2AC3D: malloc (vg_replace_malloc.c:299)
==4806==    by 0x40092E: main (in /var/www/mem/main)
==4806== 
==4806== LEAK SUMMARY:
==4806==    definitely lost: 50 bytes in 1 blocks
==4806==    indirectly lost: 0 bytes in 0 blocks
==4806==      possibly lost: 0 bytes in 0 blocks
==4806==    still reachable: 0 bytes in 0 blocks
==4806==         suppressed: 0 bytes in 0 blocks
==4806== 
==4806== For counts of detected and suppressed errors, rerun with: -v
==4806== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

How to free the memory correctly? It says that there should one more block of memory to free but where is it?

Answer 1

for (cc = 0; cc < il; cc++) {

If il is a valid index of array (and it is), the comparison in the loop should be:

for (cc = 0; cc <= il; cc++) {

in order to trigger the very last element of array (and free its memory).

Answer 2

Just replace

for (cc = 0; cc < il; cc++)

with

for (cc = 0; cc <= il; cc++)

To figure that out, imagine what happens if allocating loop for (il = 0; 1; il++) iterates only once. In this case control does not reach il++, so il remains zero and for (cc = 0; cc < il; cc++) iterates zero times. In general case the freeing loop makes one iteration less than allocating loop.

Answer 3

Your code leaks the last allocation, because il is never incremented when fgets(array[il], max_line_len-1, file) returns NULL.

Moving array[il] = malloc(max_line_len); along with its NULL check to after fgets will fix this problem. An added benefit to this approach is that you can make your allocations of the exact size, rather than allocating at max_line_len.

// Check that we will need the allocation
char temp[max_line_len];
if (fgets(temp, max_line_len-1, file)==NULL) {
    break;
}
// Allocate only when we are sure that we are going to need it
temp[max_line_len-1] = '\0';
size_t len = strlen(temp);
array[il] = malloc(len+1);
if (array[il]==NULL) {
    fprintf(stderr,"Out of memory (3).\n");
    exit(4);
}

Note: assigning realloc back to the variable being reallocated could result in leaking the memory that has been previously allocated to that variable. It is not a problem in your code, because you call exit(4) right away, but you should be aware of the general problem with this assignment.

Answer 4

If you're having miscellaneous problems, and intermixing calls to various allocators, and are awfully persnickety about things, then implement a wrapper for the various memory allocators (this can be done cleverly using macros) that caches the address of the newly allocated buffer somewhere (e.g., atop a stack) and then--at some point--navigates the stack and frees everybody. Don't intermix random calls here and there to free() or its analogues. When something is freed, overwrite the something with zero so you don't accidentally try to free() it a second time, which will upset free().

HOW TO CLEVERLY USE MACROS (I said it, so I'd better make it work now) and avoid recursive problems:

Let's use malloc() as our first victim.

In another source file, create a function _malloc() that calls malloc().

In the source file that contains all your memory allocation and deallocation, define malloc() as follows:

#define malloc( n )  ( *sp++ = _malloc( n ) )

There has to be code invoked as a preamble that sets up a stack and points sp at its base. Make it nice and big: you'd be surprised how many times malloc() and its brethren may end up being called. At various times — when it's appropriate, actually — call your own free_all() that does this:

void free_all() {  
  while( --sp >= base_of_malloc_buf_stack ) {  
    free( *sp );   
    *sp = 0; /* avoid future re-free() */    
  }  
}