more

Author: ibc

TODO_SCTP.md

@@ -70,3 +70,4 @@ NO! Instead:
 
 - If we have a FooDataItem, the app should not be able to change its id via `SetId()`, AKA make `SetId()` protected since app should use `FooXxxxItem::Factory()` static methods.
 - `FooItem` constructor: It doesn't make sense that it fills things since we want to rely on subclasses' `Parse()`, `Factory()` and constructors (that will override these calls anyway).
+- If `FooItem` has valid but unknown id, that's correct, but let's remove the `SetValue()` method so those unknown items can only be parsed but not created.

worker/include/RTC/Serializable.hpp

@@ -110,7 +110,7 @@ namespace RTC
 		 * @throw MediaSoupError - If given `bufferLength` is lower than the
 		 * current exact length of the Serializable.
 		 */
-		virtual void Serialize(const uint8_t* buffer, size_t bufferLength);
+		virtual void Serialize(uint8_t* buffer, size_t bufferLength);
 
 		/**
 		 * Clone the Serializable into a new buffer. This method returns a new
@@ -129,7 +129,7 @@ namespace RTC
 		 * @throw MediaSoupError - If given `bufferLength` is lower than the
 		 * current exact length of the Serializable.
 		 */
-		virtual std::unique_ptr<Serializable> Clone(const uint8_t* buffer, size_t bufferLength) const = 0;
+		virtual std::unique_ptr<Serializable> Clone(uint8_t* buffer, size_t bufferLength) const = 0;
 
 		/**
 		 * Methods to be used by classes inheriting from Serializable.
@@ -139,9 +139,9 @@ namespace RTC
 		 * Method to be called by the child class in case it overrides the
 		 * `Serialize()` method.
 		 */
-		virtual void SetBuffer(const uint8_t* buffer) final
+		virtual void SetBuffer(uint8_t* buffer) final
 		{
-			this->buffer = const_cast<uint8_t*>(buffer);
+			this->buffer = buffer;
 		}
 
 		/**

worker/src/RTC/Serializable.cpp

@@ -23,7 +23,7 @@ namespace RTC
 		this->bufferLength = bufferLength;
 	}
 
-	void Serializable::Serialize(const uint8_t* buffer, size_t bufferLength)
+	void Serializable::Serialize(uint8_t* buffer, size_t bufferLength)
 	{
 		MS_TRACE();
 
@@ -35,9 +35,9 @@ namespace RTC
 			  this->length);
 		}
 
-		std::memcpy(const_cast<uint8_t*>(buffer), this->buffer, this->length);
+		std::memcpy(buffer, this->buffer, this->length);
 
-		this->buffer       = const_cast<uint8_t*>(buffer);
+		this->buffer       = buffer;
 		this->bufferLength = bufferLength;
 	}
 

worker/test/include/RTC/TestSerializable/FooDataItem.hpp

@@ -2,8 +2,6 @@
 #define MS_RTC_SERIALIZABLE_FOO_DATA_ITEM_HPP
 
 #include "common.hpp"
-#include "Utils.hpp"
-#include "RTC/Serializable.hpp"
 #include "RTC/TestSerializable/FooItem.hpp"
 
 using namespace RTC;
@@ -44,7 +42,7 @@ class FooDataItem : public FooItem
 	static std::unique_ptr<FooDataItem> Parse(const uint8_t* buffer, size_t bufferLength);
 
 	static std::unique_ptr<FooDataItem> Factory(
-	  const uint8_t* buffer, size_t bufferLength, uint8_t flags, uint16_t number);
+	  uint8_t* buffer, size_t bufferLength, uint8_t flags, uint16_t number);
 
 private:
 	/**
@@ -58,18 +56,9 @@ class FooDataItem : public FooItem
 
 	virtual void Dump() const override final;
 
-	virtual uint16_t GetNumber() const final
-	{
-		return Utils::Byte::Get2Bytes(GetValuePointer(), 0);
-	}
+	virtual uint16_t GetNumber() const final;
 
-	virtual void SetNumber(uint16_t number) final
-	{
-		// Convert number to network byte order.
-		number = uint16_t{ htons(number) };
-
-		SetValue(reinterpret_cast<const uint8_t*>(std::addressof(number)), NumberLength);
-	}
+	virtual void SetNumber(uint16_t number) final;
 };
 
 #endif

worker/test/include/RTC/TestSerializable/FooItem.hpp

@@ -40,7 +40,7 @@ class FooItem : public Serializable
 	 * @remarks
 	 * - NONE is not a valid value, so a FooItem with id NONE should be
 	 *   considered invalid and parsing should fail.
-	 * - Values other than the defined must be accepted as unknown FooItem.
+	 * - Values other than the defined must be accepted as FooUnknownItem.
 	 */
 	enum class ItemId : uint8_t
 	{
@@ -92,7 +92,7 @@ class FooItem : public Serializable
 	static std::unique_ptr<FooItem> Parse(const uint8_t* buffer, size_t bufferLength);
 
 	static std::unique_ptr<FooItem> Factory(
-	  const uint8_t* buffer,
+	  uint8_t* buffer,
 	  size_t bufferLength,
 	  ItemId id,
 	  uint8_t flags,
@@ -122,7 +122,7 @@ class FooItem : public Serializable
 	/**
 	 * Could be overridden by each subclass.
 	 */
-	std::unique_ptr<Serializable> Clone(const uint8_t* buffer, size_t bufferLength) const override;
+	std::unique_ptr<Serializable> Clone(uint8_t* buffer, size_t bufferLength) const override;
 
 	virtual ItemId GetId() const final
 	{
@@ -176,13 +176,19 @@ class FooItem : public Serializable
 	virtual void SetValue(const uint8_t* value, uint8_t valueLength) final;
 
 protected:
+	/**
+	 * NOTE: Return ItemHeader* instead of const ItemHeader* since we may
+	 * want to modify its fields.
+	 */
 	ItemHeader* GetHeaderPointer() const
 	{
 		return reinterpret_cast<ItemHeader*>(const_cast<uint8_t*>(GetBuffer()));
 	}
 
-	// We make this method private because it returns the value of the Value
-	// Length field, which is not useful for the application.
+	/**
+	 * Private private because it returns the value of the Value Length field,
+	 * which is not useful for the application.
+	 */
 	uint8_t GetValueLengthField() const
 	{
 		return GetHeaderPointer()->valueLength;
@@ -193,9 +199,13 @@ class FooItem : public Serializable
 		GetHeaderPointer()->valueLength = valueLength;
 	}
 
-	const uint8_t* GetValuePointer() const
+	/**
+	 * NOTE: Return uint8_t* instead of const uint8_t* since we may want to
+	 * modify its value.
+	 */
+	uint8_t* GetValuePointer() const
 	{
-		return GetBuffer() + ItemHeaderLength;
+		return const_cast<uint8_t*>(GetBuffer()) + ItemHeaderLength;
 	}
 
 	const uint8_t* GetEndPointer() const

worker/test/include/RTC/TestSerializable/FooPacket.hpp

@@ -67,7 +67,7 @@ class FooPacket : public Serializable
 	 */
 	static std::unique_ptr<FooPacket> Parse(const uint8_t* buffer, size_t length);
 
-	static std::unique_ptr<FooPacket> Factory(const uint8_t* buffer, size_t bufferLength, uint8_t type);
+	static std::unique_ptr<FooPacket> Factory(uint8_t* buffer, size_t bufferLength, uint8_t type);
 
 private:
 	/**
@@ -81,9 +81,9 @@ class FooPacket : public Serializable
 
 	void Dump() const override;
 
-	void Serialize(const uint8_t* buffer, size_t bufferLength) override;
+	void Serialize(uint8_t* buffer, size_t bufferLength) override;
 
-	std::unique_ptr<Serializable> Clone(const uint8_t* buffer, size_t bufferLength) const override;
+	std::unique_ptr<Serializable> Clone(uint8_t* buffer, size_t bufferLength) const override;
 
 	uint8_t GetType() const
 	{
@@ -143,13 +143,19 @@ class FooPacket : public Serializable
 	void AddItem(std::unique_ptr<FooItem> item);
 
 private:
+	/**
+	 * NOTE: Return Header* instead of const Header* since we may want to
+	 * modify its fields.
+	 */
 	Header* GetHeaderPointer() const
 	{
 		return reinterpret_cast<Header*>(const_cast<uint8_t*>(GetBuffer()));
 	}
 
-	// We make this method private because it returns the value of the Length
-	// field, which is not useful for the application.
+	/**
+	 * NOTE: Private because it returns the value of the Length field, which is
+	 * not useful for the application.
+	 */
 	uint16_t GetLengthField() const
 	{
 		return uint16_t{ ntohs(GetHeaderPointer()->length) };
@@ -170,9 +176,13 @@ class FooPacket : public Serializable
 		GetHeaderPointer()->length = uint16_t{ htons(length) };
 	}
 
-	const uint8_t* GetAppendixPointer() const
+	/**
+	 * NOTE: Return uint8_t* instead of const uint8_t* since we may want to
+	 * modify its value.
+	 */
+	uint8_t* GetAppendixPointer() const
 	{
-		return GetBuffer() + HeaderLength;
+		return const_cast<uint8_t*>(GetBuffer()) + HeaderLength;
 	}
 
 	const uint8_t* GetItemsPointer() const
@@ -187,9 +197,9 @@ class FooPacket : public Serializable
 		return ptr;
 	}
 
-	const uint8_t* GetPaddingPointer() const
+	uint8_t* GetPaddingPointer() const
 	{
-		return GetBuffer() + GetLengthField();
+		return const_cast<uint8_t*>(GetBuffer()) + GetLengthField();
 	}
 
 	/**

worker/test/src/RTC/TestSerializable/FooDataItem.cpp

@@ -4,6 +4,7 @@
 #include "RTC/TestSerializable/FooDataItem.hpp"
 #include "Logger.hpp"
 #include "MediaSoupErrors.hpp"
+#include "Utils.hpp"
 
 using namespace RTC;
 
@@ -41,7 +42,7 @@ std::unique_ptr<FooDataItem> FooDataItem::Parse(const uint8_t* buffer, size_t bu
 }
 
 std::unique_ptr<FooDataItem> FooDataItem::Factory(
-  const uint8_t* buffer, size_t bufferLength, uint8_t flags, uint16_t number)
+  uint8_t* buffer, size_t bufferLength, uint8_t flags, uint16_t number)
 {
 	MS_TRACE();
 
@@ -99,3 +100,17 @@ void FooDataItem::Dump() const
 	MS_DUMP("  number: %" PRIu16, GetNumber());
 	MS_DUMP("</FooDataItem>");
 }
+
+uint16_t FooDataItem::GetNumber() const
+{
+	MS_TRACE();
+
+	return Utils::Byte::Get2Bytes(GetValuePointer(), 0);
+}
+
+void FooDataItem::SetNumber(uint16_t number)
+{
+	MS_TRACE();
+
+	Utils::Byte::Set2Bytes(GetValuePointer(), 0, number);
+}

worker/test/src/RTC/TestSerializable/FooItem.cpp

@@ -73,10 +73,10 @@ std::unique_ptr<FooItem> FooItem::Parse(const uint8_t* buffer, size_t bufferLeng
 
 	// Pointer that starts at the beginning of the buffer and it's incremented
 	// to point to different parts of the item.
-	const uint8_t* ptr = buffer;
+	auto* ptr = buffer;
 
 	// Pointer that points to the end of the buffer.
-	const uint8_t* end = buffer + bufferLength;
+	auto* end = buffer + bufferLength;
 
 	// NOTE: We are parsing so we don't want to initialize the header.
 	auto item = std::unique_ptr<FooItem>(new FooItem(buffer, bufferLength, /*initializeHeader*/ false));
