首页
学习
活动
专区
圈层
工具
发布
首页
学习
活动
专区
圈层
工具
MCP广场
社区首页 >问答首页 >C++ TcpServer类

C++ TcpServer类
EN

Code Review用户
提问于 2021-03-07 14:51:16
回答 2查看 774关注 0票数 4

我有一个实现TCP服务器的类Tcp_Server。这是我的程序中可用的其他类型服务器的基类。例如,Tcp_Chat_ServerPacket是我们发送/接收的一个结构。

以下是实现:

packet.hpp

代码语言:javascript
运行
复制
#ifndef PACKET
#define PACKET
#define MAX_NAME_LEN 24
#include <stdio.h>
struct Packet
{
    Packet();
    ~Packet();
    char name[MAX_NAME_LEN];
    char buf[BUFSIZ];
};
#endif

packet.cpp

代码语言:javascript
运行
复制
#include "packet.hpp"
Packet::Packet() {};
Packet::~Packet() {};

tcp_server.hpp

代码语言:javascript
运行
复制
#ifndef TCP_SERVER
#define TCP_SERVER
#include <netinet/in.h>
#include <sys/select.h>
#include <arpa/inet.h>
#include <unistd.h>
#include <string.h>
#include <stdio.h>
struct server_info
{
    char ip_str[INET_ADDRSTRLEN] = {'\0'};
    int port;
};

class Tcp_Server
{
public:
    Tcp_Server(int port);               // Initializer
    ~Tcp_Server();                      // Destructor
    /***
     * select(), blocking method
     * Return 0 if it's a listening socket.
     * Return 1 if it's a usual socket.
     */
    int WaitingForActions();        
    int NewConnectionHandler();         // If new client. Return socket number.
    virtual int NewMessageHandler() = 0;// This method shouldn't be implement, look at the inheritance classes.
    struct server_info GetServerInfo(); // Fill and return server_info structure.
    int GetCurrentFds();
protected:
    int listen_socket;                  // Listening socket
    struct sockaddr_in serv_addr;       // Structure with information about socket (adress, port etc.)
    struct server_info server_info;     // Information about server in readable form, returned by function.
    fd_set connections;                 // Set of clients
    int max_socket;                     // Number of maximum existing descriptor
    int current_socket;                 // Number of the processed descriptor
};
#endif

tcp_server.cpp

代码语言:javascript
运行
复制
#include "tcp_server.hpp"
Tcp_Server::Tcp_Server(int port)
{
    int param = 1;      // Parametr for setsockopt(), maybe we can change it to bool.
    char ip_str[INET_ADDRSTRLEN] = {'\0'};

    if((listen_socket = socket(AF_INET, SOCK_STREAM, 0)) < 0)
    {
        throw "Can't create a socket!\n";
    }

    serv_addr.sin_family = AF_INET;
    serv_addr.sin_addr.s_addr = htonl(INADDR_ANY);
    serv_addr.sin_port = htons(port);
    
    if(setsockopt(listen_socket, SOL_SOCKET, SO_REUSEADDR, ¶m, sizeof(param)) < 0)
    {
        throw "setsockopt() error!\n";
    }
    
    if(bind(listen_socket, (struct sockaddr*)&serv_addr, sizeof(serv_addr)) < 0)
    {
        throw "bind() error!\n";
    }

    listen(listen_socket, 5);

    FD_ZERO(&connections);
    FD_SET(listen_socket, &connections);
    max_socket = listen_socket;
}

Tcp_Server::~Tcp_Server()
{
    for(int i = 0; i <= max_socket; i++)
    {
        if(i == listen_socket)
        {
            shutdown(i, SHUT_RDWR);
            close(i);
            FD_CLR(i, &connections);
        }
        if(FD_ISSET(i, &connections))
        {
            shutdown(i, SHUT_RDWR);
            close(i);
            FD_CLR(i, &connections);
        }
    }
}

