Should I delete pointer from `new` passed to a function which makes into a `shared_ptr`?

Question

In the following code example:

#include <iostream>

class Foo{
};

class Bar{
public:
    void addFoo(Foo *foo){
        auto my_foo = std::shared_ptr<Foo>(foo);
    }
};

int main() {
    auto bar = Bar();
    bar.addFoo(new Foo());
    return 0;
}

Do I need to clean up the pointer created in main() by the bar.addFoo(new Foo) call, or will this be taken care of by Bar which creates a shared_ptr of it? My understanding is that auto my_foo = std::shared_ptr<Foo>(foo); will use the copy constructer to copy this pointer into my_foo leaving the original one dangling, is that correct?

Answer 1

The very idea of a constructor taking a raw pointer is to pass the ownership to std::shared_ptr. So, no, you don't have to delete a raw pointer passed to std::shared_ptr. Doing this will lead to a double deletions, which is UB.

Note that in general passing a raw pointer is dangerous. Consider the following more generalized example:

void addFoo(Foo *foo){
        // some code which could throw an exception
        auto my_foo = std::shared_ptr<Foo>(foo);
    }

If an exception is throw before my_foo is constructed, foo will leak.

If you have no special reason to pass a raw pointer, consider the following alternative:

class Bar {
public:
    template<class... Args>
    void addFoo(Args... args){
        auto my_foo = std::make_shared<Foo>(args...);
    }  
};

int main() {
    auto bar = Bar();
    bar.addFoo();
    return 0;
}

Here you pass arguments (if you have any) to construct Foo inside addFoo() instead of constructing Foo before invoking addFoo().

Perfect forwarding of args... could be used if it is needed:

    template<class... Args>
    void addFoo(Args&&... args){
        auto my_foo = std::make_shared<Foo>(std::forward<Args>(args)...);
    }