Memory allocation in C language - 3D array

Question

I want to allocate memory for a data cube in C language. I mean, I need to allocate a 3D array. My code, however, returns a segmentation fault and I don't know why.

I believe my loops are right, but the fact is, my code doesn't work.

This is my code:

int malloc3dfloat(float ****array, int q, int r, int s) {
    // allocate the q*r*s contiguous items 
    float *p = (float *) malloc(q*r*s*sizeof(float));
    if (!p) return -1;

    // allocate the row pointers into the memory 
    (*array) = (float ***) malloc(q*sizeof(float**));
    if (!(*array)) {
       free(p);
       return -1;
    }
    for (int i=0; i<q; i++)
    {
        (*array)[i] = (float **) malloc(r*sizeof(float*));
        if (!(*array[i])) 
        {
        free(p);
        return -1;
        }
    }

    //set up the pointers into the contiguous memory
    for (int i=0; i<q; i++)
    {
        for (int j=0; j<r; j++)
        { 
            (*array)[i][j] = &(p[(i*r+j)*s]);
        }
    }

    return 0;
}

Answer 1

H.S. already pointed out your bug. But others correctly suggested to use just a single pointer and avoid the precomputation. Then you can move to VLAs. I still like chux version, but in order to have correct malloc() failure management you could do it like this:

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

// Full out-of-memory handling included
int malloc3dfloat(float ****array, int q, int r, int s) 
{
    char *p = malloc((sizeof(float**) + (sizeof(float*) + sizeof(float) * s) * r) * q);
    if (p == NULL) {
        return -1;
    }
    float ***pq = (void*)(p);
    float **pr = (void*)(p + sizeof(float**) * q);
    float *ps = (void*)(p + (sizeof(float**) + sizeof(float*) * r) * q);
    for (int qi = 0; qi < q; ++qi) {  
        pq[qi] = pr + qi*r;
        for (int ri = 0; ri < r; ++ri) {  
            pq[qi][ri] = ps + (qi*r + ri)*s;
        }
    }
    *array = pq;
    return 0;
}

It's ugly, I've got to admit it. But all pointers and values stick together as a single malloc, allowing a simple release and a single check to verify if there was enough memory.

Performance wise I don't have a clue, but deallocation is really easy (just one free):

int main(void) 
{
    float ***x;
    int res = malloc3dfloat(&x, 3, 4, 2);
    for (int q = 0; q < 3; ++q) {
        for (int r = 0; r < 4; ++r) {
            for (int s = 0; s < 2; ++s) {
                x[q][r][s] = rand() % 10;
            }
        }
    }

    for (int q = 0; q < 3; ++q) {
        for (int r = 0; r < 4; ++r) {
            for (int s = 0; s < 2; ++s) {
                printf("%f ", x[q][r][s]);
            }
            printf("\n");
        }
        printf("---\n");
    }
    
    free(x);
    return 0;
}

Answer 2

I had in mind a solution similar to what @tstanisl posted. I've never done this, so I had some doubts about how to make it work, and so I developed a simple program to show it:

$ cat ap.c 
#include <stddef.h>
#include <stdio.h>
#include <stdlib.h>

#define ARRAY_SIZE(a)   (sizeof((a)) / sizeof((a)[0]))
#define ARRAY_SSIZE(a)  ((ptrdiff_t) ARRAY_SIZE(a))

int main(void)
{
    int (*ap)[2][3][5];
    int l = 0;

    ap = malloc(sizeof(*ap));

    printf("%zu\n", sizeof(*ap));

    for (ptrdiff_t i = 0; i < ARRAY_SSIZE(*ap); ++i) {
        for (ptrdiff_t j = 0; j < ARRAY_SSIZE((*ap)[0]); ++j) {
            for (ptrdiff_t k = 0; k < ARRAY_SSIZE((*ap)[0][0]); ++k) {
                (*ap)[i][j][k] = l++;
            }
        }
    }

    for (ptrdiff_t i = 0; i < ARRAY_SSIZE(*ap); ++i) {
        for (ptrdiff_t j = 0; j < ARRAY_SSIZE((*ap)[0]); ++j) {
            for (ptrdiff_t k = 0; k < ARRAY_SSIZE((*ap)[0][0]); ++k)
                printf("%3i", (*ap)[i][j][k]);
            putchar('\n');
        }
        putchar('\n');
    }
}

