# deleting dynamic memory from a STL map *Fixed*

## Recommended Posts

Ted_Striker    114
Greetings to all. I've been popping in and out of here for a while and figured its time to sign up. I'm having a problem deleteing dynamic memory from a map. The error occurs when I reach the last element left in the map. My guess is im not revalidating the iterator properly. Any Ideas?
typedef map <int, AnimFile*> ANIM_FILE_MAP;

ANIM_FILE_MAP::iterator fileItr;
for (fileItr = currNode->nodeAnimMap.begin();
fileItr != currNode->nodeAnimMap.end();)
{
AnimFile* tempInfo = fileItr->second;

delete [] tempInfo->anims;
delete [] tempInfo->localOffsets;
delete tempInfo;

fileItr = currNode->nodeAnimMap.erase(fileItr);
}


[Edited by - Ted_Striker on February 15, 2005 3:54:04 PM]

##### Share on other sites
MaulingMonkey    1730
1) those delete[] statements belong in AnimFile's destructor.
2) What's the error? segfault?
3) Is that your real source?
4) Is nodeAnimMap of type std::map<int,AnimFile*> ? Maybe I'm being stupid here, but SGI's docs don't seem to have a map.erase() function that returns an iterator, only void and size_type.
5) Does rewriting like so work?:

for ( fileItr = currNode->nodeAnimMap.begin() ; fileItr != currNode->nodeAnimMap.end() ; ++fileItr ){    //deletes, no erase here because...}currNode->nodeAnimMap.clear(); //...all elements erased here

##### Share on other sites
darookie    1441
I'm all for MaulingMonkey's point 5)!
This should do the trick, if not then your error (whatever it is) stems from another source, e.g. AnimFile's constructor didn't initialise its anims or localOffets member to NULL and delete [] is trying to work with an invalid pointer. Check if anims and localOffsets are something like 0xfefefefe or 0xcdcdcdcd once delete [] is called on them.

##### Share on other sites
Ted_Striker    114
Thanks for the replies,
AnimFile is acually a struct. (but you right it probably should be a class)
Yes nodeAnimMap is of the proper type.
I havent looked at SGI's map docs but visual studios libray says yes it does return the next iterator or end();

And Yes maybe the problem does stem from another source, maybe I'm treating memory like a three cent whore. In that case im on my own, but I'll post the allocation and insert code for AnimFile also.

more about what happens: if I comment out delete tempInfo I'm fine but I suffer from plenty of leaks. When left in, the code jumps out of the function to the next time its called.

// the allocation and insert	AnimFile* animInfo = new AnimFile;	animInfo->numFrames = numFrames;	animInfo->anims = new float [numFrames*(sizeof(float)*16)];	animInfo->localOffsets = new float [numFrames*(sizeof(float)*3)];	strcpy(animInfo->animName, fileName);        currNode->nodeAnimMap.insert(ANIM_FILE_MAP::value_type(HashString(fileName), animInfo));DeleteModel(){   list<node*> lst;   lst.push_back(this->geometry);	   node* currNode = NULL;   while(!lst.empty())   {	currNode = lst.front();			// remove the node, we don't need it anymore	lst.pop_front();	delete [] currNode->theVerts;	currNode->theVerts = NULL;			ANIM_FILE_MAP::iterator fileItr;	for (fileItr = currNode->nodeAnimMap.begin();           fileItr != currNode->nodeAnimMap.end(); ++fileItr)	{			AnimFile* tempInfo = fileItr->second;		delete [] tempInfo->anims;		delete [] tempInfo->localOffsets;		delete tempInfo;	}	currNode->nodeAnimMap.clear(); 	// if children add them to the list	for (int i = 0; i < currNode->childrenv.size(); i++)	{			lst.push_back(currNode->childrenv[i]);	}	delete currNode;	currNode = NULL;   }}

##### Share on other sites
JohnBolton    1372
Quote:
 Original post by Ted_Striker...AnimFile is acually a struct. (but you right it probably should be a class)...

Oh, no!!! I hope this isn't the start of another fallacy-laden class vs. struct discussion!!!

