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;
}

Always put `sizeof` first in multiplications. You may be overflowing `int` in the multiplication. If you used `float *p = malloc(sizeof(*p) * q * r * s);` the multiplication will be done on `size_t` which won't overflow. — alx - recommends codidact, Sep 18 '21 at 11:04
Don't cast the result of `malloc()`, and don't use `sizeof(type)`. See https://stackoverflow.com/a/605858 — alx - recommends codidact, Sep 18 '21 at 11:05
You have a bunch of memory leaks if `malloc()` fails, BTW. `free(p)` won't be enough. You'll have to `free()` every mallocated chunk! `goto` is usually helpful to do that sort of cleanup. `defer` (a proposal to C2x) may also be useful, but I don't think any compilers implement it yet. — alx - recommends codidact, Sep 18 '21 at 11:10
To find the source of the `SIGSEGV`, you could add lines like `fprintf(stderr, "Line %s; i = %i\n", __LINE__, i);` everywhere. I suspect where the problem is, but am not sure. With that, you'll help me find it. — alx - recommends codidact, Sep 18 '21 at 11:14
I would personally just do `float *p = malloc(q * r * s * sizeof *p);` and then access the cube at coordinates `x`, `y`, `z` via `p[ (x) + (y * q) + (z * q * r) ]`. The brackets in the last expression are not necessary but are for clarity. — Cheatah, Sep 18 '21 at 11:39
`// allocate the row pointers into the memory` <<-- You don't need to pre-compute and store these pointers. They can be computed when needed. This will also remove the need for being a *three-star* programmer. — wildplasser, Sep 18 '21 at 12:13

score 4 · Accepted Answer · answered Sep 18 '21 at 12:01

4

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.

answered Sep 18 '21 at 12:01

tstanisl

13,520
2
25
40

Nice, very close to allocating a true 3D array. – chux - Reinstate Monica Sep 18 '21 at 12:26
Nice, unless you need to put these in structs or return them from functions. – Costantino Grana Sep 18 '21 at 15:48
2

@CostantinoGrana, yes. It's problematic. However. There is a workaround. One can always cast pointer to VLA to `void*` and recast it back when needed – tstanisl Sep 18 '21 at 15:56
@tstanisl Even easier: hhttps://stackoverflow.com/a/42094467/4756299 Look for `void arr_alloc (size_t x, size_t y, int(**aptr)[x][y])` – Andrew Henle Sep 19 '21 at 18:48
@AndrewHenle, yes it is as long as you know the dimensions in advance when the function is called. It is not always the case. For example: loading a shape and data of a matrix from a file. – tstanisl Sep 19 '21 at 19:11
@tstanisl Then return a `struct` with the dimensions along with a `void *` to the actual array. – Andrew Henle Sep 19 '21 at 19:13
1

@AndrewHenle, this is exactly what I typically do :) . Then knowing dimensions I recast this `void*` to something useful – tstanisl Sep 19 '21 at 19:19

chux - Reinstate Monica · Answer 2 · 2021-09-18T12:28:10.883

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;
 }

Exactly my ideas: use a local variable and be coherent with your choice. — Costantino Grana, Sep 18 '21 at 15:39

alx - recommends codidact · Answer 3 · 2021-09-19T18:41:48.960

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

score 0 · Answer 4 · answered Sep 18 '21 at 12:39

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); 
}

score 0 · Answer 5 · answered Sep 20 '21 at 21:14

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;
}

Memory allocation in C language - 3D array

5 Answers5