int Tcp_Server::WaitingForActions()
{
    fd_set tmp_set = connections;

    if(select(max_socket + 1, &tmp_set, NULL, NULL, NULL) < 0)
    {
        throw "error in select \n";  
        fprintf(stderr, "error in select \n");
    }

    for(int i = 0; i <= max_socket; i++)
    {
        if(FD_ISSET(i, &tmp_set))
        {
            current_socket = i;
            if(i == listen_socket)  // Если запрос к слушающему сокету
            {
               return 0;
            }
            else
            {
                return 1;
            }
        }
    }
    return -1;
};

int Tcp_Server::NewConnectionHandler()
{
    int new_sock = accept(listen_socket, NULL, NULL);
    if(new_sock < 0)
    {
        throw "accept() error\n";
    }
    else
    {
        FD_SET(new_sock, &connections);
        if(new_sock > max_socket)
        {
            max_socket = new_sock;
        }
    }
    return new_sock;
}

struct server_info Tcp_Server::GetServerInfo()
{
    memset(&server_info, 0, sizeof(server_info));
    inet_ntop(AF_INET, &(serv_addr.sin_addr), server_info.ip_str, INET_ADDRSTRLEN);
    server_info.port = ntohs(serv_addr.sin_port);
    return server_info;
}

int Tcp_Server::GetCurrentFds()
{
    return current_socket;
}

tcp_chat_server.hpp

代码语言:javascript
运行
复制
#ifndef TCP_CHAT_SERVER
#define TCP_CHAT_SERVER
#include "tcp_server.hpp"
#include "packet.hpp"
class Tcp_Chat_Server : public Tcp_Server
{
public:
    Tcp_Chat_Server(int port);
    virtual int NewMessageHandler();
};
#endif

tcp_chat_server.cpp

代码语言:javascript
运行
复制
#include "tcp_chat_server.hpp"
Tcp_Chat_Server::Tcp_Chat_Server(int port) : Tcp_Server(port) {};

int Tcp_Chat_Server::NewMessageHandler()
{
    Packet packet;
    int bytes = recv(current_socket, &packet, sizeof(packet), 0);
    if(bytes <= 0)
    {
        close(current_socket);
        FD_CLR(current_socket, &connections);
        return bytes;
    }
    else
    {
        for(int i = 0; i <= max_socket; i++)
        {
            if(i == current_socket || i == listen_socket)
            {
                continue;
            }
            if(FD_ISSET(i, &connections))
            {
                if((send(i, &packet, sizeof(packet), 0)) <= 0)
                {
                    return -1;
                }
            }
        }
        fprintf(stdout, "%s : %s", packet.name, packet.buf);
        memset(&packet, '\0', sizeof(packet));
    }
    return bytes;
}

所以,首先,我想听一听:

  1. 类接口;
  2. 阶级等级;
  3. 所有关于virtual的事情;
  4. 新的细微差别的网络和数据传输通过它。
  5. 这种建筑的可支持性;

其次,不太重要的是:

  1. STL和其他图书馆
  2. 命名

最后,这个类是即时消息应用程序的一部分。目前,密码是在私人回购。听到您对如何最好地设计此类应用程序,特别是发送/接收消息的过程的想法,将会很感兴趣。

EN

回答 2

Code Review用户

回答已采纳

发布于 2021-03-08 17:27:13

简·舒尔特克说的每一句话:

设计

我没有看到类似run()的方法。所以用户需要实现这一点。它看起来像这样吗?

代码语言:javascript
运行
复制
void run(Tcp_Server& server)
{
    while(!finished) {
        int action = server.WaitingForActions();
        if (action == 0) {
            int socket = server.NewConnectionHandler()
        }
        else if (action == 1) {
            int byteCount = server.NewMessageHandler()
        }
        else {
            // error
        }
    }
}

这就引出了很多问题。

  1. 这是处理它的预期模式吗?
  2. 我该如何处理这两个方法返回的值?
  • NewConnectionHandler()已经处理了套接字。
  • NewMessageHandler()是虚拟的,为什么我需要编写run()和这个呢?
  1. 你的NewMessageHandler()为什么要摆弄Tcp_Server的成员?
  • 如果您需要访问内部成员,似乎是一个糟糕的设计。

