From 885a1f751497c265a532a42a0c98829e32ae7f82 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 21 Jul 2017 00:46:56 +0200 Subject: Calculate `nfds` parameter for `select` Doing this behind the scenes makes the API less error-prone and easier to use. It should also fix https://github.com/nix-rust/nix/issues/679#issuecomment-316838148 --- CHANGELOG.md | 2 + src/sys/select.rs | 225 ++++++++++++++++++++++++++++++++++++++++++++++-- test/sys/mod.rs | 1 - test/sys/test_select.rs | 59 ------------- 4 files changed, 222 insertions(+), 65 deletions(-) delete mode 100644 test/sys/test_select.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f1973b5..31902b24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ This project adheres to [Semantic Versioning](http://semver.org/). has changed type from `c_int` to `SockProtocol`. It accepts a `None` value for default protocol that was specified with zero using `c_int`. ([#647](https://github.com/nix-rust/nix/pull/647)) +- Made `select` easier to use, adding the ability to automatically calculate the `nfds` parameter using the new + `FdSet::highest` ([#701](https://github.com/nix-rust/nix/pull/701)) - Exposed `unistd::setresuid` and `unistd::setresgid` on FreeBSD and OpenBSD ([#721](https://github.com/nix-rust/nix/pull/721)) - Refactored the `statvfs` module removing extraneous API functions and the diff --git a/src/sys/select.rs b/src/sys/select.rs index eae191b6..82b2aad3 100644 --- a/src/sys/select.rs +++ b/src/sys/select.rs @@ -53,6 +53,42 @@ impl FdSet { *bits = 0 } } + + /// Finds the highest file descriptor in the set. + /// + /// Returns `None` if the set is empty. + /// + /// This can be used to calculate the `nfds` parameter of the [`select`] function. + /// + /// # Example + /// + /// ``` + /// # extern crate nix; + /// # use nix::sys::select::FdSet; + /// # fn main() { + /// let mut set = FdSet::new(); + /// set.insert(4); + /// set.insert(9); + /// assert_eq!(set.highest(), Some(9)); + /// # } + /// ``` + /// + /// [`select`]: fn.select.html + pub fn highest(&self) -> Option { + for (i, &block) in self.bits.iter().enumerate().rev() { + if block != 0 { + // Highest bit is located at `BITS - 1 - n.leading_zeros()`. Examples: + // 0b00000001 + // 7 leading zeros, result should be 0 (bit at index 0 is 1) + // 0b001xxxxx + // 2 leading zeros, result should be 5 (bit at index 5 is 1) - x may be 0 or 1 + + return Some((i * BITS + BITS - 1 - block.leading_zeros() as usize) as RawFd); + } + } + + None + } } mod ffi { @@ -68,11 +104,52 @@ mod ffi { } } -pub fn select(nfds: c_int, - readfds: Option<&mut FdSet>, - writefds: Option<&mut FdSet>, - errorfds: Option<&mut FdSet>, - timeout: Option<&mut TimeVal>) -> Result { +/// Monitors file descriptors for readiness (see [select(2)]). +/// +/// Returns the total number of ready file descriptors in all sets. The sets are changed so that all +/// file descriptors that are ready for the given operation are set. +/// +/// When this function returns, `timeout` has an implementation-defined value. +/// +/// # Parameters +/// +/// * `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. +/// * `writefds`: File descriptors to check for being ready to write. +/// * `errorfds`: File descriptors to check for pending error conditions. +/// * `timeout`: Maximum time to wait for descriptors to become ready (`None` to block +/// indefinitely). +/// +/// [select(2)]: http://man7.org/linux/man-pages/man2/select.2.html +/// [`FdSet::highest`]: struct.FdSet.html#method.highest +pub fn select<'a, N, R, W, E, T>(nfds: N, + readfds: R, + writefds: W, + errorfds: E, + timeout: T) -> Result +where + N: Into>, + R: Into>, + W: Into>, + E: Into>, + T: Into>, +{ + let readfds = readfds.into(); + let writefds = writefds.into(); + let errorfds = errorfds.into(); + let timeout = timeout.into(); + + let nfds = nfds.into().unwrap_or_else(|| { + readfds.iter() + .chain(writefds.iter()) + .chain(errorfds.iter()) + .map(|set| set.highest().unwrap_or(-1)) + .max() + .unwrap_or(-1) + 1 + }); + let readfds = readfds.map(|set| set as *mut FdSet).unwrap_or(null_mut()); let writefds = writefds.map(|set| set as *mut FdSet).unwrap_or(null_mut()); let errorfds = errorfds.map(|set| set as *mut FdSet).unwrap_or(null_mut()); @@ -85,3 +162,141 @@ pub fn select(nfds: c_int, Errno::result(res) } + +#[cfg(test)] +mod tests { + use super::*; + use sys::time::{TimeVal, TimeValLike}; + use unistd::{write, pipe}; + + #[test] + fn fdset_insert() { + let mut fd_set = FdSet::new(); + + for i in 0..FD_SETSIZE { + assert!(!fd_set.contains(i)); + } + + fd_set.insert(7); + + assert!(fd_set.contains(7)); + } + + #[test] + fn fdset_remove() { + let mut fd_set = FdSet::new(); + + for i in 0..FD_SETSIZE { + assert!(!fd_set.contains(i)); + } + + fd_set.insert(7); + fd_set.remove(7); + + for i in 0..FD_SETSIZE { + assert!(!fd_set.contains(i)); + } + } + + #[test] + fn fdset_clear() { + let mut fd_set = FdSet::new(); + fd_set.insert(1); + fd_set.insert(FD_SETSIZE / 2); + fd_set.insert(FD_SETSIZE - 1); + + fd_set.clear(); + + for i in 0..FD_SETSIZE { + assert!(!fd_set.contains(i)); + } + } + + #[test] + fn fdset_highest() { + let mut set = FdSet::new(); + assert_eq!(set.highest(), None); + set.insert(0); + assert_eq!(set.highest(), Some(0)); + set.insert(90); + assert_eq!(set.highest(), Some(90)); + set.remove(0); + assert_eq!(set.highest(), Some(90)); + set.remove(90); + assert_eq!(set.highest(), None); + + set.insert(4); + set.insert(5); + set.insert(7); + assert_eq!(set.highest(), Some(7)); + } + + // powerpc-unknown-linux-gnu currently fails on the first `assert_eq` because + // `select()` returns a 0 instead of a 1. Since this test has only been run on + // qemu, it's unclear if this is a OS or qemu bug. Just disable it on that arch + // for now. + // FIXME: Fix tests for powerpc and mips + // FIXME: Add a link to an upstream qemu bug if there is one + #[test] + #[cfg_attr(any(target_arch = "powerpc", target_arch = "mips"), ignore)] + fn test_select() { + let (r1, w1) = pipe().unwrap(); + write(w1, b"hi!").unwrap(); + let (r2, _w2) = pipe().unwrap(); + + let mut fd_set = FdSet::new(); + fd_set.insert(r1); + fd_set.insert(r2); + + let mut timeout = TimeVal::seconds(10); + assert_eq!(1, select(None, + &mut fd_set, + None, + None, + &mut timeout).unwrap()); + assert!(fd_set.contains(r1)); + assert!(!fd_set.contains(r2)); + } + + #[test] + #[cfg_attr(any(target_arch = "powerpc", target_arch = "mips"), ignore)] + fn test_select_nfds() { + let (r1, w1) = pipe().unwrap(); + write(w1, b"hi!").unwrap(); + let (r2, _w2) = pipe().unwrap(); + + let mut fd_set = FdSet::new(); + fd_set.insert(r1); + fd_set.insert(r2); + + let mut timeout = TimeVal::seconds(10); + assert_eq!(1, select(Some(fd_set.highest().unwrap() + 1), + &mut fd_set, + None, + None, + &mut timeout).unwrap()); + assert!(fd_set.contains(r1)); + assert!(!fd_set.contains(r2)); + } + + #[test] + #[cfg_attr(any(target_arch = "powerpc", target_arch = "mips"), ignore)] + fn test_select_nfds2() { + let (r1, w1) = pipe().unwrap(); + write(w1, b"hi!").unwrap(); + let (r2, _w2) = pipe().unwrap(); + + let mut fd_set = FdSet::new(); + fd_set.insert(r1); + fd_set.insert(r2); + + let mut timeout = TimeVal::seconds(10); + assert_eq!(1, select(::std::cmp::max(r1, r2) + 1, + &mut fd_set, + None, + None, + &mut timeout).unwrap()); + assert!(fd_set.contains(r1)); + assert!(!fd_set.contains(r2)); + } +} diff --git a/test/sys/mod.rs b/test/sys/mod.rs index 2ecc36f8..3aab9fc5 100644 --- a/test/sys/mod.rs +++ b/test/sys/mod.rs @@ -9,7 +9,6 @@ mod test_sockopt; mod test_termios; mod test_ioctl; mod test_wait; -mod test_select; mod test_uio; #[cfg(target_os = "linux")] diff --git a/test/sys/test_select.rs b/test/sys/test_select.rs deleted file mode 100644 index d50c7d74..00000000 --- a/test/sys/test_select.rs +++ /dev/null @@ -1,59 +0,0 @@ -use nix::sys::select::{FdSet, FD_SETSIZE, select}; -use nix::sys::time::{TimeVal, TimeValLike}; -use nix::unistd::{write, pipe}; - -#[test] -fn test_fdset() { - let mut fd_set = FdSet::new(); - - for i in 0..FD_SETSIZE { - assert!(!fd_set.contains(i)); - } - - fd_set.insert(7); - - assert!(fd_set.contains(7)); - - fd_set.remove(7); - - for i in 0..FD_SETSIZE { - assert!(!fd_set.contains(i)); - } - - fd_set.insert(1); - fd_set.insert(FD_SETSIZE / 2); - fd_set.insert(FD_SETSIZE - 1); - - fd_set.clear(); - - for i in 0..FD_SETSIZE { - assert!(!fd_set.contains(i)); - } -} - -// powerpc-unknown-linux-gnu currently fails on the first `assert_eq` because -// `select()` returns a 0 instead of a 1. Since this test has only been run on -// qemu, it's unclear if this is a OS or qemu bug. Just disable it on that arch -// for now. -// FIXME: Fix tests for powerpc and mips -// FIXME: Add a link to an upstream qemu bug if there is one -#[test] -#[cfg_attr(any(target_arch = "powerpc", target_arch = "mips"), ignore)] -fn test_select() { - let (r1, w1) = pipe().unwrap(); - write(w1, b"hi!").unwrap(); - let (r2, _w2) = pipe().unwrap(); - - let mut fd_set = FdSet::new(); - fd_set.insert(r1); - fd_set.insert(r2); - - let mut timeout = TimeVal::seconds(10); - assert_eq!(1, select(r2 + 1, - Some(&mut fd_set), - None, - None, - Some(&mut timeout)).unwrap()); - assert!(fd_set.contains(r1)); - assert!(!fd_set.contains(r2)); -} -- cgit v1.2.3