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

unix: Fix feature(unix_socket_ancillary_data) on macos and other BSDs #83374

Merged
merged 1 commit into from
Mar 30, 2021

Conversation

reyk
Copy link
Contributor

@reyk reyk commented Mar 22, 2021

This adds support for CMSG handling on macOS and fixes it on OpenBSD and possibly other BSDs.

When traversing the CMSG list, the previous code had an exception for Android where the next element after the last pointer could point to the first pointer instead of NULL. This is actually not specific to Android: the libc::CMSG_NXTHDR implementation for Linux and emscripten have a special case to return NULL when the length of the previous element is zero; most other implementations simply return the previous element plus a zero offset in this case.

This MR makes the check non-optional which fixes CMSG handling and a possible endless loop on such systems; tested with file descriptor passing on OpenBSD, Linux, and macOS.

This MR additionally adds SocketAncillary::is_empty because clippy is right that it should be added.

This belongs to the feature(unix_socket_ancillary_data) tracking issue: #76915

r? @joshtriplett

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @joshtriplett (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 22, 2021
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

This adds support for CMSG handling on macOS and fixes it on OpenBSD
and other BSDs.

When traversing the CMSG list, the previous code had an exception for
Android where the next element after the last pointer could point to
the first pointer instead of NULL.  This is actually not specific to
Android: the `libc::CMSG_NXTHDR` implementation for Linux and
emscripten have a special case to return NULL when the length of the
previous element is zero; most other implementations simply return the
previous element plus a zero offset in this case.

This MR additionally adds `SocketAncillary::is_empty` because clippy
is right that it should be added.
@reyk
Copy link
Contributor Author

reyk commented Mar 27, 2021

Hi @joshtriplett, I picked you to review this because you have been involved in reviewing @LinkTed's previous PRs. I hope this was OK to overwrite Highfive's auto-selection; this is my first PR to Rust so I'm new to the process.

@joshtriplett
Copy link
Member

Looks reasonable to me, as long as it passes tests on all platforms.

@bors r+

@bors
Copy link
Contributor

bors commented Mar 29, 2021

📌 Commit 3d6bd87 has been approved by joshtriplett

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 29, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 30, 2021
Rollup of 7 pull requests

Successful merges:

 - rust-lang#82331 (alloc: Added `as_slice` method to `BinaryHeap` collection)
 - rust-lang#83130 (escape_ascii take 2)
 - rust-lang#83374 (unix: Fix feature(unix_socket_ancillary_data) on macos and other BSDs)
 - rust-lang#83543 (Lint on unknown intra-doc link disambiguators)
 - rust-lang#83636 (Add a regression test for issue-82792)
 - rust-lang#83643 (Remove a FIXME resolved by rust-lang#73578)
 - rust-lang#83644 (:arrow_up: rust-analyzer)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 772582e into rust-lang:master Mar 30, 2021
@rustbot rustbot added this to the 1.53.0 milestone Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants