Add overloads to setRawData that take explicit ownership parameter. (#2036)

Author: Dimi1010

Packet++/header/RawPacket.h

@@ -9,6 +9,8 @@
 #endif
 #include <stddef.h>
 
+#include "DeprecationUtils.h"
+
 /// @file
 
 /// @namespace pcpp
@@ -262,7 +264,7 @@ namespace pcpp
 		int m_RawDataLen = 0;
 		int m_FrameLength = 0;
 		timespec m_TimeStamp{};  // Zero initialized
-		bool m_DeleteRawDataAtDestructor = true;
+		bool m_OwnsRawData = false;
 		bool m_RawPacketSet = false;
 		LinkLayerType m_LinkLayerType = LinkLayerType::LINKTYPE_ETHERNET;
 
@@ -273,7 +275,7 @@ namespace pcpp
 		/// - data pointer is set to nullptr
 		/// - data length is set to 0
 		/// - frame length is set to 0
-		/// - deleteRawDataAtDestructor is set to 'true'
+		/// - takeOwnership is set to 'false'
 		/// - timestamp is set to 0
 		/// - layer type is set to Ethernet
 		RawPacket() = default;
@@ -285,10 +287,10 @@ namespace pcpp
 		/// @param[in] pRawData A pointer to the raw data
 		/// @param[in] rawDataLen The raw data length in bytes
 		/// @param[in] timestamp The timestamp packet was received by the NIC (in usec precision)
-		/// @param[in] deleteRawDataAtDestructor An indicator whether raw data pointer should be freed when the instance
-		/// is freed or not. If set to 'true' than pRawData will be freed when instanced is being freed
+		/// @param[in] takeOwnership If 'true' the RawPacket will take ownership of the pRawData pointer and will
+		/// free it when the RawPacket instance is destroyed or the buffer is replaced.
 		/// @param[in] layerType The link layer type of this raw packet. The default is Ethernet
-		RawPacket(const uint8_t* pRawData, int rawDataLen, timeval timestamp, bool deleteRawDataAtDestructor,
+		RawPacket(const uint8_t* pRawData, int rawDataLen, timeval timestamp, bool takeOwnership,
 		          LinkLayerType layerType = LINKTYPE_ETHERNET);
 
 		/// A constructor that receives a pointer to the raw data (allocated elsewhere). This constructor is usually
@@ -298,13 +300,13 @@ namespace pcpp
 		/// @param[in] pRawData A pointer to the raw data
 		/// @param[in] rawDataLen The raw data length in bytes
 		/// @param[in] timestamp The timestamp packet was received by the NIC (in nsec precision)
-		/// @param[in] deleteRawDataAtDestructor An indicator whether raw data pointer should be freed when the instance
-		/// is freed or not. If set to 'true' than pRawData will be freed when instanced is being freed
+		/// @param[in] takeOwnership If 'true' the RawPacket will take ownership of the pRawData pointer and will
+		/// free it when the RawPacket instance is destroyed or the buffer is replaced.
 		/// @param[in] layerType The link layer type of this raw packet. The default is Ethernet
-		RawPacket(const uint8_t* pRawData, int rawDataLen, timespec timestamp, bool deleteRawDataAtDestructor,
+		RawPacket(const uint8_t* pRawData, int rawDataLen, timespec timestamp, bool takeOwnership,
 		          LinkLayerType layerType = LINKTYPE_ETHERNET);
 
-		/// A destructor for this class. Frees the raw data if deleteRawDataAtDestructor was set to 'true'
+		/// A destructor for this class. Frees the raw data if takeOwnership was set to 'true'
 		virtual ~RawPacket();
 
 		/// A copy constructor that copies all data from another instance. Notice all raw data is copied (using memcpy),
@@ -313,7 +315,7 @@ namespace pcpp
 		RawPacket(const RawPacket& other);
 
 		/// Assignment operator overload for this class. When using this operator on an already initialized RawPacket
-		/// instance, the original raw data is freed first if deleteRawDataAtDestructor was set to 'true'.
+		/// instance, the original raw data is freed first if takeOwnership was set to 'true'.
 		/// Then the other instance is copied to this instance, the same way the copy constructor works
 		/// @param[in] other The instance to copy from
 		RawPacket& operator=(const RawPacket& other);
@@ -328,7 +330,7 @@ namespace pcpp
 			return 0;
 		}
 
-		/// Set a raw data. If data was already set and deleteRawDataAtDestructor was set to 'true' the old data will be
+		/// Set a raw data. If data was already set and takeOwnership was set to 'true' the old data will be
 		/// freed first
 		/// @param[in] pRawData A pointer to the new raw data
 		/// @param[in] rawDataLen The new raw data length in bytes
@@ -338,10 +340,36 @@ namespace pcpp
 		/// actual packet length. This parameter represents the packet length. This parameter is optional, if not set or
 		/// set to -1 it is assumed both lengths are equal
 		/// @return True if raw data was set successfully, false otherwise
-		virtual bool setRawData(const uint8_t* pRawData, int rawDataLen, timeval timestamp,
-		                        LinkLayerType layerType = LINKTYPE_ETHERNET, int frameLength = -1);
+		/// @deprecated This method does not update ownership of the pRawData pointer, inheriting the previous buffer's
+		/// ownership. Use the overload that takes bool takeOwnership parameter for explicit control.
+		/// @remarks Derived classes may not support using the raw data buffer directly.
+		/// Users should not rely on the RawPacket always writing to the provided pointer location,
+		/// and instead get the raw data pointer using getRawData().
+		PCPP_DEPRECATED("Use the overload that takes bool takeOwnership parameter for explicit control.")
+		bool setRawData(const uint8_t* pRawData, int rawDataLen, timeval timestamp,
+		                LinkLayerType layerType = LINKTYPE_ETHERNET, int frameLength = -1);
 
-		/// Set a raw data. If data was already set and deleteRawDataAtDestructor was set to 'true' the old data will be
+		/// @brief Assign a buffer to use as raw data.
+		///
+		/// If the RawPacket already had raw data, it will be disposed of accordingly.
+		///
+		/// @param pRawData A pointer to the new raw data buffer.
+		/// @param rawDataLen The length of the new raw data buffer in bytes.
+		/// @param takeOwnership If true, the RawPacket will take ownership of the buffer and will be responsible for
+		/// freeing it.
+		/// @param timestamp The timestamp packet was received by the NIC (in usec precision).
+		/// @param layerType The link layer type for this raw data.
+		/// @param frameLength When reading from pcap files, sometimes the captured length is different from the
+		/// actual packet length. This parameter represents the packet length. This parameter is optional, if not set or
+		/// set to -1 it is assumed both lengths are equal
+		/// @return True if raw data was set successfully, false otherwise.
+		/// @remarks Derived classes may not support using the raw data buffer directly.
+		/// Users should not rely on the RawPacket always writing to the provided pointer location,
+		/// and instead get the raw data pointer using getRawData().
+		bool setRawData(const uint8_t* pRawData, int rawDataLen, bool takeOwnership, timeval timestamp,
+		                LinkLayerType layerType = LINKTYPE_ETHERNET, int frameLength = -1);
+
+		/// Set a raw data. If data was already set and takeOwnership was set to 'true' the old data will be
 		/// freed first
 		/// @param[in] pRawData A pointer to the new raw data
 		/// @param[in] rawDataLen The new raw data length in bytes
@@ -351,8 +379,34 @@ namespace pcpp
 		/// actual packet length. This parameter represents the packet length. This parameter is optional, if not set or
 		/// set to -1 it is assumed both lengths are equal
 		/// @return True if raw data was set successfully, false otherwise
-		virtual bool setRawData(const uint8_t* pRawData, int rawDataLen, timespec timestamp,
-		                        LinkLayerType layerType = LINKTYPE_ETHERNET, int frameLength = -1);
+		/// @deprecated This method does not update ownership of the pRawData pointer, inheriting the previous buffer's
+		/// ownership. Use the overload that takes bool takeOwnership parameter for explicit control.
+		/// @remarks Derived classes may not support using the raw data buffer directly.
+		/// Users should not rely on the RawPacket always writing to the provided pointer location,
+		/// and instead get the raw data pointer using getRawData().
+		PCPP_DEPRECATED("Use the overload that takes bool takeOwnership parameter for explicit control.")
+		bool setRawData(const uint8_t* pRawData, int rawDataLen, timespec timestamp,
+		                LinkLayerType layerType = LINKTYPE_ETHERNET, int frameLength = -1);
+
+		/// @brief Assign a buffer to use as raw data.
+		///
+		/// If the RawPacket already had raw data, it will be disposed of accordingly.
+		///
+		/// @param pRawData A pointer to the new raw data buffer.
+		/// @param rawDataLen The length of the new raw data buffer in bytes.
+		/// @param takeOwnership If true, the RawPacket will take ownership of the buffer and will be responsible for
+		/// freeing it.
+		/// @param timestamp The timestamp packet was received by the NIC (in nsec precision).
+		/// @param layerType The link layer type for this raw data.
+		/// @param frameLength When reading from pcap files, sometimes the captured length is different from the
+		/// actual packet length. This parameter represents the packet length. This parameter is optional, if not set or
+		/// set to -1 it is assumed both lengths are equal
+		/// @return True if raw data was set successfully, false otherwise.
+		/// @remarks Derived classes may not support using the raw data buffer directly.
+		/// Users should not rely on the RawPacket always writing to the provided pointer location,
+		/// and instead get the raw data pointer using getRawData().
+		bool setRawData(const uint8_t* pRawData, int rawDataLen, bool takeOwnership, timespec timestamp,
+		                LinkLayerType layerType = LINKTYPE_ETHERNET, int frameLength = -1);
 
 		/// Initialize a raw packet with data. The main difference between this method and setRawData() is that
 		/// setRawData() is meant for replacing the data in an existing raw packet, whereas this method is meant to be
@@ -362,6 +416,8 @@ namespace pcpp
 		/// @param timestamp The timestamp packet was received by the NIC (in nsec precision)
 		/// @param layerType The link layer type for this raw data
 		/// @return True if raw data was set successfully, false otherwise
+		/// @deprecated Prefer using setRawData() with takeOwnership `false`.
+		PCPP_DEPRECATED("Prefer using setRawData() with takeOwnership `false`.")
 		bool initWithRawData(const uint8_t* pRawData, int rawDataLen, timespec timestamp,
 		                     LinkLayerType layerType = LINKTYPE_ETHERNET);
 
@@ -424,7 +480,7 @@ namespace pcpp
 		}
 
 		/// Clears all members of this instance, meaning setting raw data to nullptr, raw data length to 0, etc.
-		/// Frees the raw data if deleteRawDataAtDestructor was set to 'true'
+		/// Frees the raw data if takeOwnership was set to 'true'
 		/// @todo set timestamp to a default value as well
 		virtual void clear();
 
@@ -461,6 +517,11 @@ namespace pcpp
 		/// allocate the memory
 		/// @return True if data was reallocated successfully, false otherwise
 		virtual bool reallocateData(size_t newBufferLength);
+
+	protected:
+		// Updates the raw data buffer and related members. Used by setRawData() methods.
+		virtual bool doSetRawData(const uint8_t* pRawData, int rawDataLen, bool takeOwnership, timespec timestamp,
+		                          LinkLayerType layerType, int frameLength);
 	};
 
 }  // namespace pcpp

