0

So I created a dynamically sized 2d array using the following logic:

int **newBoard;
newBoard = (int **) malloc(sizeof(int *) * rows);
newBoard[0] = (int *) malloc(sizeof(int) * cols * rows);

for (i = 0; i < rows; i++) {
    newBoard[i] = (*newBoard + cols * i);
}

But now I am having trouble freeing the memory I allocated. Whenever I try to I receive a "free(): invalid pointer: 0x00000000007343c8 ***" error message. I am currently trying to free the memory like:

for (i = 0; i < rows; i++) {
    free(newBoard[i]);
}
free(newBoard);

Can anyone help me out and tell me how to free this correctly??

m. thomas
  • 31
  • 1
  • 3
  • 1
    How many calls did you make to `malloc()`? There must be one call to `free()` for each call to `malloc()`. How many calls to `free()` do you make? You make two calls to `malloc()`; you make many more calls to `free()` in general (when there's more than one row). Oops! – Jonathan Leffler Feb 11 '18 at 00:00

1 Answers1

2

When you do

newBoard[0] = (int *) malloc(sizeof(int) * cols * rows);

you are allocating a continous block of memory for cols * rows spaces of int objects. You can only free it once using the start of the block, you cannot free it at an offset of the start of the memory and you cannot free it multiple times, only once.

for (i = 0; i < rows; i++) {
    newBoard[i] = (*newBoard + cols * i);
}

Here the newBoard[i] are getting an offset of the continues block of memory, so you cannot free more than once at different offsets, you have to free it once at the start of the block.

free(newBoard[0]);
free(newBoard);

would be correct.

Please don't cast malloc, this is bad practice. And you should check if malloc returns NULL.

I personally think that this is a clever way to do only 2 mallocs instead of row+1 mallocs, but it has downsides, too. You have to be very careful with it. Freeing single blocks of memory that are not needed is not possible, reallocation of single newBoard[i] is also not possible. You have to have a lot of discipline in order to avoid mistakes like multiple frees, that's why I think that the traditional way of doing, is better overall. I'd do:

for(i = 0; i < rows; ++i)
    newBoard[i] = malloc(cols * sizeof *newBoard[i]);

then you can free like you did.

Pablo
  • 13,271
  • 4
  • 39
  • 59
  • This was extremely helpful thank you! Sorry for the bad practice, I'm new to c. I appreciate the advice! – m. thomas Feb 10 '18 at 23:42