refactor bitfield io

include/sp/common/ByteSwapping.h

@@ -4,34 +4,13 @@
 
 namespace sp {
 
-/**
+bool IsLittleEndian();
- * \brief Serialize value to (network byte order) big endian
- */
-template <typename T>
-void ToNetwork(T& value) {}
 
-template <>
+void SwapBytes(std::uint8_t* begin, std::uint8_t* end);
-void ToNetwork<std::uint16_t>(std::uint16_t& value);
 
-template <>
+template<typename T>
-void ToNetwork<std::uint32_t>(std::uint32_t& value);
+void SwapBytes(T& a_Data) {
-
+	SwapBytes(reinterpret_cast<std::uint8_t*>(&a_Data), reinterpret_cast<std::uint8_t*>(&a_Data) + sizeof(T));
-template <>
+}
-void ToNetwork<std::uint64_t>(std::uint64_t& value);
-
-/**
- * \brief Deserialize value from (network byte order) big endian
- */
-template <typename T>
-void FromNetwork(T& value) {}
-
-template <>
-void FromNetwork<std::uint16_t>(std::uint16_t& value);
-
-template <>
-void FromNetwork<std::uint32_t>(std::uint32_t& value);
-
-template <>
-void FromNetwork<std::uint64_t>(std::uint64_t& value);
 
 }  // namespace sp

include/sp/common/DataBuffer.h

@@ -10,10 +10,11 @@
 #include <cassert>
 #include <cstdint>
 #include <cstring>
-#include <string>
-#include <sp/common/VarInt.h>
-#include <vector>
 #include <map>
