-1

I have this test code :

void test2(){

    char** twoDArray = (char**)calloc(3,sizeof(char*));
    char* element1 = (char*)calloc(3,sizeof(char));

    twoDArray[0] = element1;    
    twoDArray[1] = element1;
    twoDArray[2] = element1;

    freeArray(twoDArray,3);

}

void freeArray(char** arr,int size){
    if(arr!= NULL){
        for(int i =0 ;i < size;i++){
            if(arr[i] != NULL){
                free(arr[i]);
                arr[i] = NULL;
            }
        }
        free(arr);
        arr = NULL;
    }
}

in second iteration of loop in freeArray arr[1] != NULL give 'true' and error occurs, why ? How to properly free such array ?

trojanfoe
  • 120,358
  • 21
  • 212
  • 242
Mickey Tin
  • 3,408
  • 10
  • 42
  • 71

2 Answers2

3

Do not cast the return value of calloc()!


You are assigning the very same pointer to each element of the array. I don't know why/how you expect it to work (because then you won't have a 2D-like array, since setting one element of a row will change the element at the same column in all other rows too...)

If you still stick to this design, then you should free the inner "array" only once.

But I suspect you wanted to emulate a sane, working, sensible 2D-array. In this case, you have two options:

I. The hackish solution: almost the same as yours, but you need to callocate memory for each row in a separate step, like this:

twoDArray[0] = calloc(3, sizeof(twoDArray[0]));
twoDArray[1] = calloc(3, sizeof(twoDArray[0]));
twoDArray[2] = calloc(3, sizeof(twoDArray[0]));

(or perhaps use a loop... if you know what they are.)

II. The correct solution: why not allocate a continuous block of memory and avoid all superfluous calls to calloc() and all headache as to what and when needs to be free()d? One array -> one allocation, one deallocation:

char (*arr[3]) = calloc(3, sizeof(arr[0]));

// ...

free(arr);

Nice, simple and more maintainable.

Community
  • 1
  • 1
0

If you are going to make the freeArray function, you have to define some rules about what the array must contain. Your implementation of freeArray makes the assumption that each element of the array is pointing to a separately allocated array of characters, however each element of the array is actually pointing to the same array of characters, so you end up trying to free the same array multiple times, which is not valid.

If freeArray's assumptions are correct, then you'll need to modify how you create the array. If freeArray's assumptions are not correct, you'll need to modify freeArray.

Vaughn Cato
  • 63,448
  • 5
  • 82
  • 132