Skip to content

Commit

Permalink
feat: Safer poll timeout
Browse files Browse the repository at this point in the history
  • Loading branch information
JonathanWoollett-Light committed Nov 7, 2023
1 parent 582e773 commit e6b1a07
Show file tree
Hide file tree
Showing 3 changed files with 250 additions and 10 deletions.
55 changes: 55 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,61 @@ All notable changes to this project will be documented in this file.
This project adheres to [Semantic Versioning](https://semver.org/).

# Change Log
## [Unreleased] - ReleaseDate

### Fixed
- Fix `SigSet` incorrect implementation of `Eq`, `PartialEq` and `Hash`
([#1946](https://github.com/nix-rust/nix/pull/1946))

- Fixed the function signature of `recvmmsg`, potentially causing UB
([#2119](https://github.com/nix-rust/nix/issues/2119))

- Fix `SignalFd::set_mask`. In 0.27.0 it would actually close the file
descriptor.
([#2141](https://github.com/nix-rust/nix/pull/2141))

### Changed

- The MSRV is now 1.69
([#2144](https://github.com/nix-rust/nix/pull/2144))

- The following APIs now take an implementation of `AsFd` rather than a
`RawFd`:

- `unistd::tcgetpgrp`
- `unistd::tcsetpgrp`
- `unistd::fpathconf`
- `unistd::ttyname`
- `unistd::getpeereid`

([#2137](https://github.com/nix-rust/nix/pull/2137))

- Changed `openat()` and `Dir::openat()`, now take optional `dirfd`s
([#2139](https://github.com/nix-rust/nix/pull/2139))

- `PollFd::new` now takes a `BorrowedFd` argument, with relaxed lifetime
requirements relative to the previous version.
([#2134](https://github.com/nix-rust/nix/pull/2134))

- `FdSet::{insert, remove, contains}` now take `BorrowedFd` arguments, and have
relaxed lifetime requirements relative to 0.27.1.
([#2136](https://github.com/nix-rust/nix/pull/2136))

- Simplified the function signatures of `recvmmsg` and `sendmmsg`

- The `timeout` argument of `poll::poll` is now of type `poll::PollTimeout`.
([#1876](https://github.com/nix-rust/nix/pull/1876))

### Added
- Added `Icmp` and `IcmpV6` to `SockProtocol`.
(#[2103](https://github.com/nix-rust/nix/pull/2103))

- Added `F_GETPATH` FcntlFlags entry on Apple/NetBSD/DragonflyBSD for `::nix::fcntl`.
([#2142](https://github.com/nix-rust/nix/pull/2142))

- Added `Ipv6HopLimit` to `::nix::sys::socket::ControlMessage` for Linux,
MacOS, FreeBSD, DragonflyBSD, Android, iOS and Haiku.
([#2074](https://github.com/nix-rust/nix/pull/2074))

## [0.27.1] - 2023-08-28

Expand Down
199 changes: 192 additions & 7 deletions src/poll.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -179,6 +179,184 @@ 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<i32> {
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<Duration> {
self.as_millis().map(|x|Duration::from_millis(u64::try_from(x).unwrap()))
}
}

impl<T: Into<PollTimeout>> From<Option<T>> for PollTimeout {
fn from(x: Option<T>) -> Self {
x.map_or(Self::NONE, |x| x.into())
}
}
impl TryFrom<Duration> for PollTimeout {
type Error = <i32 as TryFrom<u128>>::Error;
fn try_from(x: Duration) -> std::result::Result<Self, Self::Error> {
Ok(Self(i32::try_from(x.as_millis())?))
}
}
impl TryFrom<u128> for PollTimeout {
type Error = <i32 as TryFrom<u128>>::Error;
fn try_from(x: u128) -> std::result::Result<Self, Self::Error> {
Ok(Self(i32::try_from(x)?))
}
}
impl TryFrom<u64> for PollTimeout {
type Error = <i32 as TryFrom<u64>>::Error;
fn try_from(x: u64) -> std::result::Result<Self, Self::Error> {
Ok(Self(i32::try_from(x)?))
}
}
impl TryFrom<u32> for PollTimeout {
type Error = <i32 as TryFrom<u32>>::Error;
fn try_from(x: u32) -> std::result::Result<Self, Self::Error> {
Ok(Self(i32::try_from(x)?))
}
}
impl From<u16> for PollTimeout {
fn from(x: u16) -> Self {
Self(i32::from(x))
}
}
impl From<u8> for PollTimeout {
fn from(x: u8) -> Self {
Self(i32::from(x))
}
}
impl TryFrom<i128> for PollTimeout {
type Error = <i32 as TryFrom<i128>>::Error;
fn try_from(x: i128) -> std::result::Result<Self, Self::Error> {
match x {
// > Specifying a negative value in timeout means an infinite timeout.
i128::MIN..=-1 => Ok(Self::NONE),
millis @ 0.. => Ok(Self(i32::try_from(millis)?)),
}
}
}
impl TryFrom<i64> for PollTimeout {
type Error = <i32 as TryFrom<i64>>::Error;
fn try_from(x: i64) -> std::result::Result<Self, Self::Error> {
match x {
i64::MIN..=-1 => Ok(Self::NONE),
millis @ 0.. => Ok(Self(i32::try_from(millis)?)),
}
}
}
impl From<i32> for PollTimeout {
fn from(x: i32) -> Self {
Self(x)
}
}
impl From<i16> for PollTimeout {
fn from(x: i16) -> Self {
Self(i32::from(x))
}
}
impl From<i8> for PollTimeout {
fn from(x: i8) -> Self {
Self(i32::from(x))
}
}
impl TryFrom<PollTimeout> for Duration {
type Error = ();
fn try_from(x: PollTimeout) -> std::result::Result<Self, ()> {
match x.timeout() {
// SAFETY: When `x.timeout()` returns `Some(a)`, `a` is always non-negative.
Some(millis) => Ok(Duration::from_millis(unsafe {
u64::try_from(millis).unwrap_unchecked()
})),
None => Err(()),
}
}
}
impl TryFrom<PollTimeout> for u128 {
type Error = <Self as TryFrom<i32>>::Error;
fn try_from(x: PollTimeout) -> std::result::Result<Self, Self::Error> {
Self::try_from(x.0)
}
}
impl TryFrom<PollTimeout> for u64 {
type Error = <Self as TryFrom<i32>>::Error;
fn try_from(x: PollTimeout) -> std::result::Result<Self, Self::Error> {
Self::try_from(x.0)
}
}
impl TryFrom<PollTimeout> for u32 {
type Error = <Self as TryFrom<i32>>::Error;
fn try_from(x: PollTimeout) -> std::result::Result<Self, Self::Error> {
Self::try_from(x.0)
}
}
impl TryFrom<PollTimeout> for u16 {
type Error = <Self as TryFrom<i32>>::Error;
fn try_from(x: PollTimeout) -> std::result::Result<Self, Self::Error> {
Self::try_from(x.0)
}
}
impl TryFrom<PollTimeout> for u8 {
type Error = <Self as TryFrom<i32>>::Error;
fn try_from(x: PollTimeout) -> std::result::Result<Self, Self::Error> {
Self::try_from(x.0)
}
}
impl From<PollTimeout> for i128 {
fn from(x: PollTimeout) -> Self {
Self::from(x.0)
}
}
impl From<PollTimeout> for i64 {
fn from(x: PollTimeout) -> Self {
Self::from(x.0)
}
}
impl From<PollTimeout> for i32 {
fn from(x: PollTimeout) -> Self {
x.0
}
}
impl TryFrom<PollTimeout> for i16 {
type Error = <Self as TryFrom<i32>>::Error;
fn try_from(x: PollTimeout) -> std::result::Result<Self, Self::Error> {
Self::try_from(x.0)
}
}
impl TryFrom<PollTimeout> for i8 {
type Error = <Self as TryFrom<i32>>::Error;
fn try_from(x: PollTimeout) -> std::result::Result<Self, Self::Error> {
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))
///
Expand All @@ -195,13 +373,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<libc::c_int> {
/// 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<T: Into<PollTimeout>>(
fds: &mut [PollFd],
timeout: T,
) -> Result<libc::c_int> {
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)
Expand Down
6 changes: 3 additions & 3 deletions test/test_poll.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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));
}
Expand Down

0 comments on commit e6b1a07

Please sign in to comment.