From cf575d24b895907c253ebfe1c69920c06bb9be7a Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Fri, 28 Feb 2025 05:57:10 -0800 Subject: [PATCH 1/2] Convert `port::nget` to `Buffer`. And, change `port::*` to use Timespec for timeouts, consistent with the other polling functions in rustix. --- CHANGES.md | 39 ++++++++----- src/backend/libc/event/syscalls.rs | 89 +++++++++++++++++++++--------- src/event/port.rs | 53 ++++++------------ 3 files changed, 104 insertions(+), 77 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 10226177a..e3c95b663 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -7,15 +7,17 @@ greater than the requested size. [`rustix::pipe::fcntl_setpipe_size`]: https://docs.rs/rustix/1.0.0/rustix/pipe/fn.fcntl_setpipe_size.html -When a `&mut Vec<_>` is passed to [`rustix::event::epoll::wait`] or -[`rustix::event::kqueue::kevent`], these functions previously adjusted the -length of the `Vec` to the number of elements written, and now do not. A common -alternative is to wrap the `&mut Vec<_>` using [`spare_capacity`], and then to -clear the `Vec` by iterating using `.drain(..)` after each call. For an example -of using `spare_capacity` in this way, see [here]. +When a `&mut Vec<_>` is passed to [`rustix::event::epoll::wait`], +[`rustix::event::kqueue::kevent`], or [`rustix::event::port::getn`], these +functions previously adjusted the length of the `Vec` to the number of elements +written, and now do not. A common alternative is to wrap the `&mut Vec<_>` +using [`spare_capacity`], and then to clear the `Vec` by iterating using +`.drain(..)` after each call. For an example of using `spare_capacity` in this +way, see [here]. [`rustix::event::epoll::wait`]: https://docs.rs/rustix/1.0.0/rustix/event/epoll/fn.wait.html -[`rustix::event::kqueue::kevent`]: https://docs.rs/rustix/1.0.0/x86_64-unknown-freebsd/rustix/event/kqueue/fn.kqueue.html +[`rustix::event::kqueue::kevent`]: https://docs.rs/rustix/1.0.0/x86_64-unknown-freebsd/rustix/event/kqueue/fn.kevent.html +[`rustix::event::port::getn`]: https://docs.rs/rustix/1.0.0/x86_64-unknown-illumos/rustix/event/port/fn.getn.html [`spare_capacity`]: https://docs.rs/rustix/1.0.0/rustix/buffer/fn.spare_capacity.html [here]: https://docs.rs/rustix/1.0.0/rustix/event/epoll/index.html#examples @@ -287,18 +289,19 @@ a `[MaybeUninit]` instead of a `[u8]`. [`RecvAncillaryBuffer`]: https://docs.rs/rustix/1.0.0/rustix/net/struct.RecvAncillaryBuffer.html [`read`], [`pread`], [`recv`], [`recvfrom`], [`getrandom`], [`epoll::wait`], -[`kqueue`], [`getxattr`], [`lgetxattr`], [`fgetxattr`], [`listxattr`], -[`llistxattr`], and [`flistxattr`] now use the new [`Buffer` trait]. +[`kevent`], [`port::getn`], [`getxattr`], [`lgetxattr`], [`fgetxattr`], +[`listxattr`], [`llistxattr`], and [`flistxattr`] now use the new +[`Buffer` trait]. This replaces `read_uninit`, `pread_uninit`, `recv_uninit`, `recvfrom_uninit`, and `getrandom_uninit`, as the `Buffer` trait supports reading into uninitialized slices. -`epoll::wait` and `kqueue` previously took a `Vec` which it implicitly cleared -before results were appended. When passing a `Vec` to `epoll::wait` or `kqueue` -using [`spare_capacity`], the `Vec` is not cleared first. Consider clearing the -vector before calling `epoll::wait` or `kqueue`, or consuming it using -`.drain(..)` before reusing it. +`epoll::wait`, `kevent`, and `port::getn` previously took a `Vec` which they +implicitly cleared before results were appended. When passing a `Vec` to +`epoll::wait`, `kevent`, or `port::getn` using [`spare_capacity`], the `Vec` is +not cleared first. Consider clearing the vector before calling `epoll::wait`, +`kevent`, or `port::getn`, or consuming it using `.drain(..)` before reusing it. [`read`]: https://docs.rs/rustix/1.0.0/rustix/io/fn.read.html [`pread`]: https://docs.rs/rustix/1.0.0/rustix/io/fn.pread.html @@ -312,7 +315,8 @@ vector before calling `epoll::wait` or `kqueue`, or consuming it using [`listxattr`]: https://docs.rs/rustix/1.0.0/rustix/fs/fn.listxattr.html [`llistxattr`]: https://docs.rs/rustix/1.0.0/rustix/fs/fn.llistxattr.html [`flistxattr`]: https://docs.rs/rustix/1.0.0/rustix/fs/fn.flistxattr.html -[`kqueue`]: https://docs.rs/rustix/1.0.0/x86_64-unknown-freebsd/rustix/event/kqueue/fn.kqueue.html +[`kevent`]: https://docs.rs/rustix/1.0.0/x86_64-unknown-freebsd/rustix/event/kqueue/fn.kevent.html +[`port::getn`]: https://docs.rs/rustix/1.0.0/x86_64-unknown-illumos/rustix/event/port/fn.getn.html [`Buffer` trait]: https://docs.rs/rustix/1.0.0/rustix/buffer/trait.Buffer.html [`spare_capacity`]: https://docs.rs/rustix/1.0.0/rustix/buffer/fn.spare_capacity.html @@ -335,5 +339,10 @@ In place of `BadOpcode`, use the opcode value directly. [`rustix::ioctl::Opcode`]: https://docs.rs/rustix/1.0.0/rustix/ioctl/type.Opcode.html [`ioctl::opcode`]: https://docs.rs/rustix/1.0.0/rustix/ioctl/opcode/index.html +[`rustix::event::port::getn`]'s `min_events` argument is now a `u32`, to +reflect the type in the underlying system API. + +[`rustix::event::port::getn`]: https://docs.rs/rustix/1.0.0/x86_64-unknown-illumos/rustix/event/port/fn.getn.html + All explicitly deprecated functions and types have been removed. Their deprecation messages will have identified alternatives. diff --git a/src/backend/libc/event/syscalls.rs b/src/backend/libc/event/syscalls.rs index 8c126bf9f..97b47150c 100644 --- a/src/backend/libc/event/syscalls.rs +++ b/src/backend/libc/event/syscalls.rs @@ -19,8 +19,6 @@ use crate::event::EventfdFlags; use crate::event::FdSetElement; use crate::event::{PollFd, Timespec}; use crate::io; -#[cfg(solarish)] -use crate::utils::as_mut_ptr; #[cfg(any(linux_kernel, target_os = "illumos", target_os = "redox"))] use crate::utils::as_ptr; #[cfg(solarish)] @@ -362,52 +360,89 @@ pub(crate) unsafe fn port_dissociate( } #[cfg(solarish)] -pub(crate) fn port_get( - port: BorrowedFd<'_>, - timeout: Option<&mut c::timespec>, -) -> io::Result { +pub(crate) fn port_get(port: BorrowedFd<'_>, timeout: Option<&Timespec>) -> io::Result { + // If we don't have to fix y2038 on this platform, `Timespec` is + // the same as `c::timespec` and it's easy. + #[cfg(not(fix_y2038))] + let timeout = crate::timespec::option_as_libc_timespec_ptr(timeout); + + // If we do have to fix y2038 on this platform, convert to + // `c::timespec`. + #[cfg(fix_y2038)] + let converted_timeout; + #[cfg(fix_y2038)] + let timeout = match timeout { + None => null(), + Some(timeout) => { + converted_timeout = c::timespec { + tv_sec: timeout.tv_sec.try_into().map_err(|_| io::Errno::OVERFLOW)?, + tv_nsec: timeout.tv_nsec as _, + }; + &converted_timeout + } + }; + let mut event = MaybeUninit::::uninit(); - let timeout = timeout.map_or(null_mut(), as_mut_ptr); + // In Rust >= 1.65, the `as _` can be `.cast_mut()`. unsafe { - ret(c::port_get(borrowed_fd(port), event.as_mut_ptr(), timeout))?; + ret(c::port_get( + borrowed_fd(port), + event.as_mut_ptr(), + timeout as _, + ))?; } // If we're done, initialize the event and return it. Ok(Event(unsafe { event.assume_init() })) } -#[cfg(all(feature = "alloc", solarish))] -pub(crate) fn port_getn( +#[cfg(solarish)] +pub(crate) unsafe fn port_getn( port: BorrowedFd<'_>, - timeout: Option<&mut c::timespec>, - events: &mut Vec, + events: (*mut Event, usize), mut nget: u32, -) -> io::Result<()> { + timeout: Option<&Timespec>, +) -> io::Result { + // If we don't have to fix y2038 on this platform, `Timespec` is + // the same as `c::timespec` and it's easy. + #[cfg(not(fix_y2038))] + let timeout = crate::timespec::option_as_libc_timespec_ptr(timeout); + + // If we do have to fix y2038 on this platform, convert to + // `c::timespec`. + #[cfg(fix_y2038)] + let converted_timeout; + #[cfg(fix_y2038)] + let timeout = match timeout { + None => null(), + Some(timeout) => { + converted_timeout = c::timespec { + tv_sec: timeout.tv_sec.try_into().map_err(|_| io::Errno::OVERFLOW)?, + tv_nsec: timeout.tv_nsec as _, + }; + &converted_timeout + } + }; + // `port_getn` special-cases a max value of 0 to be a query that returns - // the number of events. We don't want to do the `set_len` in that case, so - // so bail out early if needed. - if events.capacity() == 0 { - return Ok(()); + // the number of events, so so bail out early if needed. + if events.1 == 0 { + return Ok(0); } - let timeout = timeout.map_or(null_mut(), as_mut_ptr); + // In Rust >= 1.65, the `as _` can be `.cast_mut()`. unsafe { ret(c::port_getn( borrowed_fd(port), - events.as_mut_ptr().cast(), - events.capacity().try_into().unwrap(), + events.0.cast(), + events.1.try_into().unwrap_or(u32::MAX), &mut nget, - timeout, + timeout as _, ))?; } - // Update the vector length. - unsafe { - events.set_len(nget.try_into().unwrap()); - } - - Ok(()) + Ok(nget as usize) } #[cfg(solarish)] diff --git a/src/event/port.rs b/src/event/port.rs index a5c2e9310..fc0c0b566 100644 --- a/src/event/port.rs +++ b/src/event/port.rs @@ -29,11 +29,11 @@ use crate::backend::c; use crate::backend::event::syscalls; +use crate::buffer::Buffer; use crate::fd::{AsFd, AsRawFd, OwnedFd}; +use crate::timespec::Timespec; use crate::{ffi, io}; -use core::time::Duration; - pub use super::PollFlags; /// The structure representing a port event. @@ -130,24 +130,16 @@ pub unsafe fn dissociate_fd(port: Fd, object: RawFd) - /// [OpenSolaris]: https://www.unix.com/man-page/opensolaris/3C/port_get/ /// [illumos]: https://illumos.org/man/3C/port_get #[doc(alias = "port_get")] -pub fn get(port: Fd, timeout: Option) -> io::Result { - let mut timeout = timeout.map(|timeout| c::timespec { - tv_sec: timeout.as_secs().try_into().unwrap(), - tv_nsec: timeout.subsec_nanos() as _, - }); - - syscalls::port_get(port.as_fd(), timeout.as_mut()) +pub fn get(port: Fd, timeout: Option<&Timespec>) -> io::Result { + syscalls::port_get(port.as_fd(), timeout) } -/// `port_getn(port, events, min_events, timeout)`—Gets multiple events from a -/// port. +/// `port_getn(port, events, min_events, timeout)`—Gets multiple events from +/// a port. /// -/// This requests up to a max of `events.capacity()` events, and then resizes -/// `events` to the number of events retrieved. If `events.capacity()` is 0, -/// this does nothing and returns immediately. +/// If `events` is empty, this does nothing and returns immediately. /// -/// To query the number of events without retrieving any, use -/// [`getn_query`]. +/// To query the number of events without retrieving any, use [`getn_query`]. /// /// # References /// - [OpenSolaris] @@ -155,27 +147,18 @@ pub fn get(port: Fd, timeout: Option) -> io::Result { /// /// [OpenSolaris]: https://www.unix.com/man-page/opensolaris/3C/port_getn/ /// [illumos]: https://illumos.org/man/3C/port_getn -#[cfg(feature = "alloc")] #[doc(alias = "port_getn")] -pub fn getn( +pub fn getn>( port: Fd, - events: &mut Vec, - min_events: usize, - timeout: Option, -) -> io::Result<()> { - events.clear(); - - let mut timeout = timeout.map(|timeout| c::timespec { - tv_sec: timeout.as_secs().try_into().unwrap(), - tv_nsec: timeout.subsec_nanos() as _, - }); - - syscalls::port_getn( - port.as_fd(), - timeout.as_mut(), - events, - min_events.try_into().unwrap(), - ) + mut events: Buf, + min_events: u32, + timeout: Option<&Timespec>, +) -> io::Result { + // SAFETY: `port_getn` behaves. + let nevents = + unsafe { syscalls::port_getn(port.as_fd(), events.parts_mut(), min_events, timeout)? }; + // SAFETY: `port_getn` behaves. + unsafe { Ok(events.assume_init(nevents)) } } /// `port_getn(port, NULL, 0, NULL)`—Queries the number of events From f34a0a09b3a705be28f446a83ea4ea55adb9a6df Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Sat, 1 Mar 2025 07:08:48 -0800 Subject: [PATCH 2/2] Fix a warning. --- src/backend/libc/event/syscalls.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/backend/libc/event/syscalls.rs b/src/backend/libc/event/syscalls.rs index 97b47150c..21cdf7309 100644 --- a/src/backend/libc/event/syscalls.rs +++ b/src/backend/libc/event/syscalls.rs @@ -432,15 +432,13 @@ pub(crate) unsafe fn port_getn( } // In Rust >= 1.65, the `as _` can be `.cast_mut()`. - unsafe { - ret(c::port_getn( - borrowed_fd(port), - events.0.cast(), - events.1.try_into().unwrap_or(u32::MAX), - &mut nget, - timeout as _, - ))?; - } + ret(c::port_getn( + borrowed_fd(port), + events.0.cast(), + events.1.try_into().unwrap_or(u32::MAX), + &mut nget, + timeout as _, + ))?; Ok(nget as usize) }