Coding problem using a 2-d array of structs inside

Question

I am working with a 2-dimensional array of structs which is a part of another struct. It's not something I've done a lot with so I'm having a problem. This function ends up failing after getting to the "test" for-loop near the end. It prints out one line correctly before it seg faults.

The parts of my code which read data into a dummy 2-d array of structs works just fine, so it must be my assigning array to be part of another struct (the imageStruct).

Any help would be greatly appreciated!

/*the structure of each pixel*/
typedef struct
{
 int R,G,B;
}pixelStruct;

/*data for each image*/
typedef struct
{ 
 int height;
 int width;
 pixelStruct *arr; /*pointer to 2-d array of  pixels*/
} imageStruct;


imageStruct ReadImage(char * filename)
{
 FILE *image=fopen(filename,"r");
 imageStruct thisImage;

        /*get header data from image*/

        /*make a 2-d array of of pixels*/
 pixelStruct imageArr[thisImage.height][thisImage.width];

        /*Read in the image. */

        /*I know this works because I after storing the image data in the
          imageArr array, I printed each element from the array to the
          screen.*/

 /*so now I want to take the array called imageArr and put it in the
   imageStruct called thisImage*/

  thisImage.arr = malloc(sizeof(imageArr));
  //allocate enough space in struct for the image array. 

 *thisImage.arr = *imageArr; /*put imageArr into the thisImage imagestruct*/

//test to see if assignment worked: (this is where it fails)

 for (i = 0; i < thisImage.height; i++)
 {
  for (j = 0; j < thisImage.width; j++)
  {
   printf("\n%d: R: %d G: %d B: %d\n", i ,thisImage.arr[i][j].R,
          thisImage.arr[i][j].G, thisImage.arr[i][j].B);
  }
 } 

 return thisImage;
}

(In case you are wondering why I am using a dummy array in the first place, well it's because when I started writing this code, I couldn't figure out how to do what I am trying to do now.)

EDIT: One person suggested that I didn't initialize my 2-d array correctly in the typedef for the imageStruct. Can anyone help me correct this if it is indeed the problem?

Answer 1

*thisImage.arr = *imageArr; /*put imageArr into the thisImage imagestruct*

This won't work. You have to declare arr as colorRGB **, allocate it accordingly, etc.

Answer 2

it looks like you are trying to copy array by assignment. You cannot use simple assignment operator to do that, you have to use some function to copy things, for example memcpy.

*thisImage.arr = *imageArr;
thisimage.arr[0] = imagearr[0];

The above statements are doing the same thing. However this is not most likely what causes the memory corruption

since you are working with two dimensional arrays, do make sure you initialize them correctly. Looking at the code, should not even compile: the array is declared as one-dimensional in your image structure but you refer to as two-dimensional?

Answer 3

I don't think sizeof imageArr works as you expect it to when you're using runtime-sized arrays. Which, btw, are a sort of "niche" C99 feature. You should add some printouts of crucial values, such as that sizeof to see if it does what you think.

Clearer would be to use explicit allocation of the array:

thisImage.arr = malloc(thisImage.width * thisImage.height * sizeof *thisImage.arr);

I also think that it's hard (if even possible) to implement a "true" 2D array like this. I would recommend just doing the address computation yourself, i.e. accessing a pixel like this:

unsigned int x = 3, y = 1; // Assume image is larger.
print("pixel at (%d,%d) is r=%d g=%d b=%d\n", x, y, thisImage.arr[y * thisImage.width + x]);

I don't see how the required dimension information can be associated with an array at run-time; I don't think that's possible.

Answer 4

You seem to be able to create variable-length-arrays, so you're on a C99 system, or on a system that supports it. But not all compilers support those. If you want to use those, you don't need the arr pointer declaration in your struct. Assuming no variable-length-arrays, let's look at the relevant parts of your code:

/*data for each image*/
typedef struct
{ 
    int height;
    int width;
    pixelStruct *arr; /*pointer to 2-d array of  pixels*/
} imageStruct;

arr is a pointer to pixelStruct, and not to a 2-d array of pixels. Sure, you can use arr to access such an array, but the comment is misleading, and it hints at a misunderstanding. If you really wish to declare such a variable, you would do something like:

pixelStruct (*arr)[2][3];

and arr would be a pointer to an "array 2 of array 3 of pixelStruct", which means that arr points to a 2-d array. This isn't really what you want. To be fair, this isn't what you declare, so all is good. But your comment suggests a misunderstanding of pointers in C, and that is manifested later in your code.

At this point, you will do well to read a good introduction to arrays and pointers in C, and a really nice one is C For Smarties: Arrays and Pointers by Chris Torek. In particular, please make sure you understand the first diagram on the page and everything in the definition of the function f there.

Since you want to be able to index arr in a natural way using "column" and "row" indices, I suggest you declare arr as a pointer to pointer. So your structure becomes:

/* data for each image */
typedef struct
{ 
    int height;
    int width;
    pixelStruct **arr; /* Image data of height*width dimensions */
} imageStruct;

Then in your ReadImage function, you allocate memory you need:

int i;
thisImage.arr = malloc(thisImage.height * sizeof *thisImage.arr);
for (i=0; i < thisImage.height; ++i)
    thisImage.arr[i] = malloc(thisImage.width * sizeof *thisImage.arr[i]);

Note that for clarity, I haven't done any error-checking on malloc. In practice, you should check if malloc returned NULL and take appropriate measures.

Assuming all the memory allocation succeeded, you can now read your image in thisImage.arr (just like you were doing for imageArr in your original function).

Once you're done with thisImage.arr, make sure to free it:

for (i=0; i < thisImage.height; ++i)
    free(thisImage.arr[i]);

free(thisImage.arr);

In practice, you will want to wrap the allocation and deallocation parts above in their respective functions that allocate and free the arr object, and take care of error-checking.

Answer 5

height and width are undefined; you might want to initialise them first, as in

thisImage.height = 10; thisImage.width = 20;

also,

• what is colorRGB?