-1

Code to read bmp image using struct variables and struct array. Kindly suggest me correct way to do typecasting to malloc(errors listed below code):

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

typedef struct bands{
/* .bmp image store pixel colors in "bgr" sequence */
unsigned char b,g,r; //in 24bit bmp, we need to use 8bit datatype for each color
}bands;

int main()
{
FILE *bmpimage; //ptr to read image file
FILE *redpix,*greenpix,*bluepix; //ptr to create band/color wise file
unsigned short pix_x=223,pix_y=197; /*pix_x: no. of pixels in a row,   pix_y: no. of pixels in a column of input image*/
unsigned short n_pix=pix_x*pix_y;   /*variable to count total no. of  pixels*/

bmpimage=fopen("blocks223x197.bmp","r"); //24 bit bmpimage
redpix=fopen("redpixels.txt","w");
greenpix=fopen("greenpixels.txt","w");
bluepix=fopen("bluepixels.txt","w");

/*  Define a pointer to a memory block,'*readbuffer',
that has 'n_pix' no. of memory blocks each of size same as struct bands */  
bands *readbuffer=(char*)malloc(n_pix*sizeof(*readbuffer)); 

int n;
//Create memory for each of 'n_pix' no. of pixel array of each color 
for(n=0;n<n_pix;n++){
    unsigned char *readbuffer[n].b =  (char*) malloc(sizeof(readbuffer[n].b));
    unsigned char *readbuffer[n].g = (char*) malloc(sizeof(readbuffer[n].g));
    unsigned char *readbuffer[n].r = (char*) malloc(sizeof(readbuffer[n].r));
}

if(!bmpimage){printf("Error reading bmpimage!");return 1;}
if(readbuffer==NULL){printf("NULL buffer"); exit(1);}

/* Go to 54th byte to access pixelvalue data (since, 24bit bmp format) */
fseek(bmpimage,54,SEEK_SET);

/* Read 'n_pix' no. of 'bgr' blocks each of which are of the size same as "struct bands" */
fread(readbuffer,sizeof(bands),n_pix,bmpimage);  /*read 'n_pix' no. of 'bgr' blocks each of which are of the size same as "struct bands" to the memory address, 'readbuffer' or '&readbuffer[0]' */     

int n_blocks=(sizeof(readbuffer)/sizeof(bands));
printf("no. of blocks read= %d, n_pix=%d",n_blocks,n_pix);


int i,j; int count; count=0;
/* logic to print pixel values in correct order*/

for(i=pix_y;i>0;i--){   /*for accessing row data. Choose to print from bottom to top*/
 for(j=1;j<=pix_x;j++){ /*for accessing column data. Print from left to right*/

    if(j!=pix_x){  
    fprintf(redpix,"%d,",readbuffer[(i-1)*pix_x + j].r);
    fprintf(greenpix,"%d,",readbuffer[(i-1)*pix_x + j].g);
    fprintf(bluepix,"%d,",readbuffer[(i-1)*pix_x + j].b);
    }
    else{
        count++;
    fprintf(redpix,"%d\n",readbuffer[(i-1)*pix_x + j].r);
    fprintf(greenpix,"%d\n",readbuffer[(i-1)*pix_x + j].g);
    fprintf(bluepix,"%d\n",readbuffer[(i-1)*pix_x + j].b);
    }
  }
}

// free allocated memory 
for(n=0;n<n_pix;n++){
    free(readbuffer[n].b) ;
    free(readbuffer[n].g) ;
    free(readbuffer[n].r) ;
}


fclose(bmpimage);fclose(redpix);fclose(bluepix);fclose(greenpix);

return 0;   

}

References: How to properly malloc for array of struct in C

malloc an array of struct pointers vs array of structs

List of errors:

