Refactor of PcapRemoteDeviceList's factory functions.

Pcap++/header/PcapRemoteDeviceList.h

@@ -5,6 +5,7 @@
 #include <memory>
 #include "IpAddress.h"
 #include "PcapRemoteDevice.h"
+#include "DeprecationUtils.h"
 
 /// @file
 
@@ -33,23 +34,21 @@ namespace pcpp
 		uint16_t m_RemoteMachinePort;
 		std::shared_ptr<PcapRemoteAuthentication> m_RemoteAuthentication;
 
-		// private c'tor. User should create the list via static methods PcapRemoteDeviceList::getRemoteDeviceList()
-		PcapRemoteDeviceList() : m_RemoteMachinePort(0), m_RemoteAuthentication(nullptr) {}
-
-		void setRemoteMachineIpAddress(const IPAddress& ipAddress);
-		void setRemoteMachinePort(uint16_t port);
-		void setRemoteAuthentication(const PcapRemoteAuthentication* remoteAuth);
+		// private c'tor. User should create the list via static methods PcapRemoteDeviceList::createRemoteDeviceList()
+		PcapRemoteDeviceList(const IPAddress& ipAddress, uint16_t port,
+		                     std::shared_ptr<PcapRemoteAuthentication> remoteAuth,
+		                     std::vector<PcapRemoteDevice*> deviceList);
 
 	public:
 		/**
 		 * Iterator object that can be used for iterating all PcapRemoteDevice in list
 		 */
-		typedef typename std::vector<PcapRemoteDevice*>::iterator RemoteDeviceListIterator;
+		using RemoteDeviceListIterator = typename std::vector<PcapRemoteDevice*>::iterator;
 
 		/**
 		 * Const iterator object that can be used for iterating all PcapRemoteDevice in a constant list
 		 */
-		typedef typename std::vector<PcapRemoteDevice*>::const_iterator ConstRemoteDeviceListIterator;
+		using ConstRemoteDeviceListIterator = typename std::vector<PcapRemoteDevice*>::const_iterator;
 
 		PcapRemoteDeviceList(const PcapRemoteDeviceList&) = delete;
 		PcapRemoteDeviceList(PcapRemoteDeviceList&&) noexcept = delete;
@@ -70,9 +69,30 @@ namespace pcpp
 		 * - IP address provided is NULL or not valid
 		 * - WinPcap/Npcap encountered an error in creating the remote connection string
 		 * - WinPcap/Npcap encountered an error connecting to the rpcapd daemon on the remote machine or retrieving devices on the remote machine
+		 * @deprecated This factory function has been deprecated in favor of 'createRemoteDeviceList' factory for better memory safety.
 		 */
+		PCPP_DEPRECATED("Please use 'createRemoteDeviceList' factory method instead.")
 		static PcapRemoteDeviceList* getRemoteDeviceList(const IPAddress& ipAddress, uint16_t port);
 
+		/**
+		 * A static method for creating a PcapRemoteDeviceList instance for a specific remote machine.
+		 * This methods creates the instance and populates it with PcapRemoteDevice instances.
+		 * Each PcapRemoteDevice instance corresponds to a network interface on the remote machine.
+		 *
+		 * This method overload is for remote daemons which don't require authentication for accessing them.
+		 * For daemons which do require authentication use the other method overload.
+		 *
+		 * @param[in] ipAddress The IP address of the remote machine through which clients can connect to the rpcapd
+		 * daemon
+		 * @param[in] port The port of the remote machine through which clients can connect to the rpcapd daemon
+		 * @return A smart pointer to the newly created PcapRemoteDeviceList, or nullptr if (an appropriate error will be printed
+		 * to log in each case):
+		 * - WinPcap/Npcap encountered an error in creating the remote connection string
+		 * - WinPcap/Npcap encountered an error connecting to the rpcapd daemon on the remote machine or retrieving
+		 * devices on the remote machine
+		 */
+		static std::unique_ptr<PcapRemoteDeviceList> createRemoteDeviceList(const IPAddress& ipAddress, uint16_t port);
+
 		/**
 		 * An overload of the previous getRemoteDeviceList() method but with authentication support. This method is suitable for connecting to
 		 * remote daemons which require authentication for accessing them
@@ -84,9 +104,32 @@ namespace pcpp
 		 * - IP address provided is NULL or not valid
 		 * - WinPcap/Npcap encountered an error in creating the remote connection string
 		 * - WinPcap/Npcap encountered an error connecting to the rpcapd daemon on the remote machine or retrieving devices on the remote machine
+		 * @deprecated This factory function has been deprecated in favor of 'createRemoteDeviceList' factory for better memory safety.
 		 */
