views:

400

answers:

13

Running valgrind or purify would be the next steps But while while writing the code itself how do you ensure that it will not cause any memory leaks? You can ensure following things:- 1: Number of new equal to delete 2: Opened File descriptor is closed or not

Is there any thing else?

+25  A: 

Use the RAII idiom everywhere you can

Use smart pointers, e.g. std::auto_ptr where appropriate. (don't use auto_prt in any of the standard collections as it won't work as you think it will)

Glen
+1 from me. But why 'everywhere you can' and not just always (considering smart pointers a form of RAII)?
Paolo Tedesco
@orsogufo, I don't like absolute statements. If I said "always" someone might come up with a situation where RAII isn't appropriate (just because I haven't thought of such a situation doesn't mean someone else hasn't).
Glen
Thomi
Use boost::shared_ptr or its standard equivalent std::tr1::shared_ptr for standard collections, or any other time when you might want to copy a pointer and still use the first one.
David Thornley
tr1 is not part of the C++ standard
anon
@David we don't use boost or tr1 in our products unfortunately. It's a situation a lot of people end up in unfortunately. Rolling your own shared_ptr works well in this situation, you just have to be careful to get it right.
Glen
+7  A: 

Use Smart Pointers

Aamir
good first advice, but fails on circles
gimpf
Yes, the second advice should be "Be Smart Yourself" :)
Aamir
Use boost::weak_ptr<> to resolve cycles in the reference graph.
janm
A: 

Make sure shared memory created by your application is freed if nobody's using it anymore, clean up memory mapped files...

Basically, make sure you clean up any type of resource your application directly or indirectly creates. File descriptors are only one type of resource your application may use during runtime.

Matthew Iselin
+1  A: 

I always use std::auto_ptr when I need to create a new object on the heap.

std::auto_ptr<Foo> CreateFoo()
{
   return std::auto_ptr<Foo>(new Foo());
}

Even if you call

CreateFoo()

it won't leak

bocco
Except that a std::auto_ptr cannot be copied. This means it can't be put into the standard containers, among other things.
David Thornley
It can, of course, be copied, otherwise you wouldn't be able to write functions that return auto_ptrs.
anon
A: 

if you make any tree or graph recursively in your code for your data structure maybe eat all of your memory.

Am1rr3zA
+14  A: 

Avoid creating objects dynamically wherever possible. Programmers coming from Java and other similar languages often write stuff like:

string * s = new string( "hello world" );

when they should have written:

string s = "hello world";

Similarly, they create collections of pointers when they should create collections of values. For example, if you have a class like this:

class Person {
   public:
      Person( const string & name ) : mName( name ) {}
      ...
   private:
      string mName;
};

Rather than writing code like:

vector <Person *> vp;

or even:

vector <shared_ptr <Person> > vp;

instead use values:

vector <Person> vp;

You can easily add to such a vector:

vp.push_back( Person( "neil butterworth" ) );

and all the memory for both Person and the vector is managed for you. Of course, if you need a collection of polymorphic types, you should use (smart) pointers

anon
This makes it effectively impossible to use polymorphism, makes it difficult to pass data around, and can contribute to bloat. Don't apply this advice wholesale without thinking about it.
David Thornley
Did you read the last sentence of my answer?
anon
Or the first one, come to that - I said "where possible".
anon
A: 

There are static code analysis tools available that do this sort of thing; wikipedia is a good place to start looking. Basically, outside of being careful and choosing the correct containers you can not make guarantees about the code you write - hence the need for tools such as valgrind and gdb.

ezpz
+4  A: 

Minimize the calls to new by using the STL containers for storing your data.

Diomidis Spinellis
+1  A: 

The basic steps are twofold:

Firstly, be aware that every new requires a delete. So, when you use the new operator, up your awareness of what that object will be doing, how it will be used, and how its lifetime will be managed.

Secondly, make sure that you never overwrite a pointer. You can do this using a smart pointer class instead of raw pointers, but if you do make absolutely sure you never use it with implicit conversion. (an example: using MSXML library, I created a CCOMPtr smart pointer to hold nodes, to get a node you call the get_Node method, passing in the address of the smart pointer - which had a conversion operator that returned the underlying pointer type. Unfortunately, this meant that if the smart pointer already held data, that member data would be overwritten, leaking the previous node).

I think those 2 cases are the times when you might leak memory. If you only use the smart pointer directly - never allowing its internal data to be exposed, you're safe from the latter issue. If you wrap all your code that uses new and delete in a class (ie using RAII) then you're pretty safe from the former too.

