Connection::Receive(): receive Network Packet instead of SharedBuffer<u8>.

Because we get a Buffer<u8> from ConnectionEvent, don't convert it to SharedBuffer<u8> and return it to Server/Client::Receive which will convert it to NetworkPacket
Instead, put the Buffer<u8> directly to NetworkPacket and return it to packet processing
This remove a long existing memory copy
Also check the packet size directly into Connection::Receive instead of packet processing

diff --git a/src/client.cpp b/src/client.cpp
index 611a73bb1a36fd1daafba49bf418fa02c44adbe4..dc2b54e9b5c736fd40b71552f6e8dd3c060bd561 100644 (file)
--- a/src/client.cpp
+++ b/src/client.cpp
@@ -834,10 +834,9 @@ void Client::ReceiveAll()
 void Client::Receive()
 {
        DSTACK(__FUNCTION_NAME);
-       SharedBuffer<u8> data;
-       u16 sender_peer_id;
-       u32 datasize = m_con.Receive(sender_peer_id, data);
-       ProcessData(*data, datasize, sender_peer_id);
+       NetworkPacket pkt;
+       m_con.Receive(&pkt);
+       ProcessData(&pkt);
 }
 
 inline void Client::handleCommand(NetworkPacket* pkt)
@@ -849,19 +848,12 @@ inline void Client::handleCommand(NetworkPacket* pkt)
 /*
        sender_peer_id given to this shall be quaranteed to be a valid peer
 */
-void Client::ProcessData(u8 *data, u32 datasize, u16 sender_peer_id)
+void Client::ProcessData(NetworkPacket *pkt)
 {
        DSTACK(__FUNCTION_NAME);
 
-       // Ignore packets that don't even fit a command
-       if(datasize < 2) {
-               m_packetcounter.add(60000);
-               return;
-       }
-
-       NetworkPacket pkt(data, datasize, sender_peer_id);
-
-       ToClientCommand command = (ToClientCommand) pkt.getCommand();
+       ToClientCommand command = (ToClientCommand) pkt->getCommand();
+       u32 sender_peer_id = pkt->getPeerId();
 
        //infostream<<"Client: received command="<<command<<std::endl;
        m_packetcounter.add((u16)command);
@@ -889,7 +881,7 @@ void Client::ProcessData(u8 *data, u32 datasize, u16 sender_peer_id)
         * as a byte mask
         */
        if(toClientCommandTable[command].state == TOCLIENT_STATE_NOT_CONNECTED) {
-               handleCommand(&pkt);
+               handleCommand(pkt);
                return;
        }
 
@@ -904,7 +896,7 @@ void Client::ProcessData(u8 *data, u32 datasize, u16 sender_peer_id)
          Handle runtime commands
        */
 
-       handleCommand(&pkt);
+       handleCommand(pkt);
 }
 
 void Client::Send(NetworkPacket* pkt)

diff --git a/src/client.h b/src/client.h
index 9db9764ddd3485a2ce7a725cdf085b18702ab88d..7ec405fa6b3c9bf2842fcfe2f7a04cc5b283d0c1 100644 (file)
--- a/src/client.h
+++ b/src/client.h
@@ -392,7 +392,7 @@ public:
        void handleCommand_LocalPlayerAnimations(NetworkPacket* pkt);
        void handleCommand_EyeOffset(NetworkPacket* pkt);
 
-       void ProcessData(u8 *data, u32 datasize, u16 sender_peer_id);
+       void ProcessData(NetworkPacket *pkt);
 
        // Returns true if something was received
        bool AsyncProcessPacket();

diff --git a/src/network/connection.cpp b/src/network/connection.cpp
index b808c3ab60d7f0e6553b7e0955a97ae0c878cf30..dd69df5bbf46d1527522a58ecf3f8937baec3508 100644 (file)
--- a/src/network/connection.cpp
+++ b/src/network/connection.cpp
@@ -24,6 +24,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "serialization.h"
 #include "log.h"
 #include "porting.h"