+#include <sp/common/VarInt.h>
+#include <sp/common/ByteSwapping.h>
+#include <string>
+#include <vector>
 
 namespace sp {
 
@@ -23,16 +24,18 @@ namespace sp {
  */
 class DataBuffer {
   private:
-	typedef std::vector<std::uint8_t> Data;
+	using Data = std::vector<std::uint8_t>;
+
+  private:
 	Data m_Buffer;
 	std::size_t m_ReadOffset;
 
   public:
-	typedef Data::iterator iterator;
+	using iterator = Data::iterator;
-	typedef Data::const_iterator const_iterator;
+	using const_iterator = Data::const_iterator;
-	typedef Data::reference reference;
+	using reference = Data::reference;
-	typedef Data::const_reference const_reference;
+	using const_reference = Data::const_reference;
-	typedef Data::difference_type difference_type;
+	using difference_type = Data::difference_type;
 
 	DataBuffer();
 	DataBuffer(std::size_t a_InitialSize);
@@ -46,21 +49,23 @@ class DataBuffer {
 
 	/**
 	 * \brief Append data to the buffer
+	 * \warning No endian checks
 	 */
 	template <typename T>
-	void Append(const T& data) {
+	void Append(const T& a_Data) {
-		std::size_t size = sizeof(data);
+		std::size_t size = sizeof(a_Data);
 		std::size_t end_pos = m_Buffer.size();
 		m_Buffer.resize(m_Buffer.size() + size);
-		std::memcpy(&m_Buffer[end_pos], &data, size);
+		std::memcpy(&m_Buffer[end_pos], &a_Data, size);
 	}
 
 	/**
-	 * \brief Append data to the buffer
+	 * \brief Append data to the buffer (converted to big endian)
 	 */
 	template <typename T>
-	DataBuffer& operator<<(const T& data) {
+	DataBuffer& operator<<(T a_Data) {
-		Append(data);
+		SwapBytes(a_Data);
+		Append(a_Data);
 		return *this;
 	}
 
@@ -115,14 +120,24 @@ class DataBuffer {
 		return *this;
 	}
 
+	/**
+	 * \brief Read data into a_Data
+	 * \warning No endian checks
+	 */
+	template <typename T>
+	void Read(T& a_Data) {
+		assert(m_ReadOffset + sizeof(T) <= GetSize());
+		std::memcpy(&a_Data, m_Buffer.data() + m_ReadOffset, sizeof(T));
+		m_ReadOffset += sizeof(T);
+	}
+
 	/**
 	 * \brief Read some data from the buffer and assign to desired variable
 	 */
 	template <typename T>
-	DataBuffer& operator>>(T& data) {
+	DataBuffer& operator>>(T& a_Data) {
-		assert(m_ReadOffset + sizeof(T) <= GetSize());
+		Read(a_Data);
-		data = *(reinterpret_cast<T*>(&m_Buffer[m_ReadOffset]));
+		SwapBytes(a_Data);
-		m_ReadOffset += sizeof(T);
 		return *this;
 	}
 

include/sp/io/BitBuffer.h

@@ -0,0 +1,90 @@
+#pragma once
+
+#include <sp/common/DataBuffer.h>
+#include <sp/protocol/BitField.h>
+
+namespace sp {
+
+// TODO: flush if offset exceeds 64
+class BitBuffer {
+  private:
+	using Data = std::uint64_t;
+
+  private:
+	DataBuffer& m_Buffer;
+	Data m_Data;
+	std::size_t m_Offset;
+	bool m_WasBitField;
+
+  public:
+	BitBuffer(DataBuffer& a_Buffer) : m_Buffer(a_Buffer), m_Data(0), m_Offset(0), m_WasBitField(false) {}
+
+	void UpdateWrite(bool a_IsBitField) {
+		if (m_Offset == 0)
+			return;
+		if ((m_Offset % 8 == 0) || (!a_IsBitField && m_WasBitField)) {
+			Flush();
+		}
+		m_WasBitField = a_IsBitField;
+	}
+
+	void UpdateRead(bool a_IsBitField) {
+		if (m_Offset == 0)
+			return;
+		if ((m_Offset % 8 == 0) || (!a_IsBitField && m_WasBitField)) {
+			MoveReadOffset();
+		}
+		m_WasBitField = a_IsBitField;
+	}
+
+	template <typename T, std::size_t BitSize>
+	void Append(T a_Data) {
+		Data bin = static_cast<Data>(a_Data);
+		bin &= ((1 << BitSize) - 1);  // prevents overflow
+		std::size_t pushCount = sizeof(Data) * 8 - m_Offset - BitSize;
+		m_Data |= bin << pushCount;
+		m_Offset += BitSize;
+	}
+
+	template <typename T, std::size_t BitSize>
+	void Read(T& a_Data) {
+		std::size_t byteCount = GetByteCount(m_Offset + BitSize);
+		constexpr Data dataMask = (1 << BitSize) - 1;
+		m_Buffer.ReadSome(reinterpret_cast<std::uint8_t*>(&m_Data), byteCount);
+		SwapBytes(reinterpret_cast<std::uint8_t*>(&m_Data), reinterpret_cast<std::uint8_t*>(&m_Data) + byteCount);
+		m_Data >>= byteCount * 8 - m_Offset - BitSize;
+		m_Data &= dataMask;
+		a_Data = T(m_Data);
+		m_Buffer.SetReadOffset(m_Buffer.GetReadOffset() - byteCount);
+		m_Offset += BitSize;
+	}
+
+  private:
+	void Flush() {
+		std::size_t byteCount = GetByteCount();
+		m_Data >>= (sizeof(Data) - byteCount) * 8;
+		SwapBytes(reinterpret_cast<std::uint8_t*>(&m_Data), reinterpret_cast<std::uint8_t*>(&m_Data) + byteCount);
+		m_Buffer.WriteSome(reinterpret_cast<std::uint8_t*>(&m_Data), byteCount);
+		m_Offset = 0;
+		m_WasBitField = false;
+		m_Data = 0;
+	}
+
+	void MoveReadOffset() {
+		std::size_t byteCount = GetByteCount();
+		m_Buffer.SetReadOffset(m_Buffer.GetReadOffset() + byteCount);
+		m_Offset = 0;
+		m_WasBitField = false;
+		m_Data = 0;
+	}
+
+	std::size_t GetByteCount(std::size_t a_Offset = -1) const {
+		if (a_Offset == static_cast<std::size_t>(-1))
+			a_Offset = m_Offset;
+		if (a_Offset <= 8)
+			return 1;
+		return (a_Offset - 1) / 8 + 1;
+	}
+};
+
+}  // namespace sp

include/sp/io/MessageIO.h

@@ -1,78 +1,53 @@
 #pragma once
 
 #include <boost/pfr.hpp>
-#include <sp/common/ByteSwapping.h>
+#include <sp/io/BitBuffer.h>
-#include <sp/common/DataBuffer.h>
-#include <sp/protocol/BitField.h>
 
 namespace sp {
 namespace details {
 
+template <typename T, std::size_t BitSize>
+void WriteField(DataBuffer& a_Buffer, const BitField<T, BitSize>& a_Data, BitBuffer& a_BitBuffer) {
+	a_BitBuffer.Append<T, BitSize>(*a_Data);
+	a_BitBuffer.UpdateWrite(true);
+}
+
 template <typename T>
-void WriteBitField(DataBuffer& a_Buffer, std::uint64_t& a_DataRaw, std::size_t& a_Offset) {
+void WriteField(DataBuffer& a_Buffer, const T& a_Data, BitBuffer& a_BitBuffer) {
-	T filled = static_cast<T>(a_DataRaw);
+	a_Buffer << a_Data;
-	ToNetwork(filled);
+	a_BitBuffer.UpdateWrite(false);
-	a_Buffer << filled;
-	a_Offset = 0;
-	a_DataRaw = 0;
 }
 
 template <typename T, std::size_t BitSize>
-void WriteField(DataBuffer& a_Buffer, const BitField<T, BitSize>& a_Data, std::uint64_t& a_DataRaw, std::size_t& a_Offset) {
+void ReadField(DataBuffer& a_Buffer, BitField<T, BitSize>& a_Data, BitBuffer& a_BitBuffer) {
-	T cut = *a_Data & ((1 << a_Data.GetBitSize()) - 1);
+	a_BitBuffer.Read<T, BitSize>(*a_Data);
-	std::size_t pushCount = sizeof(T) * 8 - a_Offset - a_Data.GetBitSize();
+	a_BitBuffer.UpdateRead(true);
-	a_DataRaw |= cut << pushCount;
-	a_Offset += a_Data.GetBitSize();
-	if (a_Offset == sizeof(T) * 8) {
-		WriteBitField<T>(a_Buffer, a_DataRaw, a_Offset);
-	}
 }
 
 template <typename T>
-void WriteField(DataBuffer& a_Buffer, const T& a_Data, std::uint64_t& a_DataRaw, std::size_t& a_Offset) {
+void ReadField(DataBuffer& a_Buffer, T& a_Data, BitBuffer& a_BitBuffer) {
-	T swapped = a_Data;
-	ToNetwork(swapped);
-	a_Buffer << swapped;
-}
-
-template <typename T, std::size_t BitSize>
-void ReadField(DataBuffer& a_Buffer, BitField<T, BitSize>& a_Data, std::size_t& a_Offset) {
-	a_Buffer >> *a_Data;
-	FromNetwork(*a_Data);
-
-	*a_Data >>= sizeof(T) * 8 - a_Offset - a_Data.GetBitSize();
-	*a_Data &= (1 << a_Data.GetBitSize()) - 1;
-
-	if (a_Offset != sizeof(T) * 8) {
-		a_Buffer.SetReadOffset(a_Buffer.GetReadOffset() - sizeof(T));
-		a_Offset += a_Data.GetBitSize();
-	} else {
-		a_Offset = 0;
-	}
-}
-
-template <typename T>
-void ReadField(DataBuffer& a_Buffer, T& a_Data, std::size_t& a_Offset) {
 	a_Buffer >> a_Data;
-	FromNetwork(a_Data);
+	a_BitBuffer.UpdateRead(false);
 }
 
+}  // namespace details
+
 template <typename TData>
 DataBuffer WriteMessage(const TData& a_MessageData) {
 	DataBuffer buffer;
-	std::size_t currentOffset = 0;
+	BitBuffer bitBuffer(buffer);
-	std::uint64_t dataRaw = 0;
+	boost::pfr::for_each_field(
-	boost::pfr::for_each_field(a_MessageData,
+		a_MessageData, [&buffer, &bitBuffer](const auto& a_Field) { details::WriteField(buffer, a_Field, bitBuffer); });
-		[&buffer, &dataRaw, &currentOffset](const auto& a_Field) { WriteField(buffer, a_Field, dataRaw, currentOffset); });
+	bitBuffer.UpdateWrite(false);
 	return buffer;
 }
 
 template <typename TData>
 void ReadMessage(DataBuffer& a_Buffer, TData& a_MessageData) {
-	std::size_t currentOffset = 0;
+	BitBuffer bitBuffer(a_Buffer);
 	boost::pfr::for_each_field(
-		a_MessageData, [&a_Buffer, &currentOffset](auto& a_Field) { ReadField(a_Buffer, a_Field, currentOffset); });
+		a_MessageData, [&a_Buffer, &bitBuffer](auto& a_Field) { details::ReadField(a_Buffer, a_Field, bitBuffer); });
+	bitBuffer.UpdateRead(false);
 }
 
-}  // namespace details
 }  // namespace sp

include/sp/protocol/ConcreteMessage.h

@@ -27,11 +27,11 @@ class ConcreteMessage : public MessageBase {
 	}
 
 	virtual void Read(DataBuffer& a_Buffer) override {
-		details::ReadMessage(a_Buffer, m_Data);
+		ReadMessage(a_Buffer, m_Data);
 	}
 
 	virtual DataBuffer Write() const override {
-		return details::WriteMessage(m_Data);
+		return WriteMessage(m_Data);
 	}
 
 	DataType& operator*() {

src/sp/common/ByteSwapping.cpp

@@ -1,49 +1,21 @@
 #include <sp/common/ByteSwapping.h>
 
-#ifdef _WIN32
+#include <algorithm>
-
-#include <winsock2.h>
-
-#else
-
-#include <arpa/inet.h>
-#include <endian.h>
-
-#define htonll htobe64
-#define ntohll be64toh
-
-#endif
 
 namespace sp {
 
-template <>
+bool IsLittleEndian() {
-void ToNetwork<std::uint16_t>(std::uint16_t& value) {
+#ifdef SP_BIG_ENDIAN
-	value = htons(value);
+	return false;
+#else
+	return true;
+#endif
 }
 
-template <>
+void SwapBytes(std::uint8_t* begin, std::uint8_t* end) {
-void ToNetwork<std::uint32_t>(std::uint32_t& value) {
+	if (IsLittleEndian()) {
-	value = htonl(value);
+		std::reverse(begin, end);
-}
+	}
-
-template <>
-void ToNetwork<std::uint64_t>(std::uint64_t& value) {
-	value = htonll(value);
-}
-
-template <>
-void FromNetwork<std::uint16_t>(std::uint16_t& value) {
-	value = ntohs(value);
-}
-
-template <>
-void FromNetwork<std::uint32_t>(std::uint32_t& value) {
-	value = ntohl(value);
-}
-
-template <>
-void FromNetwork<std::uint64_t>(std::uint64_t& value) {
-	value = ntohll(value);
 }
 
 }  // namespace sp

test/test_message.cpp

@@ -12,7 +12,7 @@
 #include <fstream>
 #include <iostream>
 
-enum class PacketID { KeepAlive = 0, MDC = 1 };
+enum class PacketID : std::uint8_t { KeepAlive = 0, MDC = 1 };
 
 class PacketHandler;
 
@@ -26,8 +26,13 @@ struct KeepAlivePacket {
 	sp::BitField<std::uint16_t, 4> two;
 };
 
+struct MDCPacket {
+	sp::BitField<std::uint16_t, 12> one;
+	sp::BitField<PacketID, 4> two;
+};
+
 using KeepAliveMessage = Message<KeepAlivePacket, PacketID::KeepAlive>;
-using MDCMessage = Message<KeepAlivePacket, PacketID::MDC>;
+using MDCMessage = Message<MDCPacket, PacketID::MDC>;
 
 using AllMessages = std::tuple<KeepAliveMessage, MDCMessage>;
 
@@ -39,7 +44,7 @@ class MyHandler : public PacketHandler {
 		std::cout << "I recieved a keep alive : " << *msg->one << " : " << *msg->two << "\n";
 	}
 	virtual void Handle(const MDCMessage& msg) override {
-		std::cout << "I recieved a keep alive : " << *msg->one << " : " << *msg->two << "\n";
+		std::cout << "I recieved a mdc : " << *msg->one << " : " << static_cast<unsigned>(*msg->two) << "\n";
 	}
 };
 
@@ -57,6 +62,7 @@ int main() {
 	MyHandler h;
 	PacketDispatcher d;
 	d.RegisterHandler(PacketID::KeepAlive, &h);
+	d.RegisterHandler(PacketID::MDC, &h);
 	d.Dispatch(m);
 	PacketFactory f;
 	auto message = f.CreateMessage(PacketID::KeepAlive);
@@ -73,6 +79,7 @@ int main() {
 	PacketStream p(std::make_shared<sp::StdOuput>(file));
 
 	p.WriteMessage(m);
+	p.WriteMessage(MDCMessage{42, PacketID::MDC});
 
 	file.flush();
 
@@ -81,10 +88,10 @@ int main() {
 	PacketStream p2(std::make_shared<sp::StdInput>(file2));
 
 	auto message2 = p2.ReadMessage();
+	auto message3 = p2.ReadMessage();
 
 	d.Dispatch(*message2);
-
+	d.Dispatch(*message3);
-	// Todo : verify bitfields
 
 	// message->Write(file);
 	// file << std::endl;

xmake.lua

@@ -1,5 +1,7 @@
 add_rules("mode.debug", "mode.release")
 
+includes("@builtin/check")
+
 add_requires("boost_pfr")
 
 set_warnings("all")
@@ -76,6 +78,7 @@ target("SimpleProtocol")
     set_group("Library")
     set_kind("$(kind)")
     add_packages("boost_pfr", {public = true})
+    check_bigendian("SP_BIG_ENDIAN")
 
     add_headerfiles("include/(sp/**.h)", "include/(sp/**.inl)")