Sign in to follow this  

C++: Heap error

This topic is 3294 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hello, I have a little error in my code which I absolutely don't understand. Here is a snippet of the code that causes trouble:
Patch::indicesLODStd = new DWORD*[LODCount];  /*A*/
Patch::indicesLODMod = new DWORD*[LODCount];  /*B*/	
	
UINT LODQuadCount = cullingLODDesc.patchSize - 1;
UINT LODoffset;
for(UINT LOD = 0; LOD < LODCount; LOD++, LODQuadCount/=2) {
   UINT indicesCountLOD	 = LODQuadCount * LODQuadCount * 6;	
   Patch::indicesLODStd[LOD]  = new DWORD[indicesCountLOD]; /*C*/
   Patch::indicesLODMod[LOD]  = new DWORD[indicesCountLOD];
   Patch::indexCountsLOD[LOD] = indicesCountLOD;
   // some other stuff
}
Patch::indicesLODStd and Patch::indicesLODMod are static members of type DWORD**. For simplicity lets assume LODCount is 3. So in line A and B I allocate 3 DWORD pointers. Then the loop is entered which will iterate 3 times. In every iteration I create a DWORD array and let the appropriate pointer of indicesLODStd and indicesLODMod point to it. The problem is: The programm crashes after the third iteration (when LOD=2) on line C with the message:
Quote:
Windows has triggered a breakpoint in OutdoorRenderer.exe. This may be due to a corruption of the heap, and indicates a bug in OutdoorRenderer.exe or any of the DLLs it has loaded. The output window may have more diagnostic information
I have no idea what causes this problem. How can the problem even crash just by calling new? Does anyone understand this problem (and know how to fix it)?

Share this post


Link to post
Share on other sites
Damn, 10 seconds after I submitted my posting I have found the solution ;) I accidently read the array outside the valid boundaries. Guess I was just confused that the error occured at the allocation of new memory rather than during the wrong access of the array...

Share this post


Link to post
Share on other sites
Further to that, have you considered what will happen if cullingLODDesc.patchSize == 0 somehow? :)

Also, have you considered not doing all the memory allocation work yourself, but instead using a smart container such as std::vector? And what exactly do you need this strange allocation pattern for, anyway?

Modern C++ code would look more like (never minding the question of why you need these specific amounts of allocated space :) )


struct LODIndexPair {
DWORD Std, Mod;
};

std::vector<std::vector<DWORD> > Patch::indicesLOD;

UINT LODQuadCount = cullingLODDesc.patchSize - 1;
UINT LODIndexCount = LODQuadCount * LODQuadCount * 6;
for(UINT LOD = 0; LOD < LODCount; LOD++, LODIndexCount /= 4) {
Patch::indicesLOD.push_back(std::vector<DWORD>(LODIndexCount));
}
// The index counts are implicit; vectors know their own size.
// Do you even need a separate LODCount? Do you perhaps just want to keep
// going until the number of quads at that "level" is zero?

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman

struct LODIndexPair {
DWORD Std, Mod;
};


Struct of arrays might actually be desirable in this case - it depends on actual algorithm.

While vector is one solution, it might actually be advantageous to not use them in this case (excluding the typical implementation bugs).

IMHO, if this is pre-calculation step and these structures will not change later (but geometry may be in use a lot), I would determine all array sizes in advance, then allocate a single array big enough to hold everything, then resolve the pointers into that array.

This however is a process which could be conveniently wrapped into a class to take care of not only the process, but also allocation issues.

It's also possible that a more optimal representation exists. For example, indicesLODStd and indicesLODMod contain identical values.

This is one of those problems where YMMV.

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
Further to that, have you considered what will happen if cullingLODDesc.patchSize == 0 somehow? :)

Yes. An assert will handle this case;)

Quote:

Also, have you considered not doing all the memory allocation work yourself, but instead using a smart container such as std::vector? And what exactly do you need this strange allocation pattern for, anyway?

Well yes, but I dont see many advantages of using STL containers so I prefer using simple pointers/arrays. Since I only read and write these arrays I have no use of the "advanced" operations provided by the STL containers. And to be honest I generally don't like the STL that much;)

Share this post


Link to post
Share on other sites
If you had used a range check version of std::vector in debug mode, such as that provided by recent versions of MSVC, rather than using raw arrays in this case, it would have asserted when reading from the vector giving you a better idea of where the error in your code was.

Share this post


Link to post
Share on other sites

This topic is 3294 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this