1

I have to use a 3 dimensions array because I want to divide a pic into squares and store the average of the RGB of each square in my array. I want it to be this size, tab[height][width][3], so I did that:

i = 0; j = 0; k = 0;

    float*** tab;
    tab = malloc((hauteur+1)*sizeof(float*));

    while(i <= hauteur){
        tab[i] = malloc((largeur+1)*sizeof(float**) );
        i++;
    }
    i = 0;
    while(i <= hauteur){
        j = 0;
        while (j <= largeur){
            tab[i][j] = malloc(3*sizeof(float***));
            j++;
        }
        i++;
    }

but I have a segfault after : tab[1][30][2];.

Is there a problem in my malloc?

It's strange because it doesn't segfault when I declare tab using: tab[hauteur][largeur][3].

(Sorry: "hauteur" means "height" in French and "largeur" means "width".)

(If you think you need to check my whole function: http://pastebin.com/eqQXz8Ad; it's a writer for a JPEG file.)

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
Henley n
  • 3,593
  • 2
  • 10
  • 13
  • 4
    Please — use `for` loops for our sanity if not for yours! A `while` loop scatters the control code over 3 lines; a `for` loop compresses it all onto a single line. It's far easier to avoid trouble with `for` loops. – Jonathan Leffler Dec 13 '15 at 15:07
  • You are initializing some variable twice . So please consider that also. – Punit Vara Dec 13 '15 at 15:09
  • 1
    For the innermost array, you need to allocate `3 * sizeof(float)`; for the middle allocation, you should allocate `(largeur + 1) * sizeof(float *)`; for the outermost (single) allocation, you need to allocate `(hauteur + 1) * sizeof(float **)`. As it happens, you won't run into problems on a 32-bit or 64-bit machine — though you'd allocate twice as much space as necessary for the inner-most array. I'm not sure why you have the `+ 1` in there, but it means you'll be able to index `tab[hauteur][longeur][2]` without subscript errors. Often, you use `for (i = 0; i < hauteur; i++)` (`<` not `<=`). – Jonathan Leffler Dec 13 '15 at 15:11
  • 2
    In addition to what @JonathanLeffler wrote, you can avoid this error if you stick to the malloc best practice of always referencing the pointer you assign to in the size. Example: `tab = malloc(count * sizeof(*tab));`. The `*tab` is always the correct type, no matter how many indirections it is declared with. The next would be `tab[i] = malloc(count * sizeof (*tab[i]));` etc. – Jens Dec 13 '15 at 15:15
  • 2
    Use proper tools for the job. `for (i = 0; i < size; ++i) {...}`. Note, `<` not `<=`. Learn this pattern by heart such that you can recite it when woken up in an unfamiliar place at 4:30 am after a bar crawl. Try proving to yourself that you really need `<` and not `<=`, and explore what could happen if you use `<=`. – n. m. could be an AI Dec 13 '15 at 15:16
  • What are the values of `hauteur` and `largeur`? It would be a good idea to check the result of each `malloc()` — possibly by calling a function that calls `malloc()` and checks that the allocation was successful before returning: `void *emalloc(size_t nbytes) { void *space = malloc(size); if (space == 0) { fprintf(stderr, "Memory allocation failure for %zu bytes\n", size); exit(EXIT_FAILURE); } return space; }`. – Jonathan Leffler Dec 13 '15 at 15:26
  • You might go a bit further and compress your entire code to two lines `float (*tab)[width][3]; tab = malloc(sizeof(float)*height*width*3);` but that's a topic for another day. – n. m. could be an AI Dec 13 '15 at 15:32

3 Answers3

2

Your types aren't right in your malloc calls. I'd suggest the following:

tab = malloc( (hauteur + 1) * sizeof *tab );       // sizeof (float **)
tab[i] = malloc( (largeur + 1) * sizeof *tab[i] ); // sizeof (float *)
tab[i][j] = malloc( 3 * sizeof *tab[i][j] );       // sizeof (float)

Given the declaration of tab, the following are all true:

Expression                Type
----------                ----
       tab                float ***
      *tab                float **
    tab[i]                float **
   *tab[i]                float *
 tab[i][j]                float *
*tab[i][j]                float

I usually recommend taking the sizeof of the target expression, rather than an explicit type; that way, if you ever change the type of tab (from float to double, for example), you never have to touch the malloc calls.

John Bode
  • 119,563
  • 19
  • 122
  • 198
  • It is not clear that this fix, as necessary as it is, will solve the problem. On most platforms `sizeof(anytype*)==sizeof(anyothertype*)` and `sizeof(anytype*)>=sizeof(float)`. The code as it is may allocate a bit more memory than needed. This doesn't explain the crash. – n. m. could be an AI Dec 13 '15 at 16:00
  • yea i replaced this and it segfault right at the line it was segfaulting before... – Henley n Dec 13 '15 at 17:13
1

What you are crafting is basically an array of array of array of pointers to float, not a three dimensional array of floats. You may want have a look at this Ambiguity in 2d array declaration in C. It works out a similar problem with a bidimensional array.

Maybe the solution for your problem can looks like:

float (*tab)[hauteur][largeur][3];    //declare a pointer to a real array

tab = malloc(hauteur * largeur * 3 * sizeof(float));    //Allocate room for threedimensional array

for (int i=0; i<hauteur; i++)
    for (int j=0; j<largeur; j++)
        for (int k=0; k<3; k++)
        {
            (*tab)[i][j][k] = (float)((i*100)+j*1000+k);    //Fill elements with something using threedimensional subscripting
        }
for (int i=0; i<hauteur; i++)
    for (int j=0; j<largeur; j++)
        for (int k=0; k<3; k++)
        {
            printf("[%d][%d][%d]=%f\n", i, j, k, (*tab)[i][j][k]);    //Check back data...
        }

EDITED Looking at comments I see that it is someway 'unnatural' to access an array using the pointer to array notation (*array)[a][b]...[n] even if this notation explicitly report whole dimensions in declaration. To make more friendly the usage you can use the form below that allows the well known format:

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

int largeur = 10;
int hauteur = 10;

int main(int argc, char *argv[])
{
    float (*tab)[largeur][3];    //declare a bidimensional array of pointers to our variable
                               //this fools the compiler acting as a one more dimension array of variable

    tab = malloc(hauteur * largeur * 3 * sizeof(float));    //Allocate room for threedimensional array

    for (int i=0; i<hauteur; i++)
        for (int j=0; j<largeur; j++)
            for (int k=0; k<3; k++)
            {
                tab[i][j][k] = (float)((i*100)+j*1000+k);    //Fill elements with something using threedimensional subscripting
                //This use the natural addressing...
            }
    for (int i=0; i<hauteur; i++)
        for (int j=0; j<largeur; j++)
            for (int k=0; k<3; k++)
            {
                printf("[%d][%d][%d]=%f\n", i, j, k, tab[i][j][k]);    //Check back data...
            }
}

This kind of trick works because of the lack of multidimensional array concept in C language.
C knows only of array, or better it should have been string, of something, so a bidimensional array is simply an array of arrays of something. If we add one more dimension it is an array of an array of an array of something ... and so on for each more dimension we add.
This defines the memory layout of arrays in C and the addressing method that the compiler uses to access data. Because the data is streamed in memory to access a value at a specific subscript the compiler need to compute the space used for all dimensions but the very first.

+++ I fixed a bug, now the sample can be compiled under any C99-C11 compliant compiler and works.

Community
  • 1
  • 1
Frankie_C
  • 4,764
  • 1
  • 13
  • 30
  • i get strange compilation errors when i do that http://image.noelshack.com/fichiers/2015/50/1450028563-screen-3.png is it compatible for a tab[][][] ? – Henley n Dec 13 '15 at 17:43
  • No, `tab` is a pointer to a three dimensional array. You have to use it as in my example as `(*tab)[][][]`. You may use a macro to simplify `#define TabArray (*tab)`, then access data as `TabArray[][][]`. But you can also define `tab` as `float *tab[largeur][3];`, and allocating it as described in my answer. In this way you can access data more naturally as `tab[][][]`. – Frankie_C Dec 13 '15 at 19:07
  • oh i think i was doing something wrong so i just replaced my while code with the your (with the for's) and it was saying : "ecrire.c:114:15: error: invalid type argument of unary ‘*’ (have ‘float’)" BUT it workd by replacing "tab" by "tab**" on the first malloc – Henley n Dec 14 '15 at 00:00
  • and when i use the define or the (*tab)[][][] form it's worse more lel (i think that at first i forgot something in your code so it gave me theses errors on the screenshot) do i have to change something in my code if i use your solution ? – Henley n Dec 14 '15 at 00:14
  • Hard to say without seeing whole your code, and this is a huge job. I think the problem is in the declaration of variable while used in function calls. Please use my second sample so everything should be more familliar and easy. – Frankie_C Dec 14 '15 at 08:29
0

If you care a lot about efficiency, you should consider having a single dimension array (like in Frankie_C's answer):

 int height = something_to_compute_height();
 int width = something_to_compute_width();
 double *arr = malloc(height*width*3*sizeof(double));
 if (!arr) { perror("malloc"); exit(EXIT_FAILURE); }

(as an aside, even if we both are native French speakers, let's try to use English in questions and code here)

then you might define a macro to ease speaking of some element inside arr

#define ELEM_ARR(a,i,j,k) a[i*width*height+j*width+k]
#define ARR(i,j,k) ELEM_ARR(arr)

(You've got the idea, details can be different)

This is probably more efficient than your array of pointers to array of pointers because of cache locality and because you need only one single allocation.

Choose carefully row-major or column-major access to fit the most frequent access patterns.

If height & width are specific to each array, you could use some flexible array member as last in a struct.

As a rule of thumb, it is useful in C to use multidimensional arrays only if all the dimensions (except perhaps the last) are a compile-time constant, e.g. double arr3d[3][4][5];; otherwise, better have a single-dimension array and do the index computing by yourself.

BTW, if you care a lot about performance, you might be concerned by OpenCL and OpenMP (and both usually prefer single-dimensional arrays).

Community
  • 1
  • 1
Basile Starynkevitch
  • 223,805
  • 18
  • 296
  • 547