Clang-tidy fixes for Common++ #1639
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #1639 +/- ##
==========================================
- Coverage 83.15% 83.11% -0.04%
==========================================
Files 277 277
Lines 48185 48207 +22
Branches 10165 9913 -252
==========================================
Hits 40066 40066
- Misses 7237 7248 +11
- Partials 882 893 +11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
@egecetin please see this comment: #1637 (comment) |
.clang-format
Outdated
| @@ -21,4 +21,5 @@ BreakBeforeBraces: Custom | |||
| BraceWrapping: | |||
| SplitEmptyFunction: false | |||
| AfterCaseLabel: true | |||
| QualifierAlignment: Left | |||
There was a problem hiding this comment.
RIP, const on the right... I found that easier to read because its consistent behavior with its usage in the middle...
There was a problem hiding this comment.
Let me check the codebase again. If most of them left it is better to split PR for changing it. Anyway, lets invite more people so we can decide which is better @seladb @tigercosmos @clementperon.
Note: We still have plenty of time because there are lots of warnings even for Common++ so it will take some time to fix PR :)
There was a problem hiding this comment.
I like the left one though.
There was a problem hiding this comment.
I just reverted it and placed all new qualifiers to left for now and keep the clang-format default behavior to keep this PR simple. We can completely change by just running clang-format later if left or right decided.
There was a problem hiding this comment.
Copilot reviewed 3 out of 21 changed files in this pull request and generated no suggestions.
Files not reviewed (18)
- Common++/header/GeneralUtils.h: Language not supported
- Common++/header/IpAddress.h: Language not supported
- Common++/header/IpUtils.h: Language not supported
- Common++/header/LRUList.h: Language not supported
- Common++/header/Logger.h: Language not supported
- Common++/header/MacAddress.h: Language not supported
- Common++/header/OUILookup.h: Language not supported
- Common++/header/PointerVector.h: Language not supported
- Common++/header/SystemUtils.h: Language not supported
- Common++/header/TablePrinter.h: Language not supported
- Common++/src/GeneralUtils.cpp: Language not supported
- Common++/src/IpAddress.cpp: Language not supported
- Common++/src/IpUtils.cpp: Language not supported
- Common++/src/Logger.cpp: Language not supported
- Common++/src/MacAddress.cpp: Language not supported
- Common++/src/OUILookup.cpp: Language not supported
- Common++/src/SystemUtils.cpp: Language not supported
- Common++/src/TablePrinter.cpp: Language not supported
| @@ -0,0 +1,34 @@ | |||
| Checks: 'cert-*, | |||
There was a problem hiding this comment.
I used this configuration to fix issues. Since all of directories are not fixed this file not used for now in CI
| @@ -0,0 +1,43 @@ | |||
| #!/bin/sh | |||
There was a problem hiding this comment.
This script for easy local checking. Except Common++ all directories are ignored. It will be replacing the original script after all folders are fixed.
|
Side note looks like gcovr weirdly fails in Fedora 40. Check https://github.com/seladb/PcapPlusPlus/actions/runs/12573299593/job/35046174314. I only added a newline in commit 12e7606 and it passed. It might affect other PRs too please keep in mind that if you see similar error. |
Common++/src/IpAddress.cpp
Outdated
| @@ -364,15 +367,15 @@ namespace pcpp | |||
| { | |||
| memset(m_Mask, 0, IPV6_ADDR_SIZE); | |||
| int remainingPrefixLen = prefixLen; | |||
| for (auto byteIndex = 0; byteIndex < IPV6_ADDR_SIZE; byteIndex++) | |||
| for (unsigned char& byteIndex : m_Mask) | |||
There was a problem hiding this comment.
We can rename byteIndex to byte and use auto&:
for (auto& byte : m_Mask)
{
...
}There was a problem hiding this comment.
fixed with 3ec344e
Common++/src/IpAddress.cpp
Outdated
| @@ -484,10 +488,10 @@ namespace pcpp | |||
| uint8_t IPv6Network::getPrefixLen() const | |||
| { | |||
| uint8_t result = 0; | |||
| for (auto byteIndex = 0; byteIndex < IPV6_ADDR_SIZE; byteIndex++) | |||
| for (const unsigned char byteIndex : m_Mask) | |||
There was a problem hiding this comment.
We can rename byteIndex to byte and use auto:
for (auto byte : m_Mask)
{
...
}There was a problem hiding this comment.
fixed with 3ec344e
Common++/src/IpAddress.cpp
Outdated
| @@ -520,10 +524,10 @@ namespace pcpp | |||
| uint64_t IPv6Network::getTotalAddressCount() const | |||
| { | |||
| int numOfBitset = 0; | |||
| for (auto byteIndex = 0; byteIndex < IPV6_ADDR_SIZE; byteIndex++) | |||
| for (const unsigned char byteIndex : m_Mask) | |||
There was a problem hiding this comment.
fixed with cbe3a03
Fix various clang-tidy warnings for Common++
Requires #1677