Опрос На C++ Гнездо Оболочки Для Linux


Поэтому я создал на C++ сокет класс-оболочку с помощью опроса и событий. Создание слушающего сокета и привязка уже обрабатывается в initialize()метод. Я в основном люблю, чтобы получить обратную связь на socketLoop() способ, так как он является наиболее сложным. Если есть какие-то вредные привычки, необработанные исключения, ошибки или что-нибудь еще я могу улучшить, пожалуйста, дайте мне знать. Я ценю резкую критику, не стесняйтесь быть занудой!

bool NetworkSocket::socketLoop(){

    // Start listening on specified port
    if (listen(this->listeningSocket, this->backlog) < 0) {
        this->raiseSocketExceptionEvent("Failed to start listening.");
        return false;
    }

    std::cout << "Listening for connections on port " << this->listening_port << std::endl;

    // Setup pollfd set and add listening socket to the set
    this->pfd.fd = this->listeningSocket; //
    this->pfd.events = POLLIN;
    this->pfd.revents = 0;
    this->readfds.push_back(this->pfd);

    while(this->run){

        char buf[256];

        // Poll on all file descriptors
        if (poll(&this->readfds[0], this->readfds.size(), -1) < 0) {
            this->raiseSocketExceptionEvent("poll() failed");
            return false;
        }

        // Loop through all descriptors and check which ones are readable
        for (int j = 0; j <  this->readfds.size(); j++) {

            // If its the listening socket continue
            if (this->readfds[j].revents == 0)
                continue;

            int sd = this->readfds[j].fd;

            // There is data to read?
            if (this->readfds[j].revents & POLLIN) {

                // If its on the listening socket its an incomin connection
                if (sd == this->listeningSocket) {
                    sockaddr_in address;
                    socklen_t addrlen = sizeof(address);

                    // Accept new connection
                    int new_socket = accept(this->listeningSocket, (struct sockaddr *) &address, &addrlen);
                    if (new_socket < 0) {
                        this->raiseSocketExceptionEvent("Failed to accept incoming connection.");
                        return false;
                    }

                    // Information about the new connection
                    this->raiseClientConnectedEvent(new_socket);

                    // Add new connection to fdset
                    this->pfd.fd = new_socket;
                    this->pfd.events = POLLIN | POLLRDHUP;
                    this->pfd.revents = 0;
                    this->readfds.push_back(this->pfd);

                    // Add new connection to vectors
                    this->clients.push_back(new_socket);

                } else {

                    // Recieve data on this connection until it fails
                    ssize_t rc = recv(sd, buf, sizeof(buf), 0);
                    if (rc > 0) {

                        std::vector<char> data(static_cast<unsigned long>(rc));
                        for (int i = 0; i < rc; i++) {
                            data[i] = buf[i];
                        }

                        this->raiseDataReceivedEvent(data); // ToDo replace with Packet structure later
                    }
                    // Connection was closed by the client
                    else if (rc == 0) {
                        this->readfds[j].revents |= POLLHUP;
                    }
                    else {
                        this->readfds[j].revents |= POLLERR;
                    }
                }
            }

            // If revents is not POLLIN its an unexpected result, exit
            if (this->readfds[j].revents != POLLIN) {
                if (sd == this->listeningSocket) {
                    std::cout << "Unknown exception..?" << std::endl;
                } else {
                    if (this->readfds[j].revents & POLLERR) {
                        this->raiseSocketExceptionEvent("Error reading client");
                    } else {
                        this->raiseClientDisconnectedEvent(sd);
                    }
                    close(sd);
                    this->clients.erase(std::find(this->clients.begin(),this->clients.end(), sd));
                    this->readfds.erase(this->readfds.begin() + j);
                    continue;
                }
            }
        }
    }
    return true;
}

Ладно, как просили, вот код заголовочного файла класса, так что вам лучше понять структуру класса:

#ifndef NETWORKSOCKET_H
#define NETWORKSOCKET_H

#include <sys/ioctl.h>
#include <iostream>
#include <sys/socket.h>
#include <netinet/in.h>
#include <vector>
#include <cstring>
#include <arpa/inet.h>
#include <unistd.h>
#include <cerrno>
#include "NetworkEventListener.h"
#include <algorithm>
#include <poll.h>
#include <thread>

class NetworkSocket
{
public:
    NetworkSocket(uint16_t port);
    ~NetworkSocket();

