Consolidate shared DeviceList functionality under a common base class.

[Dimi1010]

Rework of #1431 p1 and p2.

• Added deleter extension point to PointerVector<T> now PointerVector<T, Deleter>.
• Added base class DeviceListBase<DeviceType> implementing common functionality between device lists. Common functionality includes:
  • A protected PointerVector<DeviceType> member variable.
  • Element accessor methods at/front/back
  • Iterator accessor methods begin/end, cbegin/cend.
  • Capacity accessors size/empty
• Refactored all device lists to inherit from DeviceListBase
• Removed iterator API implementation in RemoteDeviceList. Superseded by iterator API inherited from DeviceListBase.
• Deprecated getPcap*DevicesList - Superseded by the direct accessor methods provided by DeviceListBase. The list classes should provide direct functions for iteration and fetching devices. It makes no sense to have to get another 'List' from a 'List' class. Additionally this getter in particular returns a reference which ties the internal implementation to it making it difficult to change. Affected Lists:
  • PcapLiveDeviceList
  • PfRingDeviceList
  • DpdkDeviceList
• Updated usages of deprecated methods in the library with non-deprecated methods.

[Dimi1010]

@tigercosmos added PfRingDeviceList to the refactor last minute. Sry for the re-review request.

[seladb]

@Dimi1010 CI fails for some reason in generating the codecov report, and idea why? 🤔

[clementperon]

Owner

I have the same issue in my MR https://github.com/seladb/PcapPlusPlus/pull/1370

I didn't touch to the code at all

[seladb]

@clementperon gcovr complains it finds the same method pcpp::PcapLiveDeviceList::getInstance() in multiple lines: 51 ,63. Apparently line 51 is the location of the method before the change, and line 63 is the location after the change:

gcovr also complains about a new file version when it compares the file to the file in .ccache. Maybe the issue is related to ccache we just added?

Stderr of gcov was >>/__w/PcapPlusPlus/PcapPlusPlus/Examples/IcmpFileTransfer/Common.cpp:source file is newer than notes file '__w/PcapPlusPlus/PcapPlusPlus/.ccache/d/d/fac836f17d373ef9882523799d4b3d-1445515.gcno'
(the message is displayed only once per source file)
/__w/PcapPlusPlus/PcapPlusPlus/Packet++/header/Packet.h:source file is newer than notes file '__w/PcapPlusPlus/PcapPlusPlus/.ccache/d/d/fac836f17d373ef9882523799d4b3d-1445515.gcno'
/__w/PcapPlusPlus/PcapPlusPlus/Packet++/header/IcmpLayer.h:source file is newer than notes file '__w/PcapPlusPlus/PcapPlusPlus/.ccache/d/d/fac836f17d373ef9882523799d4b3d-1445515.gcno'
/__w/PcapPlusPlus/PcapPlusPlus/Packet++/header/IPv4Layer.h:source file is newer than notes file '__w/PcapPlusPlus/PcapPlusPlus/.ccache/d/d/fac836f17d373ef9882523799d4b3d-1445515.gcno'
/__w/PcapPlusPlus/PcapPlusPlus/Packet++/header/TLVData.h:source file is newer than notes file '__w/PcapPlusPlus/PcapPlusPlus/.ccache/d/d/fac836f17d373ef9882523799d4b3d-1445515.gcno'
/__w/PcapPlusPlus/PcapPlusPlus/Packet++/header/IPLayer.h:source file is newer than notes file '__w/PcapPlusPlus/PcapPlusPlus/.ccache/d/d/fac836f17d373ef9882523799d4b3d-1445515.gcno'
/__w/PcapPlusPlus/PcapPlusPlus/Packet++/header/EthLayer.h:source file is newer than notes file '__w/PcapPlusPlus/PcapPlusPlus/.ccache/d/d/fac836f17d373ef9882523799d4b3d-1445515.gcno'
/__w/PcapPlusPlus/PcapPlusPlus/Common++/header/PcapPlusPlusVersion.h:source file is newer than notes file '__w/PcapPlusPlus/PcapPlusPlus/.ccache/d/d/fac836f17d373ef9882523799d4b3d-1445515.gcno'
/__w/PcapPlusPlus/PcapPlusPlus/Pcap++/header/PcapLiveDeviceList.h:source file is newer than notes file '__w/PcapPlusPlus/PcapPlusPlus/.ccache/d/d/fac836f17d373ef9882523799d4b3d-1445515.gcno'
__w/PcapPlusPlus/PcapPlusPlus/.ccache/d/d/fac836f17d373ef9882523799d4b3d-1445515.gcda:cannot open data file, assuming not executed
<< End of stderr
Exception was >>Got function pcpp::PcapLiveDeviceList::getInstance() on multiple lines: 51, 63.
	You can run gcovr with --merge-mode-functions=MERGE_MODE.
	The available values for MERGE_MODE are described in the documentation.<< End of stderr

[Dimi1010]

@seladb @clementperon It probably has something to do with ccache. Look at #1487. Commit b13d203 passes all checks and then after merge, commit e418d41 suddenly fails its report with a similar issue. The only notable things merged to the dev branch between the 2 commits are #1476, #1492 and #1495.

[egecetin]

