Leaking memory

Started by
8 comments, last by ToohrVyk 19 years, 7 months ago
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??
Advertisement
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.
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.
When a Man lies he murder''s some part of the world. These are the pale deaths which men misscall there lives. All this I cannot bear to witness any longer. Cannot the kingdom of salvation take me home? -Cliff Burton[My Site] [Jedi Academy Mod for JK2 and JK: Jedi Academy]
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).
FTA, my 2D futuristic action MMORPG
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.
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.
Thanks for the quick responses.

I used ToohrVyk snippet and I now have no more memory leaks!!


Thanks!
Quote:You only need to define what means to copy & assign if you have data members that are pointers


Quote:If you need to perform a deep copy, you'll have to do it yourself.



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]
By deep copy I meant a copy that doesn't simply rely on calling operator= on all the member variables. If this isn't the real meaning of it, I apologize.

This topic is closed to new replies.

Advertisement