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.

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

feat(sys): High-level personality(2) wrappers #1331

Merged
merged 1 commit into from Dec 19, 2020
Merged

feat(sys): High-level personality(2) wrappers #1331

merged 1 commit into from Dec 19, 2020

Conversation

woodruffw
Copy link
Contributor

Adds a high level Persona bitflags enum, as well as personality::get() and personality::set() for interacting with libc::personality().

Closes #1330.

See also rust-lang/libc#1974.

woodruffw
woodruffw commented Nov 10, 2020
View reviewed changes
src/sys/personality.rs Outdated

libc_bitflags! {
pub struct Persona: c_ulong {
ADDR_COMPAT_LAYOUT as c_ulong;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

N.B.: These casts won't be necessary if/when rust-lang/libc#1974 lands.

@woodruffw woodruffw mentioned this pull request Nov 10, 2020
Merged
asomers
asomers requested changes Nov 15, 2020
View reviewed changes
Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be worthwhile. It just needs a few things:

  • To be rebased once the libc PR lands
  • tests (doc tests are ok)
  • CHANGELOG entry

@woodruffw
Copy link
Contributor Author

Sounds good; I can do all of those.

@woodruffw
Copy link
Contributor Author

Okay, I've added some doctests and a CHANGELOG entry. I'll also rebase once that PR goes through.

@asomers
Copy link
Member

asomers commented Nov 17, 2020

This looks good. You'll also need to rebase to pick up a Nix fix for those libc deprecations.

@woodruffw
Copy link
Contributor Author

woodruffw commented Nov 18, 2020 •
edited

Looks like musl libc doesn't support some of these persona values, so I went ahead and cfg'd them out.

Upsteam musl change: https://www.openwall.com/lists/musl/2020/11/18/1

Edit: This was merged. At some point in this distant future, musl users will be able to use these with nix 😉

@woodruffw
Copy link
Contributor Author

Last CI failure looks unrelated:

test test_drop ... error: process didn't exit successfully: `/linux-runner x86_64 /target/x86_64-unknown-linux-gnu/release/deps/test_aio_drop-d7c4918b8a06891c` (signal: 1, SIGHUP: hangup)

@asomers
Copy link
Member

asomers commented Dec 3, 2020

Please rebase to fix the CHANGELOG.

@woodruffw
Copy link
Contributor Author

I've updated this PR to not be blocked on libc: the signature of personality(2) is c_ulong -> c_int where c_int is always guaranteed to hold a valid persona, so c_int is the correct underlying type.

@asomers
Copy link
Member

asomers commented Dec 8, 2020

I've updated this PR to not be blocked on libc: the signature of personality(2) is c_ulong -> c_int where c_int is always guaranteed to hold a valid persona, so c_int is the correct underlying type.

lol. The C API doesn't make much sense, then.

@asomers
Copy link
Member

asomers commented Dec 8, 2020

This looks good now. It just needs a squash. And you'll probably want to rebase at the same time, or you may get blocked waiting for Travis.

@woodruffw
Copy link
Contributor Author

lol. The C API doesn't make much sense, then.

Yep. My educated guess is that it's unsigned long because every syscall parameter is (or was? it's been a moment since I've checked) unsigned long at the end of the day, and whoever wrapped it in glibc et al. got lazy. Oh well.

This looks good now. It just needs a squash. And you'll probably want to rebase at the same time, or you may get blocked waiting for Travis.

👍 I'll squash and rebase tomorrow.

@woodruffw
Copy link
Contributor Author

Okay, squashed and rebased. Let me know if there's anything else (&c &c)

@woodruffw
Copy link
Contributor Author

Unrelated failure?

failures:

---- src/unistd.rs - unistd::alarm (line 1636) stdout ----
thread 'src/unistd.rs - unistd::alarm (line 1636)' panicked at 'test executable failed:

thread 'main' panicked at 'assertion failed: start.elapsed() >= Duration::from_secs(1)', src/unistd.rs:21:1
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

', src/librustdoc/test.rs:341:17
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

@asomers
Copy link
Member

asomers commented Dec 15, 2020

Try rebasing again.

@woodruffw
Copy link
Contributor Author

Rebased!

asomers
asomers requested changes Dec 15, 2020
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@woodruffw
feat(sys): High-level personality(2) wrappers
2359b0e 
Adds a high level `Persona` bitflags enum, as well as `personality::get()`
and `personality::set()` for interacting with `libc::personality()`.
@woodruffw
Copy link
Contributor Author

(Same unrelated failure as before).

asomers
asomers approved these changes Dec 19, 2020
View reviewed changes
Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bors r+

@bors
Copy link
Contributor

bors bot commented Dec 19, 2020

Build succeeded:

@bors bors bot merged commit e65db13 into nix-rust:master Dec 19, 2020
@woodruffw woodruffw deleted the ww/personality branch December 19, 2020 18:24
This was referenced Mar 12, 2021
Merged
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asomers asomers asomers approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support for personality(2)?
2 participants
@woodruffw @asomers