What does this valgrind error mean?

Question

I have a basic library that I use to paint OpenGL text and when ever I use valgrind to make sure it is air tight. I keep getting an unusual error that looks to me as if the linux c++ libraries are faulty. I would like to see if you guys can either spot my error or authenticate what I fear, and that is that my c++ libraries are faulty and need to be replaced. The code is very simple but it uses both OpenGL and FreeImage, so certain lines won't make sense.

Here is fontsystem.h:

#ifndef FONTSYSTEM_H
#define FONTSYSTEM_H

/*
    The Font System works by loading all the font images (max image size 32px^2) into memory and storing
  the OpenGL texture ID's into an array that can be access at all times. The DrawString() functions will
  search through the string for the specified character requested to draw and then it will draw a quad
  and paint the texture on it. Since all the images are pre-loaded, no loading of the images at load time
  is necessary, this is memory consuming but efficiant for the CPU. ALL functions WILL return a character
  string specifing errors or success. A function will work as long as it can and when an error happens,
  unless the error is fatal, the functions will NOT rollback changes! This ensures that during testing, a
  very certain bug can be spotted.
*/

#include <cstdio>
#include <cstdlib>
#include <iostream>
#include <string.h>
#include <assert.h>
#include <GL/gl.h>
#include <GL/glu.h>
#include <GL/glext.h>
#include <FreeImage.h>

#define REPORT(x) (std::cout<<x)
#define TIME clock()

class CFont
{
public:
    CFont();
    ~CFont();

    void DrawString(char *apString, int aiLetterSize, int aiX, int aiY);
    void DrawString(long anNumber, int aiLetterSize, int aiX, int aiY);
    void SetPath(char avPath[]);
    void SetupFont(); // This function will load as many images as possible into memory.

private:
    GLuint *mpTextIDs;
    int *mpDrawIDs;
    char *mpPath;

public:
    CFont(const CFont& a):
        mpTextIDs(a.mpTextIDs),
        mpDrawIDs(a.mpDrawIDs),
        mpPath(a.mpPath)
    {
        std::copy(a.mpTextIDs, a.mpTextIDs+128, mpTextIDs);
        std::copy(a.mpDrawIDs, a.mpDrawIDs+128, mpDrawIDs);
        std::copy(a.mpPath, a.mpPath + strlen(a.mpPath), mpPath);
    }

    CFont& operator=(const CFont& a)
    {
        GLuint *iTmpTex = new GLuint[128];
        int *iTmpDraw = new int[1024];
        char *vTmpPath = new char[4096];

        delete[] mpTextIDs;
        delete[] mpDrawIDs;
        delete[] mpPath;

        std::copy(a.mpTextIDs, a.mpTextIDs+128, iTmpTex);
        std::copy(a.mpDrawIDs, a.mpDrawIDs+128, iTmpDraw);
        std::copy(a.mpPath, a.mpPath + strlen(a.mpPath), vTmpPath);

        mpTextIDs = iTmpTex;
        mpDrawIDs = iTmpDraw;
        mpPath = vTmpPath;

        return *this;
    }
};

#endif // FONTSYSTEM_H

here is fontsystem.cpp:

#include "fontsystem.h"

CFont::CFont()
{
    mpTextIDs = new GLuint[128];
    mpDrawIDs = new int[1024];
    mpPath = new char[4096];
}

CFont::~CFont()
{
    delete[] mpTextIDs;
    delete[] mpDrawIDs;
    delete[] mpPath;
}