bmpread_check.c: In function 'main': bmpread_check.c:24:19: warning: initialization from incompatible pointer type >[enabled by default] bands readbuffer=(char)malloc(n_pix*sizeof(*readbuffer)); ^ bmpread_check.c:29:33: error: expected '=', ',', ';', 'asm' or 'attribute' >before '.' token unsigned char readbuffer[n].b = (char)malloc(sizeof(readbuffer[n].b)); ^ bmpread_check.c:29:33: error: expected expression before '.' token bmpread_check.c:30:33: error: expected '=', ',', ';', 'asm' or 'attribute' >before '.' token unsigned char readbuffer[n].g = (char)malloc(sizeof(readbuffer[n].g)); ^ bmpread_check.c:30:33: error: expected expression before '.' token bmpread_check.c:31:33: error: expected '=', ',', ';', 'asm' or 'attribute' >before '.' token
unsigned char readbuffer[n].r = (char)malloc(sizeof(readbuffer[n].r)); ^ bmpread_check.c:31:33: error: expected expression before '.' token bmpread_check.c:69:5: warning: passing argument 1 of 'free' makes pointer from >integer without a cast [enabled by default] free(readbuffer[n].b) ; ^ In file included from bmpread_check.c:3:0: c:\mingw\include\stdlib.h:357:38: note: expected 'void ' but argument is of >type 'unsigned char' _CRTIMP void __cdecl __MINGW_NOTHROW free (void); ^ bmpread_check.c:70:5: warning: passing argument 1 of 'free' makes pointer from >integer without a cast [enabled by default] free(readbuffer[n].g) ; ^ In file included from bmpread_check.c:3:0: c:\mingw\include\stdlib.h:357:38: note: expected 'void ' but argument is of >type 'unsigned char' _CRTIMP void __cdecl __MINGW_NOTHROW free (void); ^ bmpread_check.c:71:5: warning: passing argument 1 of 'free' makes pointer from >integer without a cast [enabled by default] free(readbuffer[n].r) ; ^ In file included from bmpread_check.c:3:0: c:\mingw\include\stdlib.h:357:38: note: expected 'void ' but argument is of type >'unsigned char' _CRTIMP void __cdecl __MINGW_NOTHROW free (void); ^

Community
  • 1
  • 1

2 Answers2

1

This:

bands *readbuffer=(bands*)malloc(n_pix*sizeof(bands));

(Note: not *readbuffer. It's bands)

has already allocated memory for all n_pix bands.

There is no need to allocate memory for b, g, r as they are not pointers.

So,

//Create memory for each of 'n_pix' no. of pixel array of each color 
// And allocating using for loop

is not needed.

Keshava GN
  • 4,195
  • 2
  • 36
  • 47
  • And the type cast must be either changed to `(bands*)` or completely removed. – Martin Zabel Feb 08 '16 at 06:57
  • @GNKeshava As per my verification, it shouldn't matter whether I use sizeof(bands) or sizeof(*readbuffer), since they have same size. – Sai Krishna Feb 08 '16 at 07:05
  • Your suggestion was useful Martin Zabel. – Sai Krishna Feb 08 '16 at 07:10
  • @ GN Keshava And yes, "removing loop for allocating memory to each element in array" was correct step, since the program started executing correctly. But discussion "http://stackoverflow.com/questions/12334343/malloc-an-array-of-struct-pointers-vs-array-of-structs" says something on these lines which I couldn't interpret correctly. – Sai Krishna Feb 08 '16 at 07:16
  • 1
    @SaiKrishna , The link discusses about the difference between allocating a pointer and allocating pointer to pointer. Your case is just allocating a pointer. In the answer that you have given link, he is demonstrating how you can allocate, 1. Pointer to a pointer (to a structure) and 2. Pointer (to the structure). So he needs multiple mallocs. But in your case, structure doesn't contain a pointer. So, there is no need of multiple allocs.. – Keshava GN Feb 08 '16 at 08:43
  • Thank u for ur enlightening answer. – Sai Krishna Feb 08 '16 at 08:46
  • @SaiKrishna You are welcome. If answer solves your issue, please mark it as answer. Thanks, – Keshava GN Feb 08 '16 at 08:56
1

The variables b , g & r are not pointers but unsigned 8-bit variables. So, the right way of allocating memory for this case is to allocate an array of that structure with size of total number of pixels, that is width times height of the image.

This can be achieved by dynamically allocating the structure pointer bands* as follows.

bands *readbuffer = malloc(n_pix * sizeof(bands));

That statment would allocate the structure n_pix times, so that you could initialize and access pixel values b, g & r at every individual pixel location as follows.

readbuffer[i]-> b = 20;
readbuffer[i]-> g = 80;
readbuffer[i]-> r = 40;

Where i can be anything from 0 to n_pix-1

Balaraman L
  • 196
  • 2
  • 12