Why is My Bitmap Manipulating Code in C Not Storing Data Correctly?

[oreocookie14]

I am taking a beginners programming course and we have to manipulate a Bitmap image with code in C. Currently I am trying to create a function which reads the Bitmap file, and stores the contents (Width,Height,Palette Info, and Pixel Data) into structs, so that it can be accessed by other functions. I think I've managed to do an okay job at extracting data, but I am having trouble storing it into the structs. When I access the struct by another simple function it just gives me 0 as the height and width, but I can't find what is wrong with my code. I would really appreciate it if anyone could help me figure out how to do this, because I don't think I did the malloc right, which is why I can't access the struct. Do I even need a malloc? By the way, the function names/parameters are already predetermined, so I can't change any of that.
This is what I have so far:

/* Create a structure for a bitmap. */
struct BM
{
short int width;
short int height;
short int storedWidth;
int* palette;
char *data;
};

struct BM *Bitmap;
struct BM MyBM;

/**************************************************************************
* fskip *
* Skips bytes in a file. *
**************************************************************************/

void fskip(FILE *fp, int num_bytes){
int i;
for(i=0; i<num_bytes; i++){
fgetc(fp);
}
}

/*********************************************************
* readBMP
* Take the name of a bitmap file, open the file
* and store its contents into appropriate structs.
* Returns 0 if invalid filename, 1 otherwise.
*********************************************************/
int readBMP(char* filename){
FILE *fileptr;
short int num_colors;
int index, x;

/* Allocate memory for the struct */
Bitmap = (struct BM *) malloc(sizeof(struct BM));

if ((Bitmap = (struct BM *) malloc(sizeof(struct BM))) == NULL){
fclose(fileptr);
printf("Error allocating memory for file %s.\n",filename);
exit(1);
}

/* Open the file */
if ((fileptr = fopen(filename,"rb")) == NULL) {
printf("Error opening file %s.\n",filename);
return 0;
exit(1);
}

/* Check that the file is a bitmap */
if (fgetc(fileptr)!='B' || fgetc(fileptr)!='M') {
fclose(fileptr);
printf("%s is not an invalid file.\n",filename);
return 0;
exit(1);
}else{
return 1;
}

/* read in the width and height of the image, and the
number of colors used; ignore the rest */
fskip(fileptr,16);
fread(&Bitmap->width, sizeof(short int), 1, fileptr);
Bitmap->storedWidth = Bitmap->width + Bitmap->width%4;
fskip(fileptr,2);
fread(&Bitmap->height,sizeof(short int), 1, fileptr);
fskip(fileptr,22);
fread(&num_colors,sizeof(short int), 1, fileptr);
fskip(fileptr,6);

/* assume we are working with an 8-bit file */
if (num_colors==0){
num_colors=256;
}
/* The palette information . */
for (index = 0; index <= num_colors*4; index++){
fread(&Bitmap->palette[index],sizeof(int), 1, fileptr);
}

/* Read the bitmap data*/
for(index=(Bitmap->height-1)*Bitmap->storedWidth;index>=0;index-=Bitmap->storedWidth) {
for(x=0;x<Bitmap->storedWidth;x++) {
Bitmap->data[(short int)index+x]=(char)fgetc(fileptr);
}
}


fclose(fileptr);
}
/*********************************************************
* getHeight
* Get height of the original image.
*********************************************************/
int getHeight(){
printf("Bitmap->height has: %d\n", Bitmap->height);
return Bitmap->height;
}

/*********************************************************
* getWidth
* Get width of the original image.
*********************************************************/
int getWidth(){
return Bitmap->width;
}int main(){

int result = readBMP("test.bmp");
printf("Result: %d\n", result);
int booga = getHeight();
printf("Height: %d\n", booga);
int hey = getWidth();
printf("Width: %d\n", hey);

free(Bitmap);
return 0;
}

 

[Coin]

I think I've managed to do an okay job at extracting data, but I am having trouble storing it into the structs. When I access the struct by another simple function it just gives me 0 as the height and width, but I can't find what is wrong with my code.

Okay... what do you mean by the bolded part here? Do you mean GetHeight is showing 0 as the height when you call that?

I would really appreciate it if anyone could help me figure out how to do this, because I don't think I did the malloc right, which is why I can't access the struct. Do I even need a malloc?

Well, first off, the way you've done the code here, no you don't need a malloc for the Bitmap. Since Bitmap is accessible from the beginning of the program until the program ends, it would make more sense to declare it statically, rather than dynamically. (By 'declare it statically' I mean you could just say "struct BM Bitmap", or maybe you could just leave Bitmap as a pointer but set Bitmap equal to &MyBM or something.) Dynamic allocation (malloc) is most often used when you have an entity of some kind which you don't know the size of at compile-time, or when you might eventually need to allocate more than one of that entity at once. In this program there's exactly one bitmap object and it survives the entire program length, so you don't need it...

Second off, if you're going to use malloc you use it in a weird way here. You malloc() Bitmap in readBMP() but you free() it only once, in main(). If readBMP() is called more than once during the program, then this means that you will malloc() two BM structures but only free one of them!

Third off, actually it's even worse than that, readBMP actually calls malloc() twice-- if you look, you duplicate line 38 on line 40. But I assume that is a typo.

Finally, although the way you've structured this program you don't need to call malloc() for Bitmap, you **DO** need to call malloc() for Bitmap->palette and Bitmap->data! When you say int *palette, you have not actually created a place to store any ints. You have created a pointer, which stores the location of some place to store ints. This "place to store ints" might be something statically allocated, or it might be a memory address returned from malloc. In your case, you want to call malloc() for Bitmap->palette and Bitmap->data, inside of readBMP. But if you don't specifically set the place where the ints are stored, then the pointer will just be pointing out into la-la land and when you start writing to Bitmap->pallette[index] you will actually be scribbling at random all over the program's memory! It is very possible (but not very likely) that this is what is causing your problem with Height and Width being equal to zero-- but, even if this is not the cause of the Height and Width problem you need to fix it, because if you do not fix this then your program WILL crash at some point.

Does that all make sense?