C++ lifespan of factory returned smart pointers

Question

I am using a factory pattern in C++11 which is in the scope of my main() function and is invoked like this:

histogram_requests -> AddNewPostfix( Postfix_factory :: get_postfix( "Layers", ntuple_reader ) );

The Postfix_factory::get_postfix() is a static member function returning an std :: unique_ptr< Postfix>. An example return value is:

return std :: move( std :: unique_ptr<Postfix>( new Layers_postfix( ntuple_reader_p ) ));

My first question is if this is actually valid to do? Is the returned pointer destroyed in the end of my main() scope?

The second thing is that I want to pass this pointer to a function. What is the correct way of doing it? My program compiles when I set the argument type of AddNewPostFix() to either an std::unique_ptr or an std::unique_ptr&& but fails as a simple reference. I do not want to let my AddNewPostFix() to accidently delete the data in my pointer, but I not sure that either of these is safe. Can anyone tell me how to do this correctly and safely?

Answer 1

return std :: move( std :: unique_ptr<Postfix>( new Layers_postfix( ntuple_reader_p ) ));

Two things are noteworthy here:

1. You don't need std::move in your return statement.

2. You should use std::make_unique. It's officially supported only since C++14, but either your compiler supports it anyway or you can easily create your own version of it.

Is the returned pointer destroyed in the end of my main() scope?

"The returned pointer" can refer to several things.

The std::unique_ptr object is destroyed, of course. All local objects are destroyed when their scope ends. The raw-pointer data member inside of the std::unique_ptr is also destroyed, but that's trivial.

I think what you really mean is the object which is pointed to and which is managed by the std::unique_ptr, i.e. the dynamically allocated Postfix. That object will also be destroyed, yes, because the std::unique_ptr is not moved to anywhere when your main ends.

If Postfix would not be destroyed, then you'd have a memory leak and std::unique_ptr would be pretty much useless, wouldn't it?

The second thing is that I want to pass this pointer to a function. What is the correct way of doing it?

This question is way too broad. Herb Sutter once wrote an entire article about passing smart pointers to functions. It's a very long article, with interesting user comments, too. You should definitely read it. Here are Sutter's guidelines:

---

Don’t pass a smart pointer as a function parameter unless you want to use or manipulate the smart pointer itself, such as to share or transfer ownership.

---

Prefer passing objects by value, *, or &, not by smart pointer.

---

Express a “sink” function using a by-value unique_ptr parameter.

---

Use a non-const unique_ptr& parameter only to modify the unique_ptr.

---

Don’t use a const unique_ptr& as a parameter; use widget* instead

---

Express that a function will store and share ownership of a heap object using a by-value shared_ptr parameter.

---

Use a non-const shared_ptr& parameter only to modify the shared_ptr. Use a const shared_ptr& as a parameter only if you’re not sure whether or not you’ll take a copy and share ownership; otherwise use widget* instead (or if not nullable, a widget&).

---

I do not want to let my AddNewPostFix() to accidently delete the data in my pointer, but I not sure that either of these is safe.

The safest thing would be not to pass a pointer but a Postfix& or a Postfix const&. Of course, people could still try delete &postfix or employ similar bad practices. But as the saying goes, C++ protects against Murphy, not against Machiavelli.

Here is a complete example:

#include <memory>
#include <string>
#include <iostream>

struct Postfix
{
    std::string layers;
    int ntuple_reader;

    Postfix(std::string const& layers, int ntuple_reader) :
        layers(layers),
        ntuple_reader(ntuple_reader)
    {
    }

    ~Postfix()
    {
        std::cout << "dtr\n";
    }
};

std::unique_ptr<Postfix> get_postfix(std::string const& layers, int ntuple_reader)
{
    return std::make_unique<Postfix>(layers, ntuple_reader);
}

void AddNewPostfix(Postfix const& postfix)
{
    std::cout << "AddNewPostfix\n";
    // do something with postfix.layers and postfix.ntuple_reader
}

int main()
{
    int ntuple_reader = 123;
    auto postfix_ptr = get_postfix("Layers", ntuple_reader);
    AddNewPostfix(*postfix_ptr);
}

As you can see in the output, the destructor of Postfix is correctly called at the end of main.

Answer 2

Returning a smart pointer is fine until you decide that it needs to be held in a different type of smart pointer. I'd just return a raw pointer and let the caller decide what it wants to do with it. In your case, std::move is unnecessary.

To pass it to a function, make the function parameter a const T& and use the dereference operator on the smart pointer.

void someFunction(const T& t)
{
}

std::unique_ptr<T> t = ...;
SomeFunction(*t);