Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated Packet++ headers to use override and Cpp type casts. #1563

Merged
merged 74 commits into from
Sep 24, 2024

Conversation

Dimi1010
Copy link
Collaborator

@Dimi1010 Dimi1010 commented Sep 1, 2024

  • Updated classes in Packet++ to use override specifier.
  • Updated empty destructors to use = default instead of {}
  • Updated many C casts to Cpp casts in Packet++ headers.
  • Removed missingOverrides cppcheck suppression from everywhere except Examples and Pcap++ as those are the only modules that currently raise override warnings.

Copy link

codecov bot commented Sep 1, 2024

Codecov Report

Attention: Patch coverage is 92.85714% with 24 lines in your changes missing coverage. Please review.

Project coverage is 82.88%. Comparing base (0aa0c83) to head (3472012).
Report is 3 commits behind head on dev.

Files with missing lines Patch % Lines
Packet++/header/SSLLayer.h 55.55% 4 Missing ⚠️
Packet++/header/SSLHandshake.h 84.21% 3 Missing ⚠️
Packet++/header/IPSecLayer.h 75.00% 2 Missing ⚠️
Packet++/header/LLCLayer.h 50.00% 2 Missing ⚠️
Packet++/header/StpLayer.h 86.66% 2 Missing ⚠️
Packet++/header/TelnetLayer.h 50.00% 2 Missing ⚠️
Packet++/header/WakeOnLanLayer.h 60.00% 2 Missing ⚠️
Packet++/header/FtpLayer.h 75.00% 1 Missing ⚠️
Packet++/header/GreLayer.h 90.90% 1 Missing ⚠️
Packet++/header/NdpLayer.h 92.30% 1 Missing ⚠️
... and 4 more
Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #1563      +/-   ##
==========================================
- Coverage   82.90%   82.88%   -0.03%     
==========================================
  Files         273      273              
  Lines       46261    46206      -55     
  Branches     9299     9454     +155     
