From 26bb085b7c1e3181c8ba94a24fac6ba22a1f0c57 Mon Sep 17 00:00:00 2001 From: SteveLauC Date: Sun, 2 Feb 2025 18:14:47 +0800 Subject: [PATCH 1/6] ci: disable MIPS Linux CIs (#2594) --- .github/workflows/ci.yml | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f9a430d228..c0fe0fb348 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -89,10 +89,15 @@ jobs: armv7-unknown-linux-gnueabihf, i686-unknown-linux-gnu, i686-unknown-linux-musl, - mips-unknown-linux-gnu, - mips64-unknown-linux-gnuabi64, - mips64el-unknown-linux-gnuabi64, - mipsel-unknown-linux-gnu, + + # Disable MIPS CIs, see https://github.com/nix-rust/nix/issues/2593 + # for detailed info. + # + # mips-unknown-linux-gnu, + # mips64-unknown-linux-gnuabi64, + # mips64el-unknown-linux-gnuabi64, + # mipsel-unknown-linux-gnu, + powerpc64le-unknown-linux-gnu, loongarch64-unknown-linux-gnu, ] From 862eee359d4147da74d0cc3e79b29ba5a5fa0737 Mon Sep 17 00:00:00 2001 From: SteveLauC Date: Sun, 2 Feb 2025 18:25:49 +0800 Subject: [PATCH 2/6] refactor: clippy cleanup doc_overindented_list_items and macro_metavars_in_unsafe (#2592) --- src/sys/aio.rs | 23 +++++++++-------------- src/sys/ioctl/mod.rs | 4 +++- src/sys/socket/addr.rs | 16 +++++++--------- src/sys/socket/sockopt.rs | 38 +++++++++++++++++++------------------- 4 files changed, 38 insertions(+), 43 deletions(-) diff --git a/src/sys/aio.rs b/src/sys/aio.rs index 3045b67de8..39cd15cc75 100644 --- a/src/sys/aio.rs +++ b/src/sys/aio.rs @@ -455,10 +455,9 @@ impl<'a> AioFsync<'a> { /// * `fd`: File descriptor to sync. /// * `mode`: Whether to sync file metadata too, or just data. /// * `prio`: If POSIX Prioritized IO is supported, then the - /// operation will be prioritized at the process's - /// priority level minus `prio`. - /// * `sigev_notify`: Determines how you will be notified of event - /// completion. + /// operation will be prioritized at the process's priority level minus + /// `prio`. + /// * `sigev_notify`: Determines how you will be notified of event completion. pub fn new( fd: BorrowedFd<'a>, mode: AioFsyncMode, @@ -573,11 +572,9 @@ impl<'a> AioRead<'a> { /// * `fd`: File descriptor to read from /// * `offs`: File offset /// * `buf`: A memory buffer. It must outlive the `AioRead`. - /// * `prio`: If POSIX Prioritized IO is supported, then the - /// operation will be prioritized at the process's - /// priority level minus `prio` - /// * `sigev_notify`: Determines how you will be notified of event - /// completion. + /// * `prio`: If POSIX Prioritized IO is supported, then the operation + /// will be prioritized at the process's priority level minus `prio` + /// * `sigev_notify`: Determines how you will be notified of event completion. pub fn new( fd: BorrowedFd<'a>, offs: off_t, @@ -805,11 +802,9 @@ impl<'a> AioWrite<'a> { /// * `fd`: File descriptor to write to /// * `offs`: File offset /// * `buf`: A memory buffer. It must outlive the `AioWrite`. - /// * `prio`: If POSIX Prioritized IO is supported, then the - /// operation will be prioritized at the process's - /// priority level minus `prio` - /// * `sigev_notify`: Determines how you will be notified of event - /// completion. + /// * `prio`: If POSIX Prioritized IO is supported, then the operation + /// will be prioritized at the process's priority level minus `prio` + /// * `sigev_notify`: Determines how you will be notified of event completion. pub fn new( fd: BorrowedFd<'a>, offs: off_t, diff --git a/src/sys/ioctl/mod.rs b/src/sys/ioctl/mod.rs index 249643b9a4..aab48192f5 100644 --- a/src/sys/ioctl/mod.rs +++ b/src/sys/ioctl/mod.rs @@ -655,8 +655,10 @@ macro_rules! ioctl_readwrite { pub unsafe fn $name(fd: $crate::libc::c_int, data: *mut $ty) -> $crate::Result<$crate::libc::c_int> { + let ioty = $ioty; + let nr = $nr; unsafe { - convert_ioctl_res!($crate::libc::ioctl(fd, request_code_readwrite!($ioty, $nr, ::std::mem::size_of::<$ty>()) as $crate::sys::ioctl::ioctl_num_type, data)) + convert_ioctl_res!($crate::libc::ioctl(fd, request_code_readwrite!(ioty, nr, ::std::mem::size_of::<$ty>()) as $crate::sys::ioctl::ioctl_num_type, data)) } } ) diff --git a/src/sys/socket/addr.rs b/src/sys/socket/addr.rs index 003db4c3d3..ac3a234704 100644 --- a/src/sys/socket/addr.rs +++ b/src/sys/socket/addr.rs @@ -655,15 +655,13 @@ pub trait SockaddrLike: private::SockaddrLikePriv { /// /// # Arguments /// - /// - `addr`: raw pointer to something that can be cast to a - /// `libc::sockaddr`. For example, `libc::sockaddr_in`, - /// `libc::sockaddr_in6`, etc. - /// - `len`: For fixed-width types like `sockaddr_in`, it will be - /// validated if present and ignored if not. For variable-width - /// types it is required and must be the total length of valid - /// data. For example, if `addr` points to a - /// named `sockaddr_un`, then `len` must be the length of the - /// structure up to but not including the trailing NUL. + /// - `addr`: raw pointer to something that can be cast to a `libc::sockaddr`. + /// For example, `libc::sockaddr_in`, `libc::sockaddr_in6`, etc. + /// - `len`: For fixed-width types like `sockaddr_in`, it will be validated + /// if present and ignored if not. For variable-width types it is required + /// and must be the total length of valid data. For example, if `addr` + /// points to a named `sockaddr_un`, then `len` must be the length of the + /// structure up to but not including the trailing NUL. /// /// # Safety /// diff --git a/src/sys/socket/sockopt.rs b/src/sys/socket/sockopt.rs index 1b61e976f0..11c3be9e13 100644 --- a/src/sys/socket/sockopt.rs +++ b/src/sys/socket/sockopt.rs @@ -33,15 +33,15 @@ const TCP_CA_NAME_MAX: usize = 16; /// /// * `$name:ident`: name of the type you want to implement `SetSockOpt` for. /// * `$level:expr` : socket layer, or a `protocol level`: could be *raw sockets* -/// (`libc::SOL_SOCKET`), *ip protocol* (libc::IPPROTO_IP), *tcp protocol* (`libc::IPPROTO_TCP`), -/// and more. Please refer to your system manual for more options. Will be passed as the second -/// argument (`level`) to the `setsockopt` call. +/// (`libc::SOL_SOCKET`), *ip protocol* (libc::IPPROTO_IP), *tcp protocol* (`libc::IPPROTO_TCP`), +/// and more. Please refer to your system manual for more options. Will be passed as the second +/// argument (`level`) to the `setsockopt` call. /// * `$flag:path`: a flag name to set. Some examples: `libc::SO_REUSEADDR`, `libc::TCP_NODELAY`, -/// `libc::IP_ADD_MEMBERSHIP` and others. Will be passed as the third argument (`option_name`) -/// to the `setsockopt` call. +/// `libc::IP_ADD_MEMBERSHIP` and others. Will be passed as the third argument (`option_name`) +/// to the `setsockopt` call. /// * Type of the value that you are going to set. -/// * Type that implements the `Set` trait for the type from the previous item (like `SetBool` for -/// `bool`, `SetUsize` for `usize`, etc.). +/// * Type that implements the `Set` trait for the type from the previous item +/// (like `SetBool` for `bool`, `SetUsize` for `usize`, etc.). #[macro_export] macro_rules! setsockopt_impl { ($name:ident, $level:expr, $flag:path, $ty:ty, $setter:ty) => { @@ -87,15 +87,15 @@ macro_rules! setsockopt_impl { /// /// * Name of the type you want to implement `GetSockOpt` for. /// * Socket layer, or a `protocol level`: could be *raw sockets* (`lic::SOL_SOCKET`), *ip -/// protocol* (libc::IPPROTO_IP), *tcp protocol* (`libc::IPPROTO_TCP`), and more. Please refer -/// to your system manual for more options. Will be passed as the second argument (`level`) to -/// the `getsockopt` call. +/// protocol* (libc::IPPROTO_IP), *tcp protocol* (`libc::IPPROTO_TCP`), and more. Please refer +/// to your system manual for more options. Will be passed as the second argument (`level`) to +/// the `getsockopt` call. /// * A flag to set. Some examples: `libc::SO_REUSEADDR`, `libc::TCP_NODELAY`, -/// `libc::SO_ORIGINAL_DST` and others. Will be passed as the third argument (`option_name`) to -/// the `getsockopt` call. +/// `libc::SO_ORIGINAL_DST` and others. Will be passed as the third argument (`option_name`) to +/// the `getsockopt` call. /// * Type of the value that you are going to get. /// * Type that implements the `Get` trait for the type from the previous item (`GetBool` for -/// `bool`, `GetUsize` for `usize`, etc.). +/// `bool`, `GetUsize` for `usize`, etc.). #[macro_export] macro_rules! getsockopt_impl { ($name:ident, $level:expr, $flag:path, $ty:ty, $getter:ty) => { @@ -161,15 +161,15 @@ macro_rules! getsockopt_impl { /// # Arguments /// /// * `GetOnly`, `SetOnly` or `Both`: whether you want to implement only getter, only setter or -/// both of them. +/// both of them. /// * `$name:ident`: name of type `GetSockOpt`/`SetSockOpt` will be implemented for. /// * `$level:expr` : socket layer, or a `protocol level`: could be *raw sockets* -/// (`libc::SOL_SOCKET`), *ip protocol* (libc::IPPROTO_IP), *tcp protocol* (`libc::IPPROTO_TCP`), -/// and more. Please refer to your system manual for more options. Will be passed as the second -/// argument (`level`) to the `getsockopt`/`setsockopt` call. +/// (`libc::SOL_SOCKET`), *ip protocol* (libc::IPPROTO_IP), *tcp protocol* (`libc::IPPROTO_TCP`), +/// and more. Please refer to your system manual for more options. Will be passed as the second +/// argument (`level`) to the `getsockopt`/`setsockopt` call. /// * `$flag:path`: a flag name to set. Some examples: `libc::SO_REUSEADDR`, `libc::TCP_NODELAY`, -/// `libc::IP_ADD_MEMBERSHIP` and others. Will be passed as the third argument (`option_name`) -/// to the `setsockopt`/`getsockopt` call. +/// `libc::IP_ADD_MEMBERSHIP` and others. Will be passed as the third argument (`option_name`) +/// to the `setsockopt`/`getsockopt` call. /// * `$ty:ty`: type of the value that will be get/set. /// * `$getter:ty`: `Get` implementation; optional; only for `GetOnly` and `Both`. /// * `$setter:ty`: `Set` implementation; optional; only for `SetOnly` and `Both`. From c2a7b259764b99b97da289863473a383d200103b Mon Sep 17 00:00:00 2001 From: SteveLauC Date: Sun, 2 Feb 2025 19:21:37 +0800 Subject: [PATCH 3/6] docs: doc that signal-unsafe fns can be called after execve (#2591) * docs: doc that signal-unsafe fns can be called after execve * style: fmt --- src/pty.rs | 6 +++--- src/unistd.rs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/pty.rs b/src/pty.rs index a41f817594..ab449e4ab7 100644 --- a/src/pty.rs +++ b/src/pty.rs @@ -334,9 +334,9 @@ feature! { /// # Safety /// /// In a multithreaded program, only [async-signal-safe] functions like `pause` -/// and `_exit` may be called by the child (the parent isn't restricted). Note -/// that memory allocation may **not** be async-signal-safe and thus must be -/// prevented. +/// and `_exit` may be called by the child (the parent isn't restricted) until +/// a call of `execve(2)`. Note that memory allocation may **not** be +/// async-signal-safe and thus must be prevented. /// /// Those functions are only a small subset of your operating system's API, so /// special care must be taken to only invoke code you can control and audit. diff --git a/src/unistd.rs b/src/unistd.rs index 9c803d8c9a..4bb715b707 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -265,9 +265,9 @@ impl ForkResult { /// # Safety /// /// In a multithreaded program, only [async-signal-safe] functions like `pause` -/// and `_exit` may be called by the child (the parent isn't restricted). Note -/// that memory allocation may **not** be async-signal-safe and thus must be -/// prevented. +/// and `_exit` may be called by the child (the parent isn't restricted) until +/// a call of `execve(2)`. Note that memory allocation may **not** be +/// async-signal-safe and thus must be prevented. /// /// Those functions are only a small subset of your operating system's API, so /// special care must be taken to only invoke code you can control and audit. From 0c40b8d6e4ae7f9f55a9eec1f8b118dcd7db7ec1 Mon Sep 17 00:00:00 2001 From: SteveLauC Date: Sun, 2 Feb 2025 20:47:36 +0800 Subject: [PATCH 4/6] fix: posix_spawn returns errno on error, not -1 (#2595) * fix: posix_spawn returns errno when on error, not -1 * test: acquire the FORK_MTX lock in posix_spawn tests --- src/spawn.rs | 14 ++++++++++---- test/test_spawn.rs | 8 ++++++-- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/spawn.rs b/src/spawn.rs index eeb9b7871d..bd8858a922 100644 --- a/src/spawn.rs +++ b/src/spawn.rs @@ -370,7 +370,7 @@ pub fn posix_spawn, SE: AsRef>( ) -> Result { let mut pid = 0; - let res = unsafe { + let ret = unsafe { let args_p = to_exec_array(args); let env_p = to_exec_array(envp); @@ -384,7 +384,10 @@ pub fn posix_spawn, SE: AsRef>( ) }; - Errno::result(res)?; + if ret != 0 { + return Err(Errno::from_raw(ret)); + } + Ok(Pid::from_raw(pid)) } @@ -399,7 +402,7 @@ pub fn posix_spawnp, SE: AsRef>( ) -> Result { let mut pid = 0; - let res = unsafe { + let ret = unsafe { let args_p = to_exec_array(args); let env_p = to_exec_array(envp); @@ -413,6 +416,9 @@ pub fn posix_spawnp, SE: AsRef>( ) }; - Errno::result(res)?; + if ret != 0 { + return Err(Errno::from_raw(ret)); + } + Ok(Pid::from_raw(pid)) } diff --git a/test/test_spawn.rs b/test/test_spawn.rs index 1283c96ca8..58dfec31e6 100644 --- a/test/test_spawn.rs +++ b/test/test_spawn.rs @@ -1,11 +1,13 @@ -use std::ffi::CString; - +use super::FORK_MTX; use nix::spawn::{self, PosixSpawnAttr, PosixSpawnFileActions}; use nix::sys::signal; use nix::sys::wait::{waitpid, WaitPidFlag, WaitStatus}; +use std::ffi::CString; #[test] fn spawn_true() { + let _guard = FORK_MTX.lock(); + let bin = &CString::new("true").unwrap(); let args = &[ CString::new("true").unwrap(), @@ -32,6 +34,8 @@ fn spawn_true() { #[test] fn spawn_sleep() { + let _guard = FORK_MTX.lock(); + let bin = &CString::new("sleep").unwrap(); let args = &[CString::new("sleep").unwrap(), CString::new("30").unwrap()]; let vars: &[CString] = &[]; From 1c0f88f93d909879e9c12c50a99f05bfde472c97 Mon Sep 17 00:00:00 2001 From: SteveLauC Date: Sun, 2 Feb 2025 21:57:59 +0800 Subject: [PATCH 5/6] refactor: change posix_spawn path arg to use NixPath (#2596) * refactor: change posix_spawn path arg to use NixPath * style: fmt --- src/spawn.rs | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/spawn.rs b/src/spawn.rs index bd8858a922..34c8666bd2 100644 --- a/src/spawn.rs +++ b/src/spawn.rs @@ -361,27 +361,34 @@ unsafe fn to_exec_array>(args: &[S]) -> Vec<*mut libc::c_char> { /// Create a new child process from the specified process image. See /// [posix_spawn](https://pubs.opengroup.org/onlinepubs/9699919799/functions/posix_spawn.html). -pub fn posix_spawn, SE: AsRef>( - path: &CStr, +pub fn posix_spawn( + path: &P, file_actions: &PosixSpawnFileActions, attr: &PosixSpawnAttr, args: &[SA], envp: &[SE], -) -> Result { +) -> Result +where + P: NixPath + ?Sized, + SA: AsRef, + SE: AsRef, +{ let mut pid = 0; let ret = unsafe { let args_p = to_exec_array(args); let env_p = to_exec_array(envp); - libc::posix_spawn( - &mut pid as *mut libc::pid_t, - path.as_ptr(), - &file_actions.fa as *const libc::posix_spawn_file_actions_t, - &attr.attr as *const libc::posix_spawnattr_t, - args_p.as_ptr(), - env_p.as_ptr(), - ) + path.with_nix_path(|c_str| { + libc::posix_spawn( + &mut pid as *mut libc::pid_t, + c_str.as_ptr(), + &file_actions.fa as *const libc::posix_spawn_file_actions_t, + &attr.attr as *const libc::posix_spawnattr_t, + args_p.as_ptr(), + env_p.as_ptr(), + ) + })? }; if ret != 0 { From 299febac5aceda2f41825f8daa95ce469c0f74e3 Mon Sep 17 00:00:00 2001 From: SteveLauC Date: Tue, 4 Feb 2025 18:29:36 +0800 Subject: [PATCH 6/6] test: test posix_spawn() and add negative test cases (#2598) * test: test posix_spawn() * chore: downgrade which to 5.0.0 due to MSRV limit * chore: use a homemade which * refactor: clippy fix * test: test posix_spawn() * refactor: CStr literal syntax is not supported * test: skip testing error cases under QEMU --- test/test_spawn.rs | 128 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 126 insertions(+), 2 deletions(-) diff --git a/test/test_spawn.rs b/test/test_spawn.rs index 58dfec31e6..a5e69b97f3 100644 --- a/test/test_spawn.rs +++ b/test/test_spawn.rs @@ -1,13 +1,115 @@ use super::FORK_MTX; +use nix::errno::Errno; use nix::spawn::{self, PosixSpawnAttr, PosixSpawnFileActions}; use nix::sys::signal; use nix::sys::wait::{waitpid, WaitPidFlag, WaitStatus}; -use std::ffi::CString; +use std::ffi::{CStr, CString}; + +/// Helper function to find a binary in the $PATH +fn which(exe_name: &str) -> Option { + std::env::var_os("PATH").and_then(|paths| { + std::env::split_paths(&paths) + .filter_map(|dir| { + let full_path = dir.join(exe_name); + if full_path.is_file() { + Some(full_path) + } else { + None + } + }) + .next() + }) +} #[test] fn spawn_true() { let _guard = FORK_MTX.lock(); + let bin = which("true").unwrap(); + let args = &[ + CString::new("true").unwrap(), + CString::new("story").unwrap(), + ]; + let vars: &[CString] = &[]; + let actions = PosixSpawnFileActions::init().unwrap(); + let attr = PosixSpawnAttr::init().unwrap(); + + let pid = + spawn::posix_spawn(bin.as_path(), &actions, &attr, args, vars).unwrap(); + + let status = waitpid(pid, Some(WaitPidFlag::empty())).unwrap(); + + match status { + WaitStatus::Exited(wpid, ret) => { + assert_eq!(pid, wpid); + assert_eq!(ret, 0); + } + _ => { + panic!("Invalid WaitStatus"); + } + }; +} + +#[test] +fn spawn_sleep() { + let _guard = FORK_MTX.lock(); + + let bin = which("sleep").unwrap(); + let args = &[CString::new("sleep").unwrap(), CString::new("30").unwrap()]; + let vars: &[CString] = &[]; + let actions = PosixSpawnFileActions::init().unwrap(); + let attr = PosixSpawnAttr::init().unwrap(); + + let pid = + spawn::posix_spawn(bin.as_path(), &actions, &attr, args, vars).unwrap(); + + let status = + waitpid(pid, WaitPidFlag::from_bits(WaitPidFlag::WNOHANG.bits())) + .unwrap(); + match status { + WaitStatus::StillAlive => {} + _ => { + panic!("Invalid WaitStatus"); + } + }; + + signal::kill(pid, signal::SIGTERM).unwrap(); + + let status = waitpid(pid, Some(WaitPidFlag::empty())).unwrap(); + match status { + WaitStatus::Signaled(wpid, wsignal, _) => { + assert_eq!(pid, wpid); + assert_eq!(wsignal, signal::SIGTERM); + } + _ => { + panic!("Invalid WaitStatus"); + } + }; +} + +#[test] +// `posix_spawn(path_not_exist)` succeeds under QEMU, so ignore the test. No need +// to investigate the root cause, this test still works in native environments, which +// is sufficient to test the binding. +#[cfg_attr(qemu, ignore)] +fn spawn_cmd_does_not_exist() { + let _guard = FORK_MTX.lock(); + + let args = &[CString::new("buzz").unwrap()]; + let envs: &[CString] = &[]; + let actions = PosixSpawnFileActions::init().unwrap(); + let attr = PosixSpawnAttr::init().unwrap(); + + let bin = "2b7433c4-523b-470c-abb5-d7ee9fd295d5-fdasf"; + let errno = + spawn::posix_spawn(bin, &actions, &attr, args, envs).unwrap_err(); + assert_eq!(errno, Errno::ENOENT); +} + +#[test] +fn spawnp_true() { + let _guard = FORK_MTX.lock(); + let bin = &CString::new("true").unwrap(); let args = &[ CString::new("true").unwrap(), @@ -33,7 +135,7 @@ fn spawn_true() { } #[test] -fn spawn_sleep() { +fn spawnp_sleep() { let _guard = FORK_MTX.lock(); let bin = &CString::new("sleep").unwrap(); @@ -67,3 +169,25 @@ fn spawn_sleep() { } }; } + +#[test] +// `posix_spawnp(bin_not_exist)` succeeds under QEMU, so ignore the test. No need +// to investigate the root cause, this test still works in native environments, which +// is sufficient to test the binding. +#[cfg_attr(qemu, ignore)] +fn spawnp_cmd_does_not_exist() { + let _guard = FORK_MTX.lock(); + + let args = &[CString::new("buzz").unwrap()]; + let envs: &[CString] = &[]; + let actions = PosixSpawnFileActions::init().unwrap(); + let attr = PosixSpawnAttr::init().unwrap(); + + let bin = CStr::from_bytes_with_nul( + "2b7433c4-523b-470c-abb5-d7ee9fd295d5-fdasf\0".as_bytes(), + ) + .unwrap(); + let errno = + spawn::posix_spawnp(bin, &actions, &attr, args, envs).unwrap_err(); + assert_eq!(errno, Errno::ENOENT); +}