+		PCPP_DEPRECATED("Please use 'createRemoteDeviceList' factory method instead.")
 		static PcapRemoteDeviceList* getRemoteDeviceList(const IPAddress& ipAddress, uint16_t port, PcapRemoteAuthentication* remoteAuth);
 
+		/**
+		 * A static method for creating a PcapRemoteDeviceList instance for a specific remote machine.
+		 * This methods creates the instance and populates it with PcapRemoteDevice instances.
+		 * Each PcapRemoteDevice instance corresponds to a network interface on the remote machine.
+		 *
+		 * This method overload is for remote daemons which require authentication for accessing them.
+		 * If no authentication is required, use the other method overload.
+		 *
+		 * @param[in] ipAddress The IP address of the remote machine through which clients can connect to the rpcapd
+		 * daemon
+		 * @param[in] port The port of the remote machine through which clients can connect to the rpcapd daemon
+		 * @param[in] remoteAuth A pointer to the authentication object which contains the username and password for
+		 * connecting to the remote daemon
+		 * @return A smart pointer to the newly created PcapRemoteDeviceList, or nullptr if (an appropriate error will be printed
+		 * to log in each case):
+		 * - WinPcap/Npcap encountered an error in creating the remote connection string
+		 * - WinPcap/Npcap encountered an error connecting to the rpcapd daemon on the remote machine or retrieving
+		 * devices on the remote machine
+		 */
+		static std::unique_ptr<PcapRemoteDeviceList> createRemoteDeviceList(const IPAddress& ipAddress, uint16_t port, PcapRemoteAuthentication const* remoteAuth);
+
 		/**
 		 * @return The IP address of the remote machine
 		 */

Pcap++/src/PcapRemoteDeviceList.cpp

@@ -48,19 +48,40 @@ namespace pcpp
 		}
 	}
 
