Optimal way to free() a malloc'ed 2D array in C

Question

Supposing I have a 2 dimensional array which was created with something like this,

char **foo = (char **) malloc(height * sizeof(char *));
for(i = 0; i <= height; i++) 
    foo[i] = (char *) malloc (width * sizeof(char *));

First of all, Is this even the right way to create an array like this?. The catch here is, 'height' and 'width' is something that is set during runtime.

This seems to work, but which is the best strategy to free this 2d array. free(funge) sounds wrong. Going by some other posts in here, I guess I will have free each row one by one?

I did try something like this,

for (height = 0; height < ip_ptr->funge_height; height++) {
    free(funge[height]);
} 
free(funge)

This, however gives me a double free pointer exception. Does this mean, I don't have to manage this piece of memory?. I was of the impression that, for every malloc'ed memory we should call free().

Answer 1

Since all the 'rows' are the same size, you can just allocate it in one swoop, with malloc(height * width * sizeof (char *)) (it's not entirely clear whether you're creating a 2d array of char or a 2d array of char *). You can use multiplication to calculate the appropriate index (i.e. foo[i][j] becomes foo + i * height + j),

free()ing it will similarly, take a single call.

Answer 2

The mechanism to allocate is OK (though you should use sizeof(char) instead of sizeof(char *) in the allocate loop; you are overallocating the character strings) given that width and height are runtime values.

Your impression that you should call free() once for each malloc() is basically correct (things like calloc() and realloc() complicate the simple story).

The loop followed by free should be correct (or, at least, the general mechanism of 'free the sub-arrays first, then the array of pointers to sub-arrays) - so you need to review where the double free error is coming from. We can't see where the ip_ptr->funge_height was controlled; it is not immediately obvious that funge is described by ip_ptr->funge_height.

---

See the answer from 'unknown @ google' - there's an array bounds problem.

Answer 3

In the for loop for allocation you are using i <= height; instead of i < height;. So you are writing to an invalid memory location and the behavior of your code becomes unpredictable.

Answer 4

When you allocate the memory, it should be i < height as the loop condition.

When you deallocate the memory, you should iterate up to the same index as you did when allocating. ip_ptr->funge_height should be the same as the original height, but it's not obviously so.

Other than that, it should work.

Here's another way, that involves fewer mallocs and frees.

To allocate:

char **foo = malloc (height * sizeof (char **));
foo[0] = malloc (height * width * sizeof (char *));
for (i = 1;  i < height;  ++i) {
    foo[i] = foo[i-1] + width;
}

To deallocate:

free (foo[0]);
free (foo);

Answer 5

The second allocation should be:

foo[i] = (char *) malloc (width * sizeof(char));

you're also looping height+1 times while allocating.

Besides that, those two snippets seem right to me, so the error should be elsewhere.

If the array was allocated as just one big chunk of memory, then you'd have to free it just once.

char **foo = (char **) malloc(height * sizeof(char *));
*foo = malloc(height * width * sizeof(char))
for (int i = 1; i < height; i++) {
  foo[i] = *foo + i*width;
}
//and you just do 2 frees
free(*foo);
free(foo);

Answer 6

Allocation (assuming height > 0 and width > 0)

char **foo, *row;

assert(height > 0 && width > 0);
foo = malloc(height * sizeof *foo);
row = malloc(height * width * sizeof *row);
assert(foo != NULL && row != NULL);

for (i = 0; i < height; ++i, row += width) 
  foo[i] = row;

assert(row == *foo + height * width);

Deallocation

assert(foo != NULL);
free(*foo);
free(foo);

Answer 7

In such cases you can always use valgrind. Just compile your executable and run it:

valgrind --leak-check=full ./a.out

Valgrind will find all your memory validations and point to the code lines involved.

In your case it may have find the indexing problem (< vs. <=) easily.

Answer 8

If your compiler supports it, you could use a pointer to a variable-length array, ie

size_t width = 10, height = 5;
char *(*foo)[height][width] = malloc(sizeof *foo);

Keep in mind that you'll have to derefence the pointer before accessing the array elements, eg

(*foo)[1][2] = "foo";

This has the benefit that you'll only allocate a single, continuous block of memory which can be deallocated with a single call fo free().