Skip to content

Commit

Permalink
Deprecate FutexOperation instead of futex (#1118)
Browse files Browse the repository at this point in the history
With `rustix::thread::futex` being both a deprecated function and
a public module, I'm seeing warnings like this when I use it in
some simple testcases:

```console
warning: use of deprecated function `rustix::thread::futex`: There are now individual functions available to perform futex operations with improved type safety. See the futex module.
 --> test.rs:2:21
  |
2 | use rustix::thread::futex;
  |                     ^^^^^
  |
  = note: `#[warn(deprecated)]` on by default
```

To fix this, move the deprecation from the `futex` function to the
`FutexOperation` enum. This has the same effect, but doesn't
produce the warning then the `futex` module is used.

Also while using the API, I found `futex::FutexFlags` to be a little
redundant, so I think it makes sense to rename it to `futex::Flags`,
and similarly rename `futex::FUTEX_WAITERS` to `futex::WAITERS` and
so on.

This also splits `FutexOperation` into two enums internally, so that
the public `FutexOperation` which is `#[non_exhausitive]` can continue
to have its original contents, and the private enum can add new opcodes.
  • Loading branch information
sunfishcode authored Aug 27, 2024
1 parent 4bd811f commit 5ab7762
Show file tree
Hide file tree
Showing 8 changed files with 228 additions and 230 deletions.
49 changes: 41 additions & 8 deletions src/backend/libc/thread/futex.rs
Original file line number Diff line number Diff line change
@@ -1,25 +1,23 @@
use crate::backend::c;

bitflags::bitflags! {
/// `FUTEX_*` flags for use with [`futex`].
/// `FUTEX_*` flags for use with the functions in [`futex`].
///
/// [`futex`]: mod@crate::thread::futex
#[repr(transparent)]
#[derive(Copy, Clone, Eq, PartialEq, Hash, Debug)]
pub struct FutexFlags: u32 {
pub struct Flags: u32 {
/// `FUTEX_PRIVATE_FLAG`
const PRIVATE = bitcast!(c::FUTEX_PRIVATE_FLAG);
/// `FUTEX_CLOCK_REALTIME`
const CLOCK_REALTIME = bitcast!(c::FUTEX_CLOCK_REALTIME);
}
}

/// `FUTEX_*` operations for use with [`futex`].
///
/// [`futex`]: mod@crate::thread::futex
/// `FUTEX_*` operations for use with the futex syscall wrappers.
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
#[repr(u32)]
pub enum FutexOperation {
pub(crate) enum Operation {
/// `FUTEX_WAIT`
Wait = bitcast!(c::FUTEX_WAIT),
/// `FUTEX_WAKE`
Expand Down Expand Up @@ -50,8 +48,43 @@ pub enum FutexOperation {
LockPi2 = bitcast!(c::FUTEX_LOCK_PI2),
}

/// `FUTEX_*` operations for use with the [`futex`] function.
///
/// [`futex`]: fn@crate::thread::futex
#[deprecated(
since = "0.38.35",
note = "
The `futex` function and `FutexOperation` enum are deprecated. There are
individual functions available to perform futex operations with improved
type safety. See the `rustix::thread::futex` module."
)]
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
#[repr(u32)]
pub enum FutexOperation {
/// `FUTEX_WAIT`
Wait = bitcast!(c::FUTEX_WAIT),
/// `FUTEX_WAKE`
Wake = bitcast!(c::FUTEX_WAKE),
/// `FUTEX_FD`
Fd = bitcast!(c::FUTEX_FD),
/// `FUTEX_REQUEUE`
Requeue = bitcast!(c::FUTEX_REQUEUE),
/// `FUTEX_CMP_REQUEUE`
CmpRequeue = bitcast!(c::FUTEX_CMP_REQUEUE),
/// `FUTEX_WAKE_OP`
WakeOp = bitcast!(c::FUTEX_WAKE_OP),
/// `FUTEX_LOCK_PI`
LockPi = bitcast!(c::FUTEX_LOCK_PI),
/// `FUTEX_UNLOCK_PI`
UnlockPi = bitcast!(c::FUTEX_UNLOCK_PI),
/// `FUTEX_TRYLOCK_PI`
TrylockPi = bitcast!(c::FUTEX_TRYLOCK_PI),
/// `FUTEX_WAIT_BITSET`
WaitBitset = bitcast!(c::FUTEX_WAIT_BITSET),
}

/// `FUTEX_WAITERS`
pub const FUTEX_WAITERS: u32 = linux_raw_sys::general::FUTEX_WAITERS;
pub const WAITERS: u32 = linux_raw_sys::general::FUTEX_WAITERS;

/// `FUTEX_OWNER_DIED`
pub const FUTEX_OWNER_DIED: u32 = linux_raw_sys::general::FUTEX_OWNER_DIED;
pub const OWNER_DIED: u32 = linux_raw_sys::general::FUTEX_OWNER_DIED;
17 changes: 8 additions & 9 deletions src/backend/libc/thread/syscalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,10 @@ use core::mem::MaybeUninit;
use core::sync::atomic::AtomicU32;
#[cfg(linux_kernel)]
use {
super::futex::FutexOperation,
crate::backend::conv::{borrowed_fd, ret_c_int, ret_usize},
crate::fd::BorrowedFd,
crate::pid::Pid,
crate::thread::FutexFlags,
crate::thread::futex,
crate::utils::as_mut_ptr,
};
#[cfg(not(any(
Expand Down Expand Up @@ -420,14 +419,14 @@ pub(crate) fn setresgid_thread(
#[cfg(linux_kernel)]
pub(crate) unsafe fn futex_val2(
uaddr: *const AtomicU32,
op: FutexOperation,
flags: FutexFlags,
op: super::futex::Operation,
flags: futex::Flags,
val: u32,
val2: u32,
uaddr2: *const AtomicU32,
val3: u32,
) -> io::Result<usize> {
// the least significant four bytes of the timeout pointer are used as `val2`.
// The least-significant four bytes of the timeout pointer are used as `val2`.
// ["the kernel casts the timeout value first to unsigned long, then to uint32_t"](https://man7.org/linux/man-pages/man2/futex.2.html),
// so we perform that exact conversion in reverse to create the pointer.
let timeout = val2 as usize as *const Timespec;
Expand Down Expand Up @@ -493,8 +492,8 @@ pub(crate) unsafe fn futex_val2(
#[cfg(linux_kernel)]
pub(crate) unsafe fn futex_timeout(
uaddr: *const AtomicU32,
op: FutexOperation,
flags: FutexFlags,
op: super::futex::Operation,
flags: futex::Flags,
val: u32,
timeout: *const Timespec,
uaddr2: *const AtomicU32,
Expand Down Expand Up @@ -574,8 +573,8 @@ pub(crate) unsafe fn futex_timeout(
))]
unsafe fn futex_old_timespec(
uaddr: *const AtomicU32,
op: FutexOperation,
flags: FutexFlags,
op: super::futex::Operation,
flags: futex::Flags,
val: u32,
timeout: *const Timespec,
uaddr2: *const AtomicU32,
Expand Down
8 changes: 4 additions & 4 deletions src/backend/linux_raw/conv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -792,15 +792,15 @@ impl<'a, Num: ArgNumber> From<(crate::net::SocketType, crate::net::SocketFlags)>
#[cfg(feature = "thread")]
impl<'a, Num: ArgNumber>
From<(
crate::backend::thread::futex::FutexOperation,
crate::thread::FutexFlags,
crate::backend::thread::futex::Operation,
crate::thread::futex::Flags,
)> for ArgReg<'a, Num>
{
#[inline]
fn from(
pair: (
crate::backend::thread::futex::FutexOperation,
crate::thread::FutexFlags,
crate::backend::thread::futex::Operation,
crate::thread::futex::Flags,
),
) -> Self {
c_uint(pair.0 as u32 | pair.1.bits())
Expand Down
49 changes: 41 additions & 8 deletions src/backend/linux_raw/thread/futex.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
bitflags::bitflags! {
/// `FUTEX_*` flags for use with [`futex`].
/// `FUTEX_*` flags for use with the functions in [`futex`].
///
/// [`futex`]: mod@crate::thread::futex
#[repr(transparent)]
#[derive(Copy, Clone, Eq, PartialEq, Hash, Debug)]
pub struct FutexFlags: u32 {
pub struct Flags: u32 {
/// `FUTEX_PRIVATE_FLAG`
const PRIVATE = linux_raw_sys::general::FUTEX_PRIVATE_FLAG;
/// `FUTEX_CLOCK_REALTIME`
Expand All @@ -16,12 +16,10 @@ bitflags::bitflags! {
}
}

/// `FUTEX_*` operations for use with [`futex`].
///
/// [`futex`]: mod@crate::thread::futex
/// `FUTEX_*` operations for use with the futex syscall wrappers.
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
#[repr(u32)]
pub enum FutexOperation {
pub(crate) enum Operation {
/// `FUTEX_WAIT`
Wait = linux_raw_sys::general::FUTEX_WAIT,
/// `FUTEX_WAKE`
Expand Down Expand Up @@ -52,8 +50,43 @@ pub enum FutexOperation {
LockPi2 = linux_raw_sys::general::FUTEX_LOCK_PI2,
}

/// `FUTEX_*` operations for use with the [`futex`] function.
///
/// [`futex`]: fn@crate::thread::futex
#[deprecated(
since = "0.38.35",
note = "
The `futex` function and `FutexOperation` enum are deprecated. There are
individual functions available to perform futex operations with improved
type safety. See the `rustix::thread::futex` module."
)]
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
#[repr(u32)]
pub enum FutexOperation {
/// `FUTEX_WAIT`
Wait = linux_raw_sys::general::FUTEX_WAIT,
/// `FUTEX_WAKE`
Wake = linux_raw_sys::general::FUTEX_WAKE,
/// `FUTEX_FD`
Fd = linux_raw_sys::general::FUTEX_FD,
/// `FUTEX_REQUEUE`
Requeue = linux_raw_sys::general::FUTEX_REQUEUE,
/// `FUTEX_CMP_REQUEUE`
CmpRequeue = linux_raw_sys::general::FUTEX_CMP_REQUEUE,
/// `FUTEX_WAKE_OP`
WakeOp = linux_raw_sys::general::FUTEX_WAKE_OP,
/// `FUTEX_LOCK_PI`
LockPi = linux_raw_sys::general::FUTEX_LOCK_PI,
/// `FUTEX_UNLOCK_PI`
UnlockPi = linux_raw_sys::general::FUTEX_UNLOCK_PI,
/// `FUTEX_TRYLOCK_PI`
TrylockPi = linux_raw_sys::general::FUTEX_TRYLOCK_PI,
/// `FUTEX_WAIT_BITSET`
WaitBitset = linux_raw_sys::general::FUTEX_WAIT_BITSET,
}

/// `FUTEX_WAITERS`
pub const FUTEX_WAITERS: u32 = linux_raw_sys::general::FUTEX_WAITERS;
pub const WAITERS: u32 = linux_raw_sys::general::FUTEX_WAITERS;

/// `FUTEX_OWNER_DIED`
pub const FUTEX_OWNER_DIED: u32 = linux_raw_sys::general::FUTEX_OWNER_DIED;
pub const OWNER_DIED: u32 = linux_raw_sys::general::FUTEX_OWNER_DIED;
18 changes: 8 additions & 10 deletions src/backend/linux_raw/thread/syscalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
//! See the `rustix::backend` module documentation for details.
#![allow(unsafe_code, clippy::undocumented_unsafe_blocks)]

use super::futex::FutexOperation;
use crate::backend::c;
use crate::backend::conv::{
by_mut, by_ref, c_int, c_uint, ret, ret_c_int, ret_c_int_infallible, ret_usize, slice,
Expand All @@ -14,7 +13,7 @@ use crate::backend::conv::{
use crate::fd::BorrowedFd;
use crate::io;
use crate::pid::Pid;
use crate::thread::{ClockId, FutexFlags, NanosleepRelativeResult, Timespec};
use crate::thread::{futex, ClockId, NanosleepRelativeResult, Timespec};
use core::mem::MaybeUninit;
use core::sync::atomic::AtomicU32;
#[cfg(target_pointer_width = "32")]
Expand Down Expand Up @@ -208,14 +207,14 @@ pub(crate) fn gettid() -> Pid {
#[inline]
pub(crate) unsafe fn futex_val2(
uaddr: *const AtomicU32,
op: FutexOperation,
flags: FutexFlags,
op: super::futex::Operation,
flags: futex::Flags,
val: u32,
val2: u32,
uaddr2: *const AtomicU32,
val3: u32,
) -> io::Result<usize> {
// the least significant four bytes of the timeout pointer are used as `val2`.
// The least-significant four bytes of the timeout pointer are used as `val2`.
// ["the kernel casts the timeout value first to unsigned long, then to uint32_t"](https://man7.org/linux/man-pages/man2/futex.2.html),
// so we perform that exact conversion in reverse to create the pointer.
let timeout = val2 as usize as *const Timespec;
Expand Down Expand Up @@ -247,8 +246,8 @@ pub(crate) unsafe fn futex_val2(
#[inline]
pub(crate) unsafe fn futex_timeout(
uaddr: *const AtomicU32,
op: FutexOperation,
flags: FutexFlags,
op: super::futex::Operation,
flags: futex::Flags,
val: u32,
timeout: *const Timespec,
uaddr2: *const AtomicU32,
Expand Down Expand Up @@ -290,8 +289,8 @@ pub(crate) unsafe fn futex_timeout(
#[cfg(target_pointer_width = "32")]
unsafe fn futex_old_timespec(
uaddr: *const AtomicU32,
op: FutexOperation,
flags: FutexFlags,
op: super::futex::Operation,
flags: futex::Flags,
val: u32,
timeout: *const Timespec,
uaddr2: *const AtomicU32,
Expand Down Expand Up @@ -321,7 +320,6 @@ unsafe fn futex_old_timespec(
c_uint(val3)
))
}

#[inline]
pub(crate) fn setns(fd: BorrowedFd<'_>, nstype: c::c_int) -> io::Result<c::c_int> {
unsafe { ret_c_int(syscall_readonly!(__NR_setns, fd, c_int(nstype))) }
Expand Down
Loading

0 comments on commit 5ab7762

Please sign in to comment.