void CFont::DrawString(char *apString, int aiLetterSize, int aiX, int aiY)
{
    // Sanity check!
    if(apString == NULL)
    {
        REPORT("{Gfx}["<< TIME<< "]Error: Drawing string is NULL! <Font System>\n");
        return;
    }

    if(aiLetterSize <= 0)
    {
        REPORT("{Gfx}["<< TIME<< "]Error: Letter size is less than zero! <Font System>\n");
        return;
    }

    // Search the string from most significant character to least significant.
    int iSelectIndex = 0;
    int iNumOfSymb = 0;
    for(size_t i = 0; apString[i] != '\0'; ++i)
    {
        iSelectIndex = apString[i] >= '0' && apString[i] <= '9' ? (apString[i] - '0') :
                       apString[i] >= 'A' && apString[i] <= 'Z' ? (apString[i] - 'A' + 10) :
                       apString[i] >= 'a' && apString[i] <= 'z' ? (apString[i] - 'a' + 10) :
                       apString[i] == ' ' ? 36 : // This is a special case, This see's if the current character is a space or not.
                       -1;

        if(iSelectIndex == -1)
        {
            return;
        }

        // Add the current selected character to the drawing array.
        mpDrawIDs[i] = iSelectIndex;
        ++iNumOfSymb;
    }

    // Go through and draw each and every character.
    for(size_t i = 0; apString[i] != '\0'/*static_cast<size_t>(iNumOfSymb)*/; ++i)
    {
        // Paint each qaud with the X,Y coordinates. After each quad has been successfully drawn,
        // Add the size to the X coordinate. NOTE: Each character is square!!!

        glBindTexture(GL_TEXTURE_2D, mpTextIDs[(uint)apString[i]]);

        int yPos = apString[i] != 'q' || apString[i] != 'j' || apString[i] != 'y' ? aiY : aiY + (aiLetterSize/2);

        glBegin(GL_QUADS);
            glTexCoord2d(0, 0);
            glVertex2d(aiX, yPos);

            glTexCoord2d(1, 0);
            glVertex2d(aiX + aiLetterSize, yPos);

            glTexCoord2d(1, 1);
            glVertex2d(aiX + aiLetterSize, yPos + aiLetterSize);

            glTexCoord2d(0, 1);
            glVertex2d(aiX, yPos + aiLetterSize);
        glEnd();

        // Now, increase the X position by the size.
        aiX += aiLetterSize;
    }
}

void CFont::DrawString(long anNumber, int aiLetterSize, int aiX, int aiY)
{
    // Sanity Check!
    if(aiLetterSize <= 0)
    {
        REPORT("{Gfx}["<< TIME<< "]Error: Letter size is less than zero! <Font System>\n");
        return;
    }

    // Convert the supplied number to a character string via snprintf().
    char *vTempString = new char[1024];
    snprintf(vTempString, 1024, "%ld", anNumber);

    // Next, run DrawString().
    DrawString(vTempString, aiLetterSize, aiX, aiY);
}

void CFont::SetupFont()
{
    // First Load The PNG file holding the font.
    FreeImage_Initialise(false);

    FIBITMAP *spBitmap = FreeImage_Load(FIF_PNG, mpPath, BMP_DEFAULT);

    if(!spBitmap)
    {
        REPORT("{Gfx}["<< TIME<< "]Error: Was Unable to opne/decode font bitmap! <FreeImage>\n");
        return;
    }

    // Do an image sanity check.
    if(!FreeImage_HasPixels(spBitmap))
    {
        REPORT("{Gfx}["<< TIME<< "]Error: The font bitmap contains nothing! <FreeImage>\n");
        return;
    }

    // Retrieve all the image data from FreeImage.
    unsigned char *pData = FreeImage_GetBits(spBitmap);
    int iWidth = FreeImage_GetWidth(spBitmap);
    int iHeight = FreeImage_GetHeight(spBitmap);
    size_t const ciCharWidth = iHeight;
    size_t const ciCharHeight = iHeight;

    // Cutup the PNG.
    int iFontElementSize = (ciCharWidth*ciCharHeight)*4; // The first two numbers, are the dimensions fo the element, the last number (4) is the number of color channels (Red Green Blue and Alpha)
    unsigned char *pElemBuff = new unsigned char[iFontElementSize]; // The temporary element buffer.

    // Create all 37 OpenGL textures. 0-9 and A-Z and finally space (' ')
    glGenTextures(128, mpTextIDs);

    for (size_t iCharIdx = 0; 128 > iCharIdx; ++iCharIdx)
    {
        // Create character texture.
        size_t const ciScanOfst = ciCharWidth * iCharIdx * 4;
        for (size_t iScanLineIdx = 0; ciCharHeight > iScanLineIdx; ++iScanLineIdx)
        {
            memcpy(pElemBuff + ciCharWidth * iScanLineIdx * 4,
                   pData + ciScanOfst + iWidth * (ciCharHeight - iScanLineIdx - 1) * 4,
                   ciCharWidth * 4);
        }

        // Create The OpenGL Texture with the current Element.
        glBindTexture(GL_TEXTURE_2D, mpTextIDs[iCharIdx]);
        glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 32, 32, 0, GL_RGBA, GL_UNSIGNED_BYTE, pElemBuff);

        // Create the correct texture environment to the current texture.
        glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR_MIPMAP_LINEAR);
        glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
        glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP);
        glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP);
        glTexParameteri(GL_TEXTURE_2D, GL_GENERATE_MIPMAP, GL_TRUE);
    }

    // Do a little house cleaning!
    delete[] pElemBuff;
    FreeImage_Unload(spBitmap);
    FreeImage_DeInitialise();

    REPORT("{Gfx}["<< TIME<< "]Information: Font was created succesfully! <Font System>\n");
}

