refactoring (const correctness, ...), simpler server networking (+comments

for c specific stuff), removed all "using namespace std"

common/block/Blocks.h

@@ -25,7 +25,7 @@ namespace Blocks
     extern const Block GRASS;
     extern const Block DIRT;
     extern const Block STONE;
-};
+}
 
 #endif
 

common/stream/Stream.cpp

@@ -1,13 +1,15 @@
-#include "common/stream/Stream.h"
+#include <iostream>
 #include <stdexcept>
 #include <iostream>
 #include <cstring>
+
 #include <sys/socket.h>
 
-Stream::Stream(size_t minSize)
+#include "common/stream/Stream.h"
+
+Stream::Stream(size_t minSize) : dataLength(getSize(minSize)), 
+        data(new unsigned char[dataLength]), writeIndex(0), readIndex(0)
 {
-    dataLength = getSize(minSize);
-    data = new unsigned char[dataLength];
 }
 
 Stream::~Stream()
@@ -15,17 +17,13 @@ Stream::~Stream()
     delete[] data;
 }
 
-size_t Stream::getSize(size_t minSize)
+size_t Stream::getSize(size_t minSize) const
 {
     size_t size = 1;
-    while(size < minSize && size != 0)
+    while(size < minSize && size <= MAX_SIZE)
     {
         size <<= 1;
     }
-    if(size == 0)
-    {
-        throw runtime_error("size exceeds max possible size");
-    }
     return size;
 }
 
@@ -34,8 +32,12 @@ bool Stream::hasData() const
     return readIndex < writeIndex;
 }
 
-void Stream::resize(size_t minSize)
+bool Stream::resize(size_t minSize)
 {
+    if(minSize > MAX_SIZE)
+    {
+        return true;
+    }
     size_t newSize = getSize(minSize);
     //cout << "resize from " << dataLength << " to " << newSize << endl;
     unsigned char* newData = new unsigned char[newSize];
@@ -43,19 +45,23 @@ void Stream::resize(size_t minSize)
     delete[] data;
     data = newData;
     dataLength = newSize;
+    return false;
 }
 
 void Stream::write(const void* writeData, size_t length)
 {
-    if(writeIndex + length > dataLength)
-    {
-        resize(writeIndex + length);
-    }
-    for(size_t i = 0; i < length; i++)
+    if(writeIndex + length > dataLength && resize(writeIndex + length))
     {
-        data[writeIndex] = ((unsigned char*) writeData)[i];
-        writeIndex++;
+        std::cout << "write over max stream size" << std::endl;
+        return;
     }
+    memcpy(data + writeIndex, writeData, length);
+    writeIndex += length;
+}
+
+void Stream::write(const char* writeData)
+{
+    write(writeData, strlen(writeData));
 }
 
 void Stream::writeChar(char c)
@@ -63,107 +69,130 @@ void Stream::writeChar(char c)
     write(&c, sizeof(char));
 }
 
-void Stream::writeShort(short s)
+void Stream::writeShort(int16_t s)
 {
     write(&s, sizeof(short));
 }
 
-void Stream::writeInt(int i)
+void Stream::writeInt(int32_t i)
 {
     write(&i, sizeof(int));
 }
 
-void Stream::writeLong(long l)
+void Stream::writeLong(int64_t l)
 {
     write(&l, sizeof(long));
 }
 
-void Stream::writeUnsignedChar(unsigned char uc)
+void Stream::writeUnsignedChar(uint8_t uc)
 {
     write(&uc, sizeof(unsigned char));
 }
 
-void Stream::writeUnsignedShort(unsigned short us)
+void Stream::writeUnsignedShort(uint16_t us)
 {
     write(&us, sizeof(unsigned short));
 }
 
-void Stream::writeUnsignedInt(unsigned int ui)
+void Stream::writeUnsignedInt(uint32_t ui)
 {
     write(&ui, sizeof(unsigned int));
 }
 
-void Stream::writeUnsignedLong(unsigned long ul)
+void Stream::writeUnsignedLong(uint64_t ul)
 {
     write(&ul, sizeof(unsigned long));
 }
 
-void Stream::read(void* buffer, size_t length)
+bool Stream::read(void* buffer, size_t length)
 {
     if(readIndex + length > writeIndex)
     {
-        throw runtime_error("stream read over data length");
-    }
-    for(size_t i = 0; i < length; i++)
-    {
-        ((unsigned char*) buffer)[i] = data[readIndex];
-        readIndex++;
+        std::cout << "read over stream data length" << std::endl;
+        return true;
     }
+    memcpy(buffer, data + readIndex, length);
+    readIndex += length;
+    return false;
 }
 
