C++ OpenGL glTexImage2D Access Violation

Question

I'm writing an application using OpenGL (freeglut and glew).

I also wanted textures so I did some research on the Bitmap file format and wrote a struct for the main header and another for the DIB header (info header).

Then I started writing the loader. It automatically binds the texture to OpenGL. Here is the function:

static unsigned int ReadInteger(FILE *fp)
{
    int a, b, c, d;

    // Integer is 4 bytes long.
    a = getc(fp);  
    b = getc(fp);  
    c = getc(fp);  
    d = getc(fp);

    // Convert the 4 bytes to an integer.
    return ((unsigned int) a) + (((unsigned int) b) << 8) +
           (((unsigned int) c) << 16) + (((unsigned int) d) << 24);
}

static unsigned int ReadShort(FILE *fp)
{
    int a, b;

    // Short is 2 bytes long.
    a = getc(fp);  
    b = getc(fp);

    // Convert the 2 bytes to a short (int16).
    return ((unsigned int) a) + (((unsigned int) b) << 8);
}

    GLuint LoadBMP(const char* filename)
{
    FILE* file;

    // Check if a file name was provided.
    if (!filename)
        return 0;

    // Try to open file.
    fopen_s(&file, filename, "rb");

    // Return if the file could not be open.
    if (!file)
    {
        cout << "Warning: Could not find texture '" << filename << "'." << endl;
        return 0;
    }

    // Read signature.
    unsigned char signature[2];
    fread(&signature, 2, 1, file);

    // Use signature to identify a valid bitmap.
    if (signature[0] != BMPSignature[0] || signature[1] != BMPSignature[1])
    {
        fclose(file);
        return 0;
    }

    // Read width and height.
    unsigned long width, height;
    fseek(file, 16, SEEK_CUR); // After the signature we have 16bytes until the width.
    width = ReadInteger(file);
    height = ReadInteger(file);

    // Calculate data size (we'll only support 24bpp).
    unsigned long dataSize;
    dataSize = width * height * 3;

    // Make sure planes is 1.
    if (ReadShort(file) != 1)
    {
        cout << "Error: Could not load texture '" << filename << "' (planes is not 1)." << endl;
        return 0;
    }

    // Make sure bpp is 24.
    if (ReadShort(file) != 24)
    {
        cout << "Error: Could not load texture '" << filename << "' (bits per pixel is not 24)." << endl;
        return 0;
    }

    // Move pointer to beggining of data. (after the bpp we have 24 bytes until the data)
    fseek(file, 24, SEEK_CUR);

    // Allocate memory and read the image data.
    unsigned char* data = new unsigned char[dataSize];

    if (!data)
    {
        fclose(file);
        cout << "Warning: Could not allocate memory to store data of '" << filename << "'." << endl;
        return 0;
    }

    fread(data, dataSize, 1, file);

    if (data == NULL)
    {
        fclose(file);
        cout << "Warning: Could no load data from '" << filename << "'." << endl;
        return 0;
    }

    // Close the file.
    fclose(file);

    // Create the texture.
    GLuint texture;
    glGenTextures(1, &texture);
    glBindTexture(GL_TEXTURE_2D, texture);

    glPixelStorei(GL_UNPACK_ALIGNMENT, 1);
    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_REPEAT);
    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_REPEAT);
    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR_MIPMAP_LINEAR); //NEAREST);
    glTexEnvi(GL_TEXTURE_ENV, GL_TEXTURE_ENV_MODE, GL_MODULATE); 

    gluBuild2DMipmaps(GL_TEXTURE_2D, GL_RGB, width, height, GL_BGR_EXT, GL_UNSIGNED_BYTE, data);

    return texture;
}

I know that the bitmap's data is correctly read because I outputted it's data to the console and compared with the image opened in paint.

The problem here is this line:

glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, dibheader.width,
    dibheader.height, 0, GL_RGB, GL_UNSIGNED_BYTE, data);

Most of the times I run the application this line crashes with the error:

Unhandled exception at 0x008ffee9 in GunsGL.exe: 0xC0000005: Access violation reading location 0x00af7002.

This is the Disassembly of where the error occurs:

movzx       ebx,byte ptr [esi+2]

It's not an error with my loader, because I've downloaded other loaders. A downloaded loader that I used was this one from NeHe.

EDIT: (CODE UPDATED ABOVE)

I rewrote the loader, but I still get the crash on the same line. Instead of that crash, sometimes I get a crash on mlock.c (same error message is I recall correctly):

void __cdecl _lock (
        int locknum
        )
{

        /*
         * Create/open the lock, if necessary
         */
        if ( _locktable[locknum].lock == NULL ) {

            if ( !_mtinitlocknum(locknum) )
                _amsg_exit( _RT_LOCK );
        }

        /*
         * Enter the critical section.
         */

        EnterCriticalSection( _locktable[locknum].lock );
}

On the line:

EnterCriticalSection( _locktable[locknum].lock );

Also, here is a screen shot of one of those times the applications doesn't crash (the texture is obviously not right): https://i.stack.imgur.com/4Mtso.jpg

Edit2:

Updated code with the new working one. (The reply marked as an answer does not contain all that was needed for this to work, but it was vital)

Answer 1

Try glPixelStorei(GL_UNPACK_ALIGNMENT, 1) before your glTexImage2D() call.

Answer 2

I know, it's tempting to read binary data like this

BitmapHeader header;
BitmapInfoHeader dibheader;
/*...*/
// Read header.
fread(&header, sizeof(BitmapHeader), 1, file);

// Read info header.
fread(&dibheader, sizeof(BitmapInfoHeader), 1, file);

but you really shouldn't do it that way. Why? Because the memory layout of structures may be padded to meet alignment constraints (yes, I know about packing pragmas), the type size of the used compiler may not match the data size in the binary file, and last but not least endianess may not match.

Always read binary data into a intermediary buffer of which you extract the fields in a well defined way with exactly specified offsets and typing.

// Allocate memory for the image data.
data = (unsigned char*)malloc(dibheader.dataSize);

If this is C++, then use the new operator. If this is C, then don't cast from void * to the L value type, it's bad style and may cover usefull compiler warnings.

// Verify memory allocation.
if (!data)
{
    free(data);

If data is NULL you mustn't free it.

// Swap R and B because bitmaps are BGR and OpenGL uses RGB.
for (unsigned int i = 0; i < dibheader.dataSize; i += 3)
{
    B = data[i]; // Backup Blue.
    data[i] = data[i + 2]; // Place red in right place.
    data[i + 2] = B; // Place blue in right place.
}

OpenGL does indeed support BGR alignment. The format parameter is, surprise, GL_BGR

// Generate texture image.
glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, dibheader.width, dibheader.height, 0, GL_RGB, GL_UNSIGNED_BYTE, data);

Well, and this misses setting all of the pixel store parameters. Always set every pixel store parameter before doing pixel transfers, they may be left in some undesired state from a previous operation. Better safe than sorry.