How to deallocate 2d array?

Question

There's lots of stuff on this already. They all seem to suggest that all you need to do is call free for each time you call malloc, and probably free the "pointer of pointers" last - so you don't end up referencing that memory after it's been released

For example, if I have

int** myPtr

Then I just need to

free(myPtr[j])

for each j, before finally releasing the pointer of pointers

free(myPtr)

Cool. I've done this a few times in my program, and it seems to work just fine - except in one case. I've tried to figure out what's different about this case, and I was able to replicate the symptom in a minimal program.

#include "stdafx.h"
#include <stdlib.h>

signed char** getPtr()
{
    unsigned char n = 10;
    unsigned char k = 2;

    signed char** myPtr = (signed char**)malloc(n*sizeof(signed char));
    if (myPtr == NULL) {return NULL;}

    for (unsigned char i = 0; i < n; i++)
    {
        myPtr[i] = (signed char*)malloc(k*sizeof(signed char)); 
        if (myPtr[i] == NULL) {return NULL;}
    }

    for (unsigned char j = 0; j < n; j++)
    {
        for (unsigned char k = 0; k <= 1; k++)
        {
            myPtr[j][k] = 42; 
        }
    }

    return myPtr; 

}

int _tmain(int argc, _TCHAR* argv[])
{
    signed char** potato = getPtr();
    if (potato != NULL)
    {
        for (unsigned char j = 0; j < 10; j++)
        {
            free(potato[j]);
        }
        free(potato); 
    }
    return 0;
}

Answer 1

Your immediate problem is the following line:

signed char** myPtr = (signed char**)malloc(n*sizeof(signed char));

This is wrong because it allocates enough space for n characters. Instead, it should be:

signed char** myPtr = (signed char**)malloc(n*sizeof(signed char*));
//                                                              ^
//                                                      char POINTER

The size of a char is generally less than the size of a pointer but, even if it weren't, you should be using the right type. In fact, I tend to prefer the following form so that I don't have to repeat the type (repetition has the possibility that I may change one in the future, forgetting the other):

signed char** myPtr = (signed char**) malloc (n * sizeof (*myPtr));

---

Beyond that, you should consider which language you're actually coding for. While C and C++ share quite a bit of basic stuff, they are in no way the same language, and how you write your code depends on which one you're targeting.

Let's cover C first. In that language, you shouldn't cast the return value from malloc because it can hide certain subtle errors that are not fun to debug. Dropping the cast gives you:

signed char **myPtr = malloc (n * sizeof (*myPtr));

Additionally, since sizeof (char) (naked, signed and unsigned) is always one, whenever you see yourself multiplying by that, it's easier to just drop the multiplication. Combining that with the previous guideline allows you to turn:

myPtr[i] = (signed char*)malloc(k*sizeof(signed char));

into the much more readable:

myPtr[i] = malloc (k);

---

Now, if you're coding for C++, there are additional things to look into.

In C++, it's more acceptable to use the new and delete aspects of the language rather than the legacy C stuff like malloc and free. The guidelines you've already mentioned (ensuring you have a delete for each new) still hold, but there's the added complication of ensuring you use delete[] when freeing arrays of items.

That's not to say malloc and free won't work; they're still very much part of the C++ language specification. It's just that the C++ way of doing things can be much more powerful in that it can both allocate and initialise objects in one hit, and raise exceptions on out-of-memory, meaning you don't have to pepper your code with NULL checks.

In addition, C++ provides, in its standard library, a great many collections which make handling groups of things a lot easier than the use of raw arrays. One of these is a vector, basically a resizable array, and you should consider using it in situation where you may have previously used arrays.

The code you've shown would be much easier to implement as a vector of strings (C++ strings, not those puny C ones). And, in fact, I'd consider going one step further and actually encapsulating the entire data structure into its own class, hiding the fact that it uses vectors at all.

If you do that, and provide all the setter/getter methods that you need, you can later replace the entire underlying data structure without affecting the code that uses it.

Answer 2

You're not allocating enough space for your pointer to pointers. This:

signed char** myPtr = (signed char**)malloc(n*sizeof(signed char));

should be:

signed char** myPtr = malloc(n*sizeof(signed char*));

You also would leak memory here if malloc would fail:

for (unsigned char i = 0; i < n; i++)
{
    myPtr[i] = (signed char*)malloc(k*sizeof(signed char)); 
    if (myPtr[i] == NULL) {return NULL;}
}

I'm kind of assuming you want the C version of this code not fully portable to C++. There's probably a more appropriate way of accomplishing the same thing in C++ for what you really want to do.