Thread: Heap Corruption
View Single Post
Old Dec 27th, 2006, 3:47 AM   #4
grumpy
Programming Guru
 
grumpy's Avatar
 
Join Date: Jun 2005
Location: Adelaide, South Australia
Posts: 1,254
Rep Power: 5 grumpy will become famous soon enough
All sorts of problems in the original code. Some related to the "heap corruption", some not.

1) Never employ "using namespace std;" in a header file. In a header file, you usually need to suspend your laziness, and prefix anything in namespace std with "std::".

2) Your ObjModel implementation relies on a technique known as "lazy initialisation" --- eg pointers set to NULL, and then set to something useful later on. Since you are doing that with multiple pointers, it is an opportunity to forget to set one or more pointers correctly later on.

3) Make the members of ObjModel private. And then provide public member functions that can be used by ObjLoader. Yes, that will mean ObjLoader will not compile (unless you use the member functions). But it gives a means of ensuring that everything that changes an ObjModel is part of ObjModel --- and can therefore enforce any requirements (such as ensuring data is allocated before using it).

4) "delete pointer" (or "delete [] pointer") does not need to be protected with a test of "pointer != NULL", unless you are using a compiler that is very old (probably more than 15 years old).

5) To understand why you are getting heap corruption, let's have a look at some code of yours.....
void ObjLoader::ReadData(void)
{
    ifstream input(fileName->c_str());
    string buffer;

    if(!input.is_open())
        return;

    while(!input.eof())
    {
        getline(input,buffer);
            
        if(buffer.substr(0,2) == "vn")
            theObj->NumNormal++;
        else if(buffer.substr(0,2) == "vt")
            theObj->NumTexCoord++;
        else if(buffer.substr(0,1) == "v")
            theObj->NumVertex++;
        else if(buffer.substr(0,1) == "f")
            theObj->NumTriangle++;
    }

    theObj->NormalArray = new ObjNormal[theObj->NumNormal];
    theObj->TexCoordArray = new ObjTextCoord[theObj->NumTexCoord];
    theObj->TriangleArray = new ObjTriangle[theObj->NumTriangle];
    theObj->VertexArray = new ObjVertex[theObj->NumVertex];
I've cut off the end of the function. But, if this function is called twice, as in;
int main()
{
     // assume SomeLoader is an instance of ObjLoader
    SomeLoader.ReadData();
    SomeLoader.ReadData();
}
it will cause a memory leak, because the first call dynamically allocates memory and the second call dynamically allocates more memory without releasing the memory that was allocated in the first call. Some primitive memory analysers describe that as a heap corruption ... There is a basic rule of thumb you have violated: when reallocating memory, you need to release the old memory.
grumpy is offline   Reply With Quote