-char Stream::readChar()
+char Stream::readChar(char error)
 {
     char c;
-    read(&c, sizeof(char));
+    if(read(&c, sizeof(char)))
+    {
+        return error;
+    }
     return c;
 }
 
-short Stream::readShort()
+int16_t Stream::readShort(int16_t error)
 {
     short s;
-    read(&s, sizeof(short));
+    if(read(&s, sizeof(short)))
+    {
+        return error;
+    }
     return s;
 }
 
-int Stream::readInt()
+int32_t Stream::readInt(int32_t error)
 {
     int i;
-    read(&i, sizeof(int));
+    if(read(&i, sizeof(int)))
+    {
+        return error;
+    }
     return i;
 }
 
-long Stream::readLong()
+int64_t Stream::readLong(int64_t error)
 {
     long l;
-    read(&l, sizeof(long));
+    if(read(&l, sizeof(long)))
+    {
+        return error;
+    }
     return l;
 }
 
-unsigned char Stream::readUnsignedChar()
+unsigned char Stream::readUnsignedChar(unsigned char error)
 {
     unsigned char uc;
-    read(&uc, sizeof(unsigned char));
+    if(read(&uc, sizeof(unsigned char)))
+    {
+        return error;
+    }
     return uc;
 }
 
-unsigned short Stream::readUnsignedShort()
+uint16_t Stream::readUnsignedShort(uint16_t error)
 {
     unsigned short us;
-    read(&us, sizeof(unsigned short));
+    if(read(&us, sizeof(unsigned short)))
+    {
+        return error;
+    }
     return us;
 }
 
-unsigned int Stream::readUnsignedInt()
+uint32_t Stream::readUnsignedInt(uint32_t error)
 {
     unsigned int ui;
-    read(&ui, sizeof(unsigned int));
+    if(read(&ui, sizeof(unsigned int)))
+    {
+        return error;
+    }
     return ui;
 }
 
-unsigned long Stream::readUnsignedLong()
+uint64_t Stream::readUnsignedLong(uint64_t error)
 {
     unsigned long ul;
-    read(&ul, sizeof(unsigned long));
+    if(read(&ul, sizeof(unsigned long)))
+    {
+        return error;
+    }
     return ul;
 }
 

common/stream/Stream.h

@@ -3,13 +3,15 @@
 
 #include <cstddef>
 
-using namespace std;
+#include <stdint.h>
 
 class Stream
 {
 public:
     Stream(size_t minSize = 1);
     virtual ~Stream();
+    Stream(const Stream& other) = delete;
+    Stream& operator=(const Stream& other) = delete;
 
     void readSocket(int socket);
     void sendToSocket(int socket);
@@ -17,31 +19,35 @@ public:
     bool hasData() const;
     
     void write(const void* writeData, size_t length);
+    void write(const char* writeData);
     void writeChar(char c);
-    void writeShort(short s);
-    void writeInt(int i);
-    void writeLong(long l);
-    void writeUnsignedChar(unsigned char uc);
-    void writeUnsignedShort(unsigned short us);
-    void writeUnsignedInt(unsigned int ui);
-    void writeUnsignedLong(unsigned long ul);
+    void writeShort(int16_t s);
+    void writeInt(int32_t i);
+    void writeLong(int64_t l);
+    void writeUnsignedChar(uint8_t uc);
+    void writeUnsignedShort(uint16_t us);
+    void writeUnsignedInt(uint32_t ui);
+    void writeUnsignedLong(uint64_t ul);
     
-    void read(void* buffer, size_t length);
-    char readChar();
-    short readShort();
-    int readInt();
-    long readLong();
-    unsigned char readUnsignedChar();
-    unsigned short readUnsignedShort();
-    unsigned int readUnsignedInt();
-    unsigned long readUnsignedLong();
+    bool read(void* buffer, size_t length);
+    char readChar(char error);
+    int16_t readShort(int16_t error);
+    int32_t readInt(int32_t error);
+    int64_t readLong(int64_t error);
+    unsigned char readUnsignedChar(unsigned char error);
+    uint16_t readUnsignedShort(uint16_t error);
+    uint32_t readUnsignedInt(uint32_t error);
+    uint64_t readUnsignedLong(uint64_t error);
     
 private:
-    void resize(size_t minSize);
-    size_t getSize(size_t minSize);
+    bool resize(size_t minSize);
+    size_t getSize(size_t minSize) const;
     
-    size_t dataLength = 0; 
-    unsigned char* data = nullptr;
+    // limit to 50 mbyte
+    static constexpr size_t MAX_SIZE = 1024 * 1024 * 50;
+    
+    size_t dataLength; 
+    unsigned char* data;
     
     size_t writeIndex = 0;
     size_t readIndex = 0;

meson.build

@@ -1,6 +1,8 @@
 project('cubes plus plus', 'cpp')
 
-sourcesCommon = ['common/stream/Stream.cpp', 'common/utils/Face.cpp', 'common/block/Block.cpp', 'common/block/Blocks.cpp', 'common/block/BlockAir.cpp', 'common/world/Chunk.cpp', 'common/world/World.cpp']
+#sourcesCommon = ['common/stream/Stream.cpp', 'common/utils/Face.cpp', 'common/block/Block.cpp', 'common/block/Blocks.cpp', 'common/block/BlockAir.cpp', 'common/world/Chunk.cpp', 'common/world/World.cpp']
+
+sourcesCommon = ['common/stream/Stream.cpp']
 
 sourcesServer = ['server/Main.cpp', 'server/GameServer.cpp', 'server/network/Server.cpp', 'server/commands/CommandManager.cpp', 'server/commands/CommandUtils.cpp', 'server/commands/GeneralCommands.cpp']
 
@@ -15,14 +17,15 @@ pngDep = dependency('libpng')
 
 executable('game_server', 
     sources: sourcesCommon + sourcesServer,
-    dependencies : threadDep)
+    dependencies : threadDep,
+    cpp_args: ['-Wall', '-Wextra', '-pedantic', '-Werror', '-Wno-unused-parameter'])
     
