From 41f09975ab91d388b462c498c06ad3e59354dfce Mon Sep 17 00:00:00 2001 From: LaurentGom Date: Mon, 5 Apr 2010 14:06:37 +0000 Subject: [PATCH] Packets handling in UdpSocket is now more robust git-svn-id: https://sfml.svn.sourceforge.net/svnroot/sfml/branches/sfml2@1490 4e206d99-4929-0410-ac5d-dfc041789085 --- include/SFML/Network/UdpSocket.hpp | 23 ++++++ src/SFML/Network/TcpSocket.cpp | 14 ++-- src/SFML/Network/UdpSocket.cpp | 123 +++++++++++------------------ 3 files changed, 76 insertions(+), 84 deletions(-) diff --git a/include/SFML/Network/UdpSocket.hpp b/include/SFML/Network/UdpSocket.hpp index 00b0f885a..9f93dd4d9 100644 --- a/include/SFML/Network/UdpSocket.hpp +++ b/include/SFML/Network/UdpSocket.hpp @@ -44,6 +44,14 @@ class SFML_API UdpSocket : public Socket { public : + //////////////////////////////////////////////////////////// + // Constants + //////////////////////////////////////////////////////////// + enum + { + MaxDatagramSize = 65507 ///< The maximum number of bytes that can be sent in a single UDP datagram + }; + //////////////////////////////////////////////////////////// /// \brief Default constructor /// @@ -96,6 +104,10 @@ public : //////////////////////////////////////////////////////////// /// \brief Send raw data to a remote peer /// + /// Make sure that \a size is not greater than + /// UdpSocket::MaxDatagramSize, otherwise this function will + /// fail and no data will be sent. + /// /// \param data Pointer to the sequence of bytes to send /// \param size Number of bytes to send /// \param remoteAddress Address of the receiver @@ -113,6 +125,10 @@ public : /// /// In blocking mode, this function will wait until some /// bytes are actually received. + /// Be careful to use a buffer which is large enough for + /// the data that you intend to receive, if it is too small + /// then an error will be returned and *all* the data will + /// be lost. /// /// \param data Pointer to the array to fill with the received bytes /// \param size Maximum number of bytes that can be received @@ -130,6 +146,11 @@ public : //////////////////////////////////////////////////////////// /// \brief Send a formatted packet of data to a remote peer /// + /// Unlike the other version of Send, this function can accept + /// data sizes greater than UdpSocket::MaxDatagramSize. + /// If necessary, the data will be split and sent in multiple + /// datagrams. + /// /// \param packet Packet to send /// \param remoteAddress Address of the receiver /// \param remotePort Port of the receiver to send the data to @@ -146,6 +167,8 @@ public : /// /// In blocking mode, this function will wait until the whole packet /// has been received. + /// Warning: this functon doesn't properly handle mixed data + /// received from multiple peers. /// /// \param packet Packet to fill with the received data /// \param remoteAddress Address of the peer that sent the data diff --git a/src/SFML/Network/TcpSocket.cpp b/src/SFML/Network/TcpSocket.cpp index 46879a44c..19e90c4bf 100644 --- a/src/SFML/Network/TcpSocket.cpp +++ b/src/SFML/Network/TcpSocket.cpp @@ -208,7 +208,7 @@ Socket::Status TcpSocket::Send(const char* data, std::size_t size) // Check the parameters if (!data || (size == 0)) { - Err() << "Cannot send data over the network (invalid parameters)" << std::endl; + Err() << "Cannot send data over the network (no data to send)" << std::endl; return Error; } @@ -221,7 +221,7 @@ Socket::Status TcpSocket::Send(const char* data, std::size_t size) sent = send(GetHandle(), data + length, sizeToSend - length, 0); // Check for errors - if (sent <= 0) + if (sent < 0) return priv::SocketImpl::GetErrorStatus(); } @@ -235,10 +235,10 @@ Socket::Status TcpSocket::Receive(char* data, std::size_t size, std::size_t& rec // First clear the variables to fill received = 0; - // Check the parameters - if (!data || (size == 0)) + // Check the destination buffer + if (!data) { - Err() << "Cannot receive data from the network (invalid parameters)" << std::endl; + Err() << "Cannot receive data from the network (the destination buffer is invalid)" << std::endl; return Error; } @@ -265,6 +265,10 @@ Socket::Status TcpSocket::Receive(char* data, std::size_t size, std::size_t& rec //////////////////////////////////////////////////////////// Socket::Status TcpSocket::Send(Packet& packet) { + // TCP is a stream protocol, it doesn't preserve messages boundaries. + // This means that we have to send the packet size first, so that the + // receiver knows the actual end of the packet in the data stream. + // Get the data to send from the packet std::size_t size = 0; const char* data = packet.OnSend(size); diff --git a/src/SFML/Network/UdpSocket.cpp b/src/SFML/Network/UdpSocket.cpp index 0c2787934..b0fd8abb7 100644 --- a/src/SFML/Network/UdpSocket.cpp +++ b/src/SFML/Network/UdpSocket.cpp @@ -30,7 +30,7 @@ #include #include #include -#include +#include #include @@ -95,28 +95,23 @@ Socket::Status UdpSocket::Send(const char* data, std::size_t size, const IpAddre // Create the internal socket if it doesn't exist Create(); - // Check the parameters - if (!data || (size == 0)) + // Make sure that all the data will fit in one datagram + if (size > MaxDatagramSize) { - Err() << "Cannot send data over the network (invalid parameters)" << std::endl; + Err() << "Cannot send data over the network " + << "(the number of bytes to send is greater than sf::UdpSocket::MaxDatagramSize)" << std::endl; return Error; } // Build the target address sockaddr_in address = priv::SocketImpl::CreateAddress(remoteAddress.ToInteger(), remotePort); - // Loop until every byte has been sent - int sent = 0; - int sizeToSend = static_cast(size); - for (int length = 0; length < sizeToSend; length += sent) - { - // Send a chunk of data - sent = sendto(GetHandle(), data + length, sizeToSend - length, 0, reinterpret_cast(&address), sizeof(address)); + // Send the data (unlike TCP, all the data is always sent in one call) + int sent = sendto(GetHandle(), data, size, 0, reinterpret_cast(&address), sizeof(address)); - // Check for errors - if (sent <= 0) - return priv::SocketImpl::GetErrorStatus(); - } + // Check for errors + if (sent < 0) + return priv::SocketImpl::GetErrorStatus(); return Done; } @@ -130,10 +125,10 @@ Socket::Status UdpSocket::Receive(char* data, std::size_t size, std::size_t& rec remoteAddress = IpAddress(); remotePort = 0; - // Check the parameters - if (!data || (size == 0)) + // Check the destination buffer + if (!data) { - Err() << "Cannot receive data from the network (invalid parameters)" << std::endl; + Err() << "Cannot receive data from the network (the destination buffer is invalid)" << std::endl; return Error; } @@ -145,7 +140,7 @@ Socket::Status UdpSocket::Receive(char* data, std::size_t size, std::size_t& rec int sizeReceived = recvfrom(GetHandle(), data, static_cast(size), 0, reinterpret_cast(&address), &addressSize); // Check for errors - if (sizeReceived <= 0) + if (sizeReceived < 0) return priv::SocketImpl::GetErrorStatus(); // Fill the sender informations @@ -160,27 +155,29 @@ Socket::Status UdpSocket::Receive(char* data, std::size_t size, std::size_t& rec //////////////////////////////////////////////////////////// Socket::Status UdpSocket::Send(Packet& packet, const IpAddress& remoteAddress, unsigned short remotePort) { + // As the UDP protocol preserves datagrams boundaries, we don't have to + // send the packet size first (it would even be a potential source of bug, if + // that size arrives corrupted), but we must split the packet into multiple + // pieces if data size is greater than the maximum datagram size. + // Get the data to send from the packet std::size_t size = 0; const char* data = packet.OnSend(size); - // First send the packet size - Uint32 packetSize = htonl(static_cast(size)); - Status status = Send(reinterpret_cast(&packetSize), sizeof(packetSize), remoteAddress, remotePort); - - // Make sure that the size was properly sent - if (status != Done) - return status; - - // Finally send the packet data - if (packetSize > 0) + // If size is greater than MaxDatagramSize, the data must be split into multiple datagrams + while (size >= MaxDatagramSize) { - return Send(data, size, remoteAddress, remotePort); - } - else - { - return Done; + Status status = Send(data, MaxDatagramSize, remoteAddress, remotePort); + if (status != Done) + return status; + + data += MaxDatagramSize; + size -= MaxDatagramSize; } + + // It is important to send a final datagram with a size < MaxDatagramSize, + // even if it is zero, to mark the end of the packet + return Send(data, size, remoteAddress, remotePort); } @@ -192,60 +189,28 @@ Socket::Status UdpSocket::Receive(Packet& packet, IpAddress& remoteAddress, unsi remoteAddress = IpAddress(); remotePort = 0; - // We start by getting the size of the incoming packet - Uint32 packetSize = 0; + // Receive datagrams std::size_t received = 0; - if (myPendingPacket.SizeReceived < sizeof(myPendingPacket.Size)) + do { - // Loop until we've received the entire size of the packet - // (even a 4 bytes variable may be received in more than one call) - while (myPendingPacket.SizeReceived < sizeof(myPendingPacket.Size)) - { - char* data = reinterpret_cast(&myPendingPacket.Size) + myPendingPacket.SizeReceived; - std::size_t size = sizeof(myPendingPacket.Size) - myPendingPacket.SizeReceived; - Status status = Receive(data, size, received, remoteAddress, remotePort); - myPendingPacket.SizeReceived += received; + // Make room in the data buffer for a new datagram + std::size_t size = myPendingPacket.Data.size(); + myPendingPacket.Data.resize(size + MaxDatagramSize); + char* data = &myPendingPacket.Data[0] + size; - if (status != Done) - return status; - } + // Receive the datagram + Status status = Receive(data, MaxDatagramSize, received, remoteAddress, remotePort); - // The packet size has been fully received - packetSize = ntohl(myPendingPacket.Size); - } - else - { - // The packet size has already been received in a previous call - packetSize = ntohl(myPendingPacket.Size); - } - - // Use another address instance for receiving the packet data, - // chunks of data coming from a different sender will be discarded (and lost...) - IpAddress currentSender; - unsigned short currentPort; - - // Loop until we receive all the packet data - char buffer[1024]; - while (myPendingPacket.Data.size() < packetSize) - { - // Receive a chunk of data - std::size_t sizeToGet = std::min(static_cast(packetSize - myPendingPacket.Data.size()), sizeof(buffer)); - Status status = Receive(buffer, sizeToGet, received, currentSender, currentPort); + // Check for errors if (status != Done) return status; - - // Append it into the packet - if ((currentSender == remoteAddress) && (currentPort == remotePort) && (received > 0)) - { - myPendingPacket.Data.resize(myPendingPacket.Data.size() + received); - char* begin = &myPendingPacket.Data[0] + myPendingPacket.Data.size() - received; - memcpy(begin, buffer, received); - } } + while (received == MaxDatagramSize); // We have received all the packet data: we can copy it to the user packet - if (!myPendingPacket.Data.empty()) - packet.OnReceive(&myPendingPacket.Data[0], myPendingPacket.Data.size()); + std::size_t actualSize = myPendingPacket.Data.size() - MaxDatagramSize + received; + if (actualSize > 0) + packet.OnReceive(&myPendingPacket.Data[0], actualSize); // Clear the pending packet data myPendingPacket = PendingPacket();