void CFont::SetPath(char avPath[])
{
    mpPath = avPath;
}

Valgrind reports the following:

Starting the FontSystem...
FontSystem Started!
==5058== Invalid free() / delete / delete[] / realloc()
==5058==    at 0x402A8DC: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==5058==    by 0x8048F03: CFont::~CFont() (fontsystem.cpp:14)
==5058==    by 0x8048DE0: main (main.cpp:13)
==5058==  Address 0x804972f is not stack'd, malloc'd or (recently) free'd
==5058== 
==5058== 
==5058== HEAP SUMMARY:
==5058==     in use at exit: 4,172 bytes in 3 blocks
==5058==   total heap usage: 153 allocs, 151 frees, 135,457 bytes allocated
==5058== 
==5058== 20 bytes in 1 blocks are still reachable in loss record 1 of 3
==5058==    at 0x402A5E6: calloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==5058==    by 0x64DB3AD: _dlerror_run (dlerror.c:142)
==5058==    by 0x444EC64: ??? (in /usr/lib/libGL.so.295.59)
==5058== 
==5058== 56 bytes in 1 blocks are still reachable in loss record 2 of 3
==5058==    at 0x402BE68: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==5058==    by 0x442860E: ??? (in /usr/lib/libGL.so.295.59)
==5058==    by 0xBE872481: ???
==5058==    by 0x4E45504E: ???
==5058== 
==5058== 4,096 bytes in 1 blocks are definitely lost in loss record 3 of 3
==5058==    at 0x402B454: operator new[](unsigned int) (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==5058==    by 0x8048EAC: CFont::CFont() (fontsystem.cpp:7)
==5058==    by 0x8048D68: main (main.cpp:7)
==5058== 
==5058== LEAK SUMMARY:
==5058==    definitely lost: 4,096 bytes in 1 blocks
==5058==    indirectly lost: 0 bytes in 0 blocks
==5058==      possibly lost: 0 bytes in 0 blocks
==5058==    still reachable: 76 bytes in 2 blocks
==5058==         suppressed: 0 bytes in 0 blocks
==5058== 
==5058== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)

I hope you guys can help me with this because these seem to be common errors in all my programs. And as a coder, I don't very much like subtle, yet nasty, errors.

Answer 1

The most conspicuous problem is this one:

==5058== Invalid free() / delete / delete[] / realloc()
==5058==    at 0x402A8DC: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==5058==    by 0x8048F03: CFont::~CFont() (fontsystem.cpp:14)
==5058==    by 0x8048DE0: main (main.cpp:13)
==5058==  Address 0x804972f is not stack'd, malloc'd or (recently) free'd
==5058== 

As you can see, it refers to line 14 of fontsystem.cpp, in the destructor of CFont, here:

delete[] mpPath;

Apparently what you attempt to delete [] there has never been dynamically allocated, or not in the right way. How can that be? The corresponding statement in the constructor looks fine:

mpPath = new char[4096];

So the value of mpPath must have been changed somewhere after the constructor was called. Indeed, there is this function:

void CFont::SetPath(char avPath[])
{
    mpPath = avPath;
}

You probably call this somewhere, trying to transfer a stack-allocated array to it. You shouldn't do that. If CFont is responsible for the allocation and deallocation, there shouldn't be a function that attempts to take a foreign array and set a member pointer to it. Consider alternatives, such as making a copy of the avPath array into a freshly allocated array on the heap. And de-allocating the previously allocated one. Better even, use std::vector or smart pointers.

The other problem is related:

==5058== 4,096 bytes in 1 blocks are definitely lost in loss record 3 of 3
==5058==    at 0x402B454: operator new[](unsigned int) (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==5058==    by 0x8048EAC: CFont::CFont() (fontsystem.cpp:7)
==5058==    by 0x8048D68: main (main.cpp:7)

That's a memory leak related to the same member, mpPath. And of course, this leak occurs because setPath resets the pointer without de-allocating the existing array. Same problem as above.

The rest of the messages are related to various external library calls. Minor memory leaks you can probably not do much about.

Answer 2

The 4096 bytes you are losing is probably from here:

void CFont::SetPath(char avPath[])
{
    mpPath = avPath;
}

You are just copying the pointer (and overwriting your old one); you need to strcpy or use std::copy like in other parts of the code.