An owned file descriptor

Question

File descriptors are a common resource in Unix and Unix-like operating systems. They're a way to represent a resource such as a file, socket and so on.
The user should release a file descriptor as soon as (s)he's done with the corresponding resource: tipically they're limited to 1024 per process.

I implemented a RAII wrapper with some inspiration from the std::unique_ptr interface.

Usage example:

#include "FileDescriptor.h"

#include <iostream>

#include <unistd.h>
#include <fcntl.h>

int main()
{
    FileDescriptor ownedFd = open("/dev/null", O_RDWR) ;

    int fd = ownedFd.release();

    std::cout << std::boolalpha << ownedFd.isValid() << "\n";

    ownedFd = FileDescriptor(fd);

    std::cout << std::boolalpha << ownedFd.isValid() << "\n";

}

And the code:

FileDescriptor.h

#ifndef FILEDESCRIPTOR_H
#define FILEDESCRIPTOR_H

#include <vector>

#include <cstdint>
#include <sys/select.h>

class FileDescriptor
{

public:
        FileDescriptor() = default;
        /*explicit*/ FileDescriptor ( int );

        FileDescriptor ( FileDescriptor&& ) noexcept ;
        FileDescriptor& operator=(FileDescriptor &&) noexcept ;

        virtual ~FileDescriptor();

    // Copy Assignment and Copy Constructor are implicitly deleted because of user-defined move semantics.

        void close();
        int release();
        int getFd();

        bool isValid() {
                return M_fd != invalid_fd;
        }


        static constexpr int invalid_fd = -1;

        enum Monitor : std::uint8_t {
                Read   = 1, // Read activity
                Write  = 2, // Write activity
                Except = 4  // Exceptional activity (such as incoming OOB data)
        };



        using fd_list = std::vector<FileDescriptor*>;   // pointers are needed for polymorphism: containers store objects,
                            // references are not.  §23.2.1 Standard C++

    // NOTE: Static member function or free function?
        static void select ( fd_list &, fd_list &, fd_list & ); 
        static void unique_select ( fd_list &, FileDescriptor::Monitor );

protected:
        int M_fd = invalid_fd; 

private:

        bool M_isValid ( int ); // check if a fd is actually valid, hence at kernel level.
        void M_tryClose();
        void M_do_close();

        static int M_create_set ( const fd_list&, fd_set & ); // Utility: populates the fd_set with the fds in the fd_list 
                                                              // and returns the highest element.

        static void M_remove_unset ( fd_list&, const fd_set & );

};

#endif // FILEDESCRIPTOR_H

FileDescriptor.cpp

#include "FileDescriptor.h"

#include <unistd.h>
#include <fcntl.h>

#include <stdexcept>
using std::invalid_argument;

#include <system_error>
using std::system_error;
using std::system_category;

#include <algorithm>
using std::max;
using std::max_element;
using std::remove_if;

#include <iostream>
using std::cerr;
using std::endl;

#include <cerrno>

using std::vector;

FileDescriptor::FileDescriptor ( int fd ) // fd may either be an invalid_fd or an _actual_ fd
{

    if ( fd != this->invalid_fd  ) {
        if ( M_isValid(fd) )
            this->M_fd = fd;
        else
            throw invalid_argument ( "Invalid file descriptor" );
    }


}

FileDescriptor::FileDescriptor ( FileDescriptor&& fd ) noexcept : M_fd ( fd.M_fd )
{
        fd.M_fd = invalid_fd;
}

FileDescriptor& FileDescriptor::operator= ( FileDescriptor&& rhs ) noexcept 
{
        M_tryClose();

        this->M_fd = rhs.M_fd;
        rhs.M_fd = FileDescriptor::invalid_fd;


   return *this;
}


FileDescriptor::~FileDescriptor()
{
        M_tryClose();
}

void FileDescriptor::M_do_close()
{
       if ( ::close ( M_fd ) != 0 ) {

                M_fd = FileDescriptor::invalid_fd;
                throw system_error ( errno, system_category(), "Attempted to close an invalid file descriptor" );

        }


        M_fd = invalid_fd; // fd closed successfully; mark it as no longer valid
}

void FileDescriptor::close()
{
    M_tryClose();

}

void FileDescriptor::M_tryClose()
{

        if ( this->isValid() ) {

                try {
                        this->M_do_close();
                } catch ( system_error e ) {

                        // this should not happen: we thought the file descriptor as valid but close failed! 

                        cerr << "For some weird, unpredicted reason the file descriptor has turned out to be invalid."
                                "This should not happen. Please issue a bug report. \n"
                             << "Error encountered: " << e.what() << endl;


                }

        }



}


int FileDescriptor::release()
{
        int tmp_fd = M_fd;
        M_fd = invalid_fd;

        return tmp_fd;
}

