0

This is probably a basic question but I want to allocate the memory for 3 dimensional array of a struct. I'm trying to read doubles from a file and want to store in struct. The first line is block number (not relevant here as it'll be 1 always), second line denotes the number of grid points in X, Y and Z coordinate respectively. In this case 10 points in X, 5 in Y and 1 in Z direction. And from third line, are the X,Y,Z coordinates of each points which are the doubles I would like to read. First there are all X components (i.e. 10*5*1 x coordinates, then similarly Y and Z). The file format is like this:

      1
      10        5        1
  0.000000e+00   1.111111e+00   2.222222e+00   3.333333e+00 
  4.444445e+00   5.555555e+00   6.666667e+00   7.777778e+00 
  8.888889e+00   1.000000e+01   0.000000e+00   1.111111e+00 
  2.222222e+00   3.333333e+00   4.444445e+00   5.555555e+00 
  6.666667e+00   7.777778e+00   8.888889e+00   1.000000e+01 
  0.000000e+00   1.111111e+00   2.222222e+00   3.333333e+00 
  4.444445e+00   5.555555e+00   6.666667e+00   7.777778e+00 
  8.888889e+00   1.000000e+01   0.000000e+00   1.111111e+00 
  2.222222e+00   3.333333e+00   4.444445e+00   5.555555e+00 
  6.666667e+00   7.777778e+00   8.888889e+00   1.000000e+01 
  0.000000e+00   1.111111e+00   2.222222e+00   3.333333e+00 
  4.444445e+00   5.555555e+00   6.666667e+00   7.777778e+00 
  8.888889e+00   1.000000e+01...and so on...

I can read the first 4 integers and hence I know the number of points I wish to store data for. Then I'm using malloc function to allocate the memory and store the data in the variables. When I execute the program, it reads the integers but fails to read the doubles. What is the mistake I'm making?

Here's my code:

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

typedef struct{
    double x,y,z;
}Mesh;

int main(void)
{
    int nblocks, IMAX, JMAX, KMAX;

    Mesh ***grid;

    FILE *mesh = fopen("test.x","r");

    fscanf(mesh,"%i %i %i %i",&nblocks,&IMAX,&JMAX,&KMAX);
    printf("%i %i %i %i\n",nblocks,IMAX,JMAX,KMAX);

    grid = malloc(sizeof(Mesh)*nblocks*IMAX*JMAX*KMAX);

    fscanf(mesh,"%lf",&grid[0][0][0].x);
    printf("%lf\n",grid[0][0][0].x);

    fclose(mesh);

    return 0;
}

The program doesn't give any error while compiling but it does't read/write the variable I stored in the x variable of the struct. (If this works, I can put it in loop for reading all values which I've not done here.)

If I define Mesh grid[IMAX][JMAX][KMAX] after I read in IMAX,JMAX,KMAX, I get correct output. But wanted to know how pointer way of doing works.

Thank you, Pranav

Pranav
  • 560
  • 1
  • 8
  • 21
  • `Mesh ***grid` --> `Mesh (*grid)[IMAX][JMAX][KMAX];`, `grid[0][0][0][0].x` – BLUEPIXY Aug 26 '14 at 18:43
  • You need to clarify how those X,Y,Z points are stored in your file, because there is nothing in your question that shows a 10 by 5 by 1 matrix, just a bunch of floating-point values, so it is extremely vague which one of those are supposed to be the X values, which are supposed to be the Y values and which are supposed to be the Z values!!!!! BTW, a 3D coordinate is typically made of a single value of X, a single value of Y and a single value of Z. So one would expect to find the same amount of each. Having said that, the 10-5-1 "thing" makes very little sense. – barak manos Aug 26 '14 at 19:03
  • First there are all X (i.e. 10*5*1 x coordinates, then similarly Y and Z). I'll edit the post accordingly. – Pranav Aug 26 '14 at 19:22
  • if `nblocks` When you have as mere ID then `nblocks` remove from `malloc(sizeof(Mesh)*nblocks*IMAX*JMAX*KMAX)` --> `malloc(sizeof(Mesh)*IMAX*JMAX*KMAX)` and `Mesh (*grid)[JMAX][KMAX];`, `grid[0][0][0].x` – BLUEPIXY Aug 26 '14 at 19:23

3 Answers3

3

The problem is that you define grid as a pointer to a pointer to a pointer to struct, but you think that you have a three dimensional array of contiguous elements.

Explanation of the error

The following array:

Mesh array[10][5][1];   // what you would like to manage dynamically

would be stored in memory like this:

+----+----+----+----+----+----+----+----+----+----+----+----+-----
|A000|A010|A020|A020|A030|A040|A100|A110|A120|A120|1030|A140|....
+----+----+----+----+----+----+----+----+----+----+----+----+-----

But the following pointer to pointer to pointer

Mesh ***grid;           // How you decladed it 

is managed as if it would be like this:

grid-->  +--+--+--+--+--+--+--+
         | 0| 1| 2|........| 9|   some pointers to "pointers to struct"
         +--+--+--+--+--+--+--+
   grid[0] |
           +--->  +--+--+------+
                  | 1| 2| .... |   some pointers to struct 
                  +--+--+------+
         grid[0][0] |
                    +--->  +--+--+------+
                           | 1| 2| .... |   some struct 
                           +--+--+------+
               grid[0][0][0] |
                             +--->  +----+----+----+----
                                    |A000|A010|A020|... some struct  
                                    +----+----+----+----

Your malloc() allocates an array of contiguous IMAX*JMAX*KMAX struct elements. In reality, it's like a one dimensional array, because malloc() and your compiler do not know anything about the dimensions and their respecive size.

But when you write grid[0], your code looks at the adress pointed to by grid and expects to find a pointer there (but it's yet only uninitialised struct) and so on. So you might write at a random place in memory and get a segmentation fault.

Solution

You must manage grid as a one dimentional array of struct, and organise the indexing explicitely in your code.

So declare:

Mesh *grid; 

And whenever you think of element [i][j][k] of your mesh, write:

grid[(i*JMAX+j)*KMAX+k]  

Miscellaneous remarks:

You could use calloc(IMAX*JMAX*KMAX, sizeof(Mesh)) because it makes clear that it's an array, and the memory bloc is set to 0.

By the way, (may be you have it already in your real code) as a reflex always check if the allocation succeeds, and foresee a free() when you no longer need it.

Christophe
  • 68,716
  • 7
  • 72
  • 138
  • So do I have to treat the 3D struct array as 1D? Then I will have to do indexing calculations a lot in my code. Defining Mesh grid[IMAX][JMAX][KMAX], after I read for IMAX,JMAX,KMAX seems to work. – Pranav Aug 26 '14 at 19:01
  • @Pranav that will use automatic allocation, if your system has limited stack space then use `Mesh (*grid)[JMAX][KMAX] = malloc(IMAX * sizeof *grid);` (I'm assuming you want to get rid of the nblock dimension as you said in another comment) – M.M Aug 26 '14 at 20:12
  • Yes, @MattMcNabb's proposal is finally the correct one. Pranav, please "unaccept" the other answer. – Jens Gustedt Aug 26 '14 at 20:22
  • @Pranav, yes. The problem with having your grid as auto, is that your the dimebnsions must be constants. I you have a lot of indexation, you could define a function (for example using an intermediate structure keeping the adress of the grid and its dimensions). – Christophe Aug 26 '14 at 20:35
  • @Christophe since 1999 dimensions do not have to be constants – M.M Aug 26 '14 at 20:37
  • @JensGustedt I'll change mine to an answer – M.M Aug 26 '14 at 23:06
  • I agree this is a good solution, but having grid as a 3D array will be very convenient as accessing these coordinates in various calculations is one of the essential part of my code. Hence I'll prefer them in a 3D indexes than a serial array with index formula that you mentioned. Can you suggest me a way for treating it as a 3D array instead of 1D? That would be very helpful. – Pranav Aug 29 '14 at 05:40
2

You have a four dimensional array, not three. The dimensions are:

  1. nblocks
  2. IMAX
  3. JMAX
  4. KMAX

Hence, the type for grid has to be Mesh****, not Mesh***.

Mesh ****grid;

Your code to allocate memory for grid has to be:

grid = malloc(nblocks * (sizeof *grid));
for ( block = 0; block < nblocks; ++block )
{
   grid[block] = malloc(IMAX * (sizeof *grid[0]));
   for ( i = 0; i < IMAX ; ++i )
   {
      grid[block][i] = malloc(JMAX * (sizeof *grid[0][0]));
      for ( j = 0; j < JMAX ; ++j )
      {
         grid[block][i][j] = malloc(KMAX * (sizeof *grid[0][0][0]));
      }
   }
}

Now, you can access the grid data using:

grid[block][index][jindex][kindex]

These are valid usages:

fscanf(mesh,"%lf",&grid[0][0][0][0].x);
printf("%lf\n",grid[0][0][0][0].x);
R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • However, nblock will always be 1 for my case, so I don't have to make it 4D. I''ll try to implement this. Thank you. – Pranav Aug 26 '14 at 19:21
  • 1
    wrong advice, Pranav's problem seems to be the confusion between pointers of pointers of pointers ... and multidimensional arrays. Faking them with pseudo arrays is not the solution, but to define the array and allocate it correctly. – Jens Gustedt Aug 26 '14 at 20:20
  • @Sahu, I tried with your pseudo arrays, they worked without error when the the IMAX,JMAX and KMAX values were less but the program crashed when these values were 100, 40, 60 respectively. The code did not crach when grid was defined as: Mesh grid[IMAX][JMAX][KMAX]. Any reason for crashing with your way when array size becomes larger? They worked for 10, 5, 1. – Pranav Aug 29 '14 at 05:44
  • @Pranav, not sure why your program is crashing. I don't have any problems. Working code: http://ideone.com/Px48ht – R Sahu Aug 29 '14 at 15:32
  • @sahu, I wanted to ask why ++i is used in the loops instead of i++ in your code? – Pranav Sep 01 '14 at 19:39
  • @Pranav, it's matter of habit. It makes no difference for an `int`. It makes a performance difference if you are working with a `std::set::iterator` or `std::map::iterator`. For those, the prefix operator is more efficient than the postfix operator. – R Sahu Sep 01 '14 at 19:52
  • Thank you @Sahu, This may not be the correct answer to the question I asked, and other answers maybe more accurate, however my need was to use the structure as an array (which I somehow forgot to mention in my question) and this method seems to be more suitable for me, hence accepting this as answer. Thank you. – Pranav Sep 01 '14 at 20:15
2

A simple option is to write:

Mesh (*grid)[IMAX][JMAX][KMAX] = malloc( nblocks * sizeof *grid );

Then to access the items:

grid[block_num][i][j][k] = 5;

You indicated in a comment that you don't really need the nblocks since it is always 1, in that case you could go:

Mesh (*grid)[JMAX][KMAX] = malloc( IMAX * sizeof *grid );
// ...
grid[i][j][k] = 5;

Since C99 it is permitted for array dimensions to not be compile-time constants. 1

Note the use of the idiom ptr = malloc(N * sizeof *ptr);, this guarantees that we allocate N of whatever type ptr points to, so we can be sure that we allocated the right number of bytes even though the type of ptr is complicated.


1This feature was required in C99 but changed to optional in C11. If you are on a compiler that is C11 compliant but does not have VLA (I don't know of any) you would have to go for one of the other solutions.

M.M
  • 138,810
  • 21
  • 208
  • 365