Add pselect syscall #894
Add pselect syscall #894
Conversation
src/sys/select.rs
Outdated
| /// * `nfds`: The highest file descriptor set in any of the passed `FdSet`s, plus 1. If `None`, this | ||
| /// is calculated automatically by calling [`FdSet::highest`] on all descriptor sets and adding 1 | ||
| /// to the maximum of that. | ||
| /// * `readfds`: File descriptors to check for being ready to read. |
There was a problem hiding this comment.
How about s/being ready to read/read readiness here and on the next two lines?
There was a problem hiding this comment.
Excellent point. Fixed.
src/sys/select.rs
Outdated
| @@ -227,6 +300,29 @@ mod tests { | |||
| assert!(!fd_set.contains(r2)); | |||
| } | |||
|
|
|||
| #[test] | |||
| #[cfg_attr(any(target_arch = "powerpc", target_arch = "mips"), ignore)] | |||
There was a problem hiding this comment.
If you're going to ignore the test on those two arches, you should explain why in a comment.
There was a problem hiding this comment.
Ah! I forgot to drop that line when I cargo-culted this test. Thanks for spotting it!
src/sys/select.rs
Outdated
| None, | ||
| None, | ||
| &timeout, | ||
| &sigmask).unwrap()); |
There was a problem hiding this comment.
Altering the signal mask has side effects which may interfere with other concurrent tests. Nix has a way of dealing with that: you should this test into test/sys/test_select.rs and then add this line to the top:
let _mtx = ::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");Since the other test does not modify the sigmask, you don't need the mutex. It's up to you whether or not to move it.
There was a problem hiding this comment.
Thanks - I'll move both.
src/sys/select.rs
Outdated
| /// * `nfds`: The highest file descriptor set in any of the passed `FdSet`s, plus 1. If `None`, this | ||
| /// is calculated automatically by calling [`FdSet::highest`] on all descriptor sets and adding 1 | ||
| /// to the maximum of that. | ||
| /// * `readfds`: File descriptors to check for being ready to read. |
There was a problem hiding this comment.
Excellent point. Fixed.
src/sys/select.rs
Outdated
| @@ -227,6 +300,29 @@ mod tests { | |||
| assert!(!fd_set.contains(r2)); | |||
| } | |||
|
|
|||
| #[test] | |||
| #[cfg_attr(any(target_arch = "powerpc", target_arch = "mips"), ignore)] | |||
There was a problem hiding this comment.
Ah! I forgot to drop that line when I cargo-culted this test. Thanks for spotting it!
src/sys/select.rs
Outdated
| None, | ||
| None, | ||
| &timeout, | ||
| &sigmask).unwrap()); |
There was a problem hiding this comment.
Thanks - I'll move both.
test/sys/test_select.rs
Outdated
|
|
||
| #[test] | ||
| pub fn test_pselect_nfds2() { | ||
| let _mtx = ::SIGNAL_MTX |
There was a problem hiding this comment.
You put _mtx in the wrong test; only test_pselect modifies the sigmask.
This is a straight port of @abbradar's work in nix-rust#276, with two (somewhat weak) tests and a bit of documentation.
|
Updated once again, hahaha, I should probably go to bed (: |
|
thanks for your contribution! bors r+ |
894: Add pselect syscall r=asomers a=antifuchs I saw that #276 was closed, and now I need `pselect`, so here it is! I copied the function body from @abbradar's work, updated the type signatures, added two tests and added a doc comment. Hope this works! Co-authored-by: Andreas Fuchs <[email protected]>
I saw that #276 was closed, and now I need
pselect, so here it is! I copied the function body from @abbradar's work, updated the type signatures, added two tests and added a doc comment.Hope this works!