$ ./a.out 
120
  0  1  2  3  4
  5  6  7  8  9
 10 11 12 13 14

 15 16 17 18 19
 20 21 22 23 24
 25 26 27 28 29

I hope it's useful :-)

I did some further testing to check that there's no undefined behavior, and also to check that the addresses that I'm accessing are contiguous, but I removed them here to simplify the code.

---

Edit:

My solution above is slightly different from @tstanisl 's. The below is what he suggested. Use the one you prefer. Both are nice. This one is more similar to what you get in functions where an array decays to a pointer to its first element.

$ cat ap.c 
#include <stddef.h>
#include <stdio.h>
#include <stdlib.h>

#define ARRAY_SIZE(a)   (sizeof((a)) / sizeof((a)[0]))
#define ARRAY_SSIZE(a)  ((ptrdiff_t) ARRAY_SIZE(a))

int main(void)
{
    int (*ap/*[2]*/)[3][5];
    int l = 0;

    ap = malloc(sizeof(*ap) * 2);

    printf("%zu\n", sizeof(*ap) * 2);

    for (ptrdiff_t i = 0; i < 2; ++i) {
        for (ptrdiff_t j = 0; j < ARRAY_SSIZE(ap[0]); ++j) {
            for (ptrdiff_t k = 0; k < ARRAY_SSIZE(ap[0][0]); ++k) {
                ap[i][j][k] = l++;
            }
        }
    }

    for (ptrdiff_t i = 0; i < 2; ++i) {
        for (ptrdiff_t j = 0; j < ARRAY_SSIZE(ap[0]); ++j) {
            for (ptrdiff_t k = 0; k < ARRAY_SSIZE(ap[0][0]); ++k)
                printf("%3i", ap[i][j][k]);
            putchar('\n');
        }
        putchar('\n');
    }
}

$ ./a.out 
120
  0  1  2  3  4
  5  6  7  8  9
 10 11 12 13 14

 15 16 17 18 19
 20 21 22 23 24
 25 26 27 28 29

Answer 3

didn't notice you wanted contiguous memory in your post !! If usefull anyway here is some code to make a 3Dtab (and erase it) :

int malloc3dfloatBIS(float ****array, int d1, int d2, int d3) {
if (!(*array = malloc(sizeof(array) * d3)))
return -1;
for (int i = 0; i < d2 ; ++i)
    if (!((*array)[i] = malloc(sizeof(array) * d2))) {
        while (i)
            free ((*array)[i--]);
        return -1;
    }
for (int i = 0; i < d2 ; ++i)
    for (int j = 0; j < d1 ; ++j){
        if (!((*array)[i][j] = malloc(sizeof(****array) * d1))){
            for (;i;--i){
                while (j){
                    free    ((*array)[i][--j]);
                }
                j = d1; 
                free ((*array)[i]);
            }
            return -1;
        }
    }
return 0;
}

void erase(float ****array, int d1, int d2, int d3) {
    for (int i = d2;i ; --i){
        int j = d1;
        while (j)
            free ((*array)[i -1][--j]);
        free ((*array)[i - 1]);
    }
    free (*array); 
}

Answer 4

int vs. size_t math

int q, int r, int s
...

//                   v---v Product calculated using int math
// float *p = malloc(q*r*s*sizeof(float));

// Less chance of overflow
float *p = malloc(sizeof (float) * q*r*s);
// or even better 
float *p = malloc(sizeof *p * q*r*s);
//                        ^-------^ Product calculated using wider of size_t and int math

---

OP's malloc3dfloat() neither allocates a true 3D nor a jagged array, but a hybrid of the two.

To allocate a jagged one:

 // Full out-of-memory handling omitted for brevity
 int malloc3dfloat_j(float ****array, int q, int r, int s) {
   float ***a = malloc(sizeof *a * q);
   if (a == NULL) ...
   
   for (int qi = 0; qi < q; qi++) {  
     a[qi] = malloc(sizeof a[qi][0] * r);
     if (a[qi] == NULL) ...
  
     for (int ri = 0; ri < r; ri++) {  
       a[qi][ri] = malloc(sizeof a[qi][ri][0] * s);
       if (a[qi][ri] == NULL) ...
     }
   }

   *array = a;
   return 0;
 }

Answer 5

Just use Variable Length Array with dynamic storage.

float (*array)[r][s]=calloc(q, sizeof *array);

That's all!

Now use array[i][j][k] syntax to access individual elements.