@seladb @clementperon @Dimi1010 Maybe remove all codecov generated files in source tree with grep/wildcard from extension just before saving ccache?

[seladb]

@seladb @clementperon @Dimi1010 Maybe remove all codecov generated files in source tree with grep/wildcard from extension just before saving ccache?

@egecetin I reverted the codecov upgrade, please see my message on Slack. I'm not sure it'll fix this issue, but it'll probably fix the CI failure in this PR: https://github.com/seladb/PcapPlusPlus/pull/1456

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 71.83099% with 60 lines in your changes missing coverage. Please review.

Project coverage is 83.17%. Comparing base (6edb5ef) to head (9d15ce0). Report is 15 commits behind head on dev.

Files with missing lines	Patch %	Lines
Pcap++/src/PcapRemoteDeviceList.cpp	0.00%	26 Missing :warning:
Pcap++/src/PcapLiveDeviceList.cpp	73.68%	9 Missing and 1 partial :warning:
Common++/header/PointerVector.h	60.00%	2 Missing :warning:
Examples/DnsSpoofing/main.cpp	0.00%	2 Missing :warning:
Examples/HttpAnalyzer/main.cpp	0.00%	2 Missing :warning:
Examples/IcmpFileTransfer/Common.cpp	0.00%	2 Missing :warning:
...ples/IcmpFileTransfer/IcmpFileTransfer-catcher.cpp	0.00%	2 Missing :warning:
...ples/IcmpFileTransfer/IcmpFileTransfer-pitcher.cpp	0.00%	2 Missing :warning:
Examples/SSLAnalyzer/main.cpp	0.00%	2 Missing :warning:
Examples/TLSFingerprinting/main.cpp	0.00%	2 Missing :warning:
... and 7 more

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #1488      +/-   ##
==========================================
+ Coverage   83.10%   83.17%   +0.07%     
==========================================
  Files         283      285       +2     
  Lines       48933    48989      +56     
  Branches    10542    10543       +1     
==========================================
+ Hits        40666    40747      +81     
+ Misses       7168     7105      -63     
- Partials     1099     1137      +38     

Flag	Coverage Δ
alpine320	75.09% <67.36%> (+<0.01%)	:arrow_up:
fedora42	75.17% <66.30%> (-0.03%)	:arrow_down:
macos-13	80.66% <79.87%> (+0.05%)	:arrow_up:
macos-14	80.66% <79.87%> (+0.05%)	:arrow_up:
macos-15	80.63% <79.87%> (+0.05%)	:arrow_up:
mingw32	70.73% <35.48%> (-0.06%)	:arrow_down:
mingw64	70.71% <35.48%> (-0.04%)	:arrow_down:
npcap	85.15% <65.11%> (+0.07%)	:arrow_up:
rhel94	74.96% <66.66%> (+0.01%)	:arrow_up:
ubuntu2004	58.56% <55.29%> (+0.01%)	:arrow_up:
ubuntu2004-zstd	58.66% <55.29%> (-0.04%)	:arrow_down:
ubuntu2204	74.88% <66.66%> (-0.02%)	:arrow_down:
ubuntu2204-icpx	61.53% <72.46%> (+0.05%)	:arrow_up:
ubuntu2404	75.15% <67.02%> (+<0.01%)	:arrow_up:
unittest	83.17% <71.83%> (+0.07%)	:arrow_up:
windows-2019	85.17% <65.11%> (+0.08%)	:arrow_up:
windows-2022	85.19% <65.11%> (+0.07%)	:arrow_up:
winpcap	85.35% <65.11%> (+0.09%)	:arrow_up:
xdp	50.54% <15.59%> (-0.05%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[seladb]

@Dimi1010 although it's not a breaking change, it's a pretty big change that I'm not sure we should introduce just before the upcoming release. We already have a few deprecation and breaking changes...

What do you think about waiting with this PR until the release?

[Dimi1010]

@seladb Sure, no problem.

[tigercosmos]

@Dimi1010 although it's not a breaking change, it's a pretty big change that I'm not sure we should introduce just before the upcoming release. We already have a few deprecation and breaking changes...

What do you think about waiting with this PR until the release?

I'm also uncertain about the statement. Even if we don't include this in the upcoming release, the next one is likely to introduce even more breaking changes.

[seladb]

@Dimi1010 although it's not a breaking change, it's a pretty big change that I'm not sure we should introduce just before the upcoming release. We already have a few deprecation and breaking changes... What do you think about waiting with this PR until the release?

I'm also uncertain about the statement. Even if we don't include this in the upcoming release, the next one is likely to introduce even more breaking changes.

I hope that we'll release the version more frequently than the existing one, so hopefully each version will include fewer API changes and breaking changes

[Dimi1010]

@seladb can we revisit this one for merge?

[seladb]

@Dimi1010 this PR is pretty big, hard to review and affects many parts of the code. Maybe we can break it up to multiple PRs? The first one can introduce the DeviceListBase class and update one of the simpler lists (the one with the least changes). Then subsequent PRs can modify more lists. Please let me know if that's ok with you

[Dimi1010]

This should be fine since the lists themselves don't have any interconnected dependencies.

[Dimi1010]

Closing as majority is implemented through PRs: #1787 #1790 #1791 #1794 #1795 #1796 #1797