+PcapRemoteDeviceList::PcapRemoteDeviceList(const IPAddress& ipAddress, uint16_t port, std::shared_ptr<PcapRemoteAuthentication> remoteAuth, std::vector<PcapRemoteDevice*> deviceList)
+	: m_RemoteDeviceList(std::move(deviceList))
+	, m_RemoteMachineIpAddress(ipAddress)
+	, m_RemoteMachinePort(port)
+	, m_RemoteAuthentication(std::move(remoteAuth))
+{}
+
 PcapRemoteDeviceList* PcapRemoteDeviceList::getRemoteDeviceList(const IPAddress& ipAddress, uint16_t port)
 {
-	return PcapRemoteDeviceList::getRemoteDeviceList(ipAddress, port, NULL);
+	auto result = PcapRemoteDeviceList::createRemoteDeviceList(ipAddress, port);
+	return result.release();
+}
+
+std::unique_ptr<PcapRemoteDeviceList> PcapRemoteDeviceList::createRemoteDeviceList(const IPAddress& ipAddress, uint16_t port)
+{
+	return PcapRemoteDeviceList::createRemoteDeviceList(ipAddress, port, nullptr);
 }
 
 PcapRemoteDeviceList* PcapRemoteDeviceList::getRemoteDeviceList(const IPAddress& ipAddress, uint16_t port, PcapRemoteAuthentication* remoteAuth)
 {
+	auto result = PcapRemoteDeviceList::createRemoteDeviceList(ipAddress, port, remoteAuth);
+	return result.release();
+}
+
+std::unique_ptr<PcapRemoteDeviceList> PcapRemoteDeviceList::createRemoteDeviceList(const IPAddress& ipAddress, uint16_t port, PcapRemoteAuthentication const* remoteAuth)
+{
+	std::shared_ptr<PcapRemoteAuthentication> remoteAuthCopy;
 	pcap_rmtauth* pRmAuth = nullptr;
 	pcap_rmtauth rmAuth;
 	if (remoteAuth != nullptr)
 	{
 		PCPP_LOG_DEBUG("Authentication requested. Username: " << remoteAuth->userName << ", Password: " << remoteAuth->password);
-		rmAuth = remoteAuth->getPcapRmAuth();
+		remoteAuthCopy = std::make_shared<PcapRemoteAuthentication>(*remoteAuth);
+		rmAuth = remoteAuthCopy->getPcapRmAuth();
 		pRmAuth = &rmAuth;
 	}
 
@@ -75,20 +96,29 @@ PcapRemoteDeviceList* PcapRemoteDeviceList::getRemoteDeviceList(const IPAddress&
 		return nullptr;
 	}
 
-	PcapRemoteDeviceList* resultList = new PcapRemoteDeviceList();
-	resultList->setRemoteMachineIpAddress(ipAddress);
-	resultList->setRemoteMachinePort(port);
-	resultList->setRemoteAuthentication(remoteAuth);
-
-
-	for (pcap_if_t* currInterface = interfaceList.get(); currInterface != nullptr; currInterface = currInterface->next)
+	std::vector<PcapRemoteDevice*> devices;
+	try
+	{
+		for (pcap_if_t* currInterface = interfaceList.get(); currInterface != nullptr; currInterface = currInterface->next)
+		{
+			auto pNewRemoteDevice = std::unique_ptr<PcapRemoteDevice>(new PcapRemoteDevice(currInterface, remoteAuthCopy, ipAddress, port));
+			// Release is called after pushback to prevent memory leaks if vector reallocation fails.
+			// cppcheck-suppress danglingLifetime
+			devices.push_back(pNewRemoteDevice.get());
+			pNewRemoteDevice.release();
+		}
+	}
+	catch (const std::exception& e)
 	{
-		PcapRemoteDevice* pNewRemoteDevice = new PcapRemoteDevice(currInterface, resultList->m_RemoteAuthentication,
-				resultList->getRemoteMachineIpAddress(), resultList->getRemoteMachinePort());
-		resultList->m_RemoteDeviceList.push_back(pNewRemoteDevice);
+		for (auto device : devices)
+		{
+			delete device;
+		}
+		PCPP_LOG_ERROR("Error creating remote devices: " << e.what());
+		return nullptr;
 	}
 
-	return resultList;
+	return std::unique_ptr<PcapRemoteDeviceList>(new PcapRemoteDeviceList(ipAddress, port, remoteAuthCopy, devices));
 }
 
 PcapRemoteDevice* PcapRemoteDeviceList::getRemoteDeviceByIP(const std::string& ipAddrAsString) const
@@ -190,26 +220,6 @@ PcapRemoteDevice* PcapRemoteDeviceList::getRemoteDeviceByIP(const IPv6Address& i
 
 }
 
-void PcapRemoteDeviceList::setRemoteMachineIpAddress(const IPAddress& ipAddress)
-{
-	m_RemoteMachineIpAddress = ipAddress;
-}
-
-void PcapRemoteDeviceList::setRemoteMachinePort(uint16_t port)
-{
-	m_RemoteMachinePort = port;
-}
-
-void PcapRemoteDeviceList::setRemoteAuthentication(const PcapRemoteAuthentication* remoteAuth)
-{
-	if (remoteAuth != nullptr)
-		m_RemoteAuthentication = std::shared_ptr<PcapRemoteAuthentication>(new PcapRemoteAuthentication(*remoteAuth));
-	else
-	{
-		m_RemoteAuthentication = nullptr;
-	}
-}
-
 PcapRemoteDeviceList::~PcapRemoteDeviceList()
 {
 	while (m_RemoteDeviceList.size() > 0)