您的代码应该实现该运行方法,并在适当时调用NewMessageHandler(),只将需要读取的套接字传递给处理程序。

为什么要在一个线程上完成这个任务?基本上,当您现在得到一个连接时,您的服务器在处理NewMessageHandler()上的当前消息时阻止所有其他潜在的连接尝试。这意味着没有必要使用select()。您可以简单地在accept()上阻塞,处理结果请求,然后返回到accept上的块。

为了更好地做到这一点:

  1. 您有一个主线程运行您的main()版本。
  2. 当主线程接收到任何连接上的连接或输入时,它会将一个工作项添加到要单独处理的工作队列中,然后立即返回等待select()。您的主线程应该尽可能少地处理来自连接的数据。
  3. 你应该有一个线程池。池中的所有线程都应该监视工作队列。如果将一个新项添加到工作队列中,则它们将开始处理。
  4. 如果工作人员完成了套接字上的所有工作,那么它将关闭并返回到工作队列。
  5. 如果工作人员会阻止对套接字的读写,那么您就会将套接字返回到主线程,这样它就可以在select()上等待,并且当套接字准备好,线程返回到池等待另一项工作时(注意:暗示状态与套接字相关联,所以当您准备恢复时,您需要检索状态以继续)。

我会更像这样设计我的界面:

代码语言:javascript
运行
复制
class Tcp_Server
{
public:
    Tcp_Server(int port, ThreadPool workers, WorkQueue queue);         
    virtual ~Tcp_Server(); 
    void     run();
private:

    // This function will be called by a worker
    // as defined by the ThreadPool (The thread pool may be empty which
    // means it is called by the main thread).
    //
    // return true to add back to FD_SET to continue listening.
    // return false to indicate connection was closed.
    // Exception: Error on stream. TCP_Server will shut manually close
    //            the connection (any errors will be discarded).
    //            exception will be logged.
    virtual bool NewMessageHandler(int socket) = 0; // Pass socket
};

class ChatServer: public Tcp_Server {
    // Allow the handler to store some state about the connection.
    // Subsequent calls can use this information to continue
    // processing from the last know position (either read or write).
    std::map<int, void*>      socketData;

    // Handler for Chat protocol.
    // Handler should return on closed connection or a potentially blocking
    // read/write.
    virtual bool NewMessageHandler(int socket); // continue handling data.
};

int main() {
    WorkQueue     workItems;
    ThreadPool    workers(5, workItems);          // workers read from workItems.
    ChatServer    server(45, workers, workItems); // server writes to work Items.

    server.run();
}

代码评审

您不使用命名空间。

在代码周围添加一个命名空间。

见简·舒尔特克的建议

代码语言:javascript
运行
复制
#define MAX_NAME_LEN 24

见建议: Jan Schultke

代码语言:javascript
运行
复制
#include <stdio.h>

这里不需要构造函数/析构函数(他们什么也不做)。而且,这个对象使用数组,所以它是不可移动的。不知道你想怎么用它。但是在这里使用std::vector<>可能是值得的,并且允许简单地在应用程序周围移动对象。

代码语言:javascript
运行
复制
struct Packet
{
    Packet();
    ~Packet();
    char name[MAX_NAME_LEN];
    char buf[BUFSIZ];
};

其中包括警卫人手不足。而且可能并不是独一无二的。

代码语言:javascript
运行
复制
#ifndef TCP_SERVER
#define TCP_SERVER

我会将命名空间添加到宏保护中,以确保它们是唯一的。

我明白这些东西的必要性。

代码语言:javascript
运行
复制
#include <netinet/in.h>
#include <sys/select.h>
#include <arpa/inet.h>
#include <unistd.h>

但这些是C库。

代码语言:javascript
运行
复制
#include <string.h>
#include <stdio.h>

你用它们做什么?为什么不使用C++设备呢?C++ std::string类很好(并且不太容易使用错误)。C++的输入/输出工具不太容易出错(尽管我相信C编译器使这一点不再是一个问题)。但是您在C++中,所以使用C++ std::coutstd::clog和/或std::cerr

