Efficiently and elegantly returning emplaced unique_ptr

Question

I found out (thanks to a StackOverflow comment) a security flaw in my code:

std::vector<std::unique_ptr<Item>> items;

template<class... TS> Item& create(TS&&... mArgs)
{
    auto item(new Item(std::forward<TS>(mArgs)...);
    items.emplace_back(item); // Possible exception and memory leak
    return *item;
}

Basically, allocating the Item with a raw new may leak memory if the emplace_back throws.

The solution is never using a raw new, but using a std::unique_ptr right in the method body.

std::vector<std::unique_ptr<Item>> items;

template<class... TS> Item& create(TS&&... mArgs)
{
    auto item(std::make_unique<Item>(std::forward<TS>(mArgs)...);
    items.emplace_back(std::move(item));
    return *item; // `item` was moved, this is invalid!
}

As you can see, returning item is invalid, as I had to move item using std::move to emplace it into the items container.

I can't think of a solution that requires storing item's address in an additional variable. The original (buggy) solution is however very concise and easy to read.

Is there a more elegant way of returning an std::unique_ptr that was moved for emplacement in a container?

Answer 1

You may write:

template<class... TS>
Item& create(TS&&... mArgs)
{
    items.emplace_back(std::make_unique<Item>(std::forward<TS>(mArgs)...));
    return *items.back();
}

Answer 2

Caching the reference before the emplace is a more generally applicable option (e.g., for non-vector containers):

template<class... TS> Item& create(TS&&... mArgs)
{
    auto item = std::make_unique<Item>(std::forward<TS>(mArgs)...);
    auto& foo = *item;
    items.emplace_back(std::move(item));
    return foo; // This *is* valid.
}