Is it worth setting pointers to NULL in a destructor?

Question

Imagine I have a class that allocates memory (forget about smart pointers for now):

class Foo { public:   Foo() : bar(new Bar)   {   }    ~Foo()   {     delete bar;   }    void doSomething()   {     bar->doSomething();   }  private:   Bar* bar; }; 

As well as deleting the objects in the destructor is it also worth setting them to NULL?

I'm assuming that setting the pointer to NULL in the destructor of the example above is a waste of time.

Answer 1

Several answer mention that it might be worthwhile to do this in a DEBUG build to aid in debugging.

Do not do this.

You'll just potentially help hide a problem in a debug build that isn't hidden in your release builds that you actually give to customers (which is the opposite effect that your debug builds should have).

If you're going to 'clear' the pointer in the dtor, a different idiom would be better - set the pointer to a known bad pointer value. That way if there is a dangling reference to the object somewhere that ultimately tries to use the pointer, you'll get a diagnosable crash instead of the buggy code avoiding use of the pointer because it notices that it's NULL.

Say that doSomething() looked like:

void doSomething() {     if (bar) bar->doSomething(); } 

Then setting bar to NULL has just helped hide a bug if there was a dangling reference to a deleted Foo object that called Foo::doSomething().

If your pointer clean up looked like:

~Foo() {     delete bar;     if (DEBUG) bar = (bar_type*)(long_ptr)(0xDEADBEEF); } 

You might have a better chance of catching the bug (though just leaving bar alone would probably have a similar effect).

Now if anything has a dangling reference to the Foo object that's been deleted, any use of bar will not avoid referencing it due to a NULL check - it'll happily try to use the pointer and you'll get a crash that you can fix instead of nothing bad happening in debug builds, but the dangling reference still being used (to ill effect) in your customer's release builds.

When you're compiling in debug mode, chances are pretty good that the debug heap manager will already be doing this for you anyway (at least MSVC's debug runtime heap manager will overwrite the freed memory with 0xDD to indicate the memory is dead/freed).

The key thing is that if you're using raw pointers as class members, don't set the pointers to NULL when the dtor runs.

This rule might also apply to other raw pointers, but that depends on exactly how the pointer is used.