Skip to content

Commit 1debe6b

Browse files
authored
Fixes PointerVector's slicing issue for polymorphic types. (#1550)
* Fixes PointerVector's slicing issue for polymorphic types. - Adds a clone method that is enabled when the held type is non-polymorphic or contains a clone method. - Deprecates the copy constructor and copy assignment operator. * Fixed usages of post C++11 symbols. * Updated PointerVector to use Copier helper struct to facilitate a copy mechanism. * Added clone to RawPacket as it is polymorphic. * Remove unused include.
1 parent 0aa0c83 commit 1debe6b

File tree

5 files changed

+57
-10
lines changed

5 files changed

+57
-10
lines changed

Common++/header/PointerVector.h

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include <stdexcept>
77
#include <vector>
88
#include <memory>
9+
#include <type_traits>
910

1011
#include "DeprecationUtils.h"
1112

@@ -17,6 +18,34 @@
1718
*/
1819
namespace pcpp
1920
{
21+
namespace internal
22+
{
23+
/**
24+
* @brief A helper struct to facilitate the creation of a copy of an object.
25+
* @tparam T The type of object to copy.
26+
* @tparam Enable Helper parameter for SFINAE.
27+
*/
28+
template <class T, class Enable = void> struct Copier
29+
{
30+
std::unique_ptr<T> operator()(const T& obj) const
31+
{
32+
return std::unique_ptr<T>(new T(obj));
33+
}
34+
};
35+
36+
/**
37+
* @brief A specialization of Copier to facilitate the safe copying of polymorphic objects via clone() method.
38+
* @tparam T The type of object to copy.
39+
*/
40+
template <class T> struct Copier<T, typename std::enable_if<std::is_polymorphic<T>::value>::type>
41+
{
42+
std::unique_ptr<T> operator()(const T& obj) const
43+
{
44+
// Clone can return unique_ptr or raw pointer.
45+
return std::unique_ptr<T>(std::move(obj.clone()));
46+
}
47+
};
48+
} // namespace internal
2049

2150
/**
2251
* @class PointerVector
@@ -343,21 +372,16 @@ namespace pcpp
343372
static std::vector<T*> deepCopyUnsafe(std::vector<T*> const& origin)
344373
{
345374
std::vector<T*> copyVec;
375+
// Allocate the vector initially to ensure no exceptions are thrown during push_back.
376+
copyVec.reserve(origin.size());
346377

347378
try
348379
{
349380
for (const auto iter : origin)
350381
{
351-
T* objCopy = new T(*iter);
352-
try
353-
{
354-
copyVec.push_back(objCopy);
355-
}
356-
catch (const std::exception&)
357-
{
358-
delete objCopy;
359-
throw;
360-
}
382+
std::unique_ptr<T> objCopy = internal::Copier<T>()(*iter);
383+
// There shouldn't be a memory leak as the vector is reserved.
384+
copyVec.push_back(objCopy.release());
361385
}
362386
}
363387
catch (const std::exception&)

Packet++/header/RawPacket.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,12 @@ namespace pcpp
339339
*/
340340
RawPacket& operator=(const RawPacket& other);
341341

342+
/**
343+
* @brief Clones the current packet. Caller is responsible for deallocation of the memory.
344+
* @return A pointer to the new RawPacket object which is a clone of this object
345+
*/
346+
virtual RawPacket* clone() const;
347+
342348
/**
343349
* @return RawPacket object type. Each derived class should return a different value
344350
*/

Packet++/src/RawPacket.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,11 @@ namespace pcpp
6868
return *this;
6969
}
7070

71+
RawPacket* RawPacket::clone() const
72+
{
73+
return new RawPacket(*this);
74+
}
75+
7176
void RawPacket::copyDataFrom(const RawPacket& other, bool allocateData)
7277
{
7378
if (!other.m_RawPacketSet)

Pcap++/header/MBufRawPacket.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,13 @@ namespace pcpp
166166
*/
167167
MBufRawPacket& operator=(const MBufRawPacket& other);
168168

169+
/**
170+
* @brief Clone this MBufRawPacket object. See copy constructor for details.
171+
* The caller is responsible for the deallocation of the returned pointer.
172+
* @return A pointer to the new MBufRawPacket object which is a clone of this object
173+
*/
174+
MBufRawPacket* clone() const override;
175+
169176
/**
170177
* Set raw data to the mbuf by copying the data to it. In order to stay compatible with the ancestor method
171178
* which takes control of the data pointer and frees it when RawPacket is destroyed, this method frees this

Pcap++/src/MBufRawPacket.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,11 @@ namespace pcpp
175175
return *this;
176176
}
177177

178+
MBufRawPacket* MBufRawPacket::clone() const
179+
{
180+
return new MBufRawPacket(*this);
181+
}
182+
178183
bool MBufRawPacket::setRawData(const uint8_t* pRawData, int rawDataLen, timespec timestamp, LinkLayerType layerType,
179184
int frameLength)
180185
{

0 commit comments

Comments
 (0)