Sign in to follow this  
Broni

Leaking memory

Recommended Posts

I'am using the STL vector class to store a list of objects.
std::vector<CCell*> tileList;

....
....
tileList.push_bac(new CCell(screen,"pic.bmp",Vec,32,32));


When i run the program it reports that memory is being leaked. Then i tried
CCell* temp = new CCell(screen,"pic.bmp",Vec,32,32);
tileList.push_back(temp);
delete temp;

But that crashed. So how do i stop causing the leak??

Share this post


Link to post
Share on other sites
Whenever you call new, you create a new space in memory where things are stored. You then put a pointer to that space in the list.

When the list is deleted in a way or another, you lose the pointers, but the space in memory doesn't give a dollar about what is pointing to it or not, so it just stays there. But you've lost the pointers, so you won't be able to free it anymore. What you must do is before deleting the list (and losing the pointers), to call delete on each memory space through the pointers in the list (just call delete on each pointer in the list).

Your attempt to delete temp does not take into account the fact that when you add a pointer to a list, only the pointer is copied: a copy of the pointer is created, but it still points to the same area in memory as the original pointer. So by deleting temp, you delete the area in memory that was pointed to by temp, and ALSO by the copy of temp you put in the list. And now you have a pointer (a copy, actually) in your list that points to deleted (invalid) memory, which causes a crash as soon as you try to do something to that memory through the pointer.

Share this post


Link to post
Share on other sites
You are on the right track. your vector stores CCell pointers. With the second code example you pasted you delete the memory that you just allocated, and pushed into the vector. SO when you try to use that memory it is pointing to crap, nothing of use.


Still do this wen you are creating your cell

CCell* temp = new CCell(screen,"pic.bmp",Vec,32,32);
tileList.push_back(temp);




This is a example of how to clean up your memory.

while (tileList.empty() == false)
{
CCell *temp = tileList.front();
tileList.pop_front();
delete temp;
}



you only need to delete things on clean up or when you need to remove them from the vector.

Hope this helps.

Share this post


Link to post
Share on other sites
it seems like your using pointers and dynamic memory for no purpose. from the looks of it, you dont need to use dynamic memory in this situation. in fact, ive found that using STL containers, the only time i need to allocate memory myself is when i want to achieve polymorphism. it doesnt seem thats what you need here though.

instead, dont allocate the memory yourself, just have the vector do it for you. example:


std::vector<CCell> tileList;

....
....
tileList.push_back(CCell(screen,"pic.bmp",Vec,32,32));



and thats it! the beauty of the STL is you dont have to manage the memory yourself. only use pointers if you specifically need pointers for your situation (like you want to achieve run time polymorphism for example).

Share this post


Link to post
Share on other sites
A little comment on orion's code ;) pop_front( ) on a vector is suicide, performance-wise.

A faster way would be:



std::vector<CCell*>::iterator it;
for( it = tileList.begin( ); it != tileList.end( ); ++it ) {
delete *it;
}
tileList.clear( );




EDIT: graveyard filla, your method requires correct operator= and copy constructor implementations on the variables being added. If you need to perform a deep copy, you'll have to do it yourself.

Share this post


Link to post
Share on other sites
Quote:
Original post by ToohrVyk
EDIT: graveyard filla, your method requires correct operator= and copy constructor implementations on the variables being added. If you need to perform a deep copy, you'll have to do it yourself.


You only need to define what means to copy & assign if you have data members that are pointers, other than that the default copy constructor implicitly generated/defined is fine & probably better than a hand written copy constructor & assignement operator besides.

Share this post


Link to post
Share on other sites
Quote:
Original post by ToohrVyk
[If you need to perform a deep copy, you'll have to do it yourself.


thats only a valid argument for data members that are pointers to some type, the default copy constructor & assignement operator implicitly defined does a member-wise copy/assign.

[Edited by - snk_kid on September 2, 2004 4:45:37 PM]

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