I'm still learning C, and have started using it to generate images. I can't figure out why one of my programs is segfaulting. Here's the source code, cut down to 40 lines:
#include <stdio.h>
#include <stdlib.h>
struct color {
unsigned char r, g, b;
};
struct image {
int w, h/*, o*/;
struct color **data;
};
int main() {
// Declarations
int x, y;
struct color *black;
struct image *img;
// Set up color black
black = (struct color *) malloc(sizeof(struct color *));
black->r = 0;
black->g = 0;
black->b = 0;
// Set up image img
img = (struct image *) malloc(sizeof(struct image *));
img->w = 1;
img->h = 1;
/*img->o = 0;*/
img->data = (struct color **) malloc(img->h * sizeof(struct color *));
for (y = 0; y < img->h; y++) {
img->data[y] = (struct color *) malloc(img->w * sizeof(struct color));
}
// Fill in img with black
for (x = 0; x < img->w; x++) {
for (y = 0; y < img->h; y++) {
img->data[y][x].r = black->r;
img->data[y][x].g = black->g;
img->data[y][x].b = black->b;
}
}
// Free black
free(black);
// Free img
for (y = 0; y < img->h; y++)
free(img->data[y]);
free(img->data); // Segfaults
free(img); // Also segfaults
return 0;
}
It compiles and runs fine (using gcc on Ubuntu and on Vista with Cygwin), but uncommenting the two lines dealing with img->o breaks it. I have a feeling it's related to this previous question, but I'm malloc'ing everything that needs to be malloc'ed (I think). Any help would be appreciated.
-
There is a bug in your malloc statements. You are mallocing a pointer and not a struct. This gives you only 4 bytes of memory instead of the actual size required by your struct.
black = malloc(sizeof(*black));When allocating memory for a pointer, you need to allocate memory for the thing being pointed at, not the type of the pointer. If you simply write
sizeof(*black)as shown, you will always get the right type, even if the type ofblackchanges.Tiago : this way is also valid and more clear, IMHO: black = malloc(sizeof(struct black)); -
At first glance, it seems that you're using an additional level of pointer indirection, and that's causing the segfault. When you malloc memory, it's a pointer to the object, not a pointer to a pointer to an object. So you'll have:
img = (struct image *)malloc(sizeof(struct image)) img->o = 0aib : You need not (and should not) cast the return value of malloc() in C. -
Oops, the code was cut off; I forgot to escape a less-than sign. Here it is:
#include <stdio.h> #include <stdlib.h> struct color { unsigned char r, g, b; }; struct image { int w, h/*, o*/; struct color **data; }; int main() { // Declarations int x, y; struct color *black; struct image *img; // Set up color black black = (struct color *) malloc(sizeof(struct color *)); black->r = 0; black->g = 0; black->b = 0; // Set up image img img = (struct image *) malloc(sizeof(struct image *)); img->w = 1; img->h = 1; /*img->o = 0;*/ img->data = (struct color **) malloc(img->h * sizeof(struct color *)); for (y = 0; y < img->h; y++) { img->data[y] = (struct color *) malloc(img->w * sizeof(struct color)); } // Fill in img with black for (x = 0; x < img->w; x++) { for (y = 0; y < img->h; y++) { img->data[y][x].r = black->r; img->data[y][x].g = black->g; img->data[y][x].b = black->b; } } // Free black free(black); // Free img for (y = 0; y < img->h; y++) free(img->data[y]); free(img->data); free(img); // Return return 0; } -
JaredPar has the right answer, but if you get a segfault, the first thing to do is run the program under valgrind. It is a huge help for dealing with problems like this.
BTW, I've wasted days on that exact bug. Be happy you ran into it early in your C programming career and will always watch out for it in the future.
-
Wow, that was fast. Thanks a lot, it works now. valgrind? I'll try that.
0 comments:
Post a Comment