Pcap device list refactoring to use smart pointers. #1386
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| #pragma once | ||
|
|
||
| /// @file | ||
|
|
||
| #include <memory> | ||
| #include "IpAddress.h" | ||
| #include "MemoryUtils.h" | ||
|
|
||
| // Forward declaration | ||
| struct pcap_rmtauth; | ||
|
|
||
| namespace pcpp | ||
| { | ||
| namespace internal | ||
| { | ||
| /** | ||
| * Fetches a list of all network devices on the local machine that LibPcap/WinPcap/NPcap can find. | ||
| * @return A smart pointer to an interface list structure. | ||
| * @throws std::runtime_error The system encountered an error fetching the devices. | ||
| */ | ||
| std::unique_ptr<pcap_if_t, PcapFreeAllDevsDeleter> getAllLocalPcapDevices(); | ||
| #ifdef _WIN32 | ||
| /** | ||
| * Fetches a list of all network devices on a remote machine that WinPcap/NPcap can find. | ||
| * @param[in] ipAddress IP address of the remote machine. | ||
| * @param[in] port Port to use when connecting to the remote machine. | ||
| * @param[in] pRmAuth Pointer to an authentication structure to use when connecting to the remote machine. Nullptr if no authentication is required. | ||
| * @return A smart pointer to an interface list structure. | ||
| * @throws std::runtime_error The system encountered an error fetching the devices. | ||
| */ | ||
| std::unique_ptr<pcap_if_t, PcapFreeAllDevsDeleter> getAllRemotePcapDevices(const IPAddress& ipAddress, uint16_t port, pcap_rmtauth* pRmAuth = nullptr); | ||
| #endif // _WIN32 | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| #pragma once | ||
|
|
||
| // Forward declarations | ||
| struct pcap; | ||
| typedef pcap pcap_t; | ||
| struct pcap_if; | ||
| typedef pcap_if pcap_if_t; | ||
|
|
||
| namespace pcpp | ||
| { | ||
| namespace internal | ||
| { | ||
| /** | ||
| * @class PcapCloseDeleter | ||
| * A deleter that cleans up a pcap_t structure by calling pcap_close. | ||
| */ | ||
| struct PcapCloseDeleter | ||
| { | ||
| void operator()(pcap_t* ptr) const; | ||
| }; | ||
|
|
||
| /** | ||
| * @class PcapFreeAllDevsDeleter | ||
| * A deleter that frees an interface list of pcap_if_t ptr by calling 'pcap_freealldevs' function on it. | ||
| */ | ||
| struct PcapFreeAllDevsDeleter | ||
| { | ||
| void operator()(pcap_if_t* ptr) const; | ||
| }; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| #pragma once | ||
|
|
||
| #include <atomic> | ||
| #include <memory> | ||
| #include <vector> | ||
| #include <string.h> | ||
| #include <thread> | ||
|
|
@@ -24,6 +25,15 @@ typedef struct pcap_addr pcap_addr_t; | |
| */ | ||
| namespace pcpp | ||
| { | ||
| /* | ||
| * @class SmartPtrApiTag | ||
| * Helper tag to disambiguate smart pointer API. | ||
| */ | ||
| struct SmartPtrApiTag{}; | ||
| /** | ||
| * Helper tag constant for disambuguating smart pointer API. | ||
|
There was a problem hiding this comment. typo: disambiguating, btw I rarely see this word, I wonder if it is precise here. There was a problem hiding this comment. Fixed a95c946. I think it should be better now? |
||
| */ | ||
| extern const SmartPtrApiTag SmartPtrApi; | ||
|
|
||
| class PcapLiveDevice; | ||
|
|
||
|
|
@@ -114,9 +124,6 @@ namespace pcpp | |
|
|
||
| // c'tor is not public, there should be only one for every interface (created by PcapLiveDeviceList) | ||
| PcapLiveDevice(pcap_if_t* pInterface, bool calculateMTU, bool calculateMacAddress, bool calculateDefaultGateway); | ||
|
|
||
| void setDeviceMtu(); | ||
| void setDeviceMacAddress(); | ||
|
|
@@ -130,6 +137,8 @@ namespace pcpp | |
| static void onPacketArrivesNoCallback(uint8_t* user, const struct pcap_pkthdr* pkthdr, const uint8_t* packet); | ||
| static void onPacketArrivesBlockingMode(uint8_t* user, const struct pcap_pkthdr* pkthdr, const uint8_t* packet); | ||
| public: | ||
| PcapLiveDevice(const PcapLiveDevice& other) = delete; | ||
|
There was a problem hiding this comment. please also implement moving construction/assignment. There was a problem hiding this comment. btw, if the underlying are shared pointers, why do we disallow coping? There was a problem hiding this comment.
IDK. I think the reason a There was a problem hiding this comment. we can keep the old There was a problem hiding this comment. Ah, I figured out at least partly why its private / deleted. There was a problem hiding this comment.
Added 4bcb50e |
||
| PcapLiveDevice& operator=(const PcapLiveDevice& other) = delete; | ||
|
|
||
| /** | ||
| * The type of the live device | ||
|
|
@@ -571,13 +580,19 @@ namespace pcpp | |
| * @return Pointer to the copied class | ||
| */ | ||
| PcapLiveDevice* clone() const; | ||
| /** | ||
| * Clones the current device class | ||
| * @param[in] apiTag Disambiguating tag for SmartPtrAPI. | ||
| * @return Unique pointer to the copied device. | ||
| */ | ||
| std::unique_ptr<PcapLiveDevice> clone(SmartPtrApiTag apiTag) const; | ||
|
|
||
| void getStatistics(IPcapDevice::PcapStats& stats) const override; | ||
|
|
||
| protected: | ||
| pcap_t* doOpen(const DeviceConfiguration& config); | ||
|
|
||
| virtual std::unique_ptr<PcapLiveDevice> cloneInternal(pcap_if_t& devInterface) const; | ||
| }; | ||
|
|
||
| } // namespace pcpp | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,16 @@ | ||
| #pragma once | ||
|
|
||
| #include "DeprecationUtils.h" | ||
| #include "IpAddress.h" | ||
| #include "PcapLiveDevice.h" | ||
| #include <vector> | ||
| #include <memory> | ||
|
|
||
|
|
||
| /// @file | ||
|
|
||
| #define PCPP_DEPRECATED_RAW_PTR_API PCPP_DEPRECATED("This method is deprecated in favor of the SmartPtrAPI overload.") | ||
|
There was a problem hiding this comment. Doxygen generation really dislikes this macro for some reason, when used on a member function. No idea why... tried several things to fix them including mirroring the way the There was a problem hiding this comment. Doxygen test fails with the following: |
||
|
|
||
| /** | ||
| * \namespace pcpp | ||
| * \brief The main namespace for the PcapPlusPlus lib | ||
|
|
@@ -23,20 +27,24 @@ namespace pcpp | |
| class PcapLiveDeviceList | ||
| { | ||
| private: | ||
| std::vector<std::shared_ptr<PcapLiveDevice>> m_LiveDeviceList; | ||
| // Vector of raw device pointers to keep the signature of getPcapLiveDevicesList, as it returns a reference. | ||
| mutable std::vector<PcapLiveDevice*> m_LiveDeviceListView; | ||
|
|
||
| std::vector<IPv4Address> m_DnsServers; | ||
|
|
||
| // private c'tor | ||
| PcapLiveDeviceList(); | ||
|
|
||
| void init(); | ||
|
|
||
| void setDnsServers(); | ||
|
|
||
| void updateLiveDeviceListView() const; | ||
| public: | ||
| PcapLiveDeviceList(const PcapLiveDeviceList& other) = delete; | ||
| PcapLiveDeviceList& operator=(const PcapLiveDeviceList& other) = delete; | ||
|
|
||
| /** | ||
| * The access method to the singleton | ||
| * @return The singleton instance of this class | ||
|
|
@@ -49,50 +57,105 @@ namespace pcpp | |
|
|
||
| /** | ||
| * @return A vector containing pointers to all live devices currently installed on the machine | ||
| * @deprecated This method is deprecated in favor of the SmartPtrAPI overload. | ||
| */ | ||
| PCPP_DEPRECATED_RAW_PTR_API const std::vector<PcapLiveDevice*>& getPcapLiveDevicesList() const; | ||
|
There was a problem hiding this comment.
In my opinion, it would be best if both lists provided a standardized API. Which one would be better? Getter method: Iterator API: [1] As the current There was a problem hiding this comment. I think it makes sense to have both a getter and an iterator for a container-like object. Regarding the iterator type, since However, if we really don't want to break the API, as you said, it's still possible to have some conversion, but it's just troublesome. There was a problem hiding this comment.
Both is an option.
Yeah, they shouldn't really. unless they decided to save the value from the iterator somewhere. That is a breaking change, but it would only need a
I tried my hand at converting iterator a bit earlier. It is technically possible but it quickly becomes templated black magic if you try to make a general case converting iterator. There was a problem hiding this comment. I prefer to remove the old API, and have both getter and new iterator. Then we can also remove the hack of @seladb what do you say? |
||
| /** | ||
| * @param[in] apiTag Disambiguating tag for SmartPtrAPI. | ||
| * @return A reference to a vector containing shared pointers to all live devices currently installed on the machine. | ||
| */ | ||
| const std::vector<std::shared_ptr<PcapLiveDevice>>& getPcapLiveDevicesList(SmartPtrApiTag apiTag) const { return m_LiveDeviceList; }; | ||
|
|
||
| /** | ||
| * Get a pointer to the live device by its IP address. IP address can be both IPv4 or IPv6 | ||
| * @param[in] ipAddr The IP address defined for the device | ||
| * @return A pointer to the live device if this IP address exists, nullptr otherwise | ||
| * @deprecated This method is deprecated in favor of the SmartPtrAPI overload. | ||
| */ | ||
| PCPP_DEPRECATED_RAW_PTR_API PcapLiveDevice* getPcapLiveDeviceByIp(const IPAddress& ipAddr) const; | ||
| /** | ||
Dimi1010 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| * Get a pointer to the live device by its IP address. IP address can be both IPv4 or IPv6 | ||
| * @param[in] ipAddr The IP address defined for the device | ||
| * @param[in] apiTag Disambiguating tag for SmartPtrAPI. | ||
| * @return A shared pointer to the live device if this IP address exists, nullptr otherwise | ||
| */ | ||
| std::shared_ptr<PcapLiveDevice> getPcapLiveDeviceByIp(const IPAddress& ipAddr, SmartPtrApiTag apiTag) const; | ||
|
|
||
| /** | ||
| * Get a pointer to the live device by its IPv4 address | ||
| * @param[in] ipAddr The IPv4 address defined for the device | ||
| * @return A pointer to the live device if this IPv4 address exists, nullptr otherwise | ||
| * @deprecated This method is deprecated in favor of the SmartPtrAPI overload. | ||
| */ | ||
| PCPP_DEPRECATED_RAW_PTR_API PcapLiveDevice* getPcapLiveDeviceByIp(const IPv4Address& ipAddr) const; | ||
| /** | ||
| * Get a pointer to the live device by its IPv4 address | ||
| * @param[in] ipAddr The IPv4 address defined for the device | ||
| * @param[in] apiTag Disambiguating tag for SmartPtrAPI. | ||
| * @return A shared pointer to the live device if this IPv4 address exists, nullptr otherwise | ||
| */ | ||
| std::shared_ptr<PcapLiveDevice> getPcapLiveDeviceByIp(const IPv4Address& ipAddr, SmartPtrApiTag apiTag) const; | ||
|
|
||
| /** | ||
| * Get a pointer to the live device by its IPv6 address | ||
| * @param[in] ip6Addr The IPv6 address defined for the device | ||
| * @return A pointer to the live device if this IPv6 address exists, nullptr otherwise | ||
| * @deprecated This method is deprecated in favor of the SmartPtrAPI overload. | ||
| */ | ||
| PCPP_DEPRECATED_RAW_PTR_API PcapLiveDevice* getPcapLiveDeviceByIp(const IPv6Address& ip6Addr) const; | ||
| /** | ||
| * Get a pointer to the live device by its IPv6 address | ||
| * @param[in] ip6Addr The IPv6 address defined for the device | ||
| * @param[in] apiTag Disambiguating tag for SmartPtrAPI. | ||
| * @return A shared pointer to the live device if this IPv6 address exists, nullptr otherwise | ||
| * @deprecated This method is deprecated in favor of the SmartPtrAPI overload. | ||
| */ | ||
| std::shared_ptr<PcapLiveDevice> getPcapLiveDeviceByIp(const IPv6Address& ip6Addr, SmartPtrApiTag apiTag) const; | ||
|
|
||
| /** | ||
| * Get a pointer to the live device by its IP address represented as string. IP address can be both IPv4 or IPv6 | ||
| * @param[in] ipAddrAsString The IP address defined for the device as string | ||
| * @return A pointer to the live device if this IP address is valid and exists, nullptr otherwise | ||
| * @deprecated This method is deprecated in favor of the SmartPtrAPI overload. | ||
| */ | ||
| PCPP_DEPRECATED_RAW_PTR_API PcapLiveDevice* getPcapLiveDeviceByIp(const std::string& ipAddrAsString) const; | ||
| /** | ||
| * Get a pointer to the live device by its IP address represented as string. IP address can be both IPv4 or IPv6 | ||
| * @param[in] ipAddrAsString The IP address defined for the device as string | ||
| * @param[in] apiTag Disambiguating tag for SmartPtrAPI. | ||
| * @return A shared pointer to the live device if this IP address is valid and exists, nullptr otherwise | ||
| */ | ||
| std::shared_ptr<PcapLiveDevice> getPcapLiveDeviceByIp(const std::string& ipAddrAsString, SmartPtrApiTag apiTag) const; | ||
|
|
||
| /** | ||
| * Get a pointer to the live device by its name | ||
| * @param[in] name The name of the interface (e.g eth0) | ||
| * @return A pointer to the live device if this name exists, nullprt otherwise | ||
| * @deprecated This method is deprecated in favor of the SmartPtrAPI overload. | ||
| */ | ||
| PCPP_DEPRECATED_RAW_PTR_API PcapLiveDevice* getPcapLiveDeviceByName(const std::string& name) const; | ||
| /** | ||
| * Get a pointer to the live device by its name | ||
| * @param[in] name The name of the interface (e.g eth0) | ||
| * @param[in] apiTag Disambiguating tag for SmartPtrAPI. | ||
| * @return A shared pointer to the live device if this name exists, nullptr otherwise | ||
| */ | ||
| std::shared_ptr<PcapLiveDevice> getPcapLiveDeviceByName(const std::string& name, SmartPtrApiTag apiTag) const; | ||
|
|
||
| /** | ||
| * Get a pointer to the live device by its IP address or name | ||
| * @param[in] ipOrName An IP address or name of the interface | ||
| * @return A pointer to the live device if exists, nullptr otherwise | ||
| * @deprecated This method is deprecated in favor of the SmartPtrAPI overload. | ||
| */ | ||
| PCPP_DEPRECATED_RAW_PTR_API PcapLiveDevice* getPcapLiveDeviceByIpOrName(const std::string& ipOrName) const; | ||
| /** | ||
| * Get a pointer to the live device by its IP address or name | ||
| * @param[in] ipOrName An IP address or name of the interface | ||
| * @param[in] apiTag Disambiguating tag for SmartPtrAPI. | ||
| * @return A shared pointer to the live device if exists, nullptr otherwise | ||
| */ | ||
| std::shared_ptr<PcapLiveDevice> getPcapLiveDeviceByIpOrName(const std::string& ipOrName, SmartPtrApiTag apiTag) const; | ||
|
|
||
| /** | ||
| * @return A list of all DNS servers defined for this machine. If this list is empty it means no DNS servers were defined or they | ||
|
|
@@ -103,16 +166,22 @@ namespace pcpp | |
| /** | ||
| * Copies the current live device list | ||
| * @return A pointer to the cloned device list | ||
| * @deprecated This method is deprecated in favor of the SmartPtrAPI overload. | ||
| */ | ||
| PCPP_DEPRECATED_RAW_PTR_API PcapLiveDeviceList* clone() const; | ||
| /** | ||
| * Copies the current live device list | ||
| * @param[in] apiTag Disambiguating tag for SmartPtrAPI. | ||
| * @return A unique ptr managing the cloned device list | ||
| */ | ||
| std::unique_ptr<PcapLiveDeviceList> clone(SmartPtrApiTag apiTag) const; | ||
|
|
||
| /** | ||
| * Reset the live device list and DNS server list, meaning clear and refetch them | ||
| */ | ||
| void reset(); | ||
| }; | ||
|
|
||
| } // namespace pcpp | ||
|
|
||
| #undef PCPP_DEPRECATED_RAW_PTR_API | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggest to put all util functions in one file, as there is not much code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which one to merge into the other tho? I initially separated them as they have different use cases.
MemoryUtilsis for general utilities w.r.t. smart pointer utilities and deleters.DeviceUtilsis for utility functions for Pcap devices, like fetching devices, etc.MemoryUtilswill be used a lot more as any time a smart pointer to a Pcap handle is used, it would need to be included for the deleter.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. originally I thought we could merge
MemoryUtilsandDeviceUtils, but it did make sense to be separated.