-2

I have been trying to dynamically allocate memory for a bmp file. I was using constant values previously and it works perfectly but when I tried to implement code from another post: C Programming: malloc() inside another function I am trying to allocate for each memory location of a bmp with RGB values.

I get a Segmentation fault (core dump) error.

Can anyone tell me what I'm doing wrong?

int main(){
unsigned char **pixels;
scanf("%d %d", &picHeight, &picWidth);
// dynamically allocate memory for height and width.
pixels = malloc(picHeight * sizeof *pixels);
int i;
    for ( i = 0; i < picHeight * 3; i+=3) {
        pixels[i] = malloc(picWidth * sizeof *pixels[i]);
        pixels[i+1] = malloc(picWidth * sizeof *pixels[i]);
        pixels[i+2] = malloc(picWidth * sizeof *pixels[i]);
    }

fread( header, 1 , HEADER_SIZE , inputfile1);
fread( pixels, 1 , picHeight * picWidth * 3 , inputfile1);

darken(pixels, picHeight, picWidth);
fclose(inputfile1);

fclose(outputfile1);
return 0;
}

void darken(unsigned char** pixels, int picHeight, int picWidth) {
int r,c;

for( r = 0; r < picHeight; r++) {
     for ( c = 0; c < picWidth * 3; c += 3) {
         int temp1 = pixels[r][c];
         int temp2 = pixels[r][c+1];
         int temp3 = pixels[r][c+2];

         temp1 = temp1 - 50;
         temp2 = temp2 - 50;
         temp3 = temp3 - 50;
         if(temp1 < 0) temp1 = 0;
         if(temp2 < 0) temp2 = 0;
         if(temp3 < 0) temp3 = 0;

         pixels[r][c] = temp1;
         pixels[r][c+1] = temp2;
         pixels[r][c+2] = temp3;
     }
}
fwrite( header, sizeof(char)  , HEADER_SIZE  ,  outputfile1);
fwrite( pixels, sizeof(char)  , picHeight * picWidth * 3  ,  outputfile1);
}

the full code is really lengthy so I didn't want to include it all.

Community
  • 1
  • 1
WilliamA
  • 29
  • 8
  • Provide a [mcve]. There is no 2D array in your code. – too honest for this site Feb 09 '16 at 15:56
  • 2
    Is there a reason you allocate three at a time instead of one at a time in your loop? At any rate, `i < picHeight * 3` isn't going to work; you've only allocated `picHeight` pointers, `pixels[picHeight ... 3 * picHeight - 1]` access unallocated memory. Last but not least, a pointer is not an array is not a pointer: you have a pointer to a pointer, not a 2D array. – R_Kapp Feb 09 '16 at 15:59
  • Your debugger will tell you **where** the crash occurs. – Jabberwocky Feb 09 '16 at 16:06
  • I am running ubuntu in gedit I don't have a debugger to use for this class. – WilliamA Feb 09 '16 at 16:13

1 Answers1

0

1/ The size of the allocated memory for your 2D array is too small : this can trigger a segmenation fault. Try:

pixels = malloc(picHeight * sizeof(*pixels)*3);

2/ If you wish to call fread only once, the values of successive rows must be contiguous in memory. See Allocate memory 2d array in function C and try:

pixels = malloc(picHeight * sizeof(*pixels)*3);
pixels[0]=malloc(picHeight * sizeof(unsigned char)*3*picWidth);
int i;
for(i=0;i<picHeight*3;i++){
    pixels[i]=&pixels[0][i*picWidth];
}

Do not forget to free() the memory at the end !

3/ fread() needs a pointer to the first value. But pixels is a 2D array, that is an array of pointers to values. Instead, try:

fread( &pixels[0][0], sizeof(unsigned char) , picHeight * picWidth * 3 , inputfile1);

where &pixels[0][0] is a pointer to the first value of the 2D array. The same modification must be made for fwrite().

Community
  • 1
  • 1
francis
  • 9,525
  • 2
  • 25
  • 41
  • Thanks! Very good answer! I knew I wasn't too far off, just frustrating when it doesn't work the first time and you start to change stuff and mess the whole thing up. – WilliamA Feb 09 '16 at 22:06