How to properly free when there's an error allocating memory for two dimensional array

Question

I encountered something odd when freeing the memory in a two dimensional array after something goes wrong.

Case 1:

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

int main(void)
{
    int **a = NULL;
    int i;
    int j;

    if(!(a = calloc(5, sizeof(int *))))
    {
        printf("Error, could not allocate memory for a!\n");
        exit(EXIT_FAILURE);
    }

    for(i = 0; i < 5; i++)
    {
        if(i != 2)
        {
            if(!(a[i] = calloc(3, sizeof(int))))
            {
                printf("Error, could not allocate memory for a[%d]!\n",i);
                for(j = 0; j < i; j++)
                {
                    free(a[j]);
                }
                free(a);
            }
        }

        else
        {
            if(!(a[i] = calloc(MAX_INT * 1000, sizeof(int))))
            {
                printf("Error, could not allocate memory for a[%d]\n", i);
                for(j = 0; j < i; j++)
                {
                    free(a[j]);
                }
                free(a);
            }
        }
    }

    return 0;
}

Case 2:

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

int main(void)
{
    int **a = NULL;
    int i;
    int j;

    if(!(a = calloc(5, sizeof(int *))))
    {
        printf("Error, could not allocate memory for a!\n");
        exit(EXIT_FAILURE);
    }

    for(i = 0; i < 5; i++)
    {
        if(i != 2)
        {
            if(!(a[i] = calloc(3, sizeof(int))))
            {
                printf("Error, could not allocate memory for a[%d]!\n",i);
                for(j = 0; j <= i; j++)
                {
                    free(a[j]);
                }
                free(a);
            }
        }

        else
        {
            if(!(a[i] = calloc(MAX_INT * 1000, sizeof(int))))
            {
                printf("Error, could not allocate memory for a[%d]\n", i);
                for(j = 0; j <= i; j++)
                {
                    free(a[j]);
                }
                free(a);
            }
        }
    }

    return 0;
}

The only difference between the two cases is that in case 1, when something goes wrong allocating memory (I deliberately have a large allocation to cause it to fail when i == 2) I loop from j = 0 to j < i and in case 2 I loop from j = 0 to j <= i when freeing the a[j]. Neither gives me a compiler error or warning and neither results in a problem or leak when run through valgrind, so I was just wondering which was the correct way to do it? I thought it was case 1 because the allocation of element i failed, and so no memory is actually allocated, which means there's no need to free it, but the lack of a leak from valgrind has me second guessing myself. Thanks!

Hmmm, perhaps `MAX_INT * 1000` overflowed? Add `assert(SIZE_MAX/1000 >= MAX_INT);`. Are you _sure_ code needs that big an array. I think you may have wanted: `calloc(1000, sizeof(int))` — chux - Reinstate Monica, Apr 24 '18 at 18:53
Why do you need to allocate `MAX_INT` items? That just seems like you're trying to break something. — e0k, Apr 24 '18 at 19:01
Note that on many systems [`MAX_INT*1000` causes an overflow](https://ideone.com/ObVEm6), so you would get undefined behavior instead of an allocation error. — Sergey Kalinichenko, Apr 24 '18 at 19:06
@e0k, I am trying to break it. I wanted the free statement to trigger so that I could test for leaks with valgrind to see if either method produced a leak. — TimeFall, Apr 24 '18 at 21:07

score 3 · Accepted Answer · answered Apr 24 '18 at 18:54

In order to enter the branch of if with failed allocation a[i] must be set to NULL pointer. Since passing NULL pointer to free is well-defined in the C standard, including or excluding a[i] in the loop makes no difference.

Both versions have the same issue, though: when the failure happens inside the outer for loop, you do not exit the program after clean-up, letting the allocation continue. This should be fixed by adding a call to exit(EXIT_FAILURE) after free(a).

How to properly free when there's an error allocating memory for two dimensional array

1 Answers1