Skip to content

fix: cppcheck warnings#3508

Merged
airween merged 11 commits intoowasp-modsecurity:v3/masterfrom
airween:v3/cppcheckvariableh
Mar 11, 2026
Merged

fix: cppcheck warnings#3508
airween merged 11 commits intoowasp-modsecurity:v3/masterfrom
airween:v3/cppcheckvariableh

Conversation

@airween
Copy link
Copy Markdown
Member

@airween airween commented Mar 10, 2026

what

This PR contains several cppcheck warnings' fixes:

  • make toOmit() to const in src/variables/variable.h
  • make resolve() and resolveRegularExpression() functions to const in anchored_set_variable.cc|h
  • move Code variable to inner block in url_decode_uni.cc
  • make isRelevant() to const in audit_log.cc|h
  • make getParserError() to const in rules_set.cc|h
  • move ucode variable to inner block in rules_set_properties.cc
  • remove redundant initialization of ckey in transaction.cc
  • add inline cppcheck-suppress flag to avoid warning in unique_id.cc
  • make variable 's' const reference to avoid the copy in afl_fuzzer.cc

why

cppcheck's new version (2.20.0) warned these issues.

references

See #3504's failed job

other notes

where the modified functions' signature has changed, I added a wrapper with an old signature to keep the symbols in ABI. Therefore these changes don't break the library's ABI.

@airween airween requested a review from theseion March 10, 2026 10:43
Copy link
Copy Markdown
Collaborator

@theseion theseion left a comment

Choose a reason for hiding this comment

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

LGTM (AFAICT 🤷)

@airween
Copy link
Copy Markdown
Member Author

airween commented Mar 10, 2026

LGTM (AFAICT 🤷)

Thanks - unfortunately the cppcheck still warns...

@airween airween changed the title fix: make toOmit const fix: cppcheck warnings Mar 10, 2026
@airween airween requested review from Copilot and theseion March 10, 2026 21:40
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses new cppcheck (2.20.0) warnings across core and public-header code by improving const-correctness, tightening variable scope, and adding targeted suppressions while attempting to preserve ABI via wrapper overloads.

Changes:

  • Add const overloads for multiple methods (with non-const wrappers kept for ABI compatibility where needed).
  • Reduce unnecessary initializations/copies and narrow variable lifetimes.
  • Add an inline cppcheck-suppress for a platform-dependent unused label warning.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/fuzzer/afl_fuzzer.cc Avoids an extra std::string copy by binding to a const reference.
src/variables/variable.h Makes KeyExclusions::toOmit const and keeps a non-const wrapper.
src/unique_id.cc Adds an inline cppcheck-suppress on a label to silence a configuration warning.
src/transaction.cc Removes redundant empty-string initialization for ckey.
src/rules_set_properties.cc Narrows ucode scope into the mapping != NULL block.
src/rules_set.cc Adds RulesSet::getParserError() const and adjusts rules pointer constness.
src/audit_log/audit_log.cc Adds AuditLog::isRelevant(...) const and a wrapper with old signature.
src/anchored_set_variable.cc Adds const overloads for resolve helpers and ABI-preserving wrappers.
src/actions/transformations/url_decode_uni.cc Narrows Code scope to the unicode-map branch.
headers/modsecurity/rules_set.h Declares getParserError() const in the public header.
headers/modsecurity/audit_log.h Declares isRelevant(...) const in the public header.
headers/modsecurity/anchored_set_variable.h Declares new const overloads and documents ABI-compat wrappers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Mar 10, 2026

Quality Gate Passed Quality Gate passed

Issues
0 New issues
3 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@airween
Copy link
Copy Markdown
Member Author

airween commented Mar 10, 2026

@theseion could you review this again?

All tests have passed, all cppcheck are happy, Copilot is happy... so I'm ready.

Copy link
Copy Markdown
Collaborator

@theseion theseion left a comment

Choose a reason for hiding this comment

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

There's still one SonarQube warning.

@airween
Copy link
Copy Markdown
Member Author

airween commented Mar 11, 2026

There's still one SonarQube warning.

Actually, this is what I see:

kép

Thanks anyway!

@airween airween merged commit 1c1ccf6 into owasp-modsecurity:v3/master Mar 11, 2026
88 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants