0

The idea is to use only two functions that can both allocate and free a 2D array of a given data type that is not known at compile-time. The code I have written crashes in the freeing function, but I cannot see why. Here is the code:

void** new2DArray(int row, int col, int typeSize){
    void** temp = malloc(sizeof(void*)*row); //EDIT                     
    for (row-=1; row >= 0; row--)                               
        temp[row] = malloc(typeSize*col);
    return temp; 
}


void free2DArray(void** array, int row, int typeSize){
    for (row -= 1; row >= 0; row--)
        free( *(array + row*typeSize) );
     free(array); 
}

Where, for a 3x3 array, is would be called like:

double** test = (double**) new2DArray(3, 3, sizeof(double));
free2DArray(test, 3, sizeof(double));

Is there anything immediately wrong with the freeing function?

sherrellbc
  • 4,650
  • 9
  • 48
  • 77
  • Wouldn't it be easier to cast `array` in `free2DArray` do a `double **`? – Some programmer dude Mar 16 '14 at 17:12
  • Of course, if you always were going to use `double` arrays. The point is to have a generalized function in which you could make `int` 2D arrays, `unsigned short`, or anything. I had this all working for just one data type. – sherrellbc Mar 16 '14 at 17:14

3 Answers3

3

The free2DArray looks wrong. You don't need to know the size of the types; you just free the blocks that you allocated:

void free2DArray(void** array, int row){
    for (row -= 1; row >= 0; row--)
        free( array[row] );
     free(array); 
}
pat
  • 12,587
  • 1
  • 23
  • 52
  • I had this originally, but the size of the pointers must be known in order to increment accordingly. Now that I think about it, sizeof(void*) should be the same as any other pointer data type anyway. – sherrellbc Mar 16 '14 at 17:16
  • You only need to know `sizeof(void*)` since you only iterate over the first dimension, not the second. – pat Mar 16 '14 at 17:18
1

You free column arrays with free(*(array + row*typeSize));. However, it should be free(*(array + row)), or more concisely, free(array[row]). array + row*typeSize makes you skip typeSize number of column vectors per iteration, and hence, "free" columns that were never allocated.

s.bandara
  • 5,636
  • 1
  • 21
  • 36
  • I see that mistake now. I meant to do what you have said, rather than increment by the size of the data type in the array. – sherrellbc Mar 16 '14 at 17:21
  • It shouldn't be `array + row*sizeof(void*)` because C pointer arithmetic implicitly multiplies the offset by the size of the type to which the pointer points. It should be just `array + row`. – pat Mar 16 '14 at 17:26
1

Why not free it the same way you are allocating it?

for (row-=1; row >= 0; row--)                               
        free(array[row]); 
      //free(*(array + row)); <- or the way you are doing it
free(array);
brokenfoot
  • 11,083
  • 10
  • 59
  • 80
  • Makes sense now that sizeof(void*) = sizeof(double*) anyway. It would work. I wrote that with the notion that sizeof(void*) would = 1, so I manually added in the increment lengths. – sherrellbc Mar 16 '14 at 17:20