How to delete new pointer that declared on function parameters?

Question

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.

it's impossible to tell without knowing the definition of the `Object` class. You should really be using smart pointers (e. g. `unique_ptr` or `shared_ptr`) instead, though. — The Paramagnetic Croissant, May 28 '15 at 17:48
Notice that it says `72,704 bytes in 1 blocks`, which doesn't match the size of the lost pointers in the second program. Do you have an idea what could be a such a large object in your program? (since it says "1 block", it's probably an array, given such a large size) — milleniumbug, May 28 '15 at 18:00
What version of gcc do you use? [If it's 5.1, then you may suffer from this bug](http://stackoverflow.com/questions/30393229/new-libstdc-of-gcc5-1-may-allocate-large-heap-memory) — milleniumbug, May 28 '15 at 18:09
@milleniumbug those bytes is refer to `malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)` which is called by `(in /usr/lib/ld-2.21.so)` in the end of it's calling chain. — Mas Bagol, May 28 '15 at 18:09
@milleniumbug I see .. My gcc is version 5.1. It's a bug then. I'm glad to know that, even not related to my main question ;) — Mas Bagol, May 28 '15 at 18:14

score 3 · Accepted Answer · answered May 28 '15 at 17:56

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

score 2 · Answer 2 · answered May 28 '15 at 17:49

2

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.

answered May 28 '15 at 17:49

Mark Ransom

299,747
42
398
622

score 1 · Answer 3 · edited May 23 '17 at 12:00

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.

How to delete new pointer that declared on function parameters?

3 Answers3

Linked