Should I initialize a shared_ptr inside or outside the class constructor?

Question

Given the following example code:

#include <memory>

class Foo {
public:
    Foo(std::shared_ptr<int> p);
private:
    std::shared_ptr<int> ptr;
};

Foo::Foo(std::shared_ptr<int> p) : ptr(std::move(p)) {
}

class Bar {
public:
    Bar(int &p);
private:
    std::shared_ptr<int> ptr;
};

Bar::Bar(int &p) : ptr(std::make_shared<int>(p)) {
}

int main() {
    Foo foo(std::make_shared<int>(int(256)));
    Bar bar(*new int(512));

    return 0;
}

Both Foo and Bar work correctly. However, are there any differences between creating the shared_ptr when calling the constructor and then transferring ownership with std::move and just passing a reference to the object and delegating the creation of the shared_ptr to the class constructor?

I assume the second way is better since I don't have to move the pointer. However, I've mostly seen the first way being used in the code I'm reading.

Which one should I use and why?

Answer 1

Foo is correct.

Bar is an abomination. It involves a memory leak, unsafe exception behaviour and an un-necessary copy.

EDIT: explanation of memory leak.

Deconstructing this line:

Bar bar(*new int(512));

results in these operations:

1. call new int(512) which results in a call to operator new, returning a pointer to an int on the heap (memory allocation).
2. dereference the pointer in order to provide a const reference for the constructor of Bar
3. Bar then constructs its shared_ptr with one returned by make_shared (this part is efficient). This shared_ptr's int is initialised with a copy of the int passed by reference.
4. then the function returns, but since no variable has recorded the pointer returned from new, nothing can free the memory. Every new must be mirrored by a delete in order to destroy the object and deallocate its memory.
5. therefore memory leak

Answer 2

It depends on what you want to achieve.

If you need a shared_ptr internally, because you want to share the object with some other objects your create later, the second way may be better (except for that horrible constructor call obviously).

If you want shared ownership of an existing object (this is the more common case, really), you don't have a choice and you need to use the first way.

If neither of these applies, you probably don't need a shared_ptr in the first place.