C++ use Smart Pointers with changing pointer values

Question

Consider a C library that defines functions for creating, destroying and working with a custom structure

struct Foo;
void foo_action(Foo*);
Foo* foo_create();
void foo_free(Foo*);

Currently, I used the library in my C++ project as follows

Foo* myfoo = foo_create();
foo_action(myfoo);
foo_free(myfoo);

I understand why smart pointers are important and want to migrate my code to use them. That's how the code looks now.

#include <memory>
#include <functional>
typedef std::unique_ptr<Foo, std::function<void(Foo*)>> FooPtr;
// ...
FooPtr myfoo2(foo_create(), foo_free);
foo_action(myfoo2.get());

It seems to work, but the myfoo2.get() invocation seems hacky. Am I using it as intended?

There's another part of the library which creates and works with some kind of list structure. The api looks like

struct Bar;
Bar* bar_append(Bar*, int);
void bar_free_recursive(Bar*);

and is used as

// using NULL as current Bar* creates the initial structure
Bar* bar = bar_append(NULL, 1);
// each invocation leads to another 'head' structure
bar = bar_append(bar, 42);
bar = bar_append(bar, 123);

As the pointer (the address pointed to) changes with each bar_append invocation, how would I introduce smart pointers here, so that bar_free_recursive is invoked on the current pointer value when the pointer instance is freed?

Answer 1

Having to write .get() is an unfortunate consequence of using smart pointers but I think best practice if you want to pass to a function that accepts a non-owning, nullable pointer.

But, in practice I often find you don't need it to be nullable and can accept a reference instead of a raw-pointer. Then the syntax is a bit less "hacky":

void foo_action(Foo&);  // accept a reference instead of a raw-pointer

struct FooDeleter {
    void operator()(Foo* foo) const { foo_free(foo); }
};

using FooPtr = std::unique_ptr<Foo, FooDeleter>;

FooPtr make_foo() {
  return FooPtr(foo_create());
}

int main() {
    auto foo = make_foo();

    // ...  

    if (foo) {             // check for null
        foo_action(*foo);  // dereference smart-pointer
    } 
}

bar_append should work with a unique_ptr providing you use std::move:

struct BarDeleter {
    void operator()(Bar* bar) const { bar_free_recursive(bar); }
};

using BarPtr = std::unique_ptr<Bar, BarDeleter>;

BarPtr bar_append(BarPtr bar, int value) {
    return BarPtr(bar_append(bar.release(), value));
}

int main() {      
  BarPtr bar;
  bar = bar_append(std::move(bar), 42);
  bar = bar_append(std::move(bar), 123);
}

Answer 2

I would say myfoo2.get() is clunky, not hacky.

I would personally create a template based wrapper obj_ptr (you choose a more relevant name) and use traits for each type of object to model your requirement the C++ way. The wrapper can then remove the clunkyness of accessing the underlying object.

template <typename T, typename Traits>
class obj_ptr final
{
    std::unique_ptr<Foo, void(*)(T*)> ptr_{ Traits::create(), Traits::free };

public:
    operator T*() { return ptr_.get(); }

    operator const T*() const { return ptr_.get(); }

    T* operator->() { return ptr_.get(); }

    const T* operator->() const { return ptr_.get(); }
};

class foo_traits
{
public:
    static Foo* create() { return foo_create(); }

    static void free(Foo* foo) { foo_free(foo); }
};

int main()
{
    using FooPtr2 = obj_ptr<Foo, foo_traits>;

    FooPtr2 myfoo2;

    foo_action(myfoo2);

    return EXIT_SUCCESS;
}

Answer 3

but the myfoo2.get() invocation seems hacky. Am I using it as intended?

It is not hacky, you use it as intended.

I would go one step further and wrap the whole in a class:

struct Foo;
void foo_action(Foo*);
Foo* foo_create();
void foo_free(Foo*);

class FooWrapper
{
public:
    FooWrapper() : mFoo(foo_create()) {}

    void action() { foo_action(mFoo.get()); }
private:
    struct FooDeleter
    {
        void operator()(Foo* foo) const { foo_free(foo); }
    };

    std::unique_ptr<Foo, FooDeleter> mFoo;
};

In the same way:

struct Bar;
Bar* bar_append(Bar*, int);
void bar_free_recursive(Bar*);

class BarWrapper
{
public:
    explicit BarWrapper(int n) : mBar(bar_append(nullptr, n)) {}

    void append(int n) { mBar.reset(bar_append(mBar.release(), n)); }

private:
    struct BarDeleter
    {
        void operator()(Bar* bar) const { bar_free_recursive(bar); }
    };

    std::unique_ptr<Bar, BarDeleter> mBar;
};