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
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.