Skip to content

Commit

Permalink
Switched to generic endian swap
Browse files Browse the repository at this point in the history 
Added missing <array> header


Addressed review comments


Restricted templated swaps to Packet
  • Loading branch information
@gnawme
gnawme committed Jan 28, 2023
1 parent 0f2671a commit 0b5941c
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 55 deletions.
119 changes: 65 additions & 54 deletions src/SFML/Network/Packet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,57 @@
#include <SFML/Network/SocketImpl.hpp>
#include <SFML/System/String.hpp>

#include <algorithm>
#include <array>
#include <cstring>
#include <cwchar>


namespace sf
{
namespace
{
////////////////////////////////////////////////////////////
/// \brief Endian test
///
////////////////////////////////////////////////////////////
inline bool isBigEndian()
{
int one = 1;
return (*reinterpret_cast<char*>(&one)) != 1;
};


////////////////////////////////////////////////////////////
/// \brief Generic endianness conversion; if host is big-endian,
/// incoming network byte order can be handled without swap, and
/// host byte order is already network byte order
///
////////////////////////////////////////////////////////////
template <typename T>
T swapEndian(T val)
{
if (isBigEndian())
{
return val;
}

std::array<std::uint8_t, sizeof(T)> raw;

auto* cast = reinterpret_cast<std::uint8_t*>(&val);
std::memcpy(raw.data(), cast, sizeof(T));

std::array<std::uint8_t, sizeof(T)> dst;
std::reverse_copy(raw.begin(), raw.end(), dst.begin());
std::memcpy(cast, dst.data(), sizeof(T));

auto* valp = reinterpret_cast<T*>(cast);

return *valp;
}
} // namespace


////////////////////////////////////////////////////////////
Packet::Packet() = default;

Expand Down Expand Up @@ -158,7 +203,7 @@ Packet& Packet::operator>>(std::int16_t& data)
if (checkSize(sizeof(data)))
{
std::memcpy(&data, &m_data[m_readPos], sizeof(data));
data = static_cast<std::int16_t>(ntohs(static_cast<std::uint16_t>(data)));
data = swapEndian(data);
m_readPos += sizeof(data);
}

Expand All @@ -172,7 +217,7 @@ Packet& Packet::operator>>(std::uint16_t& data)
if (checkSize(sizeof(data)))
{
std::memcpy(&data, &m_data[m_readPos], sizeof(data));
data = ntohs(data);
data = swapEndian(data);
m_readPos += sizeof(data);
}

Expand All @@ -186,7 +231,7 @@ Packet& Packet::operator>>(std::int32_t& data)
if (checkSize(sizeof(data)))
{
std::memcpy(&data, &m_data[m_readPos], sizeof(data));
data = static_cast<std::int32_t>(ntohl(static_cast<std::uint32_t>(data)));
data = swapEndian(data);
m_readPos += sizeof(data);
}

Expand All @@ -200,7 +245,7 @@ Packet& Packet::operator>>(std::uint32_t& data)
if (checkSize(sizeof(data)))
{
std::memcpy(&data, &m_data[m_readPos], sizeof(data));
data = ntohl(data);
data = swapEndian(data);
m_readPos += sizeof(data);
}

Expand All @@ -213,16 +258,8 @@ Packet& Packet::operator>>(std::int64_t& data)
{
if (checkSize(sizeof(data)))
{
// Since ntohll is not available everywhere, we have to convert
// to network byte order (big endian) manually
std::uint8_t bytes[sizeof(data)];
std::memcpy(bytes, &m_data[m_readPos], sizeof(data));

data = (static_cast<std::int64_t>(bytes[0]) << 56) | (static_cast<std::int64_t>(bytes[1]) << 48) |
(static_cast<std::int64_t>(bytes[2]) << 40) | (static_cast<std::int64_t>(bytes[3]) << 32) |
(static_cast<std::int64_t>(bytes[4]) << 24) | (static_cast<std::int64_t>(bytes[5]) << 16) |
(static_cast<std::int64_t>(bytes[6]) << 8) | (static_cast<std::int64_t>(bytes[7]));

std::memcpy(&data, &m_data[m_readPos], sizeof(data));
data = swapEndian(data);
m_readPos += sizeof(data);
}

Expand All @@ -235,16 +272,8 @@ Packet& Packet::operator>>(std::uint64_t& data)
{
if (checkSize(sizeof(data)))
{
// Since ntohll is not available everywhere, we have to convert
// to network byte order (big endian) manually
std::uint8_t bytes[sizeof(data)];
std::memcpy(bytes, &m_data[m_readPos], sizeof(data));

data = (static_cast<std::uint64_t>(bytes[0]) << 56) | (static_cast<std::uint64_t>(bytes[1]) << 48) |
(static_cast<std::uint64_t>(bytes[2]) << 40) | (static_cast<std::uint64_t>(bytes[3]) << 32) |
(static_cast<std::uint64_t>(bytes[4]) << 24) | (static_cast<std::uint64_t>(bytes[5]) << 16) |
(static_cast<std::uint64_t>(bytes[6]) << 8) | (static_cast<std::uint64_t>(bytes[7]));

std::memcpy(&data, &m_data[m_readPos], sizeof(data));
data = swapEndian(data);
m_readPos += sizeof(data);
}

Expand All @@ -258,6 +287,7 @@ Packet& Packet::operator>>(float& data)
if (checkSize(sizeof(data)))
{
std::memcpy(&data, &m_data[m_readPos], sizeof(data));
data = swapEndian(data);
m_readPos += sizeof(data);
}

Expand All @@ -271,6 +301,7 @@ Packet& Packet::operator>>(double& data)
if (checkSize(sizeof(data)))
{
std::memcpy(&data, &m_data[m_readPos], sizeof(data));
data = swapEndian(data);
m_readPos += sizeof(data);
}

Expand Down Expand Up @@ -416,7 +447,7 @@ Packet& Packet::operator<<(std::uint8_t data)
////////////////////////////////////////////////////////////
Packet& Packet::operator<<(std::int16_t data)
{
auto toWrite = static_cast<std::int16_t>(htons(static_cast<std::uint16_t>(data)));
auto toWrite = swapEndian(data);
append(&toWrite, sizeof(toWrite));
return *this;
}
Expand All @@ -425,7 +456,7 @@ Packet& Packet::operator<<(std::int16_t data)
////////////////////////////////////////////////////////////
Packet& Packet::operator<<(std::uint16_t data)
{
std::uint16_t toWrite = htons(data);
auto toWrite = swapEndian(data);
append(&toWrite, sizeof(toWrite));
return *this;
}
Expand All @@ -434,7 +465,7 @@ Packet& Packet::operator<<(std::uint16_t data)
////////////////////////////////////////////////////////////
Packet& Packet::operator<<(std::int32_t data)
{
std::int32_t toWrite = static_cast<std::int32_t>(htonl(static_cast<std::uint32_t>(data)));
auto toWrite = swapEndian(data);
append(&toWrite, sizeof(toWrite));
return *this;
}
Expand All @@ -443,7 +474,7 @@ Packet& Packet::operator<<(std::int32_t data)
////////////////////////////////////////////////////////////
Packet& Packet::operator<<(std::uint32_t data)
{
std::uint32_t toWrite = htonl(data);
auto toWrite = swapEndian(data);
append(&toWrite, sizeof(toWrite));
return *this;
}
Expand All @@ -452,18 +483,7 @@ Packet& Packet::operator<<(std::uint32_t data)
////////////////////////////////////////////////////////////
Packet& Packet::operator<<(std::int64_t data)
{
// Since htonll is not available everywhere, we have to convert
// to network byte order (big endian) manually

std::uint8_t toWrite[] = {static_cast<std::uint8_t>((data >> 56) & 0xFF),
static_cast<std::uint8_t>((data >> 48) & 0xFF),
static_cast<std::uint8_t>((data >> 40) & 0xFF),
static_cast<std::uint8_t>((data >> 32) & 0xFF),
static_cast<std::uint8_t>((data >> 24) & 0xFF),
static_cast<std::uint8_t>((data >> 16) & 0xFF),
static_cast<std::uint8_t>((data >> 8) & 0xFF),
static_cast<std::uint8_t>((data)&0xFF)};

auto toWrite = swapEndian(data);
append(&toWrite, sizeof(toWrite));
return *this;
}
Expand All @@ -472,18 +492,7 @@ Packet& Packet::operator<<(std::int64_t data)
////////////////////////////////////////////////////////////
Packet& Packet::operator<<(std::uint64_t data)
{
// Since htonll is not available everywhere, we have to convert
// to network byte order (big endian) manually

std::uint8_t toWrite[] = {static_cast<std::uint8_t>((data >> 56) & 0xFF),
static_cast<std::uint8_t>((data >> 48) & 0xFF),
static_cast<std::uint8_t>((data >> 40) & 0xFF),
static_cast<std::uint8_t>((data >> 32) & 0xFF),
static_cast<std::uint8_t>((data >> 24) & 0xFF),
static_cast<std::uint8_t>((data >> 16) & 0xFF),
static_cast<std::uint8_t>((data >> 8) & 0xFF),
static_cast<std::uint8_t>((data)&0xFF)};

auto toWrite = swapEndian(data);
append(&toWrite, sizeof(toWrite));
return *this;
}
Expand All @@ -492,15 +501,17 @@ Packet& Packet::operator<<(std::uint64_t data)
////////////////////////////////////////////////////////////
Packet& Packet::operator<<(float data)
{
append(&data, sizeof(data));
auto toWrite = swapEndian(data);
append(&toWrite, sizeof(data));
return *this;
}


////////////////////////////////////////////////////////////
Packet& Packet::operator<<(double data)
{
append(&data, sizeof(data));
auto toWrite = swapEndian(data);
append(&toWrite, sizeof(data));
return *this;
}

Expand Down
2 changes: 1 addition & 1 deletion src/SFML/Network/TcpListener.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ unsigned short TcpListener::getLocalPort() const
{
if (getHandle() != priv::SocketImpl::invalidSocket())
{
// Retrieve informations about the local end of the socket
// Retrieve information about the local end of the socket
sockaddr_in address;
priv::SocketImpl::AddrLength size = sizeof(address);
if (getsockname(getHandle(), reinterpret_cast<sockaddr*>(&address), &size) != -1)
Expand Down
16 changes: 16 additions & 0 deletions test/Network/Packet.test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,5 +97,21 @@ TEST_CASE("[Network] sf::Packet")
CHECK_PACKET_STREAM_OPERATORS(std::numeric_limits<std::int64_t>::min());
CHECK_PACKET_STREAM_OPERATORS(std::numeric_limits<std::int64_t>::max());
}

SUBCASE("float")
{
CHECK_PACKET_STREAM_OPERATORS(0.f);
CHECK_PACKET_STREAM_OPERATORS(1.f);
CHECK_PACKET_STREAM_OPERATORS(std::numeric_limits<float>::min());
CHECK_PACKET_STREAM_OPERATORS(std::numeric_limits<float>::max());
}

SUBCASE("double")
{
CHECK_PACKET_STREAM_OPERATORS(0.0);
CHECK_PACKET_STREAM_OPERATORS(1.0);
CHECK_PACKET_STREAM_OPERATORS(std::numeric_limits<double>::min());
CHECK_PACKET_STREAM_OPERATORS(std::numeric_limits<double>::max());
}
}
}

0 comments on commit 0b5941c

Please sign in to comment.