Skip to main content
Code Review

Return to Question

Tweeted twitter.com/StackCodeReview/status/876986643013668866
added 4 characters in body
Source Link
alecxe
alecxe
  • 17.3k
  • 8
  • 51
  • 93

I am relatively new to socket programming, so I would like to ask if there is anything wrong with my code :) For the server socket I included the great new C++ lambdas, so you can start an initialized server using:

server.run([&](serverSock * serv) {...} For the server socket I included the great new C++ lambdas, so you can start an initialized server using:

server.run([&](serverSock * serv) {...}

I am relatively new to socket programming, so I would like to ask if there is anything wrong with my code :) For the server socket I included the great new C++ lambdas, so you can start an initialized server using:

server.run([&](serverSock * serv) {...}

I am relatively new to socket programming, so I would like to ask if there is anything wrong with my code :)

For the server socket I included the great new C++ lambdas, so you can start an initialized server using:

server.run([&](serverSock * serv) {...}
Source Link
dvs23
dvs23
  • 83
  • 1
  • 1
  • 4

C++ Linux TCP client/server socket wrapper

I am relatively new to socket programming, so I would like to ask if there is anything wrong with my code :) For the server socket I included the great new C++ lambdas, so you can start an initialized server using:

server.run([&](serverSock * serv) {...}

Moreover most things run in a new thread to be able to handle multiple clients if necessary. The readAll function is just for my project, because I tell the receiver that the message ended with the simple string "END".

Is there any bad practice, any possible bug in my code? I'm thankful for every tip! :)

clientSock.h

#ifndef CLIENTSOCK_H
#define CLIENTSOCK_H
#include <iostream>
#include <future>
#include <functional>

#include <errno.h>
#include <string.h>

#include <strings.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <netinet/tcp.h>
#include <arpa/inet.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <netdb.h>
#include <unistd.h>
#include <sys/signal.h>


using namespace std;

class clientSock {
  public:
    clientSock(string host, unsigned int port);
    clientSock();
    clientSock(int sock);
    ~clientSock();

    bool hasError();
    int connect(string host, unsigned int port);
    int disconnect();

    int write(string mesg);
    string read();
    string readAll();

    string host;
    unsigned int port;
    bool connected;

  protected:

  private:
    int enable_keepalive(int sock);

    static const unsigned int buffSize = 1000;
    int sockfd;//establish connection to ID distribution server
    struct sockaddr_in servaddr;
    char recv[buffSize];
    struct hostent* server;
};

#endif // CLIENTSOCK_H

clientSock.cpp

#include "clientSock.h"

clientSock::clientSock(string host, unsigned int port) {
    connect(host, port);
}

clientSock::clientSock() {
    connected = false;
}

clientSock::clientSock(int sock) {
    sockfd = sock;
    connected = true;
}

clientSock::~clientSock() {
    //disconnect();
}

int clientSock::connect(string host, unsigned int port) {
    clientSock::host = host;
    clientSock::port = port;

    bzero(&servaddr, sizeof(servaddr));
    servaddr.sin_family = AF_INET;
    server = gethostbyname(host.data());
    bcopy((char*) server->h_addr, (char*) &servaddr.sin_addr.s_addr, server->h_length);
    servaddr.sin_port = htons(port);

    if(connected)
        disconnect();

    sockfd = socket(AF_INET, SOCK_STREAM, 0);

    /*struct timeval tv;
    tv.tv_sec = 5;//5 Secs Timeout
    tv.tv_usec = 0;//Not init'ing this can cause strange errors
    setsockopt(sockfd, SOL_SOCKET, SO_RCVTIMEO, (char*)&tv, sizeof(struct timeval));*/

    enable_keepalive(sockfd);

    for(size_t i = 0; i < 3; i++) { //try to connect 3 times
        if(::connect(sockfd, (struct sockaddr*) &servaddr, sizeof(servaddr)) < 0)
            cerr << "Error on connecting: " << errno << "  " << strerror(errno) << endl;
        else {
            connected = true;
            return 0;
        }
    }

    connected = false;
    return 1;
}

bool clientSock::hasError() {
    if(sockfd == -1)
        return true;

    int error = 0;
    socklen_t len = sizeof(error);
    int retval = getsockopt(sockfd, SOL_SOCKET, SO_ERROR, &error, &len);

    if(retval != 0 || error != 0)
        return true;
    else
        return false;
}

int clientSock::enable_keepalive(int sock) {
    int yes = 1;

    if(setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE, &yes, sizeof(int)) == -1)
        return -1;

    int idle = 1;

    if(setsockopt(sock, IPPROTO_TCP, TCP_KEEPIDLE, &idle, sizeof(int)) == -1)
        return -1;

    int interval = 1;

    if(setsockopt(sock, IPPROTO_TCP, TCP_KEEPINTVL, &interval, sizeof(int)) == -1)
        return -1;

    int maxpkt = 10;

    if(setsockopt(sock, IPPROTO_TCP, TCP_KEEPCNT, &maxpkt, sizeof(int)) == -1)
        return -1;

    return 0;
}

int clientSock::disconnect() {
    if(!connected)
        return -1;

    close(sockfd);
    connected = false;

    return 0;
}

int clientSock::write(string mesg) {
    if(!connected)
        return 1;

    struct timeval tv;
    tv.tv_sec = 10;
    tv.tv_usec = 0;
    fd_set writefds;
    FD_ZERO(&writefds);
    FD_SET(sockfd, &writefds);

    //cout << "w: " << mesg << endl;

    int sentBytes = 0;

    for(size_t i = 0; i < mesg.length(); i += sentBytes) {
        FD_ZERO(&writefds);
        FD_SET(sockfd, &writefds);
        int rv = select(sockfd + 1, NULL, &writefds, NULL, &tv);

        if(rv == -1)
            cerr << errno << "  " << strerror(errno) << endl;
        else if(rv == 0)
            sentBytes = 0;
        else if(rv > 0 && FD_ISSET(sockfd, &writefds)) {
            sentBytes = ::write(sockfd, mesg.substr(i, mesg.length() - i).c_str(), mesg.length() - i);

            if(sentBytes == -1) {
                cerr << "Error sending IDs: " << errno << "  " << strerror(errno) << endl;
                return 1;
            }
        }
    }

    return 0;
}

string clientSock::read() {
    if(!connected)
        return "";

    struct timeval tv;
    tv.tv_sec = 10;
    tv.tv_usec = 0;
    fd_set readfds;
    FD_ZERO(&readfds);
    FD_SET(sockfd, &readfds);

    string resp = "";
    unsigned int n = 0;


    do {
        FD_ZERO(&readfds);
        FD_SET(sockfd, &readfds);
        int rv = select(sockfd + 1, &readfds, NULL, NULL, &tv);

        if(rv <= -1)
            cerr << errno << "  " << strerror(errno) << endl;
        else if(rv == 0)
            break;
        else if(rv > 0 && FD_ISSET(sockfd, &readfds)) {

            int tn = ::read(sockfd, recv, buffSize - 1);//avoid signcompare warning

            if(tn > 0) {
                n = tn;
                recv[n] = '\0';
                string tResp(recv, n);
                resp += tResp;
            }
            else if(tn == -1) {
                if(errno == 11) { //get the good part of the received stuff also if the connection closed during receive -> happens sometimes with short messages
                    string tResp(recv);

                    if(tResp.find_first_not_of("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ01234567890") == std::string::npos) //but only allow valid chars
                        resp += tResp;
                }
                else
                    cerr << errno << "  " << strerror(errno) << endl;

                break;
            }
            else
                break;

        }
        else
            cerr << "ERROR: rv: " << rv << endl;

    }
    while(n >= buffSize - 1);

    //if(resp != "")
    //cout << "r: " << resp << endl;

    return resp;
}

string clientSock::readAll() {
    string full = read();

    while(full.find("END") == string::npos)
        full += read();

    full = full.substr(0, full.find("END"));

    return full;
}

serverSock.h

#ifndef SERVERSOCK_H
#define SERVERSOCK_H
#include <iostream>
#include <future>
#include <functional>

#include <clientSock.h>

#include <errno.h>
#include <string.h>

#include <strings.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <netdb.h>
#include <unistd.h>

using namespace std;


class serverSock {
  public:
    serverSock(unsigned int port);
    serverSock();
    ~serverSock();

    clientSock accept();

    bool inited;

    int init(unsigned int port);//!<Necessary only if not already done by using constructor with port
    int start();
    int run(std::function<void(serverSock* sock)> fn);
    int runBlocking(std::function<void(serverSock* sock)> fn);
    int stop();

  protected:
    future<void> thr;

  private:
    int enable_keepalive(int sock);

    int sockfd;//start server to listen for clients to send them ids
    socklen_t clilen;
    struct sockaddr_in servaddr, cliaddr;
    static const unsigned int buffSize = 1000;
    char mesg[buffSize];

    unsigned int listeningPort;
    bool started;
    bool stopSock;

};

#endif // SERVERSOCK_H

serverSock.cpp

#include "serverSock.h"

serverSock::serverSock(unsigned int port) : stopSock(false) {
    inited = false;
    init(port);
    inited = true;
}

serverSock::serverSock() {
    listeningPort = 0;
}

serverSock::~serverSock() {
    stop();
}

int serverSock::init(unsigned int port) {
    listeningPort = port;
    sockfd = socket(AF_INET, SOCK_STREAM, 0);
    int yes = 1;

    if(setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, &yes, sizeof(yes)) == -1)
        cerr << errno << "  " << strerror(errno) << endl;

    enable_keepalive(sockfd);

    bzero(&servaddr, sizeof(servaddr));
    servaddr.sin_family = AF_INET;
    servaddr.sin_addr.s_addr = htonl(INADDR_ANY);
    servaddr.sin_port = htons(port);
    started = false;
    return 0;
}

