Check for non-deallocated pointer

Question

Assume a pointer object is being allocated on one point and it is being returned to different nested functions. At one point, I want to de-allocate this pointer after checking whether it is valid or already de-allocated by someone.

Is there any guarantee that any of these will work?

if(ptr != NULL)
   delete ptr;

OR

if(ptr)
   delete ptr;

This code does not work. It always gives Segmentation Fault

#include <iostream>
class A
{
    public:
    int x;
     A(int a){ x=a;}
     ~A()
     { 
          if(this || this != NULL) 
              delete this;
     }
};
int main()
{ 
    A *a = new A(3);
    delete a;
    a=NULL;
}

EDIT

Whenever we talk about pointers, people start asking, why not use Smart Pointers. Just because smart pointers are there, everyone cannot use it.

We may be working on systems which use old style pointers. We cannot convert all of them to smart pointers, one fine day.

Answer 1

if(ptr != NULL) delete ptr;

OR

if(ptr) delete ptr;

The two are actually equivalent, and also the same as delete ptr;, because calling delete on a NULL pointer is guaranteed to work (as in, it does nothing).

And they are not guaranteed to work if ptr is a dangling pointer.

Meaning:

int* x = new int;
int* ptr = x;
//ptr and x point to the same location
delete x;
//x is deleted, but ptr still points to the same location
x = NULL;
//even if x is set to NULL, ptr is not changed
if (ptr)  //this is true
   delete ptr;   //this invokes undefined behavior

In your specific code, you get the exception because you call delete this in the destructor, which in turn calls the destructor again. Since this is never NULL, you'll get a STACK OVERFLOW because the destructor will go uncontrollably recursive.

Answer 2

You ask: "At one point, I want to de-allocate this pointer after checking whether it is valid or already de-allocated by someone."

There is no portable way in C/C++ to check if a >naked pointer< is valid or not. That's it. End of story right there. You can't do it. Again: only if you use a naked, or C-style pointer. There are other kinds of pointers that don't have that issue, so why don't use them instead!

Now the question becomes: why the heck do you insist that you should use naked pointers? Don't use naked pointers, use std::shared_ptr and std::weak_ptr appropriately, and you won't even need to worry about deleting anything. It'll get deleted automatically when the last pointer goes out of scope. Below is an example.

The example code shows that there are two object instances allocated on the heap: an integer, and a Holder. As test() returns, the returned std::auto_ptr<Holder> is not used by the caller main(). Thus the pointer gets destructed, thus deleting the instance of the Holder class. As the instance is destructed, it destructs the pointer to the instance of the integer -- the second of the two pointers that point at that integer. Then myInt gets destructed as well, and thus the last pointer alive to the integer is destroyed, and the memory is freed. Automagically and without worries.

class Holder {
  std::auto_ptr<int> data;
public:
  Holder(const std::auto_ptr<int> & d) : data(d) {}
}

std::auto_ptr<Holder> test() {
  std::auto_ptr<int> myInt = new int;
  std::auto_ptr<Holder> myHolder = new Holder(myInt);
  return myHolder;
}

int main(int, char**) {
  test(); // notice we don't do any deallocations!
}

Simply don't use naked pointers in C++, there's no good reason for it. It only lets you shoot yourself in the foot. Multiple times. With abandon ;)

The rough guidelines for smart pointers go as follows:

• std::auto_ptr -- use when the scope is the sole owner of an object, and the lifetime of the object ends when the scope dies. Thus, if auto_ptr is a class member, it must make sense that the pointed-to object gets deletes when the instance of the class gets destroyed. Same goes for using it as an automatic variable in a function. In all other cases, don't use it.

• std::shared_ptr -- its use implies ownership, potentially shared among multiple pointers. The pointed-to object's lifetime ends when the last pointer to it gets destroyed. Makes managing lifetime of objects quite trivial, but beware of circular references. If Class1 owns an instance of Class2, and that very same instance of Class2 owns the former instance of Class1, then the pointers themselves won't ever delete the classes.

• std::weak_ptr -- its use implies non-ownership. It cannot be used directly, but has to be converted back to a shared_ptr before use. A weak_ptr will not prevent an object from being destroyed, so it doesn't present circular reference issues. It is otherwise safe in that you can't use it if it's dangling. It will assert or present you with a null pointer, causing an immediate segfault. Using dangling pointers is way worse, because they often appear to work.

  That's in fact the main benefit of weak_ptr: with a naked C-style pointer, you'll never know if someone has deleted the object or not. A weak_ptr knows when the last shared_ptr went out of scope, and it will prevent you from using the object. You can even ask it whether it's still valid: the expired() method returns true if the object was deleted.

Answer 3

Do not call delete this in the destructor:

5.3.5, Delete: If the value of the operand of the delete-expression is not a null pointer value, the delete-expression will invoke the destructor (if any) for the object or the elements of the array being deleted.

Therefore, you will have infinite recursion inside the destructor.

Then:

if (p)
    delete p;

the check for p being not null (if (x) in C++ means if x != 0) is superfluous. delete does that check already.

This would be valid:

class Foo {
public:
    Foo () : p(0) {}
    ~Foo() { delete p; }
private:
    int *p;

    // Handcrafting copy assignment for classes that store 
    // pointers is seriously non-trivial, so forbid copying:
    Foo (Foo const&) = delete;
    Foo& operator= (Foo const &) = delete;
};

Do not assume any builtin type, like int, float or pointer to something, to be initialized automatically, therefore, do not assume them to be 0 when not explicitly initializing them (only global variables will be zero-initialized):

8.5 Initializers: If no initializer is specified for an object, the object is default-initialized; if no initialization is performed, an object with automatic or dynamic storage duration has indeterminate value. [ Note: Objects with static or thread storage duration are zero-initialized

So: Always initialize builtin types!

---

My question is how should I avoid double delete of a pointer and prevent crash.

Destructors are ought to be entered and left exactly once. Not zero times, not two times, once.

And if you have multiple places that can reach the pointer, but are unsure about when you are allowed to delete, i.e. if you find yourself bookkeeping, use a more trivial algorithm, more trivial rules, or smart-pointers, like std::shared_ptr or std::unique_ptr:

class Foo {
public:
    Foo (std::shared_ptr<int> tehInt) : tehInt_(tehInt) {}
private:
    std::shared_ptr<int> tehInt_;
};

int main() {
    std::shared_ptr<int> tehInt;
    Foo foo (tehInt);
}

Answer 4

You should never use delete this. For two reasons, the destructor is in the process of deleting the memory and is giving you the opportunity to tidy up (release OS resources, do a delete any pointers in the object that the object has created). Secondly, the object may be on the stack.

Answer 5

You cannot assume that the pointer will be set to NULL after someone has deleted it. This is certainly the case with embarcadero C++ Builder XE. It may get set to NULL afterwards but do not use the fact that it is not to allow your code to delete it again.