views:

903

answers:

8

I've painfully learned during last few days a lot about programming in c++.
I love it :)
I know I should release memory - the golden "each malloc=free" or "each new=delete" rules exist now in my world, but I'm using them to rather simple objects.
What about vector ? Wherever I can, I'm using vector.clear() but that clearly isn't enough, because I'm having huge memory leaks.
Could you guys guide me on how should I treat this thing?

*Edit
Thanks guys, your comments made me think about the alghorithm of this application and I'll be able to eliminate the vector totally. :O
Sorry - I started explaining what is my use case here and I found out what I really need. It's like that when you code last 3 days for 18 hours a day :| *Edit 2
This is crazy. By small changes in code, I've eliminated memory usage from 2x130 mb (constantly growing) into 2x 13,5mb, constant size. Thanks guys for making me think about that in another way.

Btw. such self code review got a name - anyone remember that? It's when you ask anyone (even your mother or dog) and start explaining what's your problem - and suddenly you solve this 5 hour problem yourself, just by trying to look at it from other point of view, or just by trying to summarize what's it all about. I often find myself being catched on that...

+2  A: 

The rule is that when you clear a vector of objects, the destructor of each element will be called. On the other hand, if you have a vector of pointers, vector::clear() will not call delete on them, and you have to delete them yourself.

So if all you have is a vector of strings, and not pointers to strings, then your memory leaks must be caused by something else.

Dima
+4  A: 

Calling v.clear() will destroy all objects that are currently held inside v, but it will not release the memory (it is assumed that the vector will soon be filled again).

If you really want to free the memory, the idiom is

vector<string>().swap(v);

This will create a new (temporary) vector and swap its contents with v. The temporary vector is then destroyed, freeing the memory along with it.

avakar
Now I see the intention behind that, but it looks quite awkward to me. And would it be the same as v.swap(vector<string>())?
ypnos
It may seem awkward, but it is the idiomatic way of decreasing `v.capacity()`. Your snippet won't compile, since `vector<string>()` is an rvalue and cannot be bound to a non-const reference.
avakar
To clarify: in a.swap(b), a and b are not equal. Member swap has one argument, a non-const reference. You can call non-const member functions of temporaries, so in a.swap(b), a can be a temporary. But b is bound to a non-const reference and cannot be a temporary.
MSalters
A: 

Give a use case. The destructor on the string is getting called by vector::clear. Your problem lies elsewhere my friend.

also check out:

http://stackoverflow.com/questions/594089/does-stdvector-clear-do-delete-free-memory-on-each-element

Ryan Oberoi
+2  A: 

The vector (like all standard containers) owns the objects inside it.
So it is responsible for destroying them.

Note: If you vector contains pointers then it owns the pointers (not what the pointers point at). So these need to be deleted. But there are easier ways.

You could use a vector of smart pointers. In fact you should be using some form of smart pointer for nearly everything. If you are using pointers you are probably still programming like a C programmer.

So:

std::vector<int>    data; // clear is fine.

std::vector<int*>   data1; // Now things need to be deleted.
// alternative 1:
std::vector<boost::shared_ptr<int> >  data2; // The shared pointer will auto
                                             // delete the pointer.
// alternative 2:
boost::ptr_vector<int>  data3;               // Here the container knows that
                                             // it is holding pointers and will
                                             // auto de-reference them when you
                                             // its members.

But it sounds like you need to start thinking about learning about smart pointers.

int*  x = new int(5);
// Do stuff.
*x = 8;
delete x;

// --- Instead use a smart pointer:
std::auto_ptr<int>  x(new int(5));
// Do stuff.
*x = 8;
// No delete (the auto ptr handles it.
Martin York
+4  A: 

Deleting elements from STL containers is guaranteed to call destructors on these elements. However, if you have a container of some pointer-to-T type, then you still have to free the pointed-to memory yourself (in this case, the "destructor" for the pointer gets called, which is a no-operation).

If you do not want to manually manage memory in this case, consider using a smart-pointer solution or a pointer container.

ASk
+4  A: 

You don't need to be doing this. std::string cleans itself up, so the strings are not your problem. Remember that YOU didn't use new so YOU don't have to use delete.

You should probably learn about RAII - it makes allocation and deallocation much simpler. You'll avoid memory leaks this way.

rlbond
+1  A: 

If you have a vector and it goes out of scope, all objects in the vector are destroyed. There isn't really a need to call clear() unless you want to dump the contents and reuse the vector.

However if you by any chance are using something like a vector then the destructor of the objects being pointed to will not be called as the vector destructor doesn't follow the indirections represented by the pointers.

All that said, have you actually confirmed that you've got genuine memory leaks and that they are caused by the data in the vector?

Timo Geusch
A: 

As rlbond suggested, use RAII.

It's a good rule of thumb to never put new and delete calls into your main code flow. Always try to put them into objects so that the object destructor can free what needs to be freed. In this way, you avoid needing to remember to call delete and it makes your code exception safe (assuming that you make your object's operations exception safe).

For example, if you had a vector of pointers to STL strings or C-style character arrays, put that into a StringContainer (use a better name) and have the StringContainer hold a vector and in the StringContainer destructor run a for loop to delete each string in the vector.

You can make the vector inside the StringContainer a public member and mess around with it directly, but it's even better design to make it private or protected and add some member functions to manage the string* vector.

So your main C++ program should never see a new or delete anywhere. Instead it should have a lot of stack allocated objects, auto_ptrs and shared_ptrs.

Zan Lynx