cmultidimensional-arraymallocdynamic-memory-allocation

malloc & free in C with multidimensional arrays in C; Whats wrong with the code


I am a beginner and i am working on a code where the multidim arrays are allocated with the malloc, malloc variant. I have to add an Array with a higher dimension (3D instead of 2D).

I discovered a strange behavior and i would be thankful for an explanation.

I know this is not really the proper way to handle multidimensional arrays. But it is already used in the program that i am modifying.

Thats a working code i used to isolate the problem in onlinegbd.com:

#include <stdio.h>
#include <stdlib.h>

double ***AllocArray3D(short ix, int iy, int iz){
    double ***A3D; ix++; iy++;
    A3D = (double ***)malloc(ix * sizeof(double **));
    int i, j;
    for (i = 0; i < ix; i++) {
        A3D[i] = (double **)malloc(iy * sizeof(double *));

        for (j = 0; j < iy; j++) {
            A3D[i][j] = (double *)malloc(iz * sizeof(double));
            if(!(j+1 < iy))printf("alloc i=%d,j=%d\n",i,j); //debug
        }
    }
    return A3D;
}

void FreeArray3D(double *** A3D, int ix, int iy){
    ix++; iy++;
    int i, j;
    for (i = 0; i < ix; i++) {
        for (j = 0; j < iy; j++) {
            if(!(j+1 < iy))
                printf("free i=%d,j=%d\n",i,j); //wenn das der letzte durchlauf ist printe
            free(A3D[i][j]);
        }
        free(A3D[i]);
    }
    free(A3D);
}

int main(){
    int a=99, b=99;
    double *** A_xyz;
    A_xyz = AllocArray3D(a, b, 1309);
    FreeArray3D(A_xyz, a, b);
    return 0;
}

when i change one function to(note the change in the condition of the for loops):

double ***AllocArray3D(short ix, int iy, int iz){
    double ***A3D;//x++; iy++;
    A3D = (double ***)malloc(ix * sizeof(double **));
    int i, j;
    for (i = 0; i < (ix+1); i++) {
        A3D[i] = (double **)malloc(iy * sizeof(double *));

        for (j = 0; j < (iy+1); j++) {
            A3D[i][j] = (double *)malloc(iz * sizeof(double));
            if(!(j+1 < iy))printf("alloc i=%d,j=%d\n",i,j); //debug
        }
    }
    return A3D;
}

or (note the change in the condition of the for loops): EDIT: I forgot to comment out ix++; iy++;:

double ***AllocArray3D(short ix, int iy, int iz){
    double ***A3D; ix++; iy++;
    A3D = (double ***)malloc(ix * sizeof(double **));
    int i, j;
    for (i = 0; i <= ix; i++) {
        A3D[i] = (double **)malloc(iy * sizeof(double *));

        for (j = 0; j <= iy; j++) {
            A3D[i][j] = (double *)malloc(iz * sizeof(double));
            if(!(j+1 < iy))printf("alloc i=%d,j=%d\n",i,j); //debug
        }
    }
    return A3D;
}

I already checked that i can use the array and that every allocation is not NULL but the free() function is failing.

What did i miss? i would be very thankful for an explanation. Thanks in advance


Solution

  • Let's look at this segment of code:

    A3D = (double ***)malloc(ix * sizeof(double **));
    int i, j;
    for (i = 0; i <= ix; i++) {
        A3D[i] = (double **)malloc(iy * sizeof(double *));
    

    You've sized A3D to hold ix elements, but you're allocating ix + 1 items; you've gone past the end of the array.

    Similarly, you've sized A3D[i] to hold iy items, but you wind up allocating iy + 1 items, again running past the end of the array.

    Bad juju.

    If free is failing, it might mean you've overwritten some metadata associated with one or more of your dynamically-allocated blocks by going past the end of each array.

    Some malloc implementations will add some extra bytes to the allocated buffer to store size (and possibly other) metadata so that free knows how much memory to deallocate (this is just an illustration of the concept, not meant to represent any real-world malloc implementation):

             +----+ ----+
     0xffed: | 00 |     |
             +----+     +-- 4 bytes allocated
     0xffef: | 04 |     |
             +----+ ----+
     0xfff0: | ?? | <------ this is the address returned by malloc
             +----+
     0xfff1: | ?? |
             +----+
     0xfff2: | ?? |
             +----+
     0xfff3: | ?? |
             +----+
    

    So when you pass that address to free, it knows to look at the two bytes before that address to know how much memory to deallocate. If those bytes have been overwritten (say by going past the end of a previously allocated block), then that might be the cause of the problems you're seeing.

    Maybe.

    Point is, you're spilling over your array bounds and that's bad.

    Just as a stylistic note:

    In C, you do not need to cast the result of malloc; as of C89 it returns void *, and in C a void * can be assigned to any other object pointer type and vice versa without a cast (this is not true in C++, but if you're writing C++ you're not using malloc anyway).

    Furthermore, you can make your target the operand of sizeof:

    A3D = malloc( ix * sizeof *A3D );  // or sizeof A3D[0]
    ...
    A3D[i] = malloc( iy * sizeof *A3D[i] ); // or sizeof A3D[i][0]
    

    Makes things a bit less eye-stabby, and makes it easy to change the type of A3D (say from double to long double) without having to hack every stinking malloc call. You do not need to expose type information here, and you honestly shouldn't.