@@ -108,12 +108,7 @@ std::unique_ptr<FooItem> FooItem::Parse(const uint8_t* buffer, size_t bufferLeng
 }
 
 std::unique_ptr<FooItem> FooItem::Factory(
-  const uint8_t* buffer,
-  size_t bufferLength,
-  ItemId id,
-  uint8_t flags,
-  const uint8_t* value,
-  uint8_t valueLength)
+  uint8_t* buffer, size_t bufferLength, ItemId id, uint8_t flags, const uint8_t* value, uint8_t valueLength)
 {
 	MS_TRACE();
 
@@ -197,7 +192,7 @@ void FooItem::Dump() const
 	MS_DUMP("</FooItem>");
 }
 
-std::unique_ptr<Serializable> FooItem::Clone(const uint8_t* buffer, size_t bufferLength) const
+std::unique_ptr<Serializable> FooItem::Clone(uint8_t* buffer, size_t bufferLength) const
 {
 	MS_TRACE();
 
@@ -207,7 +202,7 @@ std::unique_ptr<Serializable> FooItem::Clone(const uint8_t* buffer, size_t buffe
 		  "bufferLength (%zu bytes) is lower than current length (%zu bytes)", bufferLength, GetLength());
 	}
 
-	std::memcpy(const_cast<uint8_t*>(buffer), GetBuffer(), GetLength());
+	std::memcpy(buffer, GetBuffer(), GetLength());
 
 	auto clonedFooItem =
 	  std::unique_ptr<FooItem>(new FooItem(buffer, bufferLength, /*initializeHeader*/ false));
@@ -228,7 +223,7 @@ void FooItem::SetValue(const uint8_t* value, uint8_t valueLength)
 	SetValueLengthField(valueLength);
 
 	// Copy the given value into the buffer.
-	std::memcpy(const_cast<uint8_t*>(GetValuePointer()), value, valueLength);
+	std::memcpy(GetValuePointer(), value, valueLength);
 
 	// Update Serializable length.
 	SetLength(GetLength() - previousValueLength + valueLength);