+#include "network/networkpacket.h"
 #include "util/serialize.h"
 #include "util/numeric.h"
 #include "util/string.h"
@@ -2884,30 +2885,36 @@ void Connection::Disconnect()
        putCommand(c);
 }
 
-u32 Connection::Receive(u16 &peer_id, SharedBuffer<u8> &data)
+void Connection::Receive(NetworkPacket* pkt)
 {
        for(;;) {
                ConnectionEvent e = waitEvent(m_bc_receive_timeout);
                if (e.type != CONNEVENT_NONE)
-                       LOG(dout_con<<getDesc()<<": Receive: got event: "
-                                       <<e.describe()<<std::endl);
+                       LOG(dout_con << getDesc() << ": Receive: got event: "
+                                       << e.describe() << std::endl);
                switch(e.type) {
                case CONNEVENT_NONE:
                        throw NoIncomingDataException("No incoming data");
                case CONNEVENT_DATA_RECEIVED:
-                       peer_id = e.peer_id;
-                       data = SharedBuffer<u8>(e.data);
-                       return e.data.getSize();
+                       // Data size is lesser than command size, ignoring packet
+                       if (e.data.getSize() < 2) {
+                               continue;
+                       }
+
+                       pkt->putRawPacket(*e.data, e.data.getSize(), e.peer_id);
+                       return;
                case CONNEVENT_PEER_ADDED: {
                        UDPPeer tmp(e.peer_id, e.address, this);
                        if (m_bc_peerhandler)
                                m_bc_peerhandler->peerAdded(&tmp);
-                       continue; }
+                       continue;
+               }
                case CONNEVENT_PEER_REMOVED: {
                        UDPPeer tmp(e.peer_id, e.address, this);
                        if (m_bc_peerhandler)
                                m_bc_peerhandler->deletingPeer(&tmp, e.timeout);
-                       continue; }
+                       continue;
+               }
                case CONNEVENT_BIND_FAILED:
                        throw ConnectionBindFailed("Failed to bind socket "
                                        "(port already in use?)");

diff --git a/src/network/connection.h b/src/network/connection.h
index 9c920cc015470c59312c32ae5bc13838c2a875dc..f60c6625702b5229dafc0828a5fdf5f30ef29bf9 100644 (file)
--- a/src/network/connection.h
+++ b/src/network/connection.h
@@ -34,6 +34,8 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include <list>
 #include <map>
 
+class NetworkPacket;
+
 namespace con
 {
 
@@ -1025,7 +1027,7 @@ public:
        void Connect(Address address);
        bool Connected();
        void Disconnect();
-       u32 Receive(u16 &peer_id, SharedBuffer<u8> &data);
+       void Receive(NetworkPacket* pkt);
        void Send(u16 peer_id, u8 channelnum, NetworkPacket* pkt, bool reliable);
        u16 GetPeerID() { return m_peer_id; }
        Address GetPeerAddress(u16 peer_id);

diff --git a/src/network/networkpacket.cpp b/src/network/networkpacket.cpp
index 85d39d91d679fa05b7a2962f60ccd94143a56afe..d7487af40955ad730d9699ec9e6074e29681eb95 100644 (file)
--- a/src/network/networkpacket.cpp
+++ b/src/network/networkpacket.cpp
@@ -22,17 +22,6 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "exceptions.h"
 #include "util/serialize.h"
 
-NetworkPacket::NetworkPacket(u8 *data, u32 datasize, u16 peer_id):
-m_read_offset(0), m_peer_id(peer_id)
-{
-       m_read_offset = 0;
-       m_datasize = datasize - 2;
-
-       // split command and datas
-       m_command = readU16(&data[0]);
-       m_data = std::vector<u8>(&data[2], &data[2 + m_datasize]);
-}
-
 NetworkPacket::NetworkPacket(u16 command, u32 datasize, u16 peer_id):
 m_datasize(datasize), m_read_offset(0), m_command(command), m_peer_id(peer_id)
 {
@@ -50,6 +39,20 @@ NetworkPacket::~NetworkPacket()
        m_data.clear();
 }
 
+void NetworkPacket::putRawPacket(u8 *data, u32 datasize, u16 peer_id)
+{
+       // If a m_command is already set, we are rewriting on same packet
+       // This is not permitted
+       assert(m_command == 0);
+
+       m_datasize = datasize - 2;
+       m_peer_id = peer_id;
+
+       // split command and datas
+       m_command = readU16(&data[0]);
+       m_data = std::vector<u8>(&data[2], &data[2 + m_datasize]);
+}
+
 char* NetworkPacket::getString(u32 from_offset)
 {
        if (from_offset >= m_datasize)

diff --git a/src/network/networkpacket.h b/src/network/networkpacket.h
index 4a801b44426b81d0cd60e4f9fa83732852860b63..0afb1e7e351e95cd620bf4088b70af63e9608311 100644 (file)
--- a/src/network/networkpacket.h
+++ b/src/network/networkpacket.h
@@ -28,11 +28,14 @@ class NetworkPacket
 {
 
 public:
-               NetworkPacket(u8 *data, u32 datasize, u16 peer_id);
                NetworkPacket(u16 command, u32 datasize, u16 peer_id);
                NetworkPacket(u16 command, u32 datasize);
+               NetworkPacket(): m_datasize(0), m_read_offset(0), m_command(0),
+                               m_peer_id(0) {}
                ~NetworkPacket();
 
+               void putRawPacket(u8 *data, u32 datasize, u16 peer_id);
+
                // Getters
                u32 getSize() { return m_datasize; }
                u16 getPeerId() { return m_peer_id; }

diff --git a/src/server.cpp b/src/server.cpp
index 3b4c3c3f0ce24e58e73a3548140b843fc9fab3e5..144d933b17c57f8156e46366ad624e1e19c0f4dd 100644 (file)
--- a/src/server.cpp
+++ b/src/server.cpp
@@ -1018,10 +1018,11 @@ void Server::Receive()
        DSTACK(__FUNCTION_NAME);
        SharedBuffer<u8> data;
        u16 peer_id;
-       u32 datasize;
        try {
-               datasize = m_con.Receive(peer_id,data);
-               ProcessData(*data, datasize, peer_id);
+               NetworkPacket pkt;
+               m_con.Receive(&pkt);
+               peer_id = pkt.getPeerId();
+               ProcessData(&pkt);
        }
        catch(con::InvalidIncomingDataException &e) {
                infostream<<"Server::Receive(): "
@@ -1149,13 +1150,14 @@ inline void Server::handleCommand(NetworkPacket* pkt)
        (this->*opHandle.handler)(pkt);
 }
 
-void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
+void Server::ProcessData(NetworkPacket *pkt)
 {
        DSTACK(__FUNCTION_NAME);
        // Environment is locked first.
        JMutexAutoLock envlock(m_env_mutex);
 
        ScopeProfiler sp(g_profiler, "Server::ProcessData");
+       u32 peer_id = pkt->getPeerId();
 
        try {
                Address address = getPeerAddress(peer_id);
@@ -1179,18 +1181,13 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
                 * respond for some time, your server was overloaded or
                 * things like that.
                 */
-               infostream << "Server::ProcessData(): Cancelling: peer "
+               infostream << "Server::ProcessData(): Canceling: peer "
                                << peer_id << " not found" << std::endl;
                return;
        }
 
        try {
-               if(datasize < 2)
-                       return;
-
-               NetworkPacket pkt(data, datasize, peer_id);
-
-               ToServerCommand command = (ToServerCommand) pkt.getCommand();
+               ToServerCommand command = (ToServerCommand) pkt->getCommand();
 
                // Command must be handled into ToServerCommandHandler
                if (command >= TOSERVER_NUM_MSG_TYPES) {
@@ -1199,7 +1196,7 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
                }
 
                if (toServerCommandTable[command].state == TOSERVER_STATE_NOT_CONNECTED) {
-                       handleCommand(&pkt);
+                       handleCommand(pkt);
                        return;
                }
 
@@ -1214,7 +1211,7 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
 
                /* Handle commands related to client startup */
                if (toServerCommandTable[command].state == TOSERVER_STATE_STARTUP) {
-                       handleCommand(&pkt);
+                       handleCommand(pkt);
                        return;
                }
 
@@ -1227,7 +1224,7 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
                        return;
                }
 
-               handleCommand(&pkt);
+               handleCommand(pkt);
        }
        catch(SendFailedException &e) {
                errorstream << "Server::ProcessData(): SendFailedException: "

diff --git a/src/server.h b/src/server.h
index 0c0a5f91c0022ec8f915ca39d9af5b40471ea1de..a584cbe5ad240be363eabb506bd870f59573b6b9 100644 (file)
--- a/src/server.h
+++ b/src/server.h
@@ -219,7 +219,7 @@ public:
        void handleCommand_NodeMetaFields(NetworkPacket* pkt);
        void handleCommand_InventoryFields(NetworkPacket* pkt);
 
-       void ProcessData(u8 *data, u32 datasize, u16 peer_id);
+       void ProcessData(NetworkPacket *pkt);
 
        void Send(NetworkPacket* pkt);
 

diff --git a/src/test.cpp b/src/test.cpp
index 32a6b102f9a4a715ea579aae2352e4a85bf8cc98..25809327802b8d0061b978562d3a43c0895a8ed2 100644 (file)
--- a/src/test.cpp
+++ b/src/test.cpp
@@ -1939,13 +1939,12 @@ struct TestConnection: public TestBase
 
                try
                {
-                       u16 peer_id;
-                       SharedBuffer<u8> data;
-                       infostream<<"** running client.Receive()"<<std::endl;
-                       u32 size = client.Receive(peer_id, data);
-                       infostream<<"** Client received: peer_id="<<peer_id
-                                       <<", size="<<size
-                                       <<std::endl;
+                       NetworkPacket pkt;
+                       infostream << "** running client.Receive()" << std::endl;
+                       client.Receive(&pkt);
+                       infostream << "** Client received: peer_id=" << pkt.getPeerId()
+                                       << ", size=" << pkt.getSize()
+                                       << std::endl;
                }
                catch(con::NoIncomingDataException &e)
                {
@@ -1961,13 +1960,12 @@ struct TestConnection: public TestBase
 
                try
                {
-                       u16 peer_id;
-                       SharedBuffer<u8> data;
-                       infostream<<"** running server.Receive()"<<std::endl;
-                       u32 size = server.Receive(peer_id, data);
-                       infostream<<"** Server received: peer_id="<<peer_id
-                                       <<", size="<<size
-                                       <<std::endl;
+                       NetworkPacket pkt;
+                       infostream << "** running server.Receive()" << std::endl;
+                       server.Receive(&pkt);
+                       infostream<<"** Server received: peer_id=" << pkt.getPeerId()
+                                       << ", size=" << pkt.getSize()
+                                       << std::endl;
                }
                catch(con::NoIncomingDataException &e)
                {
@@ -1988,13 +1986,12 @@ struct TestConnection: public TestBase
                {
                        try
                        {
-                               u16 peer_id;
-                               SharedBuffer<u8> data;
-                               infostream<<"** running client.Receive()"<<std::endl;
-                               u32 size = client.Receive(peer_id, data);
-                               infostream<<"** Client received: peer_id="<<peer_id
-                                               <<", size="<<size
-                                               <<std::endl;
+                               NetworkPacket pkt;
+                               infostream << "** running client.Receive()" << std::endl;
+                               client.Receive(&pkt);
+                               infostream << "** Client received: peer_id=" << pkt.getPeerId()
+                                               << ", size=" << pkt.getSize()
+                                               << std::endl;
                        }
                        catch(con::NoIncomingDataException &e)
                        {
@@ -2006,13 +2003,12 @@ struct TestConnection: public TestBase
 
                try
                {
-                       u16 peer_id;
-                       SharedBuffer<u8> data;
-                       infostream<<"** running server.Receive()"<<std::endl;
-                       u32 size = server.Receive(peer_id, data);
-                       infostream<<"** Server received: peer_id="<<peer_id
-                                       <<", size="<<size
-                                       <<std::endl;
+                       NetworkPacket pkt;
+                       infostream << "** running server.Receive()" << std::endl;
+                       server.Receive(&pkt);
+                       infostream << "** Server received: peer_id=" << pkt.getPeerId()
+                                       << ", size=" << pkt.getSize()
+                                       << std::endl;
                }
                catch(con::NoIncomingDataException &e)
                {
@@ -2022,24 +2018,26 @@ struct TestConnection: public TestBase
                        Simple send-receive test
                */
                {
-                       NetworkPacket pkt((u8*) "Hello World !", 14, 0);
+                       NetworkPacket pkt;
+                       pkt.putRawPacket((u8*) "Hello World !", 14, 0);
 
-                       SharedBuffer<u8> sentdata = pkt.oldForgePacket();
+                       Buffer<u8> sentdata = pkt.oldForgePacket();
 
                        infostream<<"** running client.Send()"<<std::endl;
                        client.Send(PEER_ID_SERVER, 0, &pkt, true);
 
                        sleep_ms(50);
 
-                       u16 peer_id;
-                       SharedBuffer<u8> recvdata;
+                       NetworkPacket recvpacket;
                        infostream << "** running server.Receive()" << std::endl;
-                       u32 size = server.Receive(peer_id, recvdata);
-                       infostream << "** Server received: peer_id=" << peer_id
-                                       << ", size=" << size
+                       server.Receive(&recvpacket);
+                       infostream << "** Server received: peer_id=" << pkt.getPeerId()
+                                       << ", size=" << pkt.getSize()
                                        << ", data=" << (const char*)pkt.getU8Ptr(0)
                                        << std::endl;
 
+                       Buffer<u8> recvdata = pkt.oldForgePacket();
+
                        UASSERT(memcmp(*sentdata, *recvdata, recvdata.getSize()) == 0);
                }
 
@@ -2061,29 +2059,33 @@ struct TestConnection: public TestBase
                                snprintf(buf, 10, "%.2X", ((int)((const char*)pkt.getU8Ptr(0))[i])&0xff);
                                infostream<<buf;
                        }
-                       if(datasize>20)
-                               infostream<<"...";
-                       infostream<<std::endl;
+                       if(datasize > 20)
+                               infostream << "...";
+                       infostream << std::endl;
 
-                       SharedBuffer<u8> sentdata = pkt.oldForgePacket();
+                       Buffer<u8> sentdata = pkt.oldForgePacket();
 
                        server.Send(peer_id_client, 0, &pkt, true);
 
                        //sleep_ms(3000);
 
-                       SharedBuffer<u8> recvdata;
-                       infostream<<"** running client.Receive()"<<std::endl;
+                       Buffer<u8> recvdata;
+                       infostream << "** running client.Receive()" << std::endl;
                        u16 peer_id = 132;
                        u16 size = 0;
                        bool received = false;
                        u32 timems0 = porting::getTimeMs();
-                       for(;;){
+                       for(;;) {
                                if(porting::getTimeMs() - timems0 > 5000 || received)
                                        break;
-                               try{
-                                       size = client.Receive(peer_id, recvdata);
+                               try {
+                                       NetworkPacket pkt;
+                                       client.Receive(&pkt);
+                                       size = pkt.getSize();
+                                       peer_id = pkt.getPeerId();
+                                       recvdata = pkt.oldForgePacket();
                                        received = true;
-                               }catch(con::NoIncomingDataException &e){
+                               } catch(con::NoIncomingDataException &e) {
                                }
                                sleep_ms(10);
                        }