##### Share on other sites
lucky_monkey    440
There seems to be no reason to be storing pointers and allocating the space yourself rather than storing by value...(especially since that's what the STL is meant to be good at)
• AnimFile becomes a class (or at least gets a Constructor/Destructor)
• the map becomes a std::map < int, AnimFile >;
• the source you posted in the original post becomes currNode->nodeAnimMap.clear();
• the code in your second post becomes
// the allocation and insert        currNode->nodeAnimMap.insert(ANIM_FILE_MAP::value_type(HashString(fileName), AnimFile( numFrames, fileName )));DeleteModel(){   list<node*> lst;   lst.push_back(this->geometry);	   node* currNode = NULL;   while(!lst.empty())   {	currNode = lst.front();			// remove the node, we don't need it anymore	lst.pop_front();	delete [] currNode->theVerts;	currNode->theVerts = NULL;	currNode->nodeAnimMap.clear(); 	// if children add them to the list	for (int i = 0; i < currNode->childrenv.size(); i++)	{			lst.push_back(currNode->childrenv[i]);	}	delete currNode;	currNode = NULL;   }}

I may have misread something, so correct me if I'm wrong.

##### Share on other sites
MaulingMonkey    1730
Quote:
 Original post by Ted_StrikerThanks for the replies,AnimFile is acually a struct. (but you right it probably should be a class)

Moot point - in C++ "struct ___" is simply short for "class ___ { public:". But ignoring that for now...
Quote:
 Yes nodeAnimMap is of the proper type.I havent looked at SGI's map docs but visual studios libray says yes it does return the next iterator or end();

Well, considering that shouldn't even compile if there's not a version of erase that returns an iterator, I'll follow that description :). It _should_ work with that snippet then, or the one I posted.

Quote:
 And Yes maybe the problem does stem from another source, maybe I'm treating memory like a three cent whore. In that case im on my own, but I'll post the allocation and insert code for AnimFile also.more about what happens: if I comment out delete tempInfo I'm fine but I suffer from plenty of leaks. When left in, the code jumps out of the function to the next time its called.*** Source Snippet Removed ***

Sorry, I don't think my morning caffinee has fully kicked in - what exactly do you mean by "the code jumps out of the function to the next time it's called"? What next time? Jumps out from where? But a note on this line:

strcpy(animInfo->animName, fileName);

Bad!! The C string api dosn't allocate memory for you, you're copying the data from fileName into an unallocated memory space (whatever's pointed to by animInfo->animName), unless something got left out. Prefer the C++ std::string, which will correctly allocate it's memory and create a copy by doing:

animInfo->animName = fileName;

(if animInfo->animName is of type std::string)

##### Share on other sites
Guest Anonymous Poster
"Bad!! The C string api dosn't allocate memory for you, you're copying the data from fileName into an unallocated memory space (whatever's pointed to by animInfo->animName), unless something got left out. Prefer the C++ std::string, which will correctly allocate it's memory and create a copy by doing:"

"doh, I totatly forgot about that"

The AnimFile pointer wasnt needed for scope anyways. That and using the String class like MaulingMonkey said worked. Good call guys, thanks.

##### Share on other sites
_the_phantom_    11250
Quote:
Original post by MaulingMonkey
Quote:
 Original post by Ted_StrikerI havent looked at SGI's map docs but visual studios libray says yes it does return the next iterator or end();

Well, considering that shouldn't even compile if there's not a version of erase that returns an iterator, I'll follow that description :). It _should_ work with that snippet then, or the one I posted.

SGI's docs really arent the best reference for the STL, as seen already they are missing a function call (which is shown in the book The C++ Standard Library) and are probably written more inline with an older, pre-standard STL with custom bits. Personally I'd trust the book I named or MS's own docs over the SGI ones

##### Share on other sites
MaulingMonkey    1730
Quote:
 Original post by Anonymous Poster"doh, I totatly forgot about that"The AnimFile pointer wasnt needed for scope anyways. That and using the String class like MaulingMonkey said worked. Good call guys, thanks.

No prob. The main advantage of std::string is you CAN'T froget do deal with memory since std::string deals with it for you :).

Quote:
Original post by _the_phantom_
Quote:
Original post by MaulingMonkey
Quote:
 Original post by Ted_StrikerI havent looked at SGI's map docs but visual studios libray says yes it does return the next iterator or end();

Well, considering that shouldn't even compile if there's not a version of erase that returns an iterator, I'll follow that description :). It _should_ work with that snippet then, or the one I posted.

SGI's docs really arent the best reference for the STL, as seen already they are missing a function call (which is shown in the book The C++ Standard Library) and are probably written more inline with an older, pre-standard STL with custom bits. Personally I'd trust the book I named or MS's own docs over the SGI ones

Good to know, I had allways heard it wasn't a good source on the basis of "It includes nonstandard stuff", which I ignored as they do document the nonstandard bits as such, because I didn't realize it also left out some stuff. I've added the MSDN's STL docs into my bookmarks folder.. thanks!