From 776aaf10dea1947229bc2bdb2d48914fe7a56094 Mon Sep 17 00:00:00 2001 From: Jonathan Date: Tue, 7 Nov 2023 07:58:07 +0000 Subject: [PATCH] feat: Safer poll timeout --- changelog/1876.changed.md | 1 + src/poll.rs | 229 ++++++++++++++++++++++++++++++++++++-- test/test_poll.rs | 6 +- 3 files changed, 224 insertions(+), 12 deletions(-) create mode 100644 changelog/1876.changed.md diff --git a/changelog/1876.changed.md b/changelog/1876.changed.md new file mode 100644 index 0000000000..858f228396 --- /dev/null +++ b/changelog/1876.changed.md @@ -0,0 +1 @@ +`poll` now takes `PollTimeout` replacing `libc::c_int`. \ No newline at end of file diff --git a/src/poll.rs b/src/poll.rs index 2a8b62d2ad..f549f9a249 100644 --- a/src/poll.rs +++ b/src/poll.rs @@ -1,9 +1,9 @@ //! Wait for events to trigger on specific file descriptors use std::os::unix::io::{AsFd, AsRawFd, BorrowedFd}; +use std::time::Duration; use crate::errno::Errno; use crate::Result; - /// This is a wrapper around `libc::pollfd`. /// /// It's meant to be used as an argument to the [`poll`](fn.poll.html) and @@ -27,13 +27,13 @@ impl<'fd> PollFd<'fd> { /// ```no_run /// # use std::os::unix::io::{AsFd, AsRawFd, FromRawFd}; /// # use nix::{ - /// # poll::{PollFd, PollFlags, poll}, + /// # poll::{PollTimeout, PollFd, PollFlags, poll}, /// # unistd::{pipe, read} /// # }; /// let (r, w) = pipe().unwrap(); /// let pfd = PollFd::new(r.as_fd(), PollFlags::POLLIN); /// let mut fds = [pfd]; - /// poll(&mut fds, -1).unwrap(); + /// poll(&mut fds, PollTimeout::NONE).unwrap(); /// let mut buf = [0u8; 80]; /// read(r.as_raw_fd(), &mut buf[..]); /// ``` @@ -179,6 +179,210 @@ libc_bitflags! { } } +/// Timeout argument for [`poll`]. +#[derive(Debug, Clone, Copy, Eq, PartialEq, Ord, PartialOrd)] +pub struct PollTimeout(i32); + +impl PollTimeout { + /// Blocks indefinitely. + /// + /// > Specifying a negative value in timeout means an infinite timeout. + pub const NONE: Self = Self(-1); + /// Returns immediately. + /// + /// > Specifying a timeout of zero causes poll() to return immediately, even if no file + /// > descriptors are ready. + pub const ZERO: Self = Self(0); + /// Blocks for at most [`std::i32::MAX`] milliseconds. + pub const MAX: Self = Self(i32::MAX); + /// Returns if `self` equals [`PollTimeout::NONE`]. + pub fn is_none(&self) -> bool { + // > Specifying a negative value in timeout means an infinite timeout. + *self <= Self::NONE + } + /// Returns if `self` does not equal [`PollTimeout::NONE`]. + pub fn is_some(&self) -> bool { + !self.is_none() + } + /// Returns the timeout in milliseconds if there is some, otherwise returns `None`. + pub fn as_millis(&self) -> Option { + self.is_some().then_some(self.0) + } + /// Returns the timeout as a `Duration` if there is some, otherwise returns `None`. + pub fn timeout(&self) -> Option { + self.as_millis().map(|x|Duration::from_millis(u64::try_from(x).unwrap())) + } +} + +/// Error type for integer conversions into `PollTimeout`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PollTimeoutTryFromError { + /// Passing a value less than -1 is invalid on some systems, see + /// . + TooNegative, + /// Passing a value greater than `i32::MAX` is invalid. + TooPositive, +} + +impl std::fmt::Display for PollTimeoutTryFromError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::TooNegative => write!(f, "Passed a negative timeout less than -1."), + Self::TooPositive => write!(f, "Passed a positive timeout greater than `i32::MAX` milliseconds.") + } + } +} + +impl std::error::Error for PollTimeoutTryFromError {} + +impl> From> for PollTimeout { + fn from(x: Option) -> Self { + x.map_or(Self::NONE, |x| x.into()) + } +} +impl TryFrom for PollTimeout { + type Error = PollTimeoutTryFromError; + fn try_from(x: Duration) -> std::result::Result { + Ok(Self(i32::try_from(x.as_millis()).map_err(|_|PollTimeoutTryFromError::TooPositive)?)) + } +} +impl TryFrom for PollTimeout { + type Error = PollTimeoutTryFromError; + fn try_from(x: u128) -> std::result::Result { + Ok(Self(i32::try_from(x).map_err(|_|PollTimeoutTryFromError::TooPositive)?)) + } +} +impl TryFrom for PollTimeout { + type Error = PollTimeoutTryFromError; + fn try_from(x: u64) -> std::result::Result { + Ok(Self(i32::try_from(x).map_err(|_|PollTimeoutTryFromError::TooPositive)?)) + } +} +impl TryFrom for PollTimeout { + type Error = PollTimeoutTryFromError; + fn try_from(x: u32) -> std::result::Result { + Ok(Self(i32::try_from(x).map_err(|_|PollTimeoutTryFromError::TooPositive)?)) + } +} +impl From for PollTimeout { + fn from(x: u16) -> Self { + Self(i32::from(x)) + } +} +impl From for PollTimeout { + fn from(x: u8) -> Self { + Self(i32::from(x)) + } +} +impl TryFrom for PollTimeout { + type Error = PollTimeoutTryFromError; + fn try_from(x: i128) -> std::result::Result { + match x { + ..=-2 => Err(PollTimeoutTryFromError::TooNegative), + -1.. => Ok(Self(i32::try_from(x).map_err(|_|PollTimeoutTryFromError::TooPositive)?)), + } + } +} +impl TryFrom for PollTimeout { + type Error = PollTimeoutTryFromError; + fn try_from(x: i64) -> std::result::Result { + match x { + ..=-2 => Err(PollTimeoutTryFromError::TooNegative), + -1.. => Ok(Self(i32::try_from(x).map_err(|_|PollTimeoutTryFromError::TooPositive)?)), + } + } +} +impl TryFrom for PollTimeout { + type Error = PollTimeoutTryFromError; + fn try_from(x: i32) -> std::result::Result { + match x { + ..=-2 => Err(PollTimeoutTryFromError::TooNegative), + -1.. => Ok(Self(x)), + } + } +} +impl TryFrom for PollTimeout { + type Error = PollTimeoutTryFromError; + fn try_from(x: i16) -> std::result::Result { + match x { + ..=-2 => Err(PollTimeoutTryFromError::TooNegative), + -1.. => Ok(Self(i32::from(x))), + } + } +} +impl TryFrom for PollTimeout { + type Error = PollTimeoutTryFromError; + fn try_from(x: i8) -> std::result::Result { + match x { + ..=-2 => Err(PollTimeoutTryFromError::TooNegative), + -1.. => Ok(Self(i32::from(x))), + } + } +} +impl TryFrom for Duration { + type Error = (); + fn try_from(x: PollTimeout) -> std::result::Result { + x.timeout().ok_or(()) + } +} +impl TryFrom for u128 { + type Error = >::Error; + fn try_from(x: PollTimeout) -> std::result::Result { + Self::try_from(x.0) + } +} +impl TryFrom for u64 { + type Error = >::Error; + fn try_from(x: PollTimeout) -> std::result::Result { + Self::try_from(x.0) + } +} +impl TryFrom for u32 { + type Error = >::Error; + fn try_from(x: PollTimeout) -> std::result::Result { + Self::try_from(x.0) + } +} +impl TryFrom for u16 { + type Error = >::Error; + fn try_from(x: PollTimeout) -> std::result::Result { + Self::try_from(x.0) + } +} +impl TryFrom for u8 { + type Error = >::Error; + fn try_from(x: PollTimeout) -> std::result::Result { + Self::try_from(x.0) + } +} +impl From for i128 { + fn from(x: PollTimeout) -> Self { + Self::from(x.0) + } +} +impl From for i64 { + fn from(x: PollTimeout) -> Self { + Self::from(x.0) + } +} +impl From for i32 { + fn from(x: PollTimeout) -> Self { + x.0 + } +} +impl TryFrom for i16 { + type Error = >::Error; + fn try_from(x: PollTimeout) -> std::result::Result { + Self::try_from(x.0) + } +} +impl TryFrom for i8 { + type Error = >::Error; + fn try_from(x: PollTimeout) -> std::result::Result { + Self::try_from(x.0) + } +} + /// `poll` waits for one of a set of file descriptors to become ready to perform I/O. /// ([`poll(2)`](https://pubs.opengroup.org/onlinepubs/9699919799/functions/poll.html)) /// @@ -195,13 +399,20 @@ libc_bitflags! { /// /// Note that the timeout interval will be rounded up to the system clock /// granularity, and kernel scheduling delays mean that the blocking -/// interval may overrun by a small amount. Specifying a negative value -/// in timeout means an infinite timeout. Specifying a timeout of zero -/// causes `poll()` to return immediately, even if no file descriptors are -/// ready. -pub fn poll(fds: &mut [PollFd], timeout: libc::c_int) -> Result { +/// interval may overrun by a small amount. Specifying a [`PollTimeout::NONE`] +/// in timeout means an infinite timeout. Specifying a timeout of +/// [`PollTimeout::ZERO`] causes `poll()` to return immediately, even if no file +/// descriptors are ready. +pub fn poll>( + fds: &mut [PollFd], + timeout: T, +) -> Result { let res = unsafe { - libc::poll(fds.as_mut_ptr().cast(), fds.len() as libc::nfds_t, timeout) + libc::poll( + fds.as_mut_ptr().cast(), + fds.len() as libc::nfds_t, + i32::from(timeout.into()), + ) }; Errno::result(res) diff --git a/test/test_poll.rs b/test/test_poll.rs index fb4291721d..4227f59994 100644 --- a/test/test_poll.rs +++ b/test/test_poll.rs @@ -1,6 +1,6 @@ use nix::{ errno::Errno, - poll::{poll, PollFd, PollFlags}, + poll::{poll, PollFd, PollFlags, PollTimeout}, unistd::{pipe, write}, }; use std::os::unix::io::{AsFd, BorrowedFd}; @@ -23,14 +23,14 @@ fn test_poll() { let mut fds = [PollFd::new(r.as_fd(), PollFlags::POLLIN)]; // Poll an idle pipe. Should timeout - let nfds = loop_while_eintr!(poll(&mut fds, 100)); + let nfds = loop_while_eintr!(poll(&mut fds, PollTimeout::from(100u8))); assert_eq!(nfds, 0); assert!(!fds[0].revents().unwrap().contains(PollFlags::POLLIN)); write(&w, b".").unwrap(); // Poll a readable pipe. Should return an event. - let nfds = poll(&mut fds, 100).unwrap(); + let nfds = poll(&mut fds, PollTimeout::from(100u8)).unwrap(); assert_eq!(nfds, 1); assert!(fds[0].revents().unwrap().contains(PollFlags::POLLIN)); }