Packet++/src/RawPacket.cpp

@@ -7,16 +7,15 @@
 
 namespace pcpp
 {
-	RawPacket::RawPacket(const uint8_t* pRawData, int rawDataLen, timeval timestamp, bool deleteRawDataAtDestructor,
+	RawPacket::RawPacket(const uint8_t* pRawData, int rawDataLen, timeval timestamp, bool takeOwnership,
 	                     LinkLayerType layerType)
-	    : RawPacket(pRawData, rawDataLen, internal::toTimespec(timestamp), deleteRawDataAtDestructor, layerType)
+	    : RawPacket(pRawData, rawDataLen, internal::toTimespec(timestamp), takeOwnership, layerType)
 	{}
 
-	RawPacket::RawPacket(const uint8_t* pRawData, int rawDataLen, timespec timestamp, bool deleteRawDataAtDestructor,
+	RawPacket::RawPacket(const uint8_t* pRawData, int rawDataLen, timespec timestamp, bool takeOwnership,
 	                     LinkLayerType layerType)
 	    : m_RawData(const_cast<uint8_t*>(pRawData)), m_RawDataLen(rawDataLen), m_FrameLength(rawDataLen),
-	      m_TimeStamp(timestamp), m_DeleteRawDataAtDestructor(deleteRawDataAtDestructor), m_RawPacketSet(true),
-	      m_LinkLayerType(layerType)
+	      m_TimeStamp(timestamp), m_OwnsRawData(takeOwnership), m_RawPacketSet(true), m_LinkLayerType(layerType)
 	{}
 
 	RawPacket::~RawPacket()
@@ -56,7 +55,7 @@ namespace pcpp
 
 		if (allocateData)
 		{
-			m_DeleteRawDataAtDestructor = true;
+			m_OwnsRawData = true;
 			m_RawData = new uint8_t[other.m_RawDataLen];
 			m_RawDataLen = other.m_RawDataLen;
 		}
@@ -70,14 +69,41 @@ namespace pcpp
 	bool RawPacket::setRawData(const uint8_t* pRawData, int rawDataLen, timeval timestamp, LinkLayerType layerType,
 	                           int frameLength)
 	{
+		DISABLE_WARNING_PUSH;
+		DISABLE_WARNING_DEPRECATED;
+
 		return setRawData(pRawData, rawDataLen, internal::toTimespec(timestamp), layerType, frameLength);
+
+		DISABLE_WARNING_POP;
+	}
+
+	bool RawPacket::setRawData(const uint8_t* pRawData, int rawDataLen, bool takeOwnership, timeval timestamp,
+	                           LinkLayerType layerType, int frameLength)
+	{
+		return setRawData(pRawData, rawDataLen, takeOwnership, internal::toTimespec(timestamp), layerType, frameLength);
 	}
 
 	bool RawPacket::setRawData(const uint8_t* pRawData, int rawDataLen, timespec timestamp, LinkLayerType layerType,
 	                           int frameLength)
+	{
+		// Legacy compatibility:
+		//  In previous versions of this method inherited the old ownership value and used it for the new buffer.
+		//  To keep backward compatibility we keep the same behavior here.
+		return doSetRawData(pRawData, rawDataLen, m_OwnsRawData, timestamp, layerType, frameLength);
+	}
+
+	bool RawPacket::setRawData(const uint8_t* pRawData, int rawDataLen, bool takeOwnership, timespec timestamp,
+	                           LinkLayerType layerType, int frameLength)
+	{
+		return doSetRawData(pRawData, rawDataLen, takeOwnership, timestamp, layerType, frameLength);
+	}
+
+	bool RawPacket::doSetRawData(const uint8_t* pRawData, int rawDataLen, bool takeOwnership, timespec timestamp,
+	                             LinkLayerType layerType, int frameLength)
 	{
 		clear();
 
+		m_OwnsRawData = takeOwnership;
 		m_FrameLength = (frameLength == -1) ? rawDataLen : frameLength;
 		m_RawData = (uint8_t*)pRawData;
 		m_RawDataLen = rawDataLen;
@@ -90,13 +116,12 @@ namespace pcpp
 	bool RawPacket::initWithRawData(const uint8_t* pRawData, int rawDataLen, timespec timestamp,
 	                                LinkLayerType layerType)
 	{
-		m_DeleteRawDataAtDestructor = false;
-		return setRawData(pRawData, rawDataLen, timestamp, layerType);
+		return setRawData(pRawData, rawDataLen, false, timestamp, layerType);
 	}
 
 	void RawPacket::clear()
 	{
-		if (m_RawData != nullptr && m_DeleteRawDataAtDestructor)
+		if (m_RawData != nullptr && m_OwnsRawData)
 			delete[] m_RawData;
 
 		m_RawData = nullptr;
@@ -144,10 +169,10 @@ namespace pcpp
 		uint8_t* newBuffer = new uint8_t[newBufferLength];
 		memset(newBuffer, 0, newBufferLength);
 		memcpy(newBuffer, m_RawData, m_RawDataLen);
-		if (m_DeleteRawDataAtDestructor)
+		if (m_OwnsRawData)
 			delete[] m_RawData;
 
-		m_DeleteRawDataAtDestructor = true;
+		m_OwnsRawData = true;
 		m_RawData = newBuffer;
 
 		return true;

Pcap++/header/MBufRawPacket.h

@@ -41,6 +41,8 @@ namespace pcpp
 	///      create a linked list of mbufs. This is good for Jumbo packets or other uses. MBufRawPacket doesn't support
 	///      this capability so there is no way to access the mbufs linked to the mbuf wrapped by MBufRawPacket
 	///      instance. I hope I'll be able to add this support in the future
+	///    - Currently class methods like MBufRawPacket::setRawData() always copy the data into the mbuf data area.
+	///      The class does not support referencing and modifying data in an external buffer like RawPacket does.
 	class MBufRawPacket : public RawPacket
 	{
 		friend class DpdkDevice;
@@ -65,7 +67,7 @@ namespace pcpp
 		/// an empty c'tor)
 		MBufRawPacket() : RawPacket(), m_MBuf(nullptr), m_Mempool(nullptr), m_MbufDataSize(0), m_FreeMbuf(true)
 		{
-			m_DeleteRawDataAtDestructor = false;
+			m_OwnsRawData = false;
 		}
 
 		/// A d'tor for this class. Once called it frees the mbuf attached to it (returning it back to the mbuf pool it
@@ -147,24 +149,6 @@ namespace pcpp
 		/// @return A pointer to the new MBufRawPacket object which is a clone of this object
 		MBufRawPacket* clone() const override;
 
-		/// Set raw data to the mbuf by copying the data to it. In order to stay compatible with the ancestor method
-		/// which takes control of the data pointer and frees it when RawPacket is destroyed, this method frees this
-		/// pointer right away after data is copied to the mbuf. So when using this method please notice that after it's
-		/// called pRawData memory is free, don't use this pointer again. In addition, if raw packet isn't initialized
-		/// (mbuf is nullptr), this method will call the init() method
-		/// @param[in] pRawData A pointer to the new raw data
-		/// @param[in] rawDataLen The new raw data length in bytes
-		/// @param[in] timestamp The timestamp packet was received by the NIC
-		/// @param[in] layerType The link layer type for this raw data. Default is Ethernet
-		/// @param[in] frameLength When reading from pcap files, sometimes the captured length is different from the
-		/// actual packet length. This parameter represents the packet length. This parameter is optional, if not set or
-		/// set to -1 it is assumed both lengths are equal
-		/// @return True if raw data was copied to the mbuf successfully, false if rawDataLen is larger than mbuf max
-		/// size, if initialization failed or if copying the data to the mbuf failed. In all of these cases an error
-		/// will be printed to log
-		bool setRawData(const uint8_t* pRawData, int rawDataLen, timespec timestamp,
-		                LinkLayerType layerType = LINKTYPE_ETHERNET, int frameLength = -1) override;
-
 		/// Clears the object and frees the mbuf
 		void clear() override;
 
@@ -207,6 +191,27 @@ namespace pcpp
 		{
 			m_FreeMbuf = val;
 		}
+
+	protected:
+		/// Set raw data to the mbuf by copying the data to it. In order to stay compatible with the ancestor method
+		/// which takes control of the data pointer and frees it when RawPacket is destroyed, this method frees this
+		/// pointer right away after data is copied to the mbuf. So when using this method please notice that after it's
+		/// called pRawData memory is free, don't use this pointer again. In addition, if raw packet isn't initialized
+		/// (mbuf is nullptr), this method will call the init() method
+		///
+		/// @param[in] pRawData A pointer to the new raw data
+		/// @param[in] rawDataLen The new raw data length in bytes
+		/// @param[in] takeOwnership Indicates whether the method should take ownership of pRawData pointer.
+		/// @param[in] timestamp The timestamp packet was received by the NIC
+		/// @param[in] layerType The link layer type for this raw data. Default is Ethernet
+		/// @param[in] frameLength When reading from pcap files, sometimes the captured length is different from the
+		/// actual packet length. This parameter represents the packet length. This parameter is optional, if not set or
+		/// set to -1 it is assumed both lengths are equal
+		/// @return True if raw data was copied to the mbuf successfully, false if rawDataLen is larger than mbuf max
+		/// size, if initialization failed or if copying the data to the mbuf failed. In all of these cases an error
+		/// will be printed to log
+		bool doSetRawData(const uint8_t* pRawData, int rawDataLen, bool takeOwnership, timespec timestamp,
+		                  LinkLayerType layerType, int frameLength) override;
 	};
 
 	/// @typedef MBufRawPacketVector