I'm very new to C++, (7 days ago I wrote "Hello World") but I really want to write good code and increase my development skill, so I ask you for some review.
This is the scheme of how my socket server works. It creates a socket, binds it, sets to listen state, creates a new thread (in order not to block the execution of the program), which accepts new connections, creates new threads for each client, reads data from them and dispatches data to observers (server is a child of Observable
class, which allows to dispatch events to Observers
.
#include <iostream>
#include <string.h>
#include <stdio.h>
#include <vector>
#include <arpa/inet.h>
#include <sys/socket.h>
#include <pthread.h>
#include "Observer.h"
using namespace std;
class SocketServer : public Observable
{
private:
string serverIp;
int serverPort;
int masterSocket;
void SocketCreate ( );
void SocketBind ( );
void SocketListen ( );
static void* SocketAccept ( void* );
static void* SocketRead ( void* );
void RequestDispatch ( int , string );
public:
SocketServer ( string , int );
static void Send ( int , string );
static void Log ( int , string );
};
struct ClientRequest
{
int socketFD;
string request;
};
struct ServerAndSocket
{
SocketServer* socketServer;
int socketFD;
};
SocketServer::SocketServer ( string serverIp , int serverPort )
{
this->serverIp = serverIp;
this->serverPort = serverPort;
this->SocketCreate();
this->SocketBind();
this->SocketListen();
pthread_create ( new pthread_t() , NULL , SocketServer::SocketAccept, this );
};
void SocketServer::SocketCreate ( )
{
this->masterSocket = socket ( AF_INET , SOCK_STREAM , 0 );
if ( this->masterSocket < 0 )
{
perror ( "socket" );
_exit(0);
}
else
{
int opt = 1;
setsockopt (this->masterSocket, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof (opt));
cout << "Socket created successfully with file descriptor " << this->masterSocket << "\n";
};
};
void SocketServer::SocketBind ( )
{
struct sockaddr_in serverAddress;
serverAddress.sin_family = AF_INET;
serverAddress.sin_port = htons ( this->serverPort );
serverAddress.sin_addr.s_addr = inet_addr ( this->serverIp.data());
if ( bind ( this->masterSocket , (sockaddr*)&serverAddress , sizeof ( serverAddress ) ) < 0 )
{
perror ( "bind" );
_exit(0);
}
else
{
cout << "Socket bound successfully to the " << this->serverIp << ":" << this->serverPort << " address\n";
};
};
void SocketServer::SocketListen ( )
{
listen ( this->masterSocket , 5 );
cout << "Socket is beeing listened now\n";
};
void* SocketServer::SocketAccept ( void* serverPointer )
{
int socketFD;
pthread_t* clientThread;
ClientRequest clientRequest;
SocketServer* this_;
ServerAndSocket serverAndSocketPtr;
this_ = (SocketServer*)serverPointer;
while ( 1 )
{
socketFD = accept ( this_->masterSocket , NULL , NULL );
if ( socketFD < 0 )
{
perror ( "accept" );
}
else
{
this_->RequestDispatch ( socketFD , "CLIENT_CONNECTED" );
serverAndSocketPtr.socketServer = this_;
serverAndSocketPtr.socketFD = socketFD;
pthread_create ( new pthread_t() , NULL , SocketServer::SocketRead, &serverAndSocketPtr );
};
};
};
void* SocketServer::SocketRead ( void* serverAndSocketPointer )
{
char input[256];
int inputLength;
SocketServer* socketServerPtr;
int socketFD;
ServerAndSocket* serverAndSocketPtr;
serverAndSocketPtr = (ServerAndSocket*)serverAndSocketPointer;
socketServerPtr = serverAndSocketPtr->socketServer;
socketFD = serverAndSocketPtr->socketFD;
while ( 1 )
{
memset ( (void*)&input , '\0' , sizeof ( input ) );
inputLength = read ( socketFD , (void*)&input , 255 );
if ( inputLength < 0 )
{
perror ( "read" );
}
else if ( inputLength == 0 || input[0] == '\0' )
{
socketServerPtr->RequestDispatch ( socketFD , "CLIENT_DISCONNECTED" );
pthread_exit( NULL );
}
else
{
socketServerPtr->RequestDispatch ( socketFD , input );
};
};
};
void SocketServer::RequestDispatch ( int socketFD , string request )
{
struct ClientRequest clientRequest;
SocketServer::Log ( socketFD , "---> " + request );
clientRequest.socketFD = socketFD;
clientRequest.request = request;
this->DispatchEvent ( (void*)&clientRequest );
};
void SocketServer::Send ( int socketFD , string message )
{
int bytesWritten;
bytesWritten = write ( socketFD , message.c_str() , message.size() + 1 );
if ( bytesWritten < 0 )
{
perror ( "write" );
}
else
{
SocketServer::Log ( socketFD , "<--- " + message );
};
};
void SocketServer::Log ( int socketFD , string message )
{
sockaddr address;
socklen_t addressLength;
sockaddr_in* addressInternet;
string ip;
int port;
getpeername ( socketFD , &address , &addressLength );
addressInternet = (struct sockaddr_in*)&address;
ip = inet_ntoa ( addressInternet->sin_addr );
port = addressInternet->sin_port;
cout << ip << ":" << port << " " << message << "\n";
};
select()
as a starting point epoll() is a natural extension to select() but takes a bit more work. It allows you to have one thread service all connections simultaneously. – Loki Astari Aug 14 '12 at 18:43sockaddr
structs are network byte-order, so usentohs()
in theLog()
method – hroptatyr Aug 15 '12 at 18:22