HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

An owned file descriptor

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
fileowneddescriptor

Problem

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 

#include 
#include 

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

#include
#include

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; // 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_lis

Solution

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.

Context

StackExchange Code Review Q#68549, answer score: 6

Revisions (0)

No revisions yet.