13

In theory, I should be able to use a custom pointer type and deleter in order to have unique_ptr manage an object that is not a pointer. I tried the following code:

#ifndef UNIQUE_FD_H
#define UNIQUE_FD_H

#include <memory>
#include <unistd.h>

struct unique_fd_deleter {
    typedef int pointer; // Internal type is a pointer

    void operator()( int fd )
    {
        close(fd);
    }
};

typedef std::unique_ptr<int, unique_fd_deleter> unique_fd;

#endif // UNIQUE_FD_H

This doesn't work (gcc 4.7 with the -std=c++11 parameter). It responds with the following errors:

In file included from /usr/include/c++/4.7/memory:86:0,
                 from test.cc:6:
/usr/include/c++/4.7/bits/unique_ptr.h: In instantiation of 'std::unique_ptr<_Tp, _Dp>::~unique_ptr() [with _Tp = int; _Dp = unique_fd_deleter]':
test.cc:22:55:   required from here
/usr/include/c++/4.7/bits/unique_ptr.h:172:2: error: invalid operands of types 'int' and 'std::nullptr_t' to binary 'operator!='

From delving into the definition of unique_ptr, I can see two problems that prevent it from working. The first, which seems in clear violation of the standard, is that the destructor for unique_ptr compares the "pointer" (which is, as per my definition, an int) to nullptr in order to see whether it is initialized or not. This is in contrast to the way it reports it through the boolean conversion, which is to compare it to "pointer()" (an uninitialized "pointer"). This is the cause of the errors I am seeing - an integer is not comparable to a nullptr.

The second problem is that I need some way to tell unique_ptr what an uninitialized value is. I want the following snippet to work:

unique_fd fd( open(something...) );

if( !fd )
    throw errno_exception("Open failed");

For that to work, unique_ptr needs to know that an "uninitialized value" is -1, as zero is a valid file descriptor.

Is this a bug in gcc, or am I trying to do something here that simply cannot be done?

curiousguy
  • 8,038
  • 2
  • 40
  • 58
Shachar Shemesh
  • 8,193
  • 6
  • 25
  • 57
  • I will add the obvious. Instead of "int", I can set the pointer type to some class I invent. This will allow me to do everything I want to. This will not, however, be a trivial class, as it requires back and forth implicit casts to int and other things I would rather avoid. – Shachar Shemesh Apr 02 '13 at 05:45
  • 5
    I suggest you stop using `std::unique_ptr` for non-pointer storage. It expects the data to actually _be_ a pointer, while you want it to be a _non-pointer_. – Some programmer dude Apr 02 '13 at 05:51
  • Wrap the file API's inside another class (RAII) that opens the file and stores the file descriptor. The class should close the descriptor when destructor is called. Then use use unique pointer of such class. – Abhijit-K Apr 02 '13 at 06:12
  • 1
    @JoachimPileborg: Then, there should be a class `std::handle` similar to `std::unique_ptr` but without `operator*`, and two extra template parameters: `class handle_type=void*`, and `handle_type null_value=nullptr`. – user877329 Jun 19 '16 at 12:26

9 Answers9

14

The type exposed by the Deleter::pointer must satisfy the NullablePointer requirements. Chief among them, this expression must be legal: Deleter::pointer p = nullptr;. Of course, nullptr is pretty much defined by the fact that it cannot be implicitly converted to a number, thus this doesn't work.

You'll have to use a type which can be implicitly constructed with std::nullptr_t. Something like this:

struct file_desc
{
  file_desc(int fd) : _desc(fd) {}
  file_desc(std::nullptr_t) : _desc(-1) {}

  operator int() {return _desc;}

  bool operator ==(const file_desc &other) const {return _desc == other._desc;}
  bool operator !=(const file_desc &other) const {return _desc != other._desc;}
  bool operator ==(std::nullptr_t) const {return _desc == -1;}
  bool operator !=(std::nullptr_t) const {return _desc != -1;}

  int _desc;
};

You can use that as the Deleter::pointer type.

Deduplicator
  • 44,692
  • 7
  • 66
  • 118
Nicol Bolas
  • 449,505
  • 63
  • 781
  • 982
10

Can you do something simple like the following?

class unique_fd {
public:
    unique_fd(int fd) : fd_(fd) {}
    unique_fd(unique_fd&& uf) { fd_ = uf.fd_; uf.fd_ = -1; }
    ~unique_fd() { if (fd_ != -1) close(fd_); }

    explicit operator bool() const { return fd_ != -1; }

private:
    int fd_;

    unique_fd(const unique_fd&) = delete;
    unique_fd& operator=(const unique_fd&) = delete;
};

I do not see why you had to use unique_ptr, which is designed to manage pointers.

Yongwei Wu
  • 5,292
  • 37
  • 49
4

Found an answer at cppreference.com. Look in the examples code:

    void close_file(std::FILE* fp) { std::fclose(fp); }
    ...
    {
      std::unique_ptr<std::FILE, decltype(&close_file)> fp(std::fopen("demo.txt",                                                             
                                                                      "r"), 
                                                           &close_file);
      if(fp)     // fopen could have failed; in which case fp holds a null pointer
         std::cout << (char)std::fgetc(fp.get()) << '\n';
    }// fclose() called here, but only if FILE* is not a null pointer
     // (that is, if fopen succeeded)

Tried it in vs2019 and it works! Also tried it with member and lambda:

FileTest.h:

class A
{
  std::unique_ptr<std::FILE, std::function<void(std::FILE*)>> fp;
}

FileTest.cpp

void A::OpenFile(const char* fname)
{
    fp = std::unique_ptr < std::FILE, std::function<void(std::FILE*)>>(
        std::fopen(fname, "wb"),
        [](std::FILE * fp) { std::fclose(fp); });
}
harel wallach
  • 57
  • 1
  • 3
3

A complete sample:

#ifdef _MSC_VER 
#define _CRT_NONSTDC_NO_WARNINGS
#define _CRT_SECURE_NO_WARNINGS
#include <io.h>
#else
#include <unistd.h>
#endif

#include <memory>
#include <fcntl.h>

template<auto nullvalue, auto delete_>
class unique
{
    using T = decltype(nullvalue);

    struct generic_delete
    {
        class pointer
        {
            T t;
        public:
            pointer(T t) : t(t) {}
            pointer(std::nullptr_t = nullptr) : t(nullvalue) { }
            explicit operator bool() { return t != nullvalue; }
            friend bool operator ==(pointer lhs, pointer rhs) { return lhs.t == rhs.t; }
            friend bool operator !=(pointer lhs, pointer rhs) { return lhs.t != rhs.t; }
            operator T() { return t; }
        };

        void operator()(T p)
        {
            delete_(p);
        }
    };
public:
    using type = std::unique_ptr<struct not_used, generic_delete>;
};

int main()
{

    using unique_fd = unique<-1, close>::type;
    static_assert(sizeof(unique_fd) == sizeof(int), "bloated unique_fd");
    unique_fd fd1(open("fd.txt", O_WRONLY | O_CREAT | O_TRUNC));
    write(fd1.get(), "hello\n", 6);
}
Bruno Martinez
  • 2,850
  • 2
  • 39
  • 47
  • 1
    Why/how is this working? I though `std::unique_ptr<>()` would transform it's first template parameter in a pointer (so `T * t` and not just `T t`) and so internally it would store `int *` which would require an extra allocation... – Alexis Wilke Jun 07 '18 at 16:55
  • 1
    The deleter::pointer type, if it exists, overrides the template parameter and is stored internally by unique_ptr. The result type of deleter::pointer::operator* still must match the template parameter. – Bruno Martinez Jun 07 '18 at 18:34
  • Okay, I tried that and it failed badly when I used the `*` operator because it's actually incorrect to apply such and your example is bogus. See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86296 -- That operator can be replaced with `operator T () const` which returns `t` as is. To get the value you do `ptr.get()`. The `*ptr` syntax will fail compiling, which is normal. As mentioned in the bug report above, that's because you're returning a reference to a temporary. The deleter needs to be fixed too, of course, since it uses `*p` instead of `static_cast(p)`. – Alexis Wilke Jun 25 '18 at 21:30
  • @AlexisWilke ouch. I think supporting operator* was a mistake. Not only you accessed dangling references; you could do *fd = 42; at any time and leak. See revised version. – Bruno Martinez Jun 26 '18 at 02:51
  • Yes. Actually there is a copy of the pointer class so the assignment was not so good but it would not really leak anything. At least the C++ library implementation that I use makes a copy of the pointer class so you were only changing the copy (although, since it was a temporary copy...) and the pointer class does not call the deleter. I have more details here: https://stackoverflow.com/questions/51015016/#51033600 – Alexis Wilke Jun 26 '18 at 04:38
