Skip to content

Fix CMSG_DATA(3) and friends on BSD #1212

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 17, 2019
Merged

Fix CMSG_DATA(3) and friends on BSD #1212

merged 1 commit into from
Jan 17, 2019

Conversation

asomers
Copy link
Contributor

@asomers asomers commented Jan 15, 2019

PR #1098 added the CMSG_DATA(3) family of functions into libc. Because
they're defined as macros in C, they had to be rewritten as Rust
functions for libc. Also, they can't be tested in CI for the same
reason. But that PR erroneously used the same definitions in BSD as in
Linux.

This commit corrects the definitions for OSX, FreeBSD, DragonflyBSD,
OpenBSD, and NetBSD. I renamed a few variables and collapsed a few
macros in order to combine the definitions where possible.

Fixes #1210

@rust-highfive
Copy link

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@asomers
Copy link
Contributor Author

asomers commented Jan 15, 2019

I'm not 100% sure about the _ALIGNBYTES definitions for NetBSD on arm and arm64, because NetBSD defines its platforms differently than Rust. And I'm not 100% sure that I got the OSX definitions correct, because I don't have a system to look at. I rely on Google for OSX headers. But using these definitions I did get a good test suite to pass on FreeBSD, NetBSD, OpenBSD, and Linux, all on amd64.

@asomers
Copy link
Contributor Author

asomers commented Jan 15, 2019

One question. The CMSG_DATA, CMSG_LEN, and CMSG_SPACE functions are always safe to use (the other two are not). But the f! macro marks them as unsafe extern, even though they are neither. Should I bypass the macro to declare them as safe Rust functions? I hesitate because I don't see any precedent for doing that, even for other obviously safe functions like WEXITSTATUS.

@asomers
Fix CMSG_DATA(3) and friends on BSD
8067378 
PR rust-lang#1098 added the CMSG_DATA(3) family of functions into libc.  Because
they're defined as macros in C, they had to be rewritten as Rust
functions for libc.  Also, they can't be tested in CI for the same
reason.  But that PR erroneously used the same definitions in BSD as in
Linux.

This commit corrects the definitions for OSX, FreeBSD, DragonflyBSD,
OpenBSD, and NetBSD.  I renamed a few variables and collapsed a few
macros in order to combine the definitions where possible.

Fixes rust-lang#1210
@alexcrichton
Copy link
Member

@bors: r+

Thanks!

I think we covered the safety in another issue where we're hoping for consistency here, but we'll see how that pays off over time!

@bors
Copy link
Contributor

bors commented Jan 17, 2019

📌 Commit 8067378 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jan 17, 2019

⌛ Testing commit 8067378 with merge 70284d7...

bors added a commit that referenced this pull request Jan 17, 2019
@bors
Auto merge of #1212 - asomers:cmsg, r=alexcrichton
70284d7 
Fix CMSG_DATA(3) and friends on BSD

PR #1098 added the CMSG_DATA(3) family of functions into libc.  Because
they're defined as macros in C, they had to be rewritten as Rust
functions for libc.  Also, they can't be tested in CI for the same
reason.  But that PR erroneously used the same definitions in BSD as in
Linux.

This commit corrects the definitions for OSX, FreeBSD, DragonflyBSD,
OpenBSD, and NetBSD.  I renamed a few variables and collapsed a few
macros in order to combine the definitions where possible.

Fixes #1210
@bors
Copy link
Contributor

bors commented Jan 17, 2019

☀️ Test successful - checks-cirrus, checks-travis, status-appveyor
Approved by: alexcrichton
Pushing 70284d7 to master...

@bors bors merged commit 8067378 into rust-lang:master Jan 17, 2019
This was referenced Jan 29, 2019
Closed
Closed
asomers added a commit to asomers/libc that referenced this pull request Jan 31, 2019
@asomers
Fix typo in CMSG_NXTHDR for OSX from PR rust-lang#1212
fc90925
@asomers asomers mentioned this pull request Feb 1, 2019
Merged
@gnzlbg gnzlbg mentioned this pull request Feb 4, 2019
Closed
asomers added a commit to asomers/libc that referenced this pull request Feb 5, 2019
@asomers
Fix CMSG_NXTHDR for OSX.
73df81f 
This was an oversight from PR rust-lang#1212.  It's been revealed by the new cmsg
test.
bors added a commit that referenced this pull request Feb 5, 2019
@bors
Auto merge of #1235 - asomers:cmsg_osx, r=gnzlbg
fcf28c6 
Fix cmsg(3) bugs for musl and OSX

This PR fixes bugs in the cmsg(3) family of functions for Linux/musl and OSX, introduced by PR #1098 and PR #1212 .  It also adds an integration test which hopefully will validate these functions on every platform.
bors added a commit that referenced this pull request Feb 5, 2019
@bors
Auto merge of #1235 - asomers:cmsg_osx, r=gnzlbg
f9b96ee 
Fix cmsg(3) bugs for musl and OSX

This PR fixes bugs in the cmsg(3) family of functions for Linux/musl and OSX, introduced by PR #1098 and PR #1212 .  It also adds an integration test which hopefully will validate these functions on every platform.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@alexcrichton alexcrichton

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@asomers @rust-highfive @alexcrichton @bors