-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add non-NULL asserts to CMSG_* returns, null-initialize AncillaryBuf #8
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.
Would be nice to describe the changes in the commit message in addition to the PR.
I see CMSG_NXTHDR()
requires the buffer to be zero-initialized, so that's a good catch.
src/ancillary.rs
Outdated
@@ -119,14 +119,29 @@ pub fn send_ancillary( | |||
} | |||
|
|||
#[cfg(not(any(target_os="illumos", target_os="solaris")))] { | |||
let mut header = &mut*CMSG_FIRSTHDR(&mut msg); | |||
let header_ptr = CMSG_FIRSTHDR(&mut msg); | |||
if header_ptr.is_null() { |
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 can only happen as result of a bug, so an assert
or debug_assert
would be better.
Speaking of bugs, I see CMSG_SPACE()
not CMSG_LEN()
should be used to calculate the size, so this could happen. But I can fix that.
I've fixed the CI failure, please rebase. |
Rebased and switched to |
Thanks! |
CMSG_FIRSTHDR() and CMSG_NXTHDR() expect the buffer to be zero-initialized. Also assert that these functions don't return NULL
Released as 0.2.7 and 0.3.0. |
Added two checks for NULL returns and one zero-ing that is technically more correct