2

The open source Android Framework defines a unique_fd class that might meet your needs: https://android.googlesource.com/platform/system/core/+/c0e6e40/base/include/android-base/unique_fd.h

Paul Crowley
  • 1,656
  • 1
  • 14
  • 26
0

This solution is based on Nicol Bolas answer:

struct FdDeleter
{
    typedef int pointer;
    void operator()(int fd)
    {
        ::close(fd);
    }
};
typedef std::unique_ptr<int, FdDeleter> UniqueFd;

It's short, but you have to avoid to compare UniqueFd instance with nullptr and use it as boolean expression:

UniqueFd fd(-1, FdDeleter()); //correct
//UniqueFd fd(nullptr, FdDeleter()); //compiler error
if (fd.get() != -1) //correct
{
    std::cout << "Ok: it is not printed" << std::endl;
}
if (fd) //incorrect, avoid
{
    std::cout << "Problem: it is printed" << std::endl;
}
if (fd != nullptr) //incorrect, avoid
{
    std::cout << "Problem: it is printed" << std::endl;
}
return 1;
Alexis Wilke
  • 19,179
  • 10
  • 84
  • 156
mooncheese
  • 124
  • 1
  • 1
  • 4
0

Making Nicol Bolas class more general:

template<class T=void*,T null_val=nullptr>
class Handle
    {
    public:
        Handle(T handle):m_handle(handle){}

        Handle(std::nullptr_t):m_handle(null_val){}

        operator T(){return m_handle;}

        bool operator==(const Handle& other) const
            {return other.m_handle==m_handle;}

    private:
        T m_handle;
    };

typedef Handle<int,-1> FileDescriptor;
typedef Handle<GLuint,0> GlResource; // according to http://stackoverflow.com/questions/7322147/what-is-the-range-of-opengl-texture-id
// ...

I am not sure if I should have default template parameter values or not.

user877329
  • 6,717
  • 8
  • 46
  • 88
0

I would suggest using shared_ptr rather than unique_ptr to manage the life time of int handles because the shared ownership semantics are usually a better fit, and because the type of the deleter is erased. You need the following helper:

namespace handle_detail
{
    template <class H, class D>
    struct deleter
    {
        deleter( H h, D d ): h_(h), d_(d) { }
        void operator()( H * h ) { (void) d_(h_); }
        H h_;
        D d_;
    };
}

template <class H,class D>
std::shared_ptr<H const>
make_handle( H h, D d )
{
    std::shared_ptr<H> p((H *)0,handle_detail::deleter<H,D>(h,d));
    return std::shared_ptr<H const>(
        p,
        &std::get_deleter<handle_detail::deleter<H,D> >(p)->h_ );
}

To use with a file descriptor:

int fh = open("readme.txt", O_RDONLY); // Check for errors though.
std::shared_ptr<int const> f = make_handle(fh, &close);
Emil
  • 363
  • 3
  • 13
  • Seems overly cumbersome, a better way would be to let `std::shared_ptr` simply allocate and `int` and then `close()` it in a custom deleter. Then you would also be able to access the descriptor by `*ptr` or `ptr.get()` as usual. – Andreas Magnusson Aug 30 '19 at 08:02
  • 1
    The `make_handle` approach also lets you access the descriptor by `*ptr` or `ptr.get()`. The handle, e.g. the `int` value, is stored in the custom deleter object, which is needed no matter what in order to call e.g. `close()`. – Emil Sep 12 '19 at 00:19
  • Ah, I see, `std::shared_ptr` _aliasing ctor_... seems esoteric enough :) Thanks, I guess I learned something new today. – Andreas Magnusson Sep 12 '19 at 07:06
0

Don't coerce a (smart) pointer to be a non-pointer object.

In theory, I should be able to use a custom pointer type and deleter in order to have unique_ptr manage an object that is not a pointer.

No, you should not. That is, in terms of getting it to compile and run, maybe, but you simply shouldn't use a unique_ptr to manage something which is not a pointer. You absolutely should write an appropriate RAII class for your resource - e.g. an OS file descriptor - or use an existing such class from some library. Only if you want a pointer to such a resource does a unique_ptr make sense; but then, you don't need a custom deleter.

einpoklum
  • 118,144
  • 57
  • 340
  • 684