What's wrong with my C code for dynamic memory allocation/free for Multi-dimensional array

I'm working on debugging a C program. A huge data array with 3 dimensionals is needed. I developed two function for memory allocation/free.

mm() is designed for allocation, with reference to an array which records the size of each dimension (you may see it in main()). ff() is used for freeing memory.

I tested my开发者_如何学Python code with top command after fr() execution. It shows memory is not freed. Can anyone shed light on it ?

Thanks in advance!

  PID USER      PR  NI  VIRT  RES  SHR S %CPU %MEM    TIME+  COMMAND
28338 fsdiag    25   0  165m 164m 1208 R 63.2 26.0   0:00.33 a.out
 3439 root      15   0 31740 1180  21m S  1.9  0.2  10:56.47 X


int main(){

unsigned char ***arr;
int dim_len[4]={8832,256,64,0};  // for 3-D array, 0 is mark of tail
unsigned char *p; 

mm( &p, dim_len, 0); arr = (unsigned char ***)p;
ff( (unsigned char **)&arr, dim_len);

while(1){}
return 0;
}

void mm(    unsigned char **a,
            int dim_len[],    //dimension size array guarded by 0 in the tail
            unsigned char data){  //preset data
    if( *dim_len ){
        int i;
        switch(*(dim_len+1)){
            case 0://when allocate memory for unsigned char
                *a = malloc( sizeof(unsigned char) * (*dim_len));
                break;
            default://when allocate memory for pointers
                *a = malloc( sizeof(unsigned char *) * (*dim_len));
                for( i=0; i<(*dim_len); i++){
                    mm( (unsigned char **)&((*a)[i*4]), dim_len+1, data);
                }
                break;
        }//end of switch
    }//end of if
    return;
}


void ff(    unsigned char **a,
            int dim_len[]){//dimension size array guarded by 0 in the tail
    if( *dim_len ){
        int i;
        switch(*(dim_len+1)){
            case 0://when free memory for unsigned char
                free( *a);
                break;
            default://when free memory for pointers
                for( i=0; i<(*dim_len); i++){
                    ff( (unsigned char **)&((*a)[i*4]), dim_len+1); //pointer needs 4 bytes storage
                }
            free( *a );
                break;
        }//end of switch
    }//end of if
    *a = NULL;
    return;
}

You should malloc just one chunk of memory with size of the product of the numbers in dim_len.

Your allocated data is fragmented and bigger than it should be and I'm struggling to imagine any scenario where your code has any benefits.

Defining a multidimensional array as an array of arrays of arrays is very handy, because you can access the elements with the familiar p[i][j][k] notation, instead of doing index arithmetics. However, others have already pointed out that it is less efficient than allocating the whole array as a big chunk.

You can have the best of both worlds with the following trick: allocate an array of dim_len[0] pointers to an array of dim_len[0]*dim_len[1] pointers to an array of dim_len[0]*dim_len[1]*dim_len[2] data cells. This way you have only three allocations (as many as dimensions) and you can still use the easy p[i][j][k] notation, provided the intermediate arrays of pointers are properly initialized. And you can also do index arithmetics on **p, at your choice.

Here is my version of your program using that trick:

/*
 * Allocation of multidimensional arrays.
 */

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

/*
 * Allocate a multidimensional array of unsigned chars.
 * Cast the returned pointer to (unsigned char **...*)
 */
void *mm(const int dim_len[])
{
    int i, j, size, nmemb = 1, prev_nmemb;
    void *p = NULL, *q = NULL;
    void **prev_q;

    for (i = 0; dim_len[i]; i++) {
        prev_nmemb = nmemb;
        nmemb *= dim_len[i];
        size = dim_len[i+1] ? sizeof(void *) : sizeof(unsigned char);
        prev_q = q;
        q = malloc(nmemb * size);
        if (i == 0) p = q;
        else for (j = 0; j < prev_nmemb; j++)
            prev_q[j] = q + j * dim_len[i] * size;
    }
    return p;
}


/* Free the multidimensional array */
void ff(void *p, int dimensions)
{
    int i;
    void **q;

    for (i = 0; i < dimensions; i++) {
        q = *((void **) p);
        free(p);
        p = q;
    }
}

int main(void)
{
    const int dims[4] = {8832, 256, 64, 0};
    unsigned char ***p;
    int i, j, k;

    printf("Allocating memory.\n");
    p = mm(dims);
    printf("Filling the array.\n");
    for (i = 0; i < dims[0]; i++)
        for (j = 0; j < dims[1]; j++)
            for (k = 0; k < dims[2]; k++)
                p[i][j][k] = (i + 3*j + 5*k) % 256;
    printf("Checking contents.\n");
    for (i = 0; i < dims[0]; i++)
        for (j = 0; j < dims[1]; j++)
            for (k = 0; k < dims[2]; k++)
                assert(p[i][j][k] == (i + 3*j + 5*k) % 256);
    printf("Waiting 10 seconds.\n");
    sleep(10);
    printf("Freeing memory.\n");
    ff(p, 3);
    printf("Waiting 10 seconds.\n");
    sleep(10);
    return 0;
}

My tests here show that when I free the multidimensional array, memory is actually returned to the OS. Here is the output of ps just before and just after freeing:

USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
edgar     5201 73.0  3.7 151852 150556 pts/0   S+   14:11   0:00 ./test
edgar     5201  6.1  0.0   1668   408 pts/0    S+   14:11   0:00 ./test