    bool initialize();
    void start();
    bool socketLoop();
    void stop();
    void pollThread();

    void addEventListener(NetworkEventListener *listener);
    void removeEventListener(NetworkEventListener *listener);
   bool run = true; // testing only will be replaced with getter/setter
private:

    void raiseDataSendEvent(const std::vector<char> &data) const;
    void raiseDataReceivedEvent(const std::vector<char> &data) const;
    void raiseClientConnectedEvent(int socket) const;
    void raiseClientDisconnectedEvent(int socket) const;
    void raiseSocketExceptionEvent(std::string message) const;

    std::vector<NetworkEventListener *> listeners;

    uint16_t listening_port = 0;
    int listeningSocket;
    int socketopts = 1;
    int backlog;

    std::thread socketLoopThread;

    fd_set master_fd_set;
    struct sockaddr_in address;
    std::vector<int> clients;
    std::vector<pollfd> readfds;
    pollfd pfd;

};

void socketLoopAuxiliary(NetworkSocket * socket);

#endif // NETWORKSOCKET_H


526
4
задан 28 марта 2018 в 03:03 Источник Поделиться
Комментарии
1 ответ

Ясность и читабельность

Я бы потерять this-> Как правило. По крайней мере, на мой взгляд, это добавляет визуальный шум, не внося ничего положительного для понимания код.

Вводящие в заблуждение комментарии

    // If its the listening socket continue
if (this->readfds[j].revents == 0)
continue;

Этот комментарий не похоже, что это соответствует тому, что делает код. Вместо того, чтобы проверить, так ли это слушающий сокет, он проверяет, является ли этот разъем на самом деле есть событие, процесс (и если не, в основном игнорирует его).

Использовать конструкторы, где это уместно

Например, в этом коде:

                // Add new connection to fdset
this->pfd.fd = new_socket;
this->pfd.events = POLLIN | POLLRDHUP;
this->pfd.revents = 0;

...В NetworkSocket знает все о ВКУ pfd. Как правило, я предпочитаю код, отражающий детали pfd в ПФО, так это закончилось тем, что-то вроде:

readfds.push_back(new pfd(new_socket, POLLIN | POLLRDHUP));
clients.push_back(new_socket);

Переменная Район

По крайней мере, судя по всему, NetworkSocket::pfd действительно нужны только как некий временной товара при получении нового соединения. Если это правильно, тогда он, наверное, должен быть локальным для кода, который принимает новое соединение, а не весь объект.

Структура Кода

Моя немедленная реакция заключается в том, что socketLoop действительно слишком большие, слишком много частей, которые лишь отчасти связаны все упакованы вместе в одном месте.

Я бы предпочел некоторые функциональные элементы внутри socketLoop были выбиты окна в функции своих собственных, и его в основном называли их делать грязную работу, так сказать. Что бы сократить код socketLoop что-то вроде этого:

while(run){
char buf[256];

// Poll on all file descriptors
if (poll(&readfds[0], readfds.size(), -1) < 0) {
raiseSocketExceptionEvent("poll() failed");
return false;
}

for (int j = 0; j < readfds.size(); j++) {

if (readfds[j].revents == 0)
continue;

int sd = readfds[j].fd;

if (readfds[j].revents & POLLIN) {
if (sd == listeningSocket) {
accept_connection(sd);
}
else {
read_data(sd);
}
}
else {
handle_error(sd);
}
}
}

Может я не очень умная, но я найти что-то вроде этого немного легче следовать.

Диапазона на основе использования цикла for, где это применимо

Цикл такой:

for (int j = 0; j <  readfds.size(); j++)

...как правило, может быть изменен на основе диапазона на цикл вроде этого:

for (auto &fd : readfds)

...затем внутри цикла fd будет эквивалентна readfds[j] в вашем цикле, так что используйте (например):

for (auto &fd : readfds)

if (fd.revents == 0)
continue;

Между прочим, это, как правило, менее склонны к ошибкам (например, с использованием размеров одной коллекции, но индексация в другое семейство).

Предпочитают данные в код

Например, вместо этого:

                else if (rc == 0) {
readfds[j].revents |= POLLHUP;
}
else {
readfds[j].revents |= POLLERR;
}

Я обычно использую что-то вроде этого:

int values = { POLLERR, POLLHUP};

readfds[j].revents = values[rc == 0];

2
ответ дан 30 марта 2018 в 10:03 Источник Поделиться