Avoiding memory leaks in C++ is very easy if you do the above.

gbjbaanb
+5  A: 
  1. Use RAII
  2. Hide default copy ctors, operator=() in EVERY CLASS, unless a) your class is trivial and only uses native types and YOU KNOW IT ALWAYS WILL BE SO b) you explicitly define your own

On 1) RAII, the idea is to have deletes happen automatically, if you find yourself thinking "I just called new, I'll need to remember to call delete somewhere" then you're doing something wrong. The delete should either be a) automatic or b) be put in a dtor (and which dtor should be obvious).

On 2) Hiding defaults. Identifying rogue default copy ctors etc can be a nightmare, the easiest thing is to avoid them by hiding them. If you have a generic "root" object that everything inherits from (can be handy for debugging / profiling anyway) hide the defaults here, then when an something tries to assign / copy an inheriting class the compiler barfs because the ctor's etc aren't available on the base class.

Binary Worrier
boost::noncopyable is very handy for #2
jalf
@jalf: Thanks, will tackle boost if I every need to do serious C++ again. Haven't used C++ in anger in 5 years or more now :)
Binary Worrier
Upvoted, but why hiding operator *() ?
Nemanja Trifunovic
@Nemanja Trifunovic: You know, now that you ask, I can't remember why, I may very well be mis-remembering something from Scott Myeres. I'll remove mentions of operator*() (it's been a while)
Binary Worrier
+1  A: 

Two simple rules of thumb:

  • Never call delete explicitly (outside a RAII class, that is). Every memory allocation should be the responsibility of a RAII class which calls delete in the destructor.
  • Almost never call new explicitly. If you do, you should immediately wrap the resulting pointer in a smart pointer, which takes ownership of the allocation, and works as above.

In your own RAII classes, two common pitfalls are:

  • Failure to handle copying correctly: Who takes ownership of the memory if the object is copied? Do they create a new allocation? Do you implement both copy constructor and assignment operator? Does the latter handle self assignment?
  • Failure to consider exception safety. What happens if an exception is thrown during an operation (an assignment, for example)? Does the object revert to a consistent state? (it should always do this, no matter what) Does it roll back to the state it had before the operation? (it should do this when possible) std::vector has to handle this, during push_back for example. It might cause the vector to resize, which means 1) a memory allocation which may throw, and 2) all the existing elements have to be copied, each of which may throw. An algorithm like std::sort has to deal with it too. It has to call a user-supplied comparer, which could potentially throw too! if that happens, is the sequence left in a valid state? Are temporary objects destructed cleanly?

If you handle the above two cases in your RAII classes, it is pretty much impossible for them to leak memory. And if you use RAII classes to wrap all resource allocations (memory allocations, file handles, database connections and any other type of resource that has to be acquired and released), then your application can not leak memory.

jalf
Use the standard and Boost smart pointers while building these classes. It'll save you work and subtle bugs.
David Thornley
Of course. When standard tools are available to do what you need, *use them*. But I'd still argue it's important to be aware of these subtle problems in the first place.
jalf
+3  A: 

I'm with Glen and jalf regarding RAII at every opportunity.

IMHO you should aim to write completely delete-free code. The only explicit "delete"s should be in your smart pointer class implementations. If you find yourself wanting to write a "delete", go and find an appropriate smart pointer type instead. If none of the "industry standard" ones (boost's etc) fit and you find yourself wanting to write some bizzare new one, chances are your architecture is broken or at the least there will be maintenance difficulties in future.

I've long held that explicit "delete" is to memory management what "goto" is to flow control. More on this in this answer.

timday
I'd say much the same for new. For many resources, I prefer writing a complete RAII class which creates the resource as well as releases it. A smart pointer is only responsible for deleting, so it requires the resource to be created elsewhere, which is a bit error-prone.
jalf
Interesting. I've gotten good results from "delete purges" on old code before, but never considered extending it on to new too, probably because I was originally coming at it from the point of view of making C++ code look more "Java-like". Suspect C++0x's variadic templates would be useful somewhere for making it easier to pass constructor arguments into smart pointer types which would also be responsible for invoking new.
timday
There is a boost::make_shared facility in boost that will create a boost::shared_ptr for you, by forwarding the passed in arguments into the constructor of the type: 'boost::shared_ptr<Type> ptr = make_shared<Type>( Arg1, Arg2, ...'
David Rodríguez - dribeas
A: 

Incorporate valgrind unit and system testing early in your development cycle and use it consistantly.

kmarsh
It's far, far better not to have memory management issues to begin with.
David Thornley