-3

I saw a common practice of deleting the pointer amd making it null in destructor even if no memory is allocated to pointer on heap. Consider below C++ code:

dummy.h

class dummy
{
     int* a;
}

dummy.cpp

  dummy::dummy():a(NULL)
   { cout<<Inside Const"; }

   dummy::~dummy()
  {
    if(a!=NULL)
    {
      delete a;
      a = NULL;
    }
  }

  bool func()
  {
     a = func1();
  }

In above code although memory to a is not allocated on heap, even then it is deleted. It should not lead to memory leak?

Mayank Jain
  • 2,504
  • 9
  • 33
  • 52
  • your question is a bit ambiguos ... – David Haim Apr 29 '15 at 17:04
  • @DavidHaim - What are you finding ambiguous over here?Don't you think your comment is ambiguous ? – Mayank Jain Apr 29 '15 at 17:06
  • Common practice to make the pointer NULL in the destructor? What for? – juanchopanza Apr 29 '15 at 17:06
  • @juanchopanza - Sorry. Didn't got your question. – Mayank Jain Apr 29 '15 at 17:07
  • 2
    @juanchopanza: When debugging, it makes it 100% clear to the debugger(person) that the memory has already been freed. Many people swear by it. Many people swear against it. – Mooing Duck Apr 29 '15 at 17:07
  • 3
    Don't do this. See: http://stackoverflow.com/a/4666535/12711 – Michael Burr Apr 29 '15 at 17:07
  • 1
    Is `func()` being called? What is `a` in it? (Is `func` supposed to be a member of `dummy`?) Where is delete being called with no memory allocated? – Cameron Apr 29 '15 at 17:07
  • @MooingDuck It would only make it 100% clear that a local pointer has been set to NULL. The memory doesn't even have to be freed for that. – juanchopanza Apr 29 '15 at 17:09
  • @juanchopanza: True. Many people develop the habit that when they free memory, they set it to NULL, in which case, the null-ness of the pointer always reflects the validity of the pointer. Is it foolproof? No, but no code is. It's simply a practice that when used properly helps when debugging. That's all. – Mooing Duck Apr 29 '15 at 17:11
  • 1
    @MooingDuck The downside is that it makes it look like they don't know what they are doing. – juanchopanza Apr 29 '15 at 17:13
  • @juanchopanza: In your opinion. In their opinion, leaving it pointing to invalid memory is a novice mistake. Subjective either way. See the question Michael Burr linked. – Mooing Duck Apr 29 '15 at 17:15

2 Answers2

2

Making it null is completely pointless, since it's about to be destroyed.

Your code isn't deleting it if it's null, due to the if (a!=NULL). However, that's also pointless: applying delete to a null pointer will simply do nothing, so you can reduce the destructor to an unconditional delete a; (assuming that you know that it's either null, or points to an object created with new).

You do need to make sure your class either isn't copyable, or has valid copy semantics, per the Rule of Three; otherwise, copying the object will lead to deleting the same memory twice, which is not allowed.

Better still, stop juggling pointers, and use smart pointers, containers and other RAII types to make life much simpler.

Community
  • 1
  • 1
Mike Seymour
  • 249,747
  • 28
  • 448
  • 644
0

You should not call delete on a pointer that points to an object that isn't heap allocated object, ever. If you do, the program may ignore that line. Or it may erase your hard drive. Or it may ignore that line on your computer, and after you give the program to a friend it erases their hard drive. Don't do it.

Related: your class is missing the copy constructor, and copy assignment which are critical when you have a pointer that manages memory. Alternatively, replace the int* member with a unique_ptr<int> member, which manages construction and movement for you.

Mooing Duck
  • 64,318
  • 19
  • 100
  • 158