From 175cddee585613e186dbbf5f9f3d3d6a54521ed3 Mon Sep 17 00:00:00 2001 From: Laurent Gomila Date: Sat, 11 Jun 2011 11:28:43 +0200 Subject: [PATCH] UdpSocket::Send(Packet) is now limited to UdpSocket::MaxDatagramSize, so that data is never split into multiple datagrams, which removes a lot of potential major problems --- include/SFML/Network/Socket.hpp | 18 -------- include/SFML/Network/TcpSocket.hpp | 20 +++++++++ include/SFML/Network/UdpSocket.hpp | 52 +++++++++++++++------- src/SFML/Network/Socket.cpp | 13 ------ src/SFML/Network/TcpSocket.cpp | 15 ++++++- src/SFML/Network/UdpSocket.cpp | 69 ++++++++---------------------- 6 files changed, 89 insertions(+), 98 deletions(-) diff --git a/include/SFML/Network/Socket.hpp b/include/SFML/Network/Socket.hpp index 403acacb3..fefd10f4e 100644 --- a/include/SFML/Network/Socket.hpp +++ b/include/SFML/Network/Socket.hpp @@ -115,19 +115,6 @@ protected : Udp ///< UDP protocol }; - //////////////////////////////////////////////////////////// - /// \brief Structure holding the data of a pending packet - /// - //////////////////////////////////////////////////////////// - struct PendingPacket - { - PendingPacket(); - - Uint32 Size; ///< Data of packet size - std::size_t SizeReceived; ///< Number of size bytes received so far - std::vector Data; ///< Data of the packet - }; - //////////////////////////////////////////////////////////// /// \brief Default constructor /// @@ -177,11 +164,6 @@ protected : //////////////////////////////////////////////////////////// void Close(); - //////////////////////////////////////////////////////////// - // Member data - //////////////////////////////////////////////////////////// - PendingPacket myPendingPacket; ///< Temporary data of the packet currently being received - private : friend class SocketSelector; diff --git a/include/SFML/Network/TcpSocket.hpp b/include/SFML/Network/TcpSocket.hpp index ebe1f60b0..45fafa721 100644 --- a/include/SFML/Network/TcpSocket.hpp +++ b/include/SFML/Network/TcpSocket.hpp @@ -182,7 +182,27 @@ public : //////////////////////////////////////////////////////////// Status Receive(Packet& packet); +private: + friend class TcpListener; + + //////////////////////////////////////////////////////////// + /// \brief Structure holding the data of a pending packet + /// + //////////////////////////////////////////////////////////// + struct PendingPacket + { + PendingPacket(); + + Uint32 Size; ///< Data of packet size + std::size_t SizeReceived; ///< Number of size bytes received so far + std::vector Data; ///< Data of the packet + }; + + //////////////////////////////////////////////////////////// + // Member data + //////////////////////////////////////////////////////////// + PendingPacket myPendingPacket; ///< Temporary data of the packet currently being received }; } // namespace sf diff --git a/include/SFML/Network/UdpSocket.hpp b/include/SFML/Network/UdpSocket.hpp index c12e4977b..56f9ef18b 100644 --- a/include/SFML/Network/UdpSocket.hpp +++ b/include/SFML/Network/UdpSocket.hpp @@ -29,6 +29,7 @@ // Headers //////////////////////////////////////////////////////////// #include +#include namespace sf @@ -146,10 +147,9 @@ 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. + /// Make sure that the packet size is not greater than + /// UdpSocket::MaxDatagramSize, otherwise this function will + /// fail and no data will be sent. /// /// \param packet Packet to send /// \param remoteAddress Address of the receiver @@ -180,6 +180,13 @@ public : /// //////////////////////////////////////////////////////////// Status Receive(Packet& packet, IpAddress& remoteAddress, unsigned short& remotePort); + +private: + + //////////////////////////////////////////////////////////// + // Member data + //////////////////////////////////////////////////////////// + std::vector myBuffer; ///< Temporary buffer holding the received data in Receive(Packet) }; } // namespace sf @@ -196,23 +203,36 @@ public : /// connecting once to a remote host, like TCP sockets, /// it can send to and receive from any host at any time. /// +/// It is a datagram protocol: bounded blocks of data (datagrams) +/// are transfered over the network rather than a continuous +/// stream of data (TCP). Therefore, one call to Send will always +/// match one call to Receive (if the datagram is not lost), +/// with the same data that was sent. +/// /// The UDP protocol is lightweight but unreliable. Unreliable -/// means that the data may be corrupted, duplicated, lost or -/// arrive out of order. UDP is generally used for real-time -/// communication (audio or video streaming, real-time games, -/// etc.) where speed is crucial and corrupted data -/// doesn't matter much. +/// means that datagrams may be duplicated, be lost or +/// arrive reordered. However, if a datagram arrives, its +/// data is guaranteed to be valid. +/// +/// UDP is generally used for real-time communication +/// (audio or video streaming, real-time games, etc.) where +/// speed is crucial and lost data doesn't matter much. /// /// Sending and receiving data can use either the low-level /// or the high-level functions. The low-level functions -/// process a raw sequence of bytes, and cannot ensure that -/// one call to Send will exactly match one call to Receive -/// at the other end of the socket. +/// process a raw sequence of bytes, whereas the high-level +/// interface uses packets (see sf::Packet), which are easier +/// to use and provide more safety regarding the data that is +/// exchanged. You can look at the sf::Packet class to get +/// more details about how they work. /// -/// The high-level interface uses packets (see sf::Packet), -/// which are easier to use and provide more safety regarding -/// the data that is exchanged. You can look at the sf::Packet -/// class to get more details about how they work. +/// It is important to note that UdpSocket is unable to send +/// datagrams bigger than MaxDatagramSize. In this case, it +/// returns an error and doesn't send anything. This applies +/// to both raw data and packets. Indeed, even packets are +/// unable to split and recompose data, due to the unreliability +/// of the protocol (dropped, mixed or duplicated datagrams may +/// lead to a big mess when trying to recompose a packet). /// /// If the socket is bound to a port, it is automatically /// unbound from it when the socket is destroyed. However, diff --git a/src/SFML/Network/Socket.cpp b/src/SFML/Network/Socket.cpp index a5d49217f..110ef71cd 100644 --- a/src/SFML/Network/Socket.cpp +++ b/src/SFML/Network/Socket.cpp @@ -131,19 +131,6 @@ void Socket::Close() priv::SocketImpl::Close(mySocket); mySocket = priv::SocketImpl::InvalidSocket(); } - - // Reset the pending packet data - myPendingPacket = PendingPacket(); -} - - -//////////////////////////////////////////////////////////// -Socket::PendingPacket::PendingPacket() : -Size (0), -SizeReceived(0), -Data () -{ - } } // namespace sf diff --git a/src/SFML/Network/TcpSocket.cpp b/src/SFML/Network/TcpSocket.cpp index 8e68e5a8a..c7a1f055b 100644 --- a/src/SFML/Network/TcpSocket.cpp +++ b/src/SFML/Network/TcpSocket.cpp @@ -197,8 +197,11 @@ Socket::Status TcpSocket::Connect(const IpAddress& remoteAddress, unsigned short //////////////////////////////////////////////////////////// void TcpSocket::Disconnect() { - // Simply close the socket + // Close the socket Close(); + + // Reset the pending packet data + myPendingPacket = PendingPacket(); } @@ -354,4 +357,14 @@ Socket::Status TcpSocket::Receive(Packet& packet) return Done; } + +//////////////////////////////////////////////////////////// +TcpSocket::PendingPacket::PendingPacket() : +Size (0), +SizeReceived(0), +Data () +{ + +} + } // namespace sf diff --git a/src/SFML/Network/UdpSocket.cpp b/src/SFML/Network/UdpSocket.cpp index 52aed7fda..36b1f84e4 100644 --- a/src/SFML/Network/UdpSocket.cpp +++ b/src/SFML/Network/UdpSocket.cpp @@ -37,7 +37,8 @@ namespace sf { //////////////////////////////////////////////////////////// UdpSocket::UdpSocket() : -Socket(Udp) +Socket (Udp), +myBuffer(MaxDatagramSize) { } @@ -154,70 +155,38 @@ 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. + // UDP is a datagram-oriented protocol (as opposed to TCP which is a stream protocol). + // Sending one datagram is almost safe: it may be lost but if it's received, then its data + // is guaranteed to be ok. However, splitting a packet into multiple datagrams would be highly + // unreliable, since datagrams may be reordered, dropped or mixed between different sources. + // That's why SFML imposes a limit on packet size so that they can be sent in a single datagram. + // This also removes the overhead associated to packets -- there's no size to send in addition + // to the packet's data. // Get the data to send from the packet std::size_t size = 0; const char* data = packet.OnSend(size); - // If size is greater than MaxDatagramSize, the data must be split into multiple datagrams - while (size >= MaxDatagramSize) - { - 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); + // Send it + return Send(data, std::min(size, static_cast(MaxDatagramSize)), remoteAddress, remotePort); } //////////////////////////////////////////////////////////// Socket::Status UdpSocket::Receive(Packet& packet, IpAddress& remoteAddress, unsigned short& remotePort) { - // First clear the variables to fill - packet.Clear(); - remoteAddress = IpAddress(); - remotePort = 0; + // See the detailed comment in Send(Packet) above. - // Receive datagrams + // Receive the datagram std::size_t received = 0; - std::size_t size = myPendingPacket.Data.size(); - do - { - // Make room in the data buffer for a new datagram - myPendingPacket.Data.resize(size + MaxDatagramSize); - char* data = &myPendingPacket.Data[0] + size; + Status status = Receive(&myBuffer[0], myBuffer.size(), received, remoteAddress, remotePort); - // Receive the datagram - Status status = Receive(data, MaxDatagramSize, received, remoteAddress, remotePort); + // If we received valid data, we can copy it to the user packet + packet.Clear(); + if ((status == Done) && (received > 0)) + packet.OnReceive(&myBuffer[0], received); - // Check for errors - if (status != Done) - { - myPendingPacket.Data.resize(size + received); - return status; - } - } - while (received == MaxDatagramSize); - - // We have received all the packet data: we can copy it to the user packet - std::size_t actualSize = size + received; - if (actualSize > 0) - packet.OnReceive(&myPendingPacket.Data[0], actualSize); - - // Clear the pending packet data - myPendingPacket = PendingPacket(); - - return Done; + return status; }