8

So I stumbled upon this bit of code and while I've been coding in C/C++ for about 5 years now, I cannot fathom why anybody would want to do this. I understand why you'd want to set the pointer to NULL after deallocating memory, but I certainly don't understand why someone would want to do the opposite (looks like a memory leak to me).

Second, I'm pretty sure it's not necessary to check if the pointer is NULL before setting it to NULL and deleting it, as discussed here.

if( m_pio )
{
    m_pio = NULL;
    delete m_pio;
}
Improve this question
10
  • 12
    That's a memory leak implementation... probably accidentally moved one line up or down. Commented Feb 12, 2016 at 20:20
  • 4
    My guess is this is the result of a bad 3-way merge, a quick and dirty edit, or some kind of refactoring that wasn't reviewed. I've made mistakes similar to this myself resulting in obviously nonsensical (but still functioning) code whose origin was only apparent after digging through the source history. Commented Feb 12, 2016 at 20:21
  • 4
    .. Also they did not even need the if statement. Deleting null is fine Commented Feb 12, 2016 at 20:22
  • 1
    @audiFanatic - there is plenty of "professional grade" software with horrendous code base. This is nothing compared to the atrocities I've seen. Commented Feb 12, 2016 at 20:27
  • 2
    @TejasPawar: If m_pio = NULL was after the delete, then it would prevent double deletes successfully. As it is, though, it's just a memory leak. Commented Feb 12, 2016 at 20:34

3 Answers 3

Reset to default
7

Double deleting an object can result in the destructor being called twice, so some people favor setting it to NULL after deletion. This prevents the destructor from being called on memory which could have been reallocated from the pointer's previous memory address.

As shown here though, setting to NULL before deleting is a memory leak. Fixed code without memory leak:

if( m_pio ) {
    delete m_pio;
    m_pio = NULL;
}

Note that calling delete on NULL (or nullptr in C++11), is legal so you code could just be written as this instead:

delete m_pio;
m_pio = NULL
Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

I prefer to set a deleted pointer to NULL as a debugging aid so I know when a pointer is valid or not. Preventing double deletes or double destruction is a side benefit.
I learned C++ "in the old days", but much of what I'm (re)learning now is to go with shared_ptr or unique_ptr if you can avoid the overhead. With those and move semantics, some cases where I used to use raw pointers are now avoided.
I do the same thing, and vector is useful too when all you needed the pointer for was a variable size array. But there are still times when a raw pointer is useful.
@MarkRansom, I agree, being close to metal is why we write in C++. But it's also designed to protect your higher level design too, which is why I said "some cases" ;) I typically run into raw pointer stuff in graphics code.
1

The author has wanted to write:

delete m_pio;
m_pio = NULL;

but messed the lines and there is bad bug. Assigning NULL after delete is good prectice to protect the variable from another delete.

Improve this answer

Comments

0

Obviously we can't discern what the original author had intended.

On a side note, you'd be surprised at the amount of people I've worked with (even industry veterans) who have it in their head that deleteing or freeing NULL is somehow a bad thing that needs to be guarded against, despite that there are no consequences for it. It just ends up creating a huge tidal wave of unnecessary checks, especially when the developer in question is especially fond of wasting their time manually managing memory.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.