Getting a random segmentation error

Question

I'm getting a weird segmentation fault: 11 error when i run this C file with gcc -std=gnu90 . From what I read, somewhere in my code it is exceeding memory? But, After doing a lot of debugging, I am not sure where is it exceeding. I'm giving my height and width as 160 and 240 respectively for a small bmp file.

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

void brighten(int,int,char*);
//void contrast(int height, int width, char* file);
//void rotation(int height, int width, char* file);

#define HEADER_SIZE 54

int main(void) {
    printf("Enter the filename: ");
    char file[256];
    scanf("%s", file);
    printf("Enter the height and width (in pixels): ");
    int height, width;
    scanf("%d %d",&height,&width);
    strcat(file,".bmp");
    brighten(height,width,file);
    //contrast(height,width,file);
    //rotation(height,width,file);
    return 0;
}

void brighten(int height, int width, char* file) {
    FILE *input = fopen(file,"rb");
    FILE *output = fopen("copy1.bmp","wb");
    char header[HEADER_SIZE];
    unsigned char pixels[height][width * 3];
    fread(header,1,HEADER_SIZE,input);
    fread(pixels,1,height * width * 3,input);

    int r, c;

    for( r = 0; r < height; r++) {
        for(c = 0; c < width * 3; c++) {
            pixels[r][c] += 50;
            if( pixels[r][c] > 255) {
                pixels[r][c] = 255;
            }
            printf(" %c \n",pixels[r][c]);
        }
    }
    fwrite(header,sizeof(char), HEADER_SIZE,output);
    fwrite(pixels,sizeof(char),height * width * 3, output);
    fclose(input);
    fclose(output);
}

You should check that the file is open (by checking if the pointer returned by `fopen` is null) — Chris, Oct 22 '17 at 01:37
You should also check the return value on your fread/frwrite calls. — jwdonahue, Oct 22 '17 at 01:45
You have a `printf` in the nested `for` loops. Does the segfault occur before, during, or after the prints? — user3386109, Oct 22 '17 at 01:51
Also, make your array `unsigned char` instead of `char`. `unsigned char` is 0 to 255 while `char` is -128 to 127. And I don't think your test `pixels[r][c] > 255` will work because the type itself can't exceed 255. Do `pixels[r][c] = (unsigned char)MIN(pixels[r][c] + 50, 255);` This should promote the intermediate values to `int`, do the arithmetic, and then force back to `unsigned char` — MFisherKDX, Oct 22 '17 at 02:34
when compiling, always enable the warnings, then fix those warnings. (for `gcc`, at a minimum use: `-Wall -Wextra -pedantic -Wconversion -std=gnu90` BTW: the std has long since passed 1990. for instance `std=gnu11` — user3629249, Oct 23 '17 at 08:36
when calling any of the `scanf()` family of functions, always check the returned value (not the parameter values) to assure the operation was successful. — user3629249, Oct 23 '17 at 08:38
when calling `fopen()`, always check (!=NULL) the returned value to assure the operation was successful. — user3629249, Oct 23 '17 at 08:39
I hope you do know that the height and width (in pixels) is already in the .bmp file header. — user3629249, Oct 23 '17 at 08:41
I hope you do know that a pixel can be anywhere from 1 to 4 bytes (consistent within a specific file) and that that information is contained in the first 54 bytes of the file. — user3629249, Oct 23 '17 at 08:42
I hope you do know that the number of bytes on a single 'row' of the image is always a multiple of 4, which often means there will be some 'fill' bytes at the end of each row in the image — user3629249, Oct 23 '17 at 08:43
regarding: `if( pixels[r][c] > 255) {` this will NEVER be true because a single character has a value range from 0x00 to 0xFF. — user3629249, Oct 23 '17 at 08:45
when calling `fread()` and/or `fwrite()`, always check the returned value to assure the desired number of items are read/written. — user3629249, Oct 23 '17 at 08:46
what happens if the user enters the whole file name, including the trailing `.bmp`? You seem to be working on linux (or similar) where capitalization counts, so just appending a `.bmp` might not result in being able to open the file. — user3629249, Oct 23 '17 at 08:49
this statement: `fread(pixels,1,height * width * 3,input);` might not read all the pixels, due to padding. this statement: `fwrite(pixels,sizeof(char),height * width * 3, output);` might not write all the pixels due to padding. — user3629249, Oct 23 '17 at 08:51

Amit Singh · Answer 1 · 2017-10-23T09:05:29.630

1

As commented by @Chris, you should check if fopen() returns NULL. Performing read/write operations on a NULL pointer leads to segmentation fault.

As per GNU C Library manual :

If the open fails, fopen() returns a null pointer.

fopen() may give NULL value due to various reasons ( but not limited to ) :

Non-existence of the file
Not having required permissions for the file

It is recommended to check errno in case fopen() returns NULL.

edited Oct 23 '17 at 09:05

answered Oct 22 '17 at 03:21

Amit Singh

2,875
14
30

the reasons listed will NOT result in a seg fault event. They will result in the returned value being NULL. Also, if the returned value is NULL, then `errno` will be appropriately set. So if the returned value is NULL, then should have a statement immediately following that says: `perror( "fopen failed" );` – user3629249 Oct 23 '17 at 08:54

Getting a random segmentation error

1 Answers1