From c70e8dd8edeca06bdae0ec4e6a5a46386f54e2bf Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Sat, 25 Oct 2025 11:37:51 +0300 Subject: [PATCH 1/7] Updates the `IFilterableDevice` interface to utilize a NVI pattern. This prevents issues such as shadowing overloads when multiple overloads are available and only 1 is overloaded. For example: `setFilter` previously required `using::setFilter` declaration on every class that included an override it. The change consolidates all device specific filter update code in `doUpdateFilter` hook. The methods `setFiltter` (+overloads) / `clearFilter` are no longer virtual and can be overridden. Their implementation is now forwarded through `doUpdateFilter`. --- Pcap++/header/Device.h | 24 ++++++++++++++++++++---- Pcap++/header/PcapDevice.h | 17 ++--------------- Pcap++/header/PcapFileDevice.h | 7 ++----- Pcap++/header/PfRingDevice.h | 13 +++++-------- Pcap++/src/PcapDevice.cpp | 16 +++++++++------- Pcap++/src/PcapFileDevice.cpp | 2 +- Pcap++/src/PfRingDevice.cpp | 16 ++++++++++++++-- 7 files changed, 53 insertions(+), 42 deletions(-) diff --git a/Pcap++/header/Device.h b/Pcap++/header/Device.h index c5a3210270..d35180eb55 100644 --- a/Pcap++/header/Device.h +++ b/Pcap++/header/Device.h @@ -53,11 +53,11 @@ namespace pcpp /// received /// @param[in] filter The filter to be set in PcapPlusPlus' GeneralFilter format /// @return True if filter set successfully, false otherwise - virtual bool setFilter(GeneralFilter& filter) + bool setFilter(GeneralFilter& filter) { std::string filterAsString; filter.parseToString(filterAsString); - return setFilter(filterAsString); + return doUpdateFilter(filterAsString); } /// Set a filter for the device. When implemented by the device, only packets that match the filter will be @@ -65,10 +65,26 @@ namespace pcpp /// @param[in] filterAsString The filter to be set in Berkeley Packet Filter (BPF) syntax /// (http://biot.com/capstats/bpf.html) /// @return True if filter set successfully, false otherwise - virtual bool setFilter(std::string filterAsString) = 0; + bool setFilter(std::string const& filterAsString) + { + return doUpdateFilter(filterAsString); + } /// Clear the filter currently set on the device /// @return True if filter was removed successfully or if no filter was set, false otherwise - virtual bool clearFilter() = 0; + bool clearFilter() + { + return doUpdateFilter(""); + } + + protected: + /// @brief Updates the filter on the device with a BPF string. + /// + /// Only packets that match the filter should be received by the device after this method is called. + /// An empty string is synonymous with ANY filter (i.e., no filtering). + /// + /// @param filterAsString A string representing the filter in BPF syntax (http://biot.com/capstats/bpf.html). + /// @return True if the operation was successful, false otherwise. + virtual bool doUpdateFilter(std::string const& filterAsString) = 0; }; } // namespace pcpp diff --git a/Pcap++/header/PcapDevice.h b/Pcap++/header/PcapDevice.h index 68d73eddf1..da28371db1 100644 --- a/Pcap++/header/PcapDevice.h +++ b/Pcap++/header/PcapDevice.h @@ -164,20 +164,7 @@ namespace pcpp PCPP_DEPRECATED("Prefer GeneralFilter::matches(...) method directly.") static bool matchPacketWithFilter(GeneralFilter& filter, RawPacket* rawPacket); - // implement abstract methods - - using IFilterableDevice::setFilter; - - /// Set a filter for the device. When implemented by the device, only packets that match the filter will be - /// received. Please note that when the device is closed the filter is reset so when reopening the device you - /// need to call this method again in order to reactivate the filter - /// @param[in] filterAsString The filter to be set in Berkeley Packet Filter (BPF) syntax - /// (http://biot.com/capstats/bpf.html) - /// @return True if filter set successfully, false otherwise - bool setFilter(std::string filterAsString) override; - - /// Clear the filter currently set on device - /// @return True if filter was removed successfully or if no filter was set, false otherwise - bool clearFilter() override; + protected: + bool doUpdateFilter(std::string const& filterAsString) override; }; } // namespace pcpp diff --git a/Pcap++/header/PcapFileDevice.h b/Pcap++/header/PcapFileDevice.h index 61513d560f..fffba9316a 100644 --- a/Pcap++/header/PcapFileDevice.h +++ b/Pcap++/header/PcapFileDevice.h @@ -476,11 +476,8 @@ namespace pcpp /// Flush and close the pcap-ng file void close() override; - /// Set a filter for PcapNG writer device. Only packets that match the filter will be persisted - /// @param[in] filterAsString The filter to be set in Berkeley Packet Filter (BPF) syntax - /// (http://biot.com/capstats/bpf.html) - /// @return True if filter set successfully, false otherwise - bool setFilter(std::string filterAsString) override; + protected: + bool doUpdateFilter(std::string const& filterAsString) override; private: /// @struct PcapNgMetadata diff --git a/Pcap++/header/PfRingDevice.h b/Pcap++/header/PfRingDevice.h index 716f91ef10..37096d5c5d 100644 --- a/Pcap++/header/PfRingDevice.h +++ b/Pcap++/header/PfRingDevice.h @@ -315,15 +315,12 @@ namespace pcpp return m_DeviceOpened; } - using IFilterableDevice::setFilter; + protected: + bool doUpdateFilter(std::string const& filterAsString) override; - /// Sets a BPF filter to the device - /// @param[in] filterAsString The BPF filter in string format - bool setFilter(std::string filterAsString); - - /// Remove a filter if currently set - /// @return True if filter was removed successfully or if no filter was set, false otherwise - bool clearFilter(); + private: + bool removeFilterForAllChannels(); + bool setFilterForAllChannels(std::string const& filterAsString); }; } // namespace pcpp diff --git a/Pcap++/src/PcapDevice.cpp b/Pcap++/src/PcapDevice.cpp index 616d76ce36..b3c03f58b0 100644 --- a/Pcap++/src/PcapDevice.cpp +++ b/Pcap++/src/PcapDevice.cpp @@ -133,7 +133,7 @@ namespace pcpp return stats; } - bool IPcapDevice::setFilter(std::string filterAsString) + bool IPcapDevice::doUpdateFilter(std::string const& filterAsString) { PCPP_LOG_DEBUG("Filter to be set: '" << filterAsString << "'"); if (!isOpened()) @@ -142,12 +142,14 @@ namespace pcpp return false; } - return m_PcapDescriptor.setFilter(filterAsString); - } - - bool IPcapDevice::clearFilter() - { - return m_PcapDescriptor.clearFilter(); + if (filterAsString.empty()) + { + return m_PcapDescriptor.clearFilter(); + } + else + { + return m_PcapDescriptor.setFilter(filterAsString); + } } bool IPcapDevice::matchPacketWithFilter(GeneralFilter& filter, RawPacket* rawPacket) diff --git a/Pcap++/src/PcapFileDevice.cpp b/Pcap++/src/PcapFileDevice.cpp index ee7cbd009e..4604d998d5 100644 --- a/Pcap++/src/PcapFileDevice.cpp +++ b/Pcap++/src/PcapFileDevice.cpp @@ -876,7 +876,7 @@ namespace pcpp PCPP_LOG_DEBUG("File writer closed for file '" << m_FileName << "'"); } - bool PcapNgFileWriterDevice::setFilter(std::string filterAsString) + bool PcapNgFileWriterDevice::doUpdateFilter(std::string const& filterAsString) { return m_BpfWrapper.setFilter(filterAsString); } diff --git a/Pcap++/src/PfRingDevice.cpp b/Pcap++/src/PfRingDevice.cpp index 0638c40e71..cf69923320 100644 --- a/Pcap++/src/PfRingDevice.cpp +++ b/Pcap++/src/PfRingDevice.cpp @@ -366,7 +366,7 @@ namespace pcpp return SystemCores::IdToSystemCore[sched_getcpu()]; } - bool PfRingDevice::setFilter(std::string filterAsString) + bool PfRingDevice::doUpdateFilter(std::string const& filterAsString) { if (!m_DeviceOpened) { @@ -374,6 +374,18 @@ namespace pcpp return false; } + if (filterAsString.empty()) + { + return removeFilterForAllChannels(); + } + else + { + return setFilterForAllChannels(filterAsString); + } + } + + bool PfRingDevice::setFilterForAllChannels(std::string const& filterAsString) + { for (pfring* rxChannel : m_PfRingDescriptors) { int res = pfring_set_bpf_filter(rxChannel, (char*)filterAsString.c_str()); @@ -394,7 +406,7 @@ namespace pcpp return true; } - bool PfRingDevice::clearFilter() + bool PfRingDevice::removeFilterForAllChannels() { if (!m_IsFilterCurrentlySet) return true; From 8f12e7c11ecff03d4a2a45135e03d008584f8a97 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Sat, 25 Oct 2025 11:48:20 +0300 Subject: [PATCH 2/7] Updated PcapNgFileReaderDevice. --- Pcap++/header/Device.h | 8 ++++---- Pcap++/header/PcapFileDevice.h | 9 +++------ Pcap++/src/PcapFileDevice.cpp | 2 +- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/Pcap++/header/Device.h b/Pcap++/header/Device.h index d35180eb55..ad4c2b4c5f 100644 --- a/Pcap++/header/Device.h +++ b/Pcap++/header/Device.h @@ -61,7 +61,7 @@ namespace pcpp } /// Set a filter for the device. When implemented by the device, only packets that match the filter will be - /// received + /// processed. /// @param[in] filterAsString The filter to be set in Berkeley Packet Filter (BPF) syntax /// (http://biot.com/capstats/bpf.html) /// @return True if filter set successfully, false otherwise @@ -79,10 +79,10 @@ namespace pcpp protected: /// @brief Updates the filter on the device with a BPF string. - /// - /// Only packets that match the filter should be received by the device after this method is called. + /// + /// Only packets that match the filter should be processed by the device after this method is called. /// An empty string is synonymous with ANY filter (i.e., no filtering). - /// + /// /// @param filterAsString A string representing the filter in BPF syntax (http://biot.com/capstats/bpf.html). /// @return True if the operation was successful, false otherwise. virtual bool doUpdateFilter(std::string const& filterAsString) = 0; diff --git a/Pcap++/header/PcapFileDevice.h b/Pcap++/header/PcapFileDevice.h index fffba9316a..29ebf52e37 100644 --- a/Pcap++/header/PcapFileDevice.h +++ b/Pcap++/header/PcapFileDevice.h @@ -370,14 +370,11 @@ namespace pcpp /// for some reason (for example: file path does not exist) bool open(); - /// Set a filter for PcapNG reader device. Only packets that match the filter will be received - /// @param[in] filterAsString The filter to be set in Berkeley Packet Filter (BPF) syntax - /// (http://biot.com/capstats/bpf.html) - /// @return True if filter set successfully, false otherwise - bool setFilter(std::string filterAsString); - /// Close the pacp-ng file void close(); + + protected: + bool doUpdateFilter(std::string const& filter) override; }; /// @class PcapNgFileWriterDevice diff --git a/Pcap++/src/PcapFileDevice.cpp b/Pcap++/src/PcapFileDevice.cpp index 4604d998d5..fe987d0d7e 100644 --- a/Pcap++/src/PcapFileDevice.cpp +++ b/Pcap++/src/PcapFileDevice.cpp @@ -624,7 +624,7 @@ namespace pcpp return getNextPacket(rawPacket, temp); } - bool PcapNgFileReaderDevice::setFilter(std::string filterAsString) + bool PcapNgFileReaderDevice::doUpdateFilter(std::string const& filterAsString) { return m_BpfWrapper.setFilter(filterAsString); } From 8e9748074d7e999feb92a8e0a26d29b9dfdce4aa Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Sat, 25 Oct 2025 12:44:18 +0300 Subject: [PATCH 3/7] Fix inconsistent override warning. --- Pcap++/header/PcapFileDevice.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Pcap++/header/PcapFileDevice.h b/Pcap++/header/PcapFileDevice.h index 29ebf52e37..bb06941fed 100644 --- a/Pcap++/header/PcapFileDevice.h +++ b/Pcap++/header/PcapFileDevice.h @@ -363,15 +363,15 @@ namespace pcpp /// @param[out] rawPacket A reference for an empty RawPacket where the packet will be written /// @return True if a packet was read successfully. False will be returned if the file isn't opened (also, an /// error log will be printed) or if reached end-of-file - bool getNextPacket(RawPacket& rawPacket); + bool getNextPacket(RawPacket& rawPacket) override; /// Open the file name which path was specified in the constructor in a read-only mode /// @return True if file was opened successfully or if file is already opened. False if opening the file failed /// for some reason (for example: file path does not exist) - bool open(); + bool open() override; /// Close the pacp-ng file - void close(); + void close() override; protected: bool doUpdateFilter(std::string const& filter) override; From 46f0374fa7eac6e1b64c178f8c95b00de231649a Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Wed, 19 Nov 2025 13:58:16 +0200 Subject: [PATCH 4/7] Update `doUpdateFilter` to account for possible valid `""` filter string; --- Pcap++/header/Device.h | 13 +++++++------ Pcap++/header/PcapDevice.h | 2 +- Pcap++/header/PcapFileDevice.h | 4 ++-- Pcap++/header/PfRingDevice.h | 2 +- Pcap++/src/PcapDevice.cpp | 6 +++--- Pcap++/src/PcapFileDevice.cpp | 18 ++++++++++++++---- Pcap++/src/PfRingDevice.cpp | 4 ++-- 7 files changed, 30 insertions(+), 19 deletions(-) diff --git a/Pcap++/header/Device.h b/Pcap++/header/Device.h index ad4c2b4c5f..5a4f57983f 100644 --- a/Pcap++/header/Device.h +++ b/Pcap++/header/Device.h @@ -57,7 +57,7 @@ namespace pcpp { std::string filterAsString; filter.parseToString(filterAsString); - return doUpdateFilter(filterAsString); + return doUpdateFilter(&filterAsString); } /// Set a filter for the device. When implemented by the device, only packets that match the filter will be @@ -67,24 +67,25 @@ namespace pcpp /// @return True if filter set successfully, false otherwise bool setFilter(std::string const& filterAsString) { - return doUpdateFilter(filterAsString); + return doUpdateFilter(&filterAsString); } /// Clear the filter currently set on the device /// @return True if filter was removed successfully or if no filter was set, false otherwise bool clearFilter() { - return doUpdateFilter(""); + return doUpdateFilter(nullptr); } protected: /// @brief Updates the filter on the device with a BPF string. /// /// Only packets that match the filter should be processed by the device after this method is called. - /// An empty string is synonymous with ANY filter (i.e., no filtering). + /// An nullptr should disable any existing filter on the device. /// - /// @param filterAsString A string representing the filter in BPF syntax (http://biot.com/capstats/bpf.html). + /// @param filterAsString A pointer to a string representing the filter in BPF syntax + /// (http://biot.com/capstats/bpf.html). /// @return True if the operation was successful, false otherwise. - virtual bool doUpdateFilter(std::string const& filterAsString) = 0; + virtual bool doUpdateFilter(std::string const* filterAsString) = 0; }; } // namespace pcpp diff --git a/Pcap++/header/PcapDevice.h b/Pcap++/header/PcapDevice.h index da28371db1..27e5b9431c 100644 --- a/Pcap++/header/PcapDevice.h +++ b/Pcap++/header/PcapDevice.h @@ -165,6 +165,6 @@ namespace pcpp static bool matchPacketWithFilter(GeneralFilter& filter, RawPacket* rawPacket); protected: - bool doUpdateFilter(std::string const& filterAsString) override; + bool doUpdateFilter(std::string const* filterAsString) override; }; } // namespace pcpp diff --git a/Pcap++/header/PcapFileDevice.h b/Pcap++/header/PcapFileDevice.h index bb06941fed..75ddd77c26 100644 --- a/Pcap++/header/PcapFileDevice.h +++ b/Pcap++/header/PcapFileDevice.h @@ -374,7 +374,7 @@ namespace pcpp void close() override; protected: - bool doUpdateFilter(std::string const& filter) override; + bool doUpdateFilter(std::string const* filter) override; }; /// @class PcapNgFileWriterDevice @@ -474,7 +474,7 @@ namespace pcpp void close() override; protected: - bool doUpdateFilter(std::string const& filterAsString) override; + bool doUpdateFilter(std::string const* filterAsString) override; private: /// @struct PcapNgMetadata diff --git a/Pcap++/header/PfRingDevice.h b/Pcap++/header/PfRingDevice.h index 37096d5c5d..eeb4f67f13 100644 --- a/Pcap++/header/PfRingDevice.h +++ b/Pcap++/header/PfRingDevice.h @@ -316,7 +316,7 @@ namespace pcpp } protected: - bool doUpdateFilter(std::string const& filterAsString) override; + bool doUpdateFilter(std::string const* filterAsString) override; private: bool removeFilterForAllChannels(); diff --git a/Pcap++/src/PcapDevice.cpp b/Pcap++/src/PcapDevice.cpp index b3c03f58b0..e4ca22756c 100644 --- a/Pcap++/src/PcapDevice.cpp +++ b/Pcap++/src/PcapDevice.cpp @@ -133,7 +133,7 @@ namespace pcpp return stats; } - bool IPcapDevice::doUpdateFilter(std::string const& filterAsString) + bool IPcapDevice::doUpdateFilter(std::string const* filterAsString) { PCPP_LOG_DEBUG("Filter to be set: '" << filterAsString << "'"); if (!isOpened()) @@ -142,13 +142,13 @@ namespace pcpp return false; } - if (filterAsString.empty()) + if (filterAsString == nullptr || filterAsString->empty()) { return m_PcapDescriptor.clearFilter(); } else { - return m_PcapDescriptor.setFilter(filterAsString); + return m_PcapDescriptor.setFilter(*filterAsString); } } diff --git a/Pcap++/src/PcapFileDevice.cpp b/Pcap++/src/PcapFileDevice.cpp index fe987d0d7e..8e239a1a46 100644 --- a/Pcap++/src/PcapFileDevice.cpp +++ b/Pcap++/src/PcapFileDevice.cpp @@ -624,9 +624,14 @@ namespace pcpp return getNextPacket(rawPacket, temp); } - bool PcapNgFileReaderDevice::doUpdateFilter(std::string const& filterAsString) + bool PcapNgFileReaderDevice::doUpdateFilter(std::string const* filterAsString) { - return m_BpfWrapper.setFilter(filterAsString); + if (filterAsString == nullptr) + { + return m_BpfWrapper.setFilter(""); + } + + return m_BpfWrapper.setFilter(*filterAsString); } void PcapNgFileReaderDevice::close() @@ -876,9 +881,14 @@ namespace pcpp PCPP_LOG_DEBUG("File writer closed for file '" << m_FileName << "'"); } - bool PcapNgFileWriterDevice::doUpdateFilter(std::string const& filterAsString) + bool PcapNgFileWriterDevice::doUpdateFilter(std::string const* filterAsString) { - return m_BpfWrapper.setFilter(filterAsString); + if (filterAsString == nullptr) + { + return m_BpfWrapper.setFilter(""); + } + + return m_BpfWrapper.setFilter(*filterAsString); } // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/Pcap++/src/PfRingDevice.cpp b/Pcap++/src/PfRingDevice.cpp index cf69923320..9f71364ad2 100644 --- a/Pcap++/src/PfRingDevice.cpp +++ b/Pcap++/src/PfRingDevice.cpp @@ -366,7 +366,7 @@ namespace pcpp return SystemCores::IdToSystemCore[sched_getcpu()]; } - bool PfRingDevice::doUpdateFilter(std::string const& filterAsString) + bool PfRingDevice::doUpdateFilter(std::string const* filterAsString) { if (!m_DeviceOpened) { @@ -374,7 +374,7 @@ namespace pcpp return false; } - if (filterAsString.empty()) + if (filterAsString == nullptr || filterAsString.empty()) { return removeFilterForAllChannels(); } From 2b7867611a33bcdefbd21c7646ab42a3ebe49f1f Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Wed, 19 Nov 2025 14:01:30 +0200 Subject: [PATCH 5/7] Fix --- Pcap++/src/PfRingDevice.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Pcap++/src/PfRingDevice.cpp b/Pcap++/src/PfRingDevice.cpp index 9f71364ad2..0310f33fc9 100644 --- a/Pcap++/src/PfRingDevice.cpp +++ b/Pcap++/src/PfRingDevice.cpp @@ -380,7 +380,7 @@ namespace pcpp } else { - return setFilterForAllChannels(filterAsString); + return setFilterForAllChannels(*filterAsString); } } From e476641b4f1087af0ad3ba8021c213f6c27eae8b Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Wed, 19 Nov 2025 14:01:55 +0200 Subject: [PATCH 6/7] Fix --- Pcap++/src/PfRingDevice.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Pcap++/src/PfRingDevice.cpp b/Pcap++/src/PfRingDevice.cpp index 0310f33fc9..49b8050cc5 100644 --- a/Pcap++/src/PfRingDevice.cpp +++ b/Pcap++/src/PfRingDevice.cpp @@ -374,7 +374,7 @@ namespace pcpp return false; } - if (filterAsString == nullptr || filterAsString.empty()) + if (filterAsString == nullptr || filterAsString->empty()) { return removeFilterForAllChannels(); } From ea58bde851fb89c95a737f8a2f257d89d70dbcfd Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Wed, 19 Nov 2025 14:23:35 +0200 Subject: [PATCH 7/7] Typo. --- Pcap++/header/Device.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Pcap++/header/Device.h b/Pcap++/header/Device.h index 5a4f57983f..41ba38e14c 100644 --- a/Pcap++/header/Device.h +++ b/Pcap++/header/Device.h @@ -81,7 +81,7 @@ namespace pcpp /// @brief Updates the filter on the device with a BPF string. /// /// Only packets that match the filter should be processed by the device after this method is called. - /// An nullptr should disable any existing filter on the device. + /// A nullptr should disable any existing filter on the device. /// /// @param filterAsString A pointer to a string representing the filter in BPF syntax /// (http://biot.com/capstats/bpf.html).