-executable('game_tests', 
-    sources: sourcesTest)
+#executable('game_tests', 
+#    sources: sourcesTest)
     
-executable('game_client', 
-    sources: sourcesCommon + sourcesClient,
-    dependencies : [threadDep, glewDep, glfwDep, pngDep])
+#executable('game_client', 
+#    sources: sourcesCommon + sourcesClient,
+#    dependencies : [threadDep, glewDep, glfwDep, pngDep])
     
 
 	

server/GameServer.cpp

@@ -1,65 +1,58 @@
+#include <iostream>
+
 #include "server/GameServer.h"
 #include "server/network/Server.h"
 
-GameServer::GameServer()
-{
-}
-
-GameServer::~GameServer()
-{
-}
-
 void GameServer::stop()
 {
     isRunning = false;
 }
 
-void GameServer::start(unsigned short port, unsigned short maxClients)
+void GameServer::start(uint16_t port, uint16_t maxClients)
 {
     std::cout << port << std::endl;
-    Server server(port, maxClients);
-    server.start(this);
+    Server server(port, maxClients, *this);
+    if(!server.isRunning())
+    {
+        return;
+    }
     
     isRunning = true;
     while(isRunning)
     {
-        cout << "> ";
-        string command;
-        getline(cin, command, '\n');
+        std::cout << "> ";
+        std::string command;
+        getline(std::cin, command, '\n');
         if(command == "q")
         {
             break;
         }
         commandManager.execute(*this, *this, command);
     }
-    
-    server.stop();
 }
 
-void GameServer::onFullServerClientConnect(int socket)
+void GameServer::onFullServerClientConnect(int socket) const
 {
-    string s = "sorry, the server is full";
     Stream answer;
-    answer.write(s.data(), s.length());
+    answer.write("Sorry, the server is full");
     answer.sendToSocket(socket);
 }
 
-void GameServer::onClientConnect(int socket)
+void GameServer::onClientConnect(int socket) const
 {
-    cout << socket << " has connected" << endl;
+    std::cout << socket << " has connected" << std::endl;
     
-    string s = "Welcome to the server.";
     Stream answer;
-    answer.write(s.data(), s.length());
+    answer.write("Welcome to the server.");
     answer.sendToSocket(socket);
 }
 
-void GameServer::onClientPackage(int socket, Stream& in)
+void GameServer::onClientPackage(int socket, Stream& in) const
 {
-    string s = "";
+    std::string s = "";
     while(in.hasData())
     {
-        s = in.readChar() + s;
+        s = in.readChar('_') + s;
     }
 
     Stream answer;
@@ -67,13 +60,12 @@ void GameServer::onClientPackage(int socket, Stream& in)
     answer.sendToSocket(socket);
 }
 
-void GameServer::onClientDisconnect(int socket)
+void GameServer::onClientDisconnect(int socket) const
 {
-    cout << socket << " has disconnected" << endl;
+    std::cout << socket << " has disconnected" << std::endl;
     
-    string s = "Bye.";
     Stream answer;
-    answer.write(s.data(), s.length());
+    answer.write("Bye.");
     answer.sendToSocket(socket);
 }
 

