Cleanup of PcapLiveDevice capture machinery.
#1838
Conversation
Removed redundant fields.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #1838 +/- ##
==========================================
- Coverage 82.81% 82.76% -0.06%
==========================================
Files 291 291
Lines 51596 51640 +44
Branches 11448 11251 -197
==========================================
+ Hits 42730 42738 +8
- Misses 7708 8071 +363
+ Partials 1158 831 -327
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Can we review this? It has been sitting here for a while. |
|
@Dimi1010 It's a big one... That's why... I will look into this recently. |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the PcapLiveDevice capture machinery to improve encapsulation and thread safety by replacing static member function callbacks with internal linkage free functions. The changes move capture-specific state from device member variables into local context objects, reducing the risk of data races and simplifying the threading model.
- Replaces static member callback functions with internal linkage free functions that use specific context objects
- Eliminates thread-specific member variables from the device class in favor of local context objects
- Consolidates the StatisticsUpdateWorker class into a simpler thread function approach
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| PcapLiveDevice.cpp | Implements new internal capture context structures and free functions, removes StatisticsUpdateWorker class |
| PcapLiveDevice.h | Removes StatisticsUpdateWorker class and capture-related member variables, adds stats thread member |
| { | ||
| PCPP_LOG_ERROR("Unable to extract PcapLiveDevice instance"); | ||
| return; | ||
| OnStatsUpdateCallback cbOnStatsUpdate; |
There was a problem hiding this comment.
Not sure what the convention is for this, no m_ prefix?
There was a problem hiding this comment.
From what I understand, the m_ prefix is only for private / protected fields?
Other structures with public fields also don't use it.
|
@seladb can you look through this one when you have time? It has been sitting for a while. |
This PR moves the implementation of the capture procedures to internally linked functions in
PcapLiveDevice.cppand aims to reduce the the need for extensive bookkeeping of capture thread only variables inside the device object's fields.The static member function callback routines for
pcap_dispatchhave been replaced with internal linkage free functions to improve readability and encapsulation. The user token is now expected to be a pointer to a specific capture context object instead of the device instance.Replacement list:
onPacketArrivesCallbackreplacesonPacketArrives. Expects user token of typeCaptureContextonPacketArrivesAccumulatorreplacesonPacketArrivesNoCallback. Expects user token of typeAccumulatorCaptureContextonPacketArrivesCallbackWithCancellationreplacesonPacketArrivesBlockingMode. Expects user token of typeCaptureContextSTonPacketArrivesNoopto be used when no handling is required.Replaced
captureThreadMainstatic private function with internal linkage free functions. The free functions take their inputs as parameters instead of relying on data from the implicitthispointer, simplifying data ownership and reducing risk of data races.captureThreadMain- used for callback or noop capture.captureThreadMainAccumulator- used for accumulating the packets into aRawPacketVector.Changed blocking mode capture to utilize
onPacketArrivesCallbackWithCancellation. The new infrastructure provides a layer of separation between the device threading machinery and the cancellation requests by relying onrequestStopflag to record a cancellation request that is then forwarded tom_StopThreadafter dispatch finishes instead of directly writing tom_StopThread.Removed
PcapLiveDevicemember fields previously only used to store data for the capture thread as that data is now stored in context objects created on demand during capture startup.