Server socket "wrap" functions

Question

This is a small socket library I made to avoid complicated C boilerplate code and several #includes for connecting sockets, bind, listening etc.

Please note that socket_create() does not support bind to specific IP addresses and that the IP parameter is ignored.

The only C++11 feature I am using is constexpr. Even though it is C++11, I decided not to wrap everything in a class, but to work with standard int.

In the future, I might add Unix domain socket in a way similar to socket_create() and / or SSL support, but at the moment I do not need those.

sockets.h

#ifndef NET_SOCKETS_H_
#define NET_SOCKETS_H_

#include <cstdint>

namespace net{

constexpr int SOCKET_NONBLOCK   = 0x01;
constexpr int SOCKET_TCPNODELAY = 0x02;

// ===========================

constexpr int SOCKET_DEFAULTOPT = SOCKET_NONBLOCK & SOCKET_TCPNODELAY;

// ===========================

int socket_create(const char *ip, uint16_t port, uint16_t backlog = 0, int options = SOCKET_DEFAULTOPT) noexcept;

bool socket_makeNonBlocking(int fd) noexcept;
bool socket_makeTCPNoDelay(int fd) noexcept;

int socket_accept(int fd) noexcept;

void socket_close(int fd) noexcept;

bool socket_check_eagain() noexcept;

}

#endif

sockets.cc

#include "sockets.h"

#include <sys/socket.h>     // EAGAIN
#include <arpa/inet.h>      // AF_INET
#include <netinet/tcp.h>    // TCP_NODELAY
#include <fcntl.h>      // fcntl

#include <unistd.h>     // close

#include <errno.h>      // errno

namespace net{



bool socket_check_eagain() noexcept{
    return errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK;
}

bool socket_makeNonBlocking(int const fd) noexcept{
    if (fd >= 0)
        return fcntl(fd, F_SETFL, O_NONBLOCK) >= 0;

    return false;
}

bool socket_makeTCPNoDelay(int const fd) noexcept{
    int const opt = 1;
    return (setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, & opt, sizeof opt) >= 0);
}

void socket_close(int const fd) noexcept{
    if (fd >= 0)
        ::close(fd);
}

int socket_accept(int const fd) noexcept{
    if (fd >= 0)
        return ::accept(fd, NULL, NULL);

    return -1;
}

int socket_create(const char *ip, uint16_t const port, uint16_t const backlog, int const options) noexcept{
    int fd = socket(AF_INET , SOCK_STREAM , 0);

    if(fd < 0)
        return -1;

    int const opt = 1;
    if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, & opt, sizeof opt) < 0){
        ::close(fd);
        return -2;
    }

    if (options & SOCKET_NONBLOCK)
    if (! socket_makeNonBlocking(fd) ){
        ::close(fd);
        return -3;
    }

    if (options & SOCKET_TCPNODELAY)
    if (! socket_makeTCPNoDelay(fd) ){
        ::close(fd);
        return -4;
    }

    struct sockaddr_in address;

    address.sin_family = AF_INET;
    address.sin_addr.s_addr = INADDR_ANY;
    address.sin_port = htons(port);

    if (bind(fd, (struct sockaddr *) & address, sizeof address) < 0){
        ::close(fd);
        return -5;
    }

    if (listen(fd, backlog ? backlog : SOMAXCONN) < 0){
        ::close(fd);
        return -6;
    }

    return fd;
}



} // namespace

Answer 1

NO. NO. NO. NO.

This does not do you any good none at all. In fact I think this is worse than doing nothing. All you have done is created a new layer that provides no advantages over the old and exposed a new set of error codes (that are not standard).

This is definitely NOT C++ code. You have just written another C layer that has all the same disadvantages of C and absolutely none of the benefits provided by C++.

The main thing to learn about C++ (in fact the overriding absolutely essential thing) is RAII. Resource acquisition is initialization. When you create an object it gets the resource, if it fails to get the resource during initialization you throw and the object does not exist and thus can not be used. Because the object only exists if it was correctly constructed most of the runtime checking to make sure the object is in the correct state disappears (because we know it is in the correct state because it exists).

I would expect the interface to a socket class to look like this:

Socket   webserver(8080);

while(webserver.accept(connection))
{
    InBoundConnection connection(webserver.buildConnection());

    connection.send(HttpObject(404, "<html><head>Miss</head><body>404 not found</body></html>"));
}

Answer 2

I think you should expose the values of your errors as either constants or some sort of enum.

You should keep the "negative is error, positive is socket descriptor" return value of socket_create, but it'd be better for users of your code to be able to check why socket creation failed. And that's better done by checking for SOCKET_MAKE_TCP_NO_DELAY_FAILED of some sort than -4.