int serverSock::enable_keepalive(int sock) {
    int yes = 1;

    if(setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE, &yes, sizeof(int)) == -1) {
        cerr << errno << "  " << strerror(errno) << endl;
        return -1;
    }

    int idle = 1;

    if(setsockopt(sock, IPPROTO_TCP, TCP_KEEPIDLE, &idle, sizeof(int)) == -1) {
        cerr << errno << "  " << strerror(errno) << endl;
        return -1;
    }

    int interval = 1;

    if(setsockopt(sock, IPPROTO_TCP, TCP_KEEPINTVL, &interval, sizeof(int)) == -1) {
        cerr << errno << "  " << strerror(errno) << endl;
        return -1;
    }

    int maxpkt = 10;

    if(setsockopt(sock, IPPROTO_TCP, TCP_KEEPCNT, &maxpkt, sizeof(int)) == -1) {
        cerr << errno << "  " << strerror(errno) << endl;
        return -1;
    }

    return 0;
}

clientSock serverSock::accept() {
    if(started) {
        int newsockfd = ::accept(sockfd, (struct sockaddr*) &cliaddr, &clilen);

        if(newsockfd == -1)
            cerr << errno << "  " << strerror(errno) << endl;

        return clientSock(newsockfd);
    }
    else return clientSock();
}

int serverSock::start() {
    if(listeningPort == 0) {
        cerr << "ERROR No port defined to listen to" << endl;
        return 1;
    }

    if(bind(sockfd, (struct sockaddr*) &servaddr, sizeof(servaddr)) < 0) {
        cerr << errno << "  " << strerror(errno) << endl;
        return 1;
    }

    listen(sockfd, 5);
    clilen = sizeof(cliaddr);

    started = true;
    stopSock = false;

    return 0;
}

int serverSock::run(std::function<void(serverSock* sock)> fn) {
    thr = std::async([this, fn]() {
        while(!stopSock)
            fn(this);
    });
    return 0;
}

int serverSock::runBlocking(std::function<void(serverSock* sock)> fn) {
    while(!stopSock)
        fn(this);

    return 0;
}

int serverSock::stop() {
    stopSock = true;
    int res = close(sockfd);

    if(res < 0)
        cerr << errno << "  " << strerror(errno) << endl;

    return 0;
}