server/GameServer.h

@@ -1,27 +1,20 @@
 #ifndef GAMESERVER_H
 #define GAMESERVER_H
 
-#include <iostream>
-
 #include "server/network/IServerListener.h"
 #include "server/commands/ICommandSource.h"
 #include "server/commands/CommandManager.h"
 #include "server/IGameServer.h"
 
-using namespace std;
-
 class GameServer : public IServerListener, public ICommandSource, public IGameServer
 {
 public:
-    GameServer();
-    virtual ~GameServer();
-    
-    void start(unsigned short port, unsigned short maxClients);
+    void start(uint16_t port, uint16_t maxClients);
     
-    void onFullServerClientConnect(int socket) override;
-    void onClientConnect(int socket) override;
-    void onClientPackage(int socket, Stream& in) override;
-    void onClientDisconnect(int socket) override;
+    void onFullServerClientConnect(int socket) const override;
+    void onClientConnect(int socket) const override;
+    void onClientPackage(int socket, Stream& in) const override;
+    void onClientDisconnect(int socket) const override;
     
     void stop() override;
     bool isServer() const override;

server/Main.cpp

@@ -1,12 +1,9 @@
-#include <iostream>
 #include "server/GameServer.h"
 
-using namespace std;
-
 int main(int argc, char** argv)
 {
     GameServer server;
-    server.start(25565, 10);
+    server.start(25565, 2);
     return 0;
 }
 

server/commands/CommandManager.cpp

@@ -1,3 +1,5 @@
+#include <iostream>
+
 #include "server/commands/CommandManager.h"
 #include "server/commands/CommandUtils.h"
 #include "server/commands/GeneralCommands.h"
@@ -8,30 +10,26 @@ CommandManager::CommandManager()
     registerCommand("stop", GeneralCommands::stop);
 }
 
-CommandManager::~CommandManager()
-{
-}
-
-void CommandManager::registerCommand(const string& name, void (*command) (IGameServer& gs, ICommandSource&, vector<string>&))
+void CommandManager::registerCommand(const std::string& name, void (*command) (IGameServer& gs, ICommandSource&, const std::vector<std::string>&))
 {
     commands[name] = command;
 }
 
-void CommandManager::execute(IGameServer& gs, ICommandSource& cs, const string& rawCommand) const
+void CommandManager::execute(IGameServer& gs, ICommandSource& cs, const std::string& rawCommand) const
 {
-    vector<string> args;
+    std::vector<std::string> args;
     
-    string command;
+    std::string command;
     if(CommandUtils::splitString(rawCommand, command, args))
     {
-        cout << "Invalid command syntax: '" << rawCommand << "'" << endl;
+        std::cout << "Invalid command syntax: '" << rawCommand << "'" << std::endl;
         return;
     }
 
     if(commands.find(command) == commands.end())
     {
-        cout << "Unknown command: '" << command << "'" << endl;
+        std::cout << "Unknown command: '" << command << "'" << std::endl;
         return;
     }
     commands.begin()->second(gs, cs, args);
-}
+}

server/commands/CommandManager.h

@@ -1,26 +1,22 @@
 #ifndef COMMANDMANAGER_H
 #define COMMANDMANAGER_H
 
-#include <iostream>
 #include <unordered_map>
 #include <vector>
 
 #include "server/commands/ICommandSource.h"
 
-using namespace std;
-
 class CommandManager
 {
 public:
     CommandManager();
-    virtual ~CommandManager();
     
-    void execute(IGameServer& gs, ICommandSource& cs, const string& rawCommand) const;
+    void execute(IGameServer& gs, ICommandSource& cs, const std::string& rawCommand) const;
 
 private:
-    unordered_map<string, void (*) (IGameServer& gs, ICommandSource&, vector<string>&)> commands;
+    std::unordered_map<std::string, void (*) (IGameServer& gs, ICommandSource&, const std::vector<std::string>&)> commands;
     
-    void registerCommand(const string& name, void (*command) (IGameServer& gs, ICommandSource&, vector<string>&));
+    void registerCommand(const std::string& name, void (*command) (IGameServer& gs, ICommandSource&, const std::vector<std::string>&));
 };
 
 #endif

server/commands/CommandUtils.cpp

@@ -1,7 +1,6 @@
 #include "server/commands/CommandUtils.h"
-#include <iostream>
 
