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

fix: Sonarcloud memleak fixes #3114

Open
wants to merge 6 commits into
base: v3/master
Choose a base branch
from

Conversation

airween
Copy link
Member

@airween airween commented Mar 27, 2024

This pull request fixes two memory leaks, reported by SonarCloud.

None of them modified the library itself.

@airween airween added the 3.x Related to ModSecurity version 3.x label Mar 27, 2024
test/regression/regression.cc Outdated Show resolved Hide resolved
tools/rules-check/rules-check.cc Outdated Show resolved Hide resolved
tools/rules-check/rules-check.cc Outdated Show resolved Hide resolved
@airween
Copy link
Member Author

airween commented Mar 27, 2024

This block makes no sense.

The variable is initialized as an empty string in line 49 in each cycle.

49.      std::string argFull("");

therefore the condition's left side (argFull.empty()) is always true which means the condition itself

54.        if (argFull.empty() == false) {

is always false. Moreover, in the two branches in that block after the append() the code jumps to the next label, where the cycle ends...

So I'm going to remove it.

Copy link

sonarcloud bot commented Mar 27, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarCloud

Copy link
Contributor

@MirkoDziadzka MirkoDziadzka left a comment

Choose a reason for hiding this comment

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

Looks a lot simpler, thanks.

@airween airween self-assigned this Mar 31, 2024
@airween airween requested a review from MirkoDziadzka May 3, 2024 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.x Related to ModSecurity version 3.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants