Correctly applying the "rule of five" to a RAII socket wrapper I was trying to create a simple RAII wrapper with rule of 5 for a TCP POSIX socket. My aim was to try learn how to apply rule of five in different situations, but this one was somehow tricky.

class tcp_socket {
public:
    tcp_socket() {
        _s = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
    }

    tcp_socket(const tcp_socket&amp; other) = delete;
    tcp_socket&amp; operator=(const tcp_socket&amp; other) = delete;

    tcp_socket(tcp_socket&amp;&amp; other) noexcept {
        _s = other._s;
        other._s = -1;
    }

    tcp_socket&amp; operator=(tcp_socket&amp;&amp; other) noexcept {
        if (this != &amp;other) {        
            _s = other._s;
            other._s = -1;
        }
        return (*this);
    }

    ~tcp_socket() {
        close(_s);
    }

private:
    int _s = -1;
};


In this class, I like to create socket in constructor and close it in destructor, a logical RAII approach. But I noticed closing socket in destructor creates a lot of side effects for writing other 4 routines of rule of five.

Move constructor/assignment:

As you see, I use other._s = -1; in move constructor/assignment. Because if I don't, when the moved object deletes, it will also close my socket in moved-to object. I think this will solve my problem, because when moved object will be destroyed, it will try to close -1 file descriptor which is an error, but will not ruin anything.

Copy constructor/assignment:

As you see, the only method that I could think of for copy constructor/assignment is deleting them. Because at first I thought to use dup() to duplicate file descriptor for copying, but if I do so, operations on one one instance (for example shutdown()) will affect another instance.

What do you think about this class? Do you think if this class does provide a simple RAII with correct rule of five or I need to adjust it somehow? Is it possible to write Copy constructor/assignment somehow or it is not logical to provide them?
These are good questions!

1. You should indeed have the copy constructor &amp; assignment operator deleted.

The "rule of five" tells you to specifically define a copy c'tor and assignment operator - but it doesn't tell you that you have to make the available. It is a perfectly valid choice to decide to not allow your object to be copied or non-move-assigned - only moved. An example of this: std::unique_ptr.

It's also what I would recommend in your case, because:


Like you said, duplicating a file descriptor is weird and unexpected.
It's not obvious to the user of your class what the copy behavior should be.
There don't seem to be - AFAICT - common scenarios in which you would copy, rather than move or pass by reference, a TCP socket.


2. Consider using std::optional to indicate "no valid value" or "missing" or "none"

In C (and the C system call bindings on Unix-like systems), it is a convention to use the -1 value for an invalid/missing file descriptor is a convention. We know that the int type is actually larger than the actual space of possible file descriptor values, so we use a junk value, which we assume the OS never uses, to indicate "no valid value". Now, this works fine; and you could choose to, say, define a static class constant:

static constexpr const int no_file_descriptor { -1 };


and then write:

close(_s);
_s = other._s;
other._s = no_file_descriptor;


As @HolyBlackCat suggests, you must get _s to be closed somehow. You could also just swap the two descriptors, but I find that to contradict the element of least surprise.

but you might want to consider the more general (though less space-efficient) solution, which is the std::optional&lt;T&gt; type template. It is intended for exactly your case: Either holding some value of type T (in your case, int), or holding some indication of "no value". Using an optional, you could write:

close(_s.get());
_s = other._s;
other._s = std::nullopt;


You'll still need to write your move assignment and move construction code, unfortunately (thanks @CassioRenan for noticing these are both necessary).

Other suggestions


Don't use a plain int; either find a type definition of a file descriptor from some library you're using, or if you have no definition to borrow, have using file_descriptor = int; or using file_descriptor_index = int.
_s is a bad member name. Use something more explicit, e.g. descriptor_index_ or posix_descriptor_index_.
Always check the return value of library/system calls! And handle errors.


The modified code

class tcp_socket {
protected:
    void close_if_neccessary() {
       constexpr auto socket_close_failed { -1 };
        if (descriptor_index_.has_value()) { 
            auto retval = close(descriptor_index_.value()); 
            if (retval == socket_close_failed) { 
               // throw something here, e.g.:
               // throw std::system_error(errno, std::system_category(), "close()");
            }
    };
public:
    using file_descriptor_index = int;

    tcp_socket() : descriptor_index_() { 
       constexpr auto socket_creation_failed { -1 };
       auto retval = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
       if (retval == socket_creation_failed) { 
           // throw std::system_error(errno, std::system_category(), "socket()");
       }
       descriptor_index = retval;
    };

    tcp_socket(const tcp_socket&amp; other) = delete;
    tcp_socket&amp; operator=(const tcp_socket&amp; other) = delete;
    tcp_socket&amp; operator=(tcp_socket&amp;&amp; other) noexcept {
        if (other.descriptor_index_ != descriptor_index_) {
            close_if_neccessary();
            descriptor_index_ = other.descriptor_index_;
            other.descriptor_index_ = std::nullopt;
        }
        return this;
    };
    tcp_socket(tcp_socket&amp;&amp; other) noexcept :
        descriptor_index_(other.descriptor_index_) 
    {
        other.descriptor_index = std::nullopt;
    }
    ~tcp_socket() { 
        // You might want to wrap this in a try-catch, since
        // destructors shouldn't throw. Otherwise you're risking
        // a double-exception and immediate program termination.
        close_if_necessary(); 
    }

protected:
    std::optional&lt;file_descriptor_index&gt; descriptor_index_;
};


See this StackOverflow question about the weird exception code in the comments: