Post History

Your understanding is correct: if the deleter has a member type `pointer`, then `std::unique_ptr` uses that to define its own member type `pointer`, and from that, most other operations. So long as you don’t do anything too complicated, it should “work”. Since you only seem to be doing sets and gets… then, yeah, it shouldn’t be a problem.

But is it a good idea? I would say no.

Try this:

```c++
using unique_float_ptr = std::unique_ptr<float>;
using unique_magic_ptr = std::unique_ptr<magic_t, MagicDeleter>;

static_assert(
    not std::same_as<
        unique_float_ptr::element_type,
        unique_float_ptr::pointer
    >
);
static_assert(
    not std::same_as<
        unique_magic_ptr::element_type,
        unique_magic_ptr::pointer
    >
);
```

As you can see, you’ve kinda warped the abstraction. `element_type` and `pointer` should not be the same in a `unique_ptr`; that just breaks the model.

If `magic_t` is an opaque handle type, and you don’t want to crack it apart to its constituent types, you can still do this:

```c++
struct MagicDeleter
{
    using pointer = ::magic_t;

    void operator()(pointer cookie) noexcept
    {
        if (cookie)
            ::magic_close(cookie);
    }
};

using unique_magic_ptr = std::unique_ptr<std::remove_pointer_t<::magic_t>, MagicDeleter>;
```

If `magic_t` really is a pointer, then the unique pointer will be “normal”. If it is not, then it will still “work”, but will have the weird behaviour that `element_type` and `pointer` are the same… but that can’t be helped if `magic_t` really is opaque. In that case, it probably shouldn’t be in a `unique_ptr`… because it’s not a pointer.

---

If I were going to use something like libmagic in a C++ program, I would wrap the C interface in a proper C++ interface, like so:

```c++
namespace magic {

struct deleter
{
    auto operator()(::magic_t p) const noexcept
    {
        if (p)
            ::magic_close(p);
    }

    // Any other types in the library that need a release function...
    auto operator()(::magic_other_t p) const noexcept
    {
        // ...
    }
};

// Good for a start, if `::magic_t` really is a pointer. If not, then
// a custom wrapper should be used.
//
// A custom wrapper should probably be used in the long run in any case
// for even more safety and power.
//
// For example, in the long run, instead of:
//  ::magic_load(m_cookie.get(), nullptr)
// or even: 
//  load(m_cookie)
// we might prefer:
//  m_cookie.load()
using magic_t = std::unique_ptr<std::remove_pointer_t<::magic_t>, deleter>;

enum class open_flags : int
{
    none,
    mime_type,
    // ... etc. ...
};

auto open(int flags) -> magic_t; // legacy function
auto open(open_flags) -> magic_t;

// Not necessary, but helpful when porting legacy code:
auto close(magic_t& m)
{
    m.reset();
}

auto load(magic_t& m) -> void;
auto load(magic_t& m, std::filesystem::path const&) -> void;
// legacy functions:
auto load(magic_t& m, std::nullptr_t) -> int;
auto load(magic_t& m, char const*) -> int;

} // namespace magic
```

Then you could start with C-ish code like:

```c++
auto m = ::magic_open(MAGIC_MIME_TYPE);
if (not m)
    throw error{};

if (::magic_load(m, nullptr) == -1)
{
    ::magic_close(m);
    throw error{};
}

::magic_close(m);
```

… and gradually change it like so:

```c++
auto m = magic::open(MAGIC_MIME_TYPE);
if (not m)          // this is now unnecessary, because open throws
    throw error{};  // on failure, but it's harmless

if (magic::load(m, nullptr) == -1)  // the if, the check, and the
{                                   // error block are now unnecessary
    magic::close(m);                // but harmless for now
    throw error{};
}

magic::close(m); // unnecessary:
```

… and then eventually start removing the unnecessary code to get:

```c++
auto m = open(magic::open_flags::mime_type);

load(m);
```

… where everything is strongly-typed, and “smart”, and errors become impossible to miss or ignore.

By making the new, better C++ interface mostly track the original C interface, you can make adoption gradual. And one of the keys to doing that is to make sure that abstractions line up. In particular, the “new” `magic_t` should not be a (unique) *pointer* to a `magic_t`, it should *be* a `magic_t` (even if it happens to be a (unique) pointer to something (to whatever the original `magic_t` points to) as an implementation detail).