-
Notifications
You must be signed in to change notification settings - Fork 175
fix(userspace/libsinsp): avoid bogus error in process_recvmsg_ancilla… #2381
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
…ry_data_fds(). Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
/milestone 0.21.0 |
Perf diff from master - unit tests
Heap diff from master - unit tests
Heap diff from master - scap file
Benchmarks diff from master
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2381 +/- ##
=======================================
Coverage 77.15% 77.15%
=======================================
Files 231 231
Lines 30347 30347
Branches 4656 4657 +1
=======================================
Hits 23415 23415
Misses 6932 6932
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
X64 kernel testing matrix
ARM64 kernel testing matrix
|
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.
/lgtm
@@ -3673,7 +3673,7 @@ inline void sinsp_parser::process_recvmsg_ancillary_data_fds(scap_platform *scap | |||
int const *fds, | |||
size_t const fds_len, | |||
scap_threadinfo *scap_tinfo) { | |||
char error[SCAP_LASTERR_SIZE]; | |||
char error[SCAP_LASTERR_SIZE] = {}; |
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.
I don't think you need the extra assignment here, though the compiler probably gets rid of it anyways
char error[SCAP_LASTERR_SIZE] = {}; | |
char error[SCAP_LASTERR_SIZE]{}; |
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.
That feels black magic syntax to me :O
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.
It's explicitly calling the default constructor of the char[SCAP_LASTERR_SIZE]
type, initializing its contents to zero. The behavior when leaving that out is the same as it would be on C, just leave any garbage in the stack laying around.
There's a better explanation here
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.
Again, the compiler probably catches that you are immediately copying new value into the array with the assignment and might just call the constructor for you and skip the copy, so either syntax will probably have the same result.
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.
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Andreagit97, FedeDP, Molter73 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind cleanup
Any specific area of the project related to this PR?
/area libsinsp
Does this PR require a change in the driver versions?
What this PR does / why we need it:
Since
error
is not always set, initialize it to empty.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: