Use unique_ptr for ownership and raw pointer otherwise?

Question

I am C++11-ing some code. I have

class X { /* */ };

class A {
    std::vector<X*> va_x;
};

class B {
    std::vector<X*> vb_x;
    std::vector<A> vb_a;
};

The X*s of "va_x" inside my class A point to objects that are also pointed to by the X*s of "vb_x" inside my class B.

Now I would like to use smart pointers. For me, it seems clear that class B has the ownership of the objects pointed by the X* (in particular because my A instances belong to B)

So I should use a unique_ptr for X inside B:

class B {
    std::vector<unique_ptr<X>> vb_x;
    std::vector<A> vb_a;
};

My question is, what should I do for class A? Should I keep raw pointers? By doing so, in my unit tests, I must admit that it leads to awkward things (imo), for instance (don't worry about encapsulation, that's not the point):

unique_ptr<X> x(new X());
A a;
a.va_x.push_back(&(*x)); //awkward, but what else can I do?

A.vb_a.push_back(a); //ok
B.vb_x.push_back(move(x)); //ok

Answer 1

You can use x.get(), which will return the internal pointer.

Other than that, yes, using raw pointers to handle non-owning references is the way to go, see also this question.

Answer 2

As Xeo says in his answer, the solution is generally to use x.get().

There is an example of a FILE which uses x.get() whenever it needs to be accessed:

void file_deleter(FILE *f)
{
    fclose(f);
}

[...]

{
    std::unique_ptr<FILE, decltype(&file_deleter)>
                       f(fopen("/tmp/test.tmp", "rw"), &file_deleter);

    // read/write use f.get() as in:
    fread(buf, 1, sizeof(buf), f.get());
    fwrite(buf, 1, sizeof(buf), f.get());

    // fclose() gets called in the deleter, no need to do anything
}

However, in your case, you need to use x.release().

A a;
{
    unique_ptr<X> x(new X());
    a.va_x.push_back(&(*x)); // this is wrong
}
// here a[0] is a dangling pointer

The &(*x) will get the raw pointer and make a copy in the a vector. However, at the time the unique_ptr<> goes out of scope, that pointer gets deleted. So after the }, the pointer in the a vector is not good anymore (although it might work for a little while.)

The correct way of transferring the bare pointer is to use the release() function as in:

    a.va_x.push_back(x.release()); // this works

After that one line, the pointer is only in the a vector. It was released from the unique pointer with the idea that the caller now becomes responsible for managing that resource.

IMPORTANT NOTE: The push_back() may throw (bad_alloc) and if that happens, the resource is lost. To avoid that problem (in case your software catches the bad_alloc and continues to run) you need to first reserve space in the vector as in:

    a.va_x.reserve(a.va_x.size() + 1);  // malloc() happens here
    a.va_x.push_back(x.release());      // no `bad_alloc` possible here

That way, the bad_alloc would happen on that statement while the resource is still attached to the unique_ptr and you do not leak it in case of an exception.

All of that said, you were probably in need of a shared_ptr instead. Those can be copied without trouble. A unique_ptr is more for a resource being allocated once and then forgotten one a function returns or an object is deleted. When (many) copies are involved, a shared_ptr makes more sense.

class X
{
    typedef std::shared_ptr<X> pointer_t;
    [...]
}

class A
{
    std::vector<X::pointer_t> va_x;
}

X::pointer_t x(new X());
A a;
a.va_x.push_back(x); // much cleaner and the pointer is still managed