-
Notifications
You must be signed in to change notification settings - Fork 920
dnsdist: Fix a few warnings reported by clang's static analyzer and cppcheck #10035
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approved with one unimportant question
@@ -52,6 +52,7 @@ bool addXPF(DNSQuestion& dq, uint16_t optionCode) | |||
pos += sizeof(drh); | |||
memcpy(reinterpret_cast<char*>(&data.at(pos)), payload.data(), payload.size()); | |||
pos += payload.size(); | |||
(void) pos; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to mark the result from the previous line used, and you don't want to delete the previous line in case we ever add more code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly. I can live with deleting the previous line if you think it's cleaner, though :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What you did makes sense :)
CodeQL reports 4 fixed alerts (all 'Potentially uninitialized local variable ') |
Where does this get reported? I don't see it in the check details? |
It's a few clicks into the Checks tab, under Code scanning results > CodeQL |
Fun fact, when I click on "All checks have passed" then "CodeQL / Analyze..." -> Details the "Code scanning results" menu is empty. But it's there when I go through the "Checks" tag. Thanks :) Unfortunately these 4 alerts seem to randomly get fixed and found again, I don't think it's related to my PR.. |
Short description
Mostly false positive that I'm tired of seeing, or performance tips in places we don't really care about performance.
But also a real bug in the code loading the ring buffers from a file (only used for debugging) and a few hardening measures (additional checks, initializations) that will not hurt and might prevent an issue later.
Checklist
I have: