CODEWITHSUNDEEP
c++pointersnew-operator
Mas Bagol
Mas Bagol

Reputation: 4617

How to delete new pointer that declared on function parameters?

My class have member function that take pointer of it's own type as it's argument.

When I do this:

Object* obj1 = new Object();
Object* obj2 = new Object();

obj1->add_child(obj2)

delete obj1;
delete obj2;
obj1 = NULL;
obj2 = NULL;

and run valgrind, the report says:

HEAP SUMMARY:
    in use at exit: 72,704 bytes in 1 blocks
  total heap usage: 6 allocs, 5 frees, 73,098 bytes allocated

LEAK SUMMARY:
   definitely lost: 0 bytes in 0 blocks
   indirectly lost: 0 bytes in 0 blocks
     possibly lost: 0 bytes in 0 blocks
   still reachable: 72,704 bytes in 1 blocks
        suppressed: 0 bytes in 0 blocks

I read an answer says that still reachable is fine, no leak. Then, when I try this:

Object* obj = new Object();

obj1->add_child(new Object());

delete obj;
obj = NULL;

valgrind's report says:

HEAP SUMMARY:
    in use at exit: 72,877 bytes in 3 blocks
  total heap usage: 6 allocs, 3 frees, 73,098 bytes allocated

LEAK SUMMARY:
   definitely lost: 144 bytes in 1 blocks
   indirectly lost: 29 bytes in 1 blocks
     possibly lost: 0 bytes in 0 blocks
   still reachable: 72,704 bytes in 1 blocks
        suppressed: 0 bytes in 0 blocks

It's obvious that I didn't delete the new Object() pointer that passed as an argument. So, how do I delete that pointer?

DETAILED UPDATE

definition of add_child(Object* obj):

void add_child(Object* obj) {

    container.add_child(obj);
}

container is a member of Object which is an instance of template class.

Container<Object> container;

The Container definition is:

template<class T>
class Container {
public:
    void add_child(const std::string& key, T* child) {
        childrens.insert(std::pair<std::string,T*>(key, child));
    }
private:
    std::multimap<std::string,T*> childrens;
}

Then, the childrens is actually a std::multimap.

I hope this not too long for just a question.

Upvotes: 1

Views: 343

Answers (3)

milleniumbug
milleniumbug

Reputation: 15814

The valgrind report about your still reachable memory block isn't your fault - it's the bug in gcc 5.1.

How did I come up with this:

The block size was very large (72704) and didn't match the size of allocated objects, so next I tried to divide 72704 by 144 and 29 to see if it's an array of these objects.

But it wasn't since neither 72704, nor 72704-4, nor 72704-8 (array, array + 32-bit integer to store the size, array + 64-bit integer to store the size) weren't divisible. So then I googled "72,704 still reachable", which showed the mentioned question.

The other answerers' suggestions to use smart pointers (like std::unique_ptr and std::shared_ptr) are sound, and I too recommend using them.

Upvotes: 1

Brian Bi
Brian Bi

Reputation: 119219

You need to make a clear and consistent choice about who owns pointers. If you decide that add_child takes ownership, then the caller should expect that they don't need to delete the pointer passed in. If you decide that add_child does not take ownership, then the caller retains ownership and deletes the pointer.

You can make Object take ownership, so its destructor deletes all the child pointers that have been added to it. Then your first example should read

Object* obj1 = new Object();
Object* obj2 = new Object();

obj1->add_child(obj2);

delete obj1; // also deletes obj2 automatically

and your second:

Object* obj = new Object();

obj->add_child(new Object());

delete obj; // automatically deletes the unnamed pointer passed in on previous line

If you don't want Object to take ownership, then you have a problem, since you have no way of deleting the new Object() in the context of the caller.

If you use smart pointers, managing ownership becomes much easier. add_child can take a std::unique_ptr<Object> argument.

auto obj1 = std::make_unique<Object>();
auto obj2 = std::make_unique<Object>();
obj1->add_child(std::move(obj2)); // obj1 takes ownership
// both pointers automatically deleted at scope exit

auto obj = std::make_unique<Object>();
obj->add_child(std::make_unique<Object>());
// both pointers automatically deleted at scope exit

Upvotes: 3

Mark Ransom
Mark Ransom

Reputation: 308206

Once the function call is finished, you no longer have the pointer that new created. If the function you called doesn't delete the pointer, then you have a memory leak.

Upvotes: 2

Related Questions