-static bool splitStringIntern(const string& rawCommand, string& command, vector<string>& args)
+static bool splitStringIntern(const std::string& rawCommand, std::string& command, std::vector<std::string>& args)
 {
     unsigned long old = 0;
     unsigned long index = 0;
@@ -76,7 +75,7 @@ static bool splitStringIntern(const string& rawCommand, string& command, vector<
     return false;
 }
 
-bool CommandUtils::splitString(const string& rawCommand, string& command, vector<string>& args)
+bool CommandUtils::splitString(const std::string& rawCommand, std::string& command, std::vector<std::string>& args)
 {
     bool b = splitStringIntern(rawCommand, command, args);
     if(b)

server/commands/CommandUtils.h

@@ -4,11 +4,9 @@
 #include <string>
 #include <vector>
 
-using namespace std;
-
 namespace CommandUtils
 {
-    bool splitString(const string& rawCommand, string& command, vector<string>& args);
+    bool splitString(const std::string& rawCommand, std::string& command, std::vector<std::string>& args);
 }
 
 #endif

server/commands/GeneralCommands.cpp

@@ -1,16 +1,18 @@
+#include <iostream>
+
 #include "server/commands/GeneralCommands.h"
 
-void GeneralCommands::test(IGameServer& gs, ICommandSource& cs, vector<string>& args)
+void GeneralCommands::test(IGameServer& gs, ICommandSource& cs, const std::vector<std::string>& args)
 {
-    cout << "test command" << endl;
+    std::cout << "test command" << std::endl;
     for(unsigned long i = 0; i < args.size(); i++)
     {
-        cout << " - " << args[i] << endl;
+        std::cout << " - " << args[i] << std::endl;
     }
 }
 
-void GeneralCommands::stop(IGameServer& gs, ICommandSource& cs, vector<string>& args)
+void GeneralCommands::stop(IGameServer& gs, ICommandSource& cs, const std::vector<std::string>& args)
 {
-    cout << cs.isServer() << endl;
+    std::cout << cs.isServer() << std::endl;
     gs.stop();
 }

server/commands/GeneralCommands.h

@@ -2,17 +2,15 @@
 #define GENERALCOMMANDS_H
 
 #include <vector>
-#include <iostream>
+#include <string>
 
 #include "server/commands/ICommandSource.h"
 
-using namespace std;
-
 namespace GeneralCommands
 {
-    void test(IGameServer& gs, ICommandSource& cs, vector<string>& args);
-    void stop(IGameServer& gs, ICommandSource& cs, vector<string>& args);
-};
+    void test(IGameServer& gs, ICommandSource& cs, const std::vector<std::string>& args);
+    void stop(IGameServer& gs, ICommandSource& cs, const std::vector<std::string>& args);
+}
 
 #endif
 

server/network/IServerListener.h

@@ -7,10 +7,10 @@ class IServerListener
 {
 public:
     virtual ~IServerListener() = default;
-    virtual void onFullServerClientConnect(int socket) = 0;
-    virtual void onClientConnect(int socket) = 0;
-    virtual void onClientPackage(int socket, Stream& in) = 0;
-    virtual void onClientDisconnect(int socket) = 0;
+    virtual void onFullServerClientConnect(int socket) const = 0;
+    virtual void onClientConnect(int socket) const = 0;
+    virtual void onClientPackage(int socket, Stream& in) const = 0;
+    virtual void onClientDisconnect(int socket) const = 0;
 };
 
 #endif

server/network/Server.cpp

@@ -1,208 +1,245 @@
-#include "server/network/Server.h"
 #include <iostream>
-#include <sys/socket.h>
-#include <cstdio>
 #include <cstring>
-#include <stdexcept>
+
+#include <sys/socket.h>
 #include <netinet/in.h>
 #include <unistd.h>
-#include <arpa/inet.h>
 #include <poll.h>
-#include <time.h>
-#include <signal.h>
-#include "common/stream/Stream.h"
 
-using namespace std;
+#include "server/network/Server.h"
 
-Server::Server(unsigned short port, unsigned short maxClients) : port(port), maxClients(maxClients) 
+// the empty function ensures the thread is joinable
+Server::ConnectedClient::ConnectedClient() : th([]() {}), socket(-1)
 {
-    try
-    {
-        // create socket for incoming connections
-        listenerSocket = socket(AF_INET, SOCK_STREAM, 0);
-        if(listenerSocket == -1)
-        {
-            throw runtime_error(string("cannot create socket: ") + strerror(errno));
-        }
-
-        // prevents clients from blocking the port if the server exits
-        struct linger sl;
-        sl.l_onoff = 1;
-        sl.l_linger = 0;
-        if(setsockopt(listenerSocket, SOL_SOCKET, SO_LINGER, &sl, sizeof(struct linger)) == -1)
-        {
-            throw runtime_error(string("cannot set non lingering: ") + strerror(errno));
-        }
-
-        // specifies data of the port ...
-        struct sockaddr_in connectSocketData;
-        memset(&connectSocketData, 0, sizeof(struct sockaddr_in));
-        connectSocketData.sin_family = AF_INET;
-        connectSocketData.sin_addr.s_addr = INADDR_ANY;
-        connectSocketData.sin_port = htons(port);
-        // ... and binds it
-        if(bind(listenerSocket, (struct sockaddr*) &connectSocketData, sizeof(struct sockaddr_in)) == -1)
-        {
-            throw runtime_error(string("cannot bind socket: ") + strerror(errno));
-        }
+}
 
-        // mark this socket as handler for connection requests
-        if(listen(listenerSocket, 5) != 0)
-        {
-            throw runtime_error(string("cannot start listening: ") + strerror(errno));
-        }
-        
-        // array for client connections, pointer to pointer to change placement
-        clients = new ConnectedClient*[maxClients];
-        for(int i = 0; i < maxClients; i++)
-        {
-            clients[i] = nullptr;
-        }
+Server::Server(uint16_t port, uint16_t maxClients, const IServerListener& listener) : 
+    shouldRun(false), port(port), maxClients(maxClients), serverListener(listener), 
+    listenerSocket(-1), clientAmount(0), clients(nullptr)
+{
+    // create socket for incoming connections
+    // domain - AF_INET - IPv4 Internet protocols
+    // type - SOCK_STREAM - two-way, connection-based byte streams
+    // protocol - 0 - use standard protocol for the given socket type
+    listenerSocket = socket(AF_INET, SOCK_STREAM, 0);
+    if(listenerSocket == -1)
+    {
+        printError("cannot create listener socket");
+        return;
+    }
+    
+    // prevents clients from blocking the port if the server exits
+    // this is useful if server and client run on the same system
+    struct linger sl;
+    sl.l_onoff = 1; // nonzero to linger on close
+    sl.l_linger = 0; // time to linger
+    // sockfd - listenerSocket - modified socket
+    // level - SOL_SOCKET - manipulate options at the sockets API level
+    // optname - SO_LINGER - identifier of the option
+    if(setsockopt(listenerSocket, SOL_SOCKET, SO_LINGER, &sl, sizeof(struct linger)) == -1)
+    {
+        printError("cannot set non lingering");
+        return;
     }
-    catch(runtime_error& err)
+    
+    // specify binding data
+    struct sockaddr_in connectSocketData;
+    // clear padding
+    memset(&connectSocketData, 0, sizeof(struct sockaddr_in));
+    // IPv4 Internet protocols
+    connectSocketData.sin_family = AF_INET; 
+    // port in network byte order
+    connectSocketData.sin_port = htons(port); 
+    // address in network byte order, accept any incoming messages
+    connectSocketData.sin_addr.s_addr = htons(INADDR_ANY); 
+    // bind the socket
+    if(bind(listenerSocket, (struct sockaddr*) &connectSocketData, sizeof(struct sockaddr_in)) == -1)
     {
-        clean();
-        throw err; 
+        printError("cannot bind listener socket");
+        return;
+    }
+    
+    // mark the socket as handler for connection requests
+    // backlog - 5 - max queue length of pending connections
+    if(listen(listenerSocket, 5) == -1)
+    {
+        printError("cannot start listening on socket");
+        return;
     }
+    
+    shouldRun = true;
+    listenerThread = std::thread(&Server::listenForClients, this);
+    
+    clients = new ConnectedClient[maxClients];
 }
 
 Server::~Server()
 {
-    clean();
-}
-
-void Server::clean()
-{
+    shouldRun = false;
+    listenerThread.join();
+    
     if(listenerSocket != -1)
     {
-        // ignore error
-        close(listenerSocket);
+        if(close(listenerSocket) == -1)
+        {
+            printError("cannot close listener socket");
+        }
     }
+    
     if(clients != nullptr)
     {
-        for(int i = 0; i < maxClients; i++)
+        for(uint16_t i = 0; i < maxClients; i++)
         {
-            if(clients[i] != nullptr)
+            if(clients[i].socket != -1)
             {
-                if(clients[i]->socket != -1)
+                if(close(clients[i].socket) == -1)
                 {
-                    close(clients[i]->socket);
+                    printError("cannot close client socket");
                 }
-                delete clients[i];
+            }
+            if(clients[i].th.joinable())
+            {
+                clients[i].th.join();
+            }
+            else
+            {
+                std::cout << "cannot join client connection thread " << std::endl;
             }
         }
         delete[] clients;
     }
 }
 
-void Server::start(IServerListener* listener)
+bool Server::isRunning() const
 {
-    serverListener = listener;
-    shouldRun = true;
-    listenerThread = thread(&Server::listenForClients, this);
+    return shouldRun;
 }
 
-void Server::stop()
+void Server::printError(const char* message) const
 {
-    shouldRun = false;
-    
-    listenerThread.join();
-    for(int i = 0; i < maxClients; i++)
-    {
-        if(clients[i] != nullptr)
-        {
-            clients[i]->th.join();
-        }
-    }
+    std::cout << message << ": " << strerror(errno) << std::endl;
 }
 
 void Server::listenForClients()
 {
     while(shouldRun)
     {
+        // wait until a connection arrives with timeout, this prevents being 
+        // stuck in accept
         struct pollfd fds;
-        fds.fd = listenerSocket;
-        fds.events = POLLIN;
-        fds.revents = 0;
+        fds.fd = listenerSocket; // file descriptor for polling
+        fds.events = POLLIN; // wait until data is ready to read
+        fds.revents = 0; // return events - none
+        // nfds_t - 1 - amount of passed in structs
+        // timeout - 100 - milliseconds to wait until an event occurs
+        // returns 0 on timeout, -1 on error, and >0 on success
         int pollData = poll(&fds, 1, 100);
         if(pollData > 0)
         {
             struct sockaddr_in clientSocketData;
+            // accepts an incoming client connection and stores the data in the
+            // given struct, returns a nonnegative file descriptor on success
             socklen_t addrlen = sizeof(struct sockaddr_in);
             int clientSocket = accept(listenerSocket, (struct sockaddr*) &clientSocketData, &addrlen);
             if(clientSocket >= 0)
             {
-                //cout << "Client connected from " << inet_ntoa(clientSocketData.sin_addr) << ":" << (int) ntohs(clientSocketData.sin_port) << endl;
-                
-                clientMutex.lock();
-                addClient(clientSocket);
-                clientMutex.unlock();
+                //std::cout << inet_ntoa(clientSocketData.sin_addr) << ":" << (int) ntohs(clientSocketData.sin_port) << std::endl;
+                if(addClientThreadsafe(clientSocket))
+                {
+                    if(close(clientSocket) == -1)
+                    {
+                        printError("cannot close client socket");
+                    }
+                }
             }
             else
             {
+                printError("accept error");
                 break;
             }
         }
         else if(pollData == -1)
         {
-            cerr << "poll error: " << strerror(errno) << endl;
+            printError("poll error");
+            break;
         }
     }
 }
 
-void Server::addClient(int clientSocket)
+bool Server::addClientThreadsafe(int clientSocket)
+{
+    clientMutex.lock();
+    bool b = addClient(clientSocket);
+    clientMutex.unlock();
+    return b;
+}
+
+bool Server::addClient(int clientSocket)
 {
-    if(clientIndex >= maxClients)
+    if(clientAmount >= maxClients)
     {
-        serverListener->onFullServerClientConnect(clientSocket);
-        close(clientSocket);
+        serverListener.onFullServerClientConnect(clientSocket);
+        return true;
     }
     else
     {
-        if(clients[clientIndex] == nullptr)
+        // search for free slot
+        uint16_t index = 0;
+        while(index < maxClients)
         {
-            clients[clientIndex] = new ConnectedClient();
-        }
-        else
-        {
-            //ensure old thread has ended
-            if(!clients[clientIndex]->th.joinable())
+            if(clients[index].socket == -1)
             {
-                cerr << "cannot join thread of non used client connection" << endl;
-                close(clientSocket);
-                return;
+                break;
             }
-            clients[clientIndex]->th.join();
+            index++;
         }
         
-        clients[clientIndex]->index = clientIndex;
-        clients[clientIndex]->socket = clientSocket;
-        clients[clientIndex]->th = thread(&Server::listenOnClient, this, clients[clientIndex]);
+        if(index >= maxClients)
+        {
+            std::cout << "cannot find free slot - even if there should be one" << std::endl;
+            return true;
+        }
 
-        clientIndex++;
+        //ensure old thread has ended
+        if(!clients[index].th.joinable())
+        {
+            std::cout << "cannot join thread of non used client connection" << std::endl;
+            return true;
+        }
+        clients[index].th.join();
+
+        clients[index].socket = clientSocket;
+        clients[index].th = std::thread(&Server::listenOnClient, this, std::ref(clients[index]));
+
+        clientAmount++;
+
+        return false;
     }
 }
 
-void Server::listenOnClient(ConnectedClient* cc)
+void Server::listenOnClient(ConnectedClient& cc)
 {
+    // poll data
     struct pollfd fds;
-    fds.fd = cc->socket;
-    fds.events = POLLIN;
-    fds.revents = 0;
+    fds.fd = cc.socket; // file descriptor for polling
+    fds.events = POLLIN; // wait until data is ready to read
+    fds.revents = 0; // return events - none
     
-    serverListener->onClientConnect(cc->socket);
+    serverListener.onClientConnect(cc.socket);
     
     Stream st;
-    
     while(shouldRun)
     {
+        // nfds_t - 1 - amount of passed in structs
+        // timeout - 100 - milliseconds to wait until an event occurs
+        // returns 0 on timeout, -1 on error, and >0 on success
         int pollData = poll(&fds, 1, 100);
         if(pollData > 0)
         {
-            st.readSocket(cc->socket);
+            st.readSocket(cc.socket);
             if(st.hasData())
             {
-                serverListener->onClientPackage(cc->socket, st);
+                serverListener.onClientPackage(cc.socket, st);
             }
             else
             {
@@ -212,39 +249,20 @@ void Server::listenOnClient(ConnectedClient* cc)
         }
         else if(pollData == -1)
         {
-            cout << "poll error: " << strerror(errno) << endl;
+            printError("cannot poll from client");
+            break;
         }
     }
     
-    serverListener->onClientDisconnect(cc->socket);
+    serverListener.onClientDisconnect(cc.socket);
     
-    // do not swap on server shutdown
-    if(!shouldRun)
-    {
-        return;
-    }
-    
-    // remove client from list, move last client to empty slot
+    // reset slot for another client
     clientMutex.lock();
-    
-    clientIndex--;
-    if(cc->index != clientIndex) // client is not the last connected client
+    if(close(cc.socket) == -1)
     {
-        // move last element to empty slot
-        ConnectedClient* tmp = clients[clientIndex];
-        clients[clientIndex] = clients[cc->index];
-        clients[cc->index] = tmp;
-        // set indices
-        int i = cc->index;
-        clients[i]->index = i;
-        clients[clientIndex]->index = clientIndex;
+        perror("cannot close socket of client");
     }
-    
-    if(close(cc->socket) == -1)
-    {
-        cerr << "cannot close socket of client: " << strerror(errno) << endl;
-    }
-    cc->socket = -1;
-    
+    cc.socket = -1;
+    clientAmount--;
     clientMutex.unlock();
-}
+}

server/network/Server.h

@@ -3,45 +3,45 @@
 
 #include <thread>
 #include <mutex>
-#include "server/network/IServerListener.h"
 
-using namespace std;
+#include "server/network/IServerListener.h"
 
 class Server
 {
+private:
+    struct ConnectedClient
+    {
+        ConnectedClient();
+        std::thread th;
+        int socket;
+    };
+    
 public:
-    Server(unsigned short port, unsigned short maxClients);
+    Server(uint16_t port, uint16_t maxClients, const IServerListener& listener);
     virtual ~Server();
-    
-    void start(IServerListener* listener);
-    void stop();
+
+    bool isRunning() const;
     
 private:
+    void printError(const char* message) const;
     void listenForClients();
-    void clean();
-    void addClient(int clientSocket);
+    bool addClientThreadsafe(int clientSocket);
+    bool addClient(int clientSocket);
+    void listenOnClient(ConnectedClient& cc);
     
-    unsigned short port;
-    unsigned short maxClients;
+    volatile bool shouldRun;
     
-    struct ConnectedClient
-    {
-        thread th;
-        int socket;
-        unsigned int index;
-    };
-    unsigned int clientIndex = 0;
-    mutex clientMutex;
-    ConnectedClient** clients = nullptr;
+    uint16_t port;
+    uint16_t maxClients;
     
-    void listenOnClient(ConnectedClient* cc);
+    const IServerListener& serverListener;
     
-    volatile bool shouldRun = false;
     int listenerSocket;
-    thread listenerThread;
+    std::thread listenerThread;
     
-    IServerListener* serverListener = nullptr;
+    uint16_t clientAmount;
+    ConnectedClient* clients;
+    std::mutex clientMutex;
 };
 
-#endif
-
+#endif