views:

628

answers:

8

I'm searching for a proper way to clean my pointers. Here the example code:

class Parent {
   protected:
      int m_Var;
   public:
      Parent() : m_Var(0) {}
      virtual ~Parent() {}
      void PubFunc();
};

class Child : public Parent {
   protected:
      bool m_Bool;
   public:
      Child() : m_Bool(false) {}
      virtual ~Child() {}
      void ChildFunc();
};

void RemoveObj(Parent **ppObj)
{
   *ppObj->PubFunc();
   delete *ppObj;
   ppObj = NULL;
}

int main()
{
   Parent* pPObj = NULL;
   Child*  pCObj = NULL;
   pPObj = new Parent();
   pCObj = new Child();

   RemoveObj(&pPObj);
   RemoveObj(&pCObj); // This is line 33
   return 1;
}

But the compiler gives error:

classes.cpp:33: error: invalid conversion from ‘Child**’ to ‘Parent**’
classes.cpp:33: error:   initializing argument 1 of ‘void RemoveObj(Parent**)’
+11  A: 

There are soo many ways to handle memory correctly.

The one close to your example would be:

template <typename T>
RemoveObj(T **p)
{
    if (p == NULL) return;
    delete *p;
    *p = NULL;
}

Additionally you might want to use std::auto_ptr instead. It would look like:

int main()
{
   std::auto_ptr<Parent*> pPObj(new Parent);
   std::auto_ptr<Child*> pCObj(new Child);
   // no deletes needed anymore
About the first solution: What's the improvement comparing to: void RemoveObj(void **ppObj)
To1ne
The first suggestion compiles and works. void RemoveObj(void **ppObj) can be defined but if you try calling it your call shouldn7t compile.
Windows programmer
@To1ne: I think deleting void * is actually undefined. With the template solution, you have a correct static type. What nontrivial destructor should be called for a void *?
Pontus Gagge
deleting 'void*' is undefined
I think a dynamic_cast or reinterpret_cast would not be bad either? only really needed when I want to call *p->PubFunc();
To1ne
BTW The test for p == NULL is unnecessary. delete on a null pointer is required to do nothing.
Rob K
The test is not done on the pointer which is deleted, but on the one which points to the pointer to be deleted. *p if p is NULL must be tested.
+1  A: 

You can find some useful information from the book < C++ common knowledge> Item 8. Pointers to Pointers.

Angus
+3  A: 

You don't need a wrapper for delete, keep it simple:

int main()
{
  Parent* pPObj = NULL;
  Child*  pCObj = NULL;
  pPObj = new Parent();
  pCObj = new Child();

  delete pPObj;
  delete pCObj; // This is line 33
  return 1;
}

And remember you will run into issues deleting array type objects with your RemoveObj (since you are always using a scalar delete). An alternative is of course to pass a flag around to indicate you want delete []. But as I said: KISS.

dirkgently
Maybe my example was too simple... I wanted to call PubFunc() in that function too...
To1ne
+3  A: 

To put it simple :

Child is a subclass of Parent so that means that Child* can be substituted with Parent*

BUT

Child* is NOT a subclass of Parent* so that means that Child** can't be substituted with Parent**

"Child" and "Child*" are not the same types.

+2  A: 

If your problem is dealing with memory and resources, the best advice would be to forget your approach completely and use smart pointers. std::auto_ptr or boost::shared_ptr would be a start point.

If you hold all your heap allocated resources with smart pointers your code will be more robust.

David Rodríguez - dribeas
+2  A: 

What you need to do is nullify all the pointers to the object you just deleted. The idea of pointers is that there will be more than one pointer storing the address of the same object. If not, there is little reason to use a bare pointer, and so the pattern you're trying to capture is not very useful - but you are far from the first person to try this. As other answers have mentioned, the only way to deal with pointers is to carefully control access to them.

The title of your question is absolutely correct! There's a good reason for it. A pointer identifies a location that stores an object of a specific type. A pointer to a pointer gives you the ability to change what object a pointer points to.

void Foo(Parent **pp)
{
    *pp = new OtherChild();
}

Your Child class derives from Parent, and so does my OtherChild class. Suppose the compiler allowed you to do this:

Child *c = 0;
Foo(&c);

You expected that to work, but if it had, then we would now have a Child pointer c that in fact pointers to an instance of OtherChild. Who says those two types are compatible?

Again, this is a very frequent misunderstanding - it crops up repeatedly here for other languages, especially with regard to List<Parent> and List<Child> in C#.

Daniel Earwicker
Thx a lot, great explanation!!
To1ne
No problem! And remember - nothing says "thank you" like an up-vote.
Daniel Earwicker
A: 

Probably the most simple solution I have found:

#define __REMOVE_OBJ(pObj) RemoveObj(pObj); pObj = NULL;

And just call this one:

   __REMOVE_OBJ(pPObj);
   __REMOVE_OBJ(pCObj);

But I don't really like it myself...

To1ne
A: 

From discussion on make shared_ptr not use delete

Shared pointer will ensure you cleanup when you should and that you don't access something that is destroyed. Further you can specialise and provide an alternate destruction method.

boost::shared_ptr<T> ptr( new T, std::mem_fun_ref(&T::deleteMe) );
Greg Domjan