int FileDescriptor::getFd()
{
        return M_fd;
}


bool FileDescriptor::M_isValid ( int fd )
{
        return fcntl ( fd, F_GETFL ) != -1; // is the fd _actually_ valid? Only the kernel can provide this information.
}


int FileDescriptor::M_create_set ( const fd_list& v, fd_set& empty_set )
{
        int max_fd = invalid_fd;

        for ( const auto&i : v ) {
                FD_SET ( i->M_fd, &empty_set );
                max_fd = max ( max_fd, i->M_fd );
        }

        return max_fd;
}

void FileDescriptor::M_remove_unset ( fd_list& v, const fd_set& set_fds )
{
        v.erase ( remove_if( v.begin(), v.end(), [&] ( FileDescriptor *fd ) { return !FD_ISSET ( fd->M_fd, &set_fds ); } ), v.end() );
}


void FileDescriptor::unique_select ( fd_list& unique_list, Monitor mode )
{
        fd_set list_set;
        FD_ZERO ( &list_set );

        int max_fd = M_create_set ( unique_list, list_set );

        int rc = ::select ( max_fd + 1, ( mode & Monitor::Read ? &list_set : nullptr ),
                        ( mode & Monitor::Write ? &list_set : nullptr ),
                    ( mode & Monitor::Except ? &list_set : nullptr ), nullptr );

        if ( rc == -1 )
                throw system_error ( errno, system_category(), "Could not select" );

        M_remove_unset ( unique_list, list_set );

}


void FileDescriptor::select ( fd_list& readList, fd_list& writeList, fd_list& exceptList )
{
        fd_set readSet, writeSet, exceptSet;

        FD_ZERO ( &readSet );
        exceptSet = writeSet = readSet;

        auto ilist = { M_create_set ( readList, readSet ), M_create_set ( writeList, writeSet ), M_create_set ( exceptList, exceptSet ), FileDescriptor::invalid_fd };

        int max_fd = *max_element ( ilist.begin(), ilist.end() );

        if ( max_fd == invalid_fd )
                throw invalid_argument ( "No file descriptor provided." );

        // We now have the highest file descriptor and the fd_sets set up: we can select.

        int rc = ::select ( max_fd + 1, &readSet, &writeSet, &exceptSet, nullptr ); // TODO: Check the effect of void fd_sets in fs/select.c

        if ( rc == -1 )
                throw system_error ( errno, system_category(), "Could not select" );


        // select was successful: we'll remove unset fds.
        M_remove_unset ( readList, readSet );
        M_remove_unset ( writeList, writeSet );
        M_remove_unset ( exceptList, exceptSet );


}

My worries:

1. Should select and unique_select be static member functions or global ones?
2. Should FileDescriptor(int) be explicit? I can find motivations for it to both be and not to be.
3. Usability
4. Efficiency (with particular regard to select/unique_select)

Any other suggestion will be also appreciated.

Inheritance explainations

This class is inherited from other classes to "specialize" a file descriptor: as said before, a file descriptor can be a socket, file and so on.
When you want to select, you select on fds but they might be a Socket for instance: hence the polymorphism. I had an hard time to go for inheritance: because a socket, for example, is represented by a fd thus composition and inheritance would be excluded but inheritance at the end seemed to fit better. Any thoughts?

Answer 1

I'm not a Unix guy so some of these points may reflect my ignorance!

I'm a little puzzled what business your select / unique_select functions or indeed the fd_list alias have as part of the FileDescriptor class. This seems to violate the Single Responsibility Principle. To me, the FileDescriptor class should be purely about managing the lifetime of the file descriptor and nothing more. If you want utility functions like select they should not be part of the FileDescriptor class.

A related point: it doesn't seem very idiomatic C++ to make a specific container a part of the interface for select. It would be more idiomatic to take a pair of templated iterators so you could work with any container.

I'm also unclear why your fd_list holds FileDescriptors by pointer. It's not clear from the code here what the use of polymorphism would be for a FileDescriptor. In general RAII types are inherently value types and so a polymorphic RAII type doesn't make much sense to me.

Finally, I would think you would want to provide a constructor that takes the arguments to open and performs the open, or a utility function similar to make_unique() that does the same. This way you never have to have raw / unowned int file descriptors floating around.

Answer 2

I don't think this implementation actually counts as RAII, as you can initialize your FileDescriptor in an "invalid" state. Why? Either you have an object, and an open file, or you don't, and no open file. Thus in my opinion there should thus be no default constructor and no is_valid() method.

If you want a variable for holding a file descriptor, but which you don't want initialized just yet - use an std::optional<> around a proper RAII wrapper (or an std::future in some rare cases where that's relevant).