Blur image function causing segfault in C

Question

I am receiving a Segmentation fault (core dumped) error when trying to blur an image, but I cannot find out why. To achieve a blur, I loop through each element in the 2x2 image array. I then check each of the 9x9 squares around & including it - if they exist, their RGB values are added to a sum (sumRed, sumGreen, sumBlue) for each color. I also increment a counter called numPixel each time this is successful so I can average the RGB values at the end.

There are other parts of the code, but I am certain that this blur() function is causing the segfault. This is because when I comment out the body of the function, the segfault goes away.

However, within the function I do not see what is triggering the segfault. I don't think I'm going out of bound in an array, which has been the cause of most of my segfaults in the past. From commenting out certain portions of the code, I also gathered that memcpy() is not the cause of the error (or at least not the only cause).

There's also a custom header file, which includes definitions for BYTE and RGBTRIPLE:

typedef uint8_t  BYTE;
...
typedef struct
{
    BYTE  rgbtBlue;
    BYTE  rgbtGreen;
    BYTE  rgbtRed;
} __attribute__((__packed__))
RGBTRIPLE;

The actual code is:

// TODO: Blur image

void blur(int height, int width, RGBTRIPLE image[height][width])

{
    RGBTRIPLE new_image[height][width];
    BYTE sumRed, sumGreen, sumBlue;
    BYTE numPixels;

    for (int i = 0; i < height; i++)
    {
        for (int j = 0; j < width; w++)
        {
            sumRed = sumGreen = sumBlue = 0;
            numPixels = 0;
            // Check from 1 higher to 1 lower
            for (int h = i - 1; h <= i + 1; h++)
            {
                // Check from 1 left to 1 right
                for (int w = j - 1; w <= j + 1; j++)
                {
                    // If neither index is out of bound, add neighboring RGB values
                    if (0 <= h < height && 0 <= w < width)
                    {
                        sumRed += image[h][w].rgbtRed;
                        sumGreen += image[h][w].rgbtGreen;
                        sumBlue += image[h][w].rgbtBlue;
                        numPixels++;
                    }
                }
            }

            new_image[i][j].rgbtRed = (BYTE) sumRed / numPixels;
            new_image[i][j].rgbtGreen = (BYTE) sumGreen / numPixels;
            new_image[i][j].rgbtBlue = (BYTE) sumBlue / numPixels;
        }
    }

    memcpy(&image[0][0], &new_image[0][0], sizeof(image[0][0]) * height * width);
    return;
}

Fe2O3 · Answer

Be sure of your logic, not relying on braces to save the day. Use simple short names in "local context". "Ease of reading" trumps being "Overly explicit."

for (int h = 0; h < height; h++)
    for (int w = 0; w < width; w++) {
        // bigger accumulators, short names, declared & init'd locally
        uint16_t sumR = 0;
        uint16_t sumG = 0;
        uint16_t sumB = 0;
        int nPix = 0;

        for (int hO = -1; hO <= 1; hO++) // height offset range
            for (int wO = -1; wO <= 1; wO++) { // width offset range
                int indH = h + hO; // Simple!
                int indW = w + wO;

                if (0 <= indH && indH < height &&  0 <= indW && indW < width) {
                    RGBTRIPLE *p = &image[ indH ][ indW ]; // short alias
                    sumR += p->rgbtRed;
                    sumG += p->rgbtGreen;
                    sumB += p->rgbtBlue;
                    nPix++;
                }
            }

        new_image[i][j].rgbtRed =   (BYTE)( sumR / nPix );
        new_image[i][j].rgbtGreen = (BYTE)( sumG / nPix );
        new_image[i][j].rgbtBlue =  (BYTE)( sumB / nPix );
    }

/* memcpy....*/

I'm still uneasy with possible confusion between "Height/width" and "vertical/Horizontal".

Here's an alternative for the two inner loops. Don't bother to set-up the width if the height is out-of-frame...

// From -1 offset, examine >>3<< pixels: -1, 0, 1...
for( int ih = h-1, limH = ih+3; ih < limH; ih++ ) { // height range
    if( ih < 0 || height <= ih ) continue;
    for( int iw = w-1, limW = iw+3;  iw < limW; iw++) { // width range
        if( iw < 0 || width <= iw ) continue;

        RGBTRIPLE *p = &image[ ih ][ iw ]; // short alias
        sumR += p->rgbtRed;
        sumG += p->rgbtGreen;
        sumB += p->rgbtBlue;
        nPix++;
    }
}

Blur image function causing segfault in C

1 Answers