Sometimes I get EXEC_BAD_ACCESS (Access violation) when reversing an array

Question

Hello, I am loading an image using the OpenEXR library.

This works fine, except the image is loaded rotated 180 degrees. I use the loop shown below to reverse the array but sometimes the program will quit and xcode will give me an EXEC_BAD_ACCESS error (Which I assume is the same as an access violation in msvc). It does not happen everytime, just once every 5-10 times.

Ideally I'd want to reverse the array in place, although that led to errors everytime and using memcpy would fail but without causing an error, just a blank image. I'd like to know what's causing this problem first.

Here is the code I am using: (Rgba is a struct of 4 "Half"s r, g, b, and a, defined in OpenEXR)

Rgba* readRgba(const char filename[], int& width, int& height){
    Rgba* pixelBuffer = new Rgba[width * height];
    Rgba* temp = new Rgba[width * height];

    // ....EXR Loading code....

    // TODO: *Sometimes* the following code results in a bad memory access error. No idea why.
    // Flip the image to conform with OpenGL coordinates.
    for (int i = 0; i < height; i++){
        for(int j = 0; j < width; j++){
            temp[(i*width)+j] = pixelBuffer[(width*height)-(i*width)+j];
        }
    }

        delete pixelBuffer;

        return temp;
}

Thanks in advance!

Answer 1

Change:

        temp[(i*width)+j] = pixelBuffer[(width*height)-(i*width)+j];

to:

        temp[(i*width)+j] = pixelBuffer[(width*height)-(i*width)+j - 1];

(Hint: think about what happens when i = 0 and j = 0 !)

Answer 2

And here's how you can optimize this code, to save memory and for cycles:

 Rgba* readRgba(const char filename[], int& width, int& height)
 {
    Rgba* pixelBuffer = new Rgba[width * height];
    Rgba tempPixel;

    // ....EXR Loading code....

    // Flip the image to conform with OpenGL coordinates.
    for (int i = 0; i <= height/2; i++)
      for(int j = 0; j < width && (i*width + j) <= (height*width/2); j++)
      {
        tempPixel =  pixelBuffer[i*width + j];
        pixelBuffer[i*width + j] = pixelBuffer[height*width - (i*width + j) -1];
        pixelBuffer[height*width - (i*width + j) -1] = tempPixel;
      }

    return pixelBuffer;
 }

Note that optimal (from a memory usage best practices point of view) would be to pass pixelBuffer* as a parameter and already allocated. It's a good practice to allocate and release the memory in the same piece of code.