不要写无用的评论。

代码语言:javascript
运行
复制
    Tcp_Server(int port);               // Initializer
    ~Tcp_Server();                      // Destructor

我看得出来他们是什么。

在这里使用整数是一种糟糕的策略。

代码语言:javascript
运行
复制
    /***
     * select(), blocking method
     * Return 0 if it's a listening socket.
     * Return 1 if it's a usual socket.
     */
    int WaitingForActions();        

创建一个enum并对其进行具体说明。使用enum允许您编写更好的自文档代码。

protected是语言设计中的一个错误。别用它。如果必须使用它,请确保本节中只有防止派生类型滥用的方法。变量应该始终是私有的。

如果不需要,不要通过protected提供可用的东西。你应该把它紧紧地控制在必要的东西上。

代码语言:javascript
运行
复制
protected:
    int listen_socket;                  // Listening socket
    struct sockaddr_in serv_addr;       // Structure with information about socket (adress, port etc.)
    struct server_info server_info;     // Information about server in readable form, returned by function.
    fd_set connections;                 // Set of clients
    int max_socket;                     // Number of maximum existing descriptor
    int current_socket;                 // Number of the processed descriptor
};

在这些异常中包含更多信息可能是值得的。

代码语言:javascript
运行
复制
        throw "Can't create a socket!\n";
        throw "setsockopt() error!\n";
        throw "bind() error!\n";

用户将很难纠正这些语句中的错误。添加错误号和生成的特定错误消息。这可能导致您派生一个异常类(只有在异常允许您修复错误时才这样做。否则,使用泛型异常类)。

这一印永远无法达到:

代码语言:javascript
运行
复制
        throw "error in select \n";  
        fprintf(stderr, "error in select \n");
票数 4
EN

Code Review用户

发布于 2021-03-08 10:08:15

这里有很多事情要讨论,所以我将从一条线到另一条线,解释我看到的所有问题:

#define MAX_NAME_LEN 24

你真的不应该在C++中使用宏,除非它是必要的。它们污染了全局命名空间,没有类型安全性。考虑一下constexpr std::size_t MAX_NAME_LEN

#include <stdio.h>

不推荐包含C头,它们甚至可能不存在,这取决于实现。使用<cstdio>代替。

Packet()

不必要的构造函数(和析构函数)。如果在头文件中定义它,并在源文件中给它一个空体,这意味着它仍然会像任何函数一样被调用,即使它什么也不做。如果没有链接时间优化,这些调用就不会被优化掉。要么遵循零的规则,要么遵循五的规则。

char buf[BUFSIZ];

具有实现定义大小的缓冲区。如果您这样做,每个数据包都会相当大,可能是4096或8192字节。如果您真的想要这么大的大小,至少要使用您自己的,而不是使用libc中的宏。

packet.cpp

删除不必要的构造函数和析构函数后,不需要此文件。你可能只是想把它删除。您的Packet可以设计为一个只使用头的实用程序。

~Tcp_Server();

虚拟类中的非虚拟析构函数。如果启用了适当的警告,您的编译器可能已经对此发出了警告。这里的问题是,销毁Tcp_Server不会调用其派生类的析构函数,从而导致潜在的内存泄漏。标记它virtual

virtual int NewMessageHandler() = 0;

如果唯一的Tcp_Server是消息处理,那么可能不值得将整个virtual抽象起来。考虑使用std::function或函数指针回调来处理初始化后传递给类的消息。这可能会消除大量的样板,也意味着您的服务器不再需要是一个虚拟类。

throw "error in select \n";

有些人会争辩说,您不应该仅仅抛出字符串,而应该使用std::exception类层次结构。另外,为什么要提交一个已经使用int结果代码的类?似乎是多余的错误处理。

票数 3
EN
页面原文内容由Code Review提供。腾讯云小微IT领域专用引擎提供翻译支持
原文链接:

https://codereview.stackexchange.com/questions/256840

复制
相关文章

相似问题

领券
问题归档专栏文章快讯文章归档关键词归档开发者手册归档开发者手册 Section 归档