==========================================
- Hits        38354    38297      -57     
- Misses       7053     7306     +253     
+ Partials      854      603     -251     
Flag Coverage Δ
fedora39 74.48% <85.44%> (+<0.01%) ⬆️
macos-12 80.86% <92.85%> (-0.03%) ⬇️
macos-13 80.27% <92.85%> (-0.03%) ⬇️
macos-14 80.20% <92.85%> (-0.03%) ⬇️
mingw32 70.22% <72.58%> (-0.09%) ⬇️
mingw64 70.17% <72.58%> (-0.10%) ⬇️
npcap 84.81% <96.95%> (-0.03%) ⬇️
rhel94 74.23% <85.13%> (-0.03%) ⬇️
ubuntu2004 57.78% <83.69%> (-0.07%) ⬇️
ubuntu2004-zstd 57.90% <83.69%> (-0.07%) ⬇️
ubuntu2204 74.16% <85.13%> (-0.06%) ⬇️
ubuntu2204-icpx 58.37% <88.69%> (-0.02%) ⬇️
ubuntu2404 74.46% <86.06%> (-0.02%) ⬇️
unittest 82.88% <92.85%> (-0.03%) ⬇️
windows-2019 84.86% <96.95%> (-0.03%) ⬇️
windows-2022 84.85% <96.95%> (-0.04%) ⬇️
winpcap 84.83% <96.95%> (-0.02%) ⬇️
xdp 49.13% <83.28%> (-0.08%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Dimi1010 Dimi1010 changed the title Updated Layer headers to use override and Cpp type casts. Updated Packet++ headers to use override and Cpp type casts. Sep 1, 2024
@Dimi1010 Dimi1010 marked this pull request as ready for review September 1, 2024 13:41
Copy link
Collaborator

@tigercosmos tigercosmos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for working on this tedious but important refactoring.

@clementperon
Copy link
Collaborator

LGTM but didn't check all the files, @Dimi1010 did you use CPPcheck to do this ?

@Dimi1010
Copy link
Collaborator Author

Dimi1010 commented Sep 8, 2024

LGTM but didn't check all the files, @Dimi1010 did you use CPPcheck to do this ?

Yes, when I remembered about it. Started by searching for them manually initially.

{
return 4 * (getAHHeader()->payloadLen + 2);
return static_cast<size_t>(4) * (getAHHeader()->payloadLen + 2);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this casting really needed? 🤔

Copy link
Collaborator Author

@Dimi1010 Dimi1010 Sep 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VS analysis gives me a "subexpression may overflow before being assigned to a wider type" notice without the cast.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok 👍

Packet++/header/NdpLayer.h Outdated Show resolved Hide resolved
Packet++/header/NdpLayer.h Outdated Show resolved Hide resolved
Packet++/header/NdpLayer.h Outdated Show resolved Hide resolved
Packet++/header/NdpLayer.h Outdated Show resolved Hide resolved
@Dimi1010 Dimi1010 merged commit 20974e6 into seladb:dev Sep 24, 2024
40 checks passed
@Dimi1010 Dimi1010 deleted the refactor/layer-overrides branch September 24, 2024 07:09
tigercosmos pushed a commit to tigercosmos/PcapPlusPlus that referenced this pull request Sep 24, 2024
…b#1563)

* Added 'override' specifier to getOsiModelLayer implementations.

* Added `override` specifier to getDataPtr implementation in Layer.

* APRLayer - overrides + Cpp casts.

* BgpLayer - overrides + Cpp casts.

* CotpLayer - override + Cpp casts.

* DhcpLayer - Overrides + Cpp casts.

* DhcpV6Layer - Overrides + Cpp casts.

* DnsLayer - Overrides + Cpp casts

* EthDot3Layer - Overrides + Cpp casts.

* EthLayer - Overrides + Cpp casts.

* FtpLayer - Overrides

* GreLayer - Overrides + Cpp casts.

* GtpLayer - Overrides + Cpp casts.

* HttpLayer - Overrides

* IcmpLayer - Overrides + Cpp casts

* IcmpV6Layer - Overrides + Cpp casts.

* IgmpLayer - Overrides + Cpp casts.

* IPLayer - Explicitly defaulted ctor/dtor.

* IPv4Layer - Overrides + Cpp casts.

* IPv6Layer - Overrides + Cpp casts.

* Layer.h

- IDataContainer - defaulted destructor.
- Layer - destructor override.

* LdapLayer - Overrides.

* LLCLayer - Overrides + Cpp casts.

* MplsLayer - Overrides + Cpp casts.

* NdpLayer - Overrides + Cpp casts.

* NflogLayer - Overrides + Cpp casts.

* NtpLayer - Overrides.

* NullLoopbackLayer - Overrides.

* PayloadLayer - Overrides.

* PPPoELayer - Overrides + Cpp casts.

* RadiusLayer - Overrides + Cpp casts.

* SdpLayer - Overrides.

* SipLayer - Overrides.

* SllLayer - Overrides + Cpp casts.

* Sll2Layer - Overrides + Cpp casts.

* SmtpLayer - Overrides.

* SomeIpLayer - Overrides + Cpp casts.

* SomeIpSdLayer - Overrides.

* SshLayer - Overrides + Cpp casts.

* SSLLayer - Overrides + Cpp casts.

* StpLayer - Overrides + Cpp casts.

* S7CommLayer - Overrides + Cpp casts.

* TcpLayer - Overrides + Cpp casts.

* TelnetLayer - Overrides.

* TextBasedProtocol - Overrides.

* TLVData - Explicit defaults + Cpp casts.

* TpktLayer - Explicit defaults + Cpp casts.

* UdpLayer - Overrides + Cpp casts.

* VlanLayer - Overrides + Cpp casts.

* VrrpLayer - Overrides.

* VxlanLayer - Overrides + Cpp casts.

* WakeOnLanLayer - Overrides + Cpp casts.

* DnsResourceData - Overrides.

* DnsResource - Overrides.

* IPv4Layer - Fixed missed overrides.

* TLVData - Fixed missed c cast.

* DhcpLayer - Fixed missed overrides.

* DhcpV6Layer - Fixed missed overrides.

* HttpLayer - Fixed missed overrides.

* IPSecLayer - Overrides + Cpp casts

* NdpLayer - Fixed missed overrides.

* PacketTrailerLayer - Overrides

* SomeIpLayer - Fixed missed overrides.

* TcpLayer - Fixed missed overrides.

* SSLHandshake - Overrides + Cpp casts.

* Removed 'missingOverride' cppcheck suppression from Packet++ and Common++.

* DhcpV6Layer - Fixed missing override

* IPSecLayer - Fixed missing override

* IPv6Extensions - Fixed missing override

* IPReassembly - Overrides

* Simplify return literals.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants