Sign in to follow this  
Ted_Striker

deleting dynamic memory from a STL map *Fixed*

Recommended Posts

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 this post


Link to post
Share on other sites
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 this post


Link to post
Share on other sites
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 this post


Link to post
Share on other sites
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 this post


Link to post
Share on other sites
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 this post


Link to post
Share on other sites
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 this post


Link to post
Share on other sites
Quote:
Original post by Ted_Striker
Thanks 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 this post


Link to post
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 this post


Link to post
Share on other sites
Quote:
Original post by MaulingMonkey
Quote:
Original post by Ted_Striker
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.


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 this post


Link to post
Share on other sites
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_Striker
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.


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!

Share this post


Link to post
Share on other sites

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