From dbe0de4200855a123a8db664e96187d99cf3a267 Mon Sep 17 00:00:00 2001 From: Steve Lau Date: Sat, 8 Jun 2024 21:11:14 +0800 Subject: [PATCH 01/10] refactor: I/O safety for module fcntl --- src/dir.rs | 48 ++++++--- src/fcntl.rs | 207 ++++++++++++++++++++++++++++----------- test/sys/test_socket.rs | 8 +- test/sys/test_termios.rs | 4 +- test/test_dir.rs | 6 -- test/test_fcntl.rs | 108 +++++++++----------- test/test_pty.rs | 2 +- test/test_stat.rs | 36 +++++-- test/test_unistd.rs | 62 ++++++++---- 9 files changed, 310 insertions(+), 171 deletions(-) diff --git a/src/dir.rs b/src/dir.rs index 20c5593702..f04a77cba8 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -3,7 +3,7 @@ use crate::errno::Errno; use crate::fcntl::{self, OFlag}; use crate::sys; -use crate::{Error, NixPath, Result}; +use crate::{NixPath, Result}; use cfg_if::cfg_if; use std::ffi; use std::os::unix::io::{AsRawFd, IntoRawFd, RawFd}; @@ -43,8 +43,8 @@ impl Dir { } /// Opens the given path as with `fcntl::openat`. - pub fn openat( - dirfd: Option, + pub fn openat( + dirfd: Fd, path: &P, oflag: OFlag, mode: sys::stat::Mode, @@ -54,21 +54,29 @@ impl Dir { } /// Converts from a descriptor-based object, closing the descriptor on success or failure. + /// + /// # Safety + /// + /// It is only safe if `fd` is an owned file descriptor. #[inline] - pub fn from(fd: F) -> Result { - Dir::from_fd(fd.into_raw_fd()) + #[deprecated(since = "0.30.0", note = "Deprecate this since it is not I/O-safe, use from_fd instead.")] + pub unsafe fn from(fd: F) -> Result { + use std::os::fd::FromRawFd; + use std::os::fd::OwnedFd; + + // SAFETY: + // + // This is indeed unsafe is `fd` it not an owned fd. + let owned_fd = unsafe { OwnedFd::from_raw_fd(fd.into_raw_fd()) }; + Dir::from_fd(owned_fd) } /// Converts from a file descriptor, closing it on failure. #[doc(alias("fdopendir"))] - pub fn from_fd(fd: RawFd) -> Result { - let d = ptr::NonNull::new(unsafe { libc::fdopendir(fd) }).ok_or_else( - || { - let e = Error::last(); - unsafe { libc::close(fd) }; - e - }, - )?; + pub fn from_fd(fd: std::os::fd::OwnedFd) -> Result { + use std::os::fd::AsRawFd; + + let d = ptr::NonNull::new(unsafe { libc::fdopendir(fd.as_raw_fd()) }).ok_or(Errno::last())?; Ok(Dir(d)) } @@ -86,6 +94,20 @@ impl Dir { // `Dir` is safe to pass from one thread to another, as it's not reference-counted. unsafe impl Send for Dir {} +impl std::os::fd::AsFd for Dir { + fn as_fd(&self) -> std::os::fd::BorrowedFd { + let raw_fd = self.as_raw_fd(); + + // SAFETY: + // + // `raw_fd` should be open and valid for the lifetime of the returned + // `BorrowedFd` as it is extracted from `&self`. + unsafe { + std::os::fd::BorrowedFd::borrow_raw(raw_fd) + } + } +} + impl AsRawFd for Dir { fn as_raw_fd(&self) -> RawFd { unsafe { libc::dirfd(self.0.as_ptr()) } diff --git a/src/fcntl.rs b/src/fcntl.rs index cf87926c7b..9e717570e4 100644 --- a/src/fcntl.rs +++ b/src/fcntl.rs @@ -18,7 +18,7 @@ use std::os::raw; use std::os::unix::ffi::OsStringExt; use std::os::unix::io::RawFd; #[cfg(not(any(target_os = "redox", target_os = "solaris")))] -use std::os::unix::io::{AsRawFd, OwnedFd}; +use std::os::unix::io::OwnedFd; #[cfg(any( target_os = "netbsd", apple_targets, @@ -27,7 +27,11 @@ use std::os::unix::io::{AsRawFd, OwnedFd}; ))] use std::path::PathBuf; #[cfg(any(linux_android, target_os = "freebsd"))] -use std::{os::unix::io::AsFd, ptr}; +use std::ptr; +use std::os::fd::BorrowedFd; + +#[cfg(feature = "fs")] +use std::os::fd::FromRawFd; #[cfg(feature = "fs")] use crate::{sys::stat::Mode, NixPath, Result}; @@ -43,6 +47,14 @@ use crate::{sys::stat::Mode, NixPath, Result}; #[cfg(feature = "fs")] pub use self::posix_fadvise::{posix_fadvise, PosixFadviseAdvice}; +/// A file descriptor referring to the working directory of the current process. +// +// SAFETY: +// 1. `AT_FDCWD` is usable for the whole process life, so it is `'static`. +// 2. It is not a valid file descriptor, but OS will handle it for us when passed +// to `xxat(2)` calls. +pub const AT_FDCWD: BorrowedFd<'static> = unsafe { BorrowedFd::borrow_raw(libc::AT_FDCWD) }; + #[cfg(not(target_os = "redox"))] #[cfg(any(feature = "fs", feature = "process", feature = "user"))] libc_bitflags! { @@ -221,12 +233,16 @@ pub fn open( path: &P, oflag: OFlag, mode: Mode, -) -> Result { +) -> Result { let fd = path.with_nix_path(|cstr| unsafe { libc::open(cstr.as_ptr(), oflag.bits(), mode.bits() as c_uint) })?; + Errno::result(fd)?; - Errno::result(fd) + // SAFETY: + // + // `open(2)` should return a valid owned fd on success + Ok( unsafe { OwnedFd::from_raw_fd(fd) } ) } /// open or create a file for reading, writing or executing @@ -240,16 +256,23 @@ pub fn open( // The conversion is not identical on all operating systems. #[allow(clippy::useless_conversion)] #[cfg(not(target_os = "redox"))] -pub fn openat( - dirfd: Option, +pub fn openat( + dirfd: Fd, path: &P, oflag: OFlag, mode: Mode, -) -> Result { +) -> Result { + use std::os::fd::AsRawFd; + let fd = path.with_nix_path(|cstr| unsafe { - libc::openat(at_rawfd(dirfd), cstr.as_ptr(), oflag.bits(), mode.bits() as c_uint) + libc::openat(dirfd.as_fd().as_raw_fd(), cstr.as_ptr(), oflag.bits(), mode.bits() as c_uint) })?; - Errno::result(fd) + Errno::result(fd)?; + + // SAFETY: + // + // `openat(2)` should return a valid owned fd on success + Ok( unsafe { OwnedFd::from_raw_fd(fd) } ) } cfg_if::cfg_if! { @@ -345,22 +368,29 @@ cfg_if::cfg_if! { /// # See also /// /// [openat2](https://man7.org/linux/man-pages/man2/openat2.2.html) - pub fn openat2( - dirfd: RawFd, + pub fn openat2( + dirfd: Fd, path: &P, mut how: OpenHow, - ) -> Result { + ) -> Result { + use std::os::fd::AsRawFd; + use std::os::fd::FromRawFd; + let fd = path.with_nix_path(|cstr| unsafe { libc::syscall( libc::SYS_openat2, - dirfd, + dirfd.as_fd().as_raw_fd(), cstr.as_ptr(), &mut how as *mut OpenHow, std::mem::size_of::(), ) - })?; + })? as RawFd; + Errno::result(fd)?; - Errno::result(fd as RawFd) + // SAFETY: + // + // `openat2(2)` should return a valid owned fd on success + Ok( unsafe { OwnedFd::from_raw_fd(fd) } ) } } } @@ -374,18 +404,20 @@ cfg_if::cfg_if! { /// # See Also /// [`renameat`](https://pubs.opengroup.org/onlinepubs/9699919799/functions/rename.html) #[cfg(not(target_os = "redox"))] -pub fn renameat( - old_dirfd: Option, +pub fn renameat( + old_dirfd: Fd1, old_path: &P1, - new_dirfd: Option, + new_dirfd: Fd2, new_path: &P2, ) -> Result<()> { + use std::os::fd::AsRawFd; + let res = old_path.with_nix_path(|old_cstr| { new_path.with_nix_path(|new_cstr| unsafe { libc::renameat( - at_rawfd(old_dirfd), + old_dirfd.as_fd().as_raw_fd(), old_cstr.as_ptr(), - at_rawfd(new_dirfd), + new_dirfd.as_fd().as_raw_fd(), new_cstr.as_ptr(), ) }) @@ -422,19 +454,21 @@ feature! { /// # See Also /// * [`rename`](https://man7.org/linux/man-pages/man2/rename.2.html) #[cfg(all(target_os = "linux", target_env = "gnu"))] -pub fn renameat2( - old_dirfd: Option, +pub fn renameat2( + old_dirfd: Fd1, old_path: &P1, - new_dirfd: Option, + new_dirfd: Fd2, new_path: &P2, flags: RenameFlags, ) -> Result<()> { + use std::os::fd::AsRawFd; + let res = old_path.with_nix_path(|old_cstr| { new_path.with_nix_path(|new_cstr| unsafe { libc::renameat2( - at_rawfd(old_dirfd), + old_dirfd.as_fd().as_raw_fd(), old_cstr.as_ptr(), - at_rawfd(new_dirfd), + new_dirfd.as_fd().as_raw_fd(), new_cstr.as_ptr(), flags.bits(), ) @@ -449,7 +483,16 @@ fn wrap_readlink_result(mut v: Vec, len: ssize_t) -> Result { Ok(OsString::from_vec(v.to_vec())) } -fn readlink_maybe_at( +/// Read the symlink specified by `path` and `dirfd` and put the contents in `v`. +/// Return the number of bytes placed in `v`. +/// +/// This function can call `readlink(2)` or `readlinkat(2)` depending on if `dirfd` +/// is some, if it is, then `readlinkat(2)` is called, otherwise, call `readlink(2)`. +/// +/// # Safety +/// +/// This function is not I/O-safe considering it employs the `RawFd` type. +unsafe fn readlink_maybe_at( dirfd: Option, path: &P, v: &mut Vec, @@ -457,7 +500,7 @@ fn readlink_maybe_at( path.with_nix_path(|cstr| unsafe { match dirfd { #[cfg(target_os = "redox")] - Some(_) => unreachable!(), + Some(_) => unreachable!("redox does not have readlinkat(2)"), #[cfg(not(target_os = "redox"))] Some(dirfd) => libc::readlinkat( dirfd, @@ -474,7 +517,15 @@ fn readlink_maybe_at( }) } -fn inner_readlink( +/// The actual implementation of [`readlink(2)`] or [`readlinkat(2)`]. +/// +/// This function can call `readlink(2)` or `readlinkat(2)` depending on if `dirfd` +/// is some, if it is, then `readlinkat(2)` is called, otherwise, call `readlink(2)`. +/// +/// # Safety +/// +/// This function is marked unsafe because it uses `RawFd`. +unsafe fn inner_readlink( dirfd: Option, path: &P, ) -> Result { @@ -486,7 +537,12 @@ fn inner_readlink( { // simple case: result is strictly less than `PATH_MAX` - let res = readlink_maybe_at(dirfd, path, &mut v)?; + + // SAFETY: + // + // If this call of `readlink_maybe_at()` is safe or not depends on the + // usage of `unsafe fn inner_readlink()`. + let res = unsafe { readlink_maybe_at(dirfd, path, &mut v)? }; let len = Errno::result(res)?; debug_assert!(len >= 0); if (len as usize) < v.capacity() { @@ -499,7 +555,7 @@ fn inner_readlink( let mut try_size = { let reported_size = match dirfd { #[cfg(target_os = "redox")] - Some(_) => unreachable!(), + Some(_) => unreachable!("redox does not have readlinkat(2)"), #[cfg(any(linux_android, target_os = "freebsd", target_os = "hurd"))] Some(dirfd) => { let flags = if path.is_empty() { @@ -543,7 +599,11 @@ fn inner_readlink( loop { { v.reserve_exact(try_size); - let res = readlink_maybe_at(dirfd, path, &mut v)?; + // SAFETY: + // + // If this call of `readlink_maybe_at()` is safe or not depends on the + // usage of `unsafe fn inner_readlink()`. + let res = unsafe { readlink_maybe_at(dirfd, path, &mut v)? }; let len = Errno::result(res)?; debug_assert!(len >= 0); if (len as usize) < v.capacity() { @@ -566,7 +626,16 @@ fn inner_readlink( /// # See Also /// * [`readlink`](https://pubs.opengroup.org/onlinepubs/9699919799/functions/readlink.html) pub fn readlink(path: &P) -> Result { - inner_readlink(None, path) + // argument `dirfd` should be `None` since we call it from `readlink()` + // + // Do NOT call it with `Some(AT_CWD)` as in that way, we are emulating + // `readlink(2)` with `readlinkat(2)`, which will make us lose `readlink(2)` + // on Redox. + // + // SAFETY: + // + // It is definitely safe because the argument involving `RawFd` is `None` + unsafe { inner_readlink(None, path) } } /// Read value of a symbolic link. @@ -577,12 +646,18 @@ pub fn readlink(path: &P) -> Result { /// # See Also /// * [`readlink`](https://pubs.opengroup.org/onlinepubs/9699919799/functions/readlink.html) #[cfg(not(target_os = "redox"))] -pub fn readlinkat( - dirfd: Option, +pub fn readlinkat( + dirfd: Fd, path: &P, ) -> Result { - let dirfd = at_rawfd(dirfd); - inner_readlink(Some(dirfd), path) + use std::os::fd::AsRawFd; + + // argument `dirfd` should be `Some` since we call it from `readlinkat()` + // + // SAFETY: + // + // The passed `RawFd` should be valid since it is borrowed from `Fd: AsFd`. + unsafe { inner_readlink(Some(dirfd.as_fd().as_raw_fd()), path) } } } @@ -720,7 +795,10 @@ pub use self::FcntlArg::*; /// # See Also /// * [`fcntl`](https://pubs.opengroup.org/onlinepubs/9699919799/functions/fcntl.html) // TODO: Figure out how to handle value fcntl returns -pub fn fcntl(fd: RawFd, arg: FcntlArg) -> Result { +pub fn fcntl(fd: Fd, arg: FcntlArg) -> Result { + use std::os::fd::AsRawFd; + + let fd = fd.as_fd().as_raw_fd(); let res = unsafe { match arg { F_DUPFD(rawfd) => libc::fcntl(fd, libc::F_DUPFD, rawfd), @@ -853,7 +931,7 @@ pub fn flock(fd: RawFd, arg: FlockArg) -> Result<()> { /// # Safety /// Types implementing this must not be `Clone`. #[cfg(not(any(target_os = "redox", target_os = "solaris")))] -pub unsafe trait Flockable: AsRawFd {} +pub unsafe trait Flockable: std::os::fd::AsRawFd {} /// Represents an owned flock, which unlocks on drop. /// @@ -1037,13 +1115,15 @@ feature! { // define it as "loff_t". But on both OSes, on all supported platforms, those // are 64 bits. So Nix uses i64 to make the docs simple and consistent. #[cfg(any(linux_android, target_os = "freebsd"))] -pub fn copy_file_range( +pub fn copy_file_range( fd_in: Fd1, off_in: Option<&mut i64>, fd_out: Fd2, off_out: Option<&mut i64>, len: usize, ) -> Result { + use std::os::fd::AsRawFd; + let off_in = off_in .map(|offset| offset as *mut i64) .unwrap_or(ptr::null_mut()); @@ -1087,7 +1167,7 @@ pub fn copy_file_range( /// # See Also /// *[`splice`](https://man7.org/linux/man-pages/man2/splice.2.html) #[cfg(linux_android)] -pub fn splice( +pub fn splice( fd_in: Fd1, off_in: Option<&mut libc::loff_t>, fd_out: Fd2, @@ -1095,6 +1175,8 @@ pub fn splice( len: usize, flags: SpliceFFlags, ) -> Result { + use std::os::fd::AsRawFd; + let off_in = off_in .map(|offset| offset as *mut libc::loff_t) .unwrap_or(ptr::null_mut()); @@ -1113,12 +1195,14 @@ pub fn splice( /// # See Also /// *[`tee`](https://man7.org/linux/man-pages/man2/tee.2.html) #[cfg(linux_android)] -pub fn tee( +pub fn tee( fd_in: Fd1, fd_out: Fd2, len: usize, flags: SpliceFFlags, ) -> Result { + use std::os::fd::AsRawFd; + let ret = unsafe { libc::tee(fd_in.as_fd().as_raw_fd(), fd_out.as_fd().as_raw_fd(), len, flags.bits()) }; Errno::result(ret).map(|r| r as usize) } @@ -1128,11 +1212,13 @@ pub fn tee( /// # See Also /// *[`vmsplice`](https://man7.org/linux/man-pages/man2/vmsplice.2.html) #[cfg(linux_android)] -pub fn vmsplice( +pub fn vmsplice( fd: F, iov: &[std::io::IoSlice<'_>], flags: SpliceFFlags, ) -> Result { + use std::os::fd::AsRawFd; + let ret = unsafe { libc::vmsplice( fd.as_fd().as_raw_fd(), @@ -1187,13 +1273,15 @@ feature! { /// file referred to by fd. #[cfg(target_os = "linux")] #[cfg(feature = "fs")] -pub fn fallocate( - fd: RawFd, +pub fn fallocate( + fd: Fd, mode: FallocateFlags, offset: libc::off_t, len: libc::off_t, ) -> Result<()> { - let res = unsafe { libc::fallocate(fd, mode.bits(), offset, len) }; + use std::os::fd::AsRawFd; + + let res = unsafe { libc::fallocate(fd.as_fd().as_raw_fd(), mode.bits(), offset, len) }; Errno::result(res).map(drop) } @@ -1268,14 +1356,14 @@ impl SpacectlRange { /// ``` #[cfg(target_os = "freebsd")] #[inline] // Delays codegen, preventing linker errors with dylibs and --no-allow-shlib-undefined -pub fn fspacectl(fd: RawFd, range: SpacectlRange) -> Result { +pub fn fspacectl(fd: Fd, range: SpacectlRange) -> Result { let mut rqsr = libc::spacectl_range { r_offset: range.0, r_len: range.1, }; let res = unsafe { libc::fspacectl( - fd, + fd.as_fd().as_raw_fd(), libc::SPACECTL_DEALLOC, // Only one command is supported ATM &rqsr, 0, // No flags are currently supported @@ -1317,11 +1405,13 @@ pub fn fspacectl(fd: RawFd, range: SpacectlRange) -> Result { /// ``` #[cfg(target_os = "freebsd")] #[inline] // Delays codegen, preventing linker errors with dylibs and --no-allow-shlib-undefined -pub fn fspacectl_all( - fd: RawFd, +pub fn fspacectl_all( + fd: Fd, offset: libc::off_t, len: libc::off_t, ) -> Result<()> { + use std::os::fd::AsRawFd; + let mut rqsr = libc::spacectl_range { r_offset: offset, r_len: len, @@ -1329,7 +1419,7 @@ pub fn fspacectl_all( while rqsr.r_len > 0 { let res = unsafe { libc::fspacectl( - fd, + fd.as_fd().as_raw_fd(), libc::SPACECTL_DEALLOC, // Only one command is supported ATM &rqsr, 0, // No flags are currently supported @@ -1352,7 +1442,6 @@ pub fn fspacectl_all( mod posix_fadvise { use crate::errno::Errno; use crate::Result; - use std::os::unix::io::RawFd; #[cfg(feature = "fs")] libc_enum! { @@ -1384,13 +1473,15 @@ mod posix_fadvise { /// /// # See Also /// * [`posix_fadvise`](https://pubs.opengroup.org/onlinepubs/9699919799/functions/posix_fadvise.html) - pub fn posix_fadvise( - fd: RawFd, + pub fn posix_fadvise( + fd: Fd, offset: libc::off_t, len: libc::off_t, advice: PosixFadviseAdvice, ) -> Result<()> { - let res = unsafe { libc::posix_fadvise(fd, offset, len, advice as libc::c_int) }; + use std::os::fd::AsRawFd; + + let res = unsafe { libc::posix_fadvise(fd.as_fd().as_raw_fd(), offset, len, advice as libc::c_int) }; if res == 0 { Ok(()) @@ -1412,12 +1503,14 @@ mod posix_fadvise { target_os = "fuchsia", target_os = "wasi", ))] -pub fn posix_fallocate( - fd: RawFd, +pub fn posix_fallocate( + fd: Fd, offset: libc::off_t, len: libc::off_t, ) -> Result<()> { - let res = unsafe { libc::posix_fallocate(fd, offset, len) }; + use std::os::fd::AsRawFd; + + let res = unsafe { libc::posix_fallocate(fd.as_fd().as_raw_fd(), offset, len) }; match Errno::result(res) { Err(err) => Err(err), Ok(0) => Ok(()), diff --git a/test/sys/test_socket.rs b/test/sys/test_socket.rs index 79c97c8720..b1a8646fa0 100644 --- a/test/sys/test_socket.rs +++ b/test/sys/test_socket.rs @@ -1122,7 +1122,13 @@ pub fn test_af_alg_aead() { // authentication tag memory is only needed in the output buffer for encryption // and in the input buffer for decryption. // Do not block on read, as we may have fewer bytes than buffer size - fcntl(session_socket, FcntlArg::F_SETFL(OFlag::O_NONBLOCK)) + + // SAFETY: + // + // `session_socket` will be valid for the lifetime of this test + // TODO: remove this workaround when accept(2) becomes I/O-safe. + let borrowed_fd = unsafe { std::os::fd::BorrowedFd::borrow_raw(session_socket) }; + fcntl(borrowed_fd, FcntlArg::F_SETFL(OFlag::O_NONBLOCK)) .expect("fcntl non_blocking"); let num_bytes = read(session_socket.as_raw_fd(), &mut decrypted).expect("read decrypt"); diff --git a/test/sys/test_termios.rs b/test/sys/test_termios.rs index 35cc7ab739..cbcb1b0f72 100644 --- a/test/sys/test_termios.rs +++ b/test/sys/test_termios.rs @@ -100,10 +100,10 @@ fn test_local_flags() { let pty = openpty(None, &termios).unwrap(); // Set the master is in nonblocking mode or reading will never return. - let flags = fcntl::fcntl(pty.master.as_raw_fd(), fcntl::F_GETFL).unwrap(); + let flags = fcntl::fcntl(pty.master.as_fd(), fcntl::F_GETFL).unwrap(); let new_flags = fcntl::OFlag::from_bits_truncate(flags) | fcntl::OFlag::O_NONBLOCK; - fcntl::fcntl(pty.master.as_raw_fd(), fcntl::F_SETFL(new_flags)).unwrap(); + fcntl::fcntl(pty.master.as_fd(), fcntl::F_SETFL(new_flags)).unwrap(); // Write into the master let string = "foofoofoo\r"; diff --git a/test/test_dir.rs b/test/test_dir.rs index 4445508f01..c9713c6455 100644 --- a/test/test_dir.rs +++ b/test/test_dir.rs @@ -56,9 +56,3 @@ fn rewind() { assert_eq!(entries1, entries2); assert_eq!(entries2, entries3); } - -#[cfg(not(target_os = "haiku"))] -#[test] -fn ebadf() { - assert_eq!(Dir::from_fd(-1).unwrap_err(), nix::Error::EBADF); -} diff --git a/test/test_fcntl.rs b/test/test_fcntl.rs index 5d320769d3..31cce0a9d6 100644 --- a/test/test_fcntl.rs +++ b/test/test_fcntl.rs @@ -21,7 +21,7 @@ use nix::fcntl::{renameat2, RenameFlags}; #[cfg(not(target_os = "redox"))] use nix::sys::stat::Mode; #[cfg(not(target_os = "redox"))] -use nix::unistd::{close, read}; +use nix::unistd::read; #[cfg(not(target_os = "redox"))] use std::fs::File; #[cfg(not(target_os = "redox"))] @@ -37,6 +37,8 @@ use tempfile::NamedTempFile; // https://gitlab.com/qemu-project/qemu/-/issues/829 #[cfg_attr(qemu, ignore)] fn test_openat() { + use std::os::fd::AsRawFd; + const CONTENTS: &[u8] = b"abcd"; let mut tmp = NamedTempFile::new().unwrap(); tmp.write_all(CONTENTS).unwrap(); @@ -45,7 +47,7 @@ fn test_openat() { open(tmp.path().parent().unwrap(), OFlag::empty(), Mode::empty()) .unwrap(); let fd = openat( - Some(dirfd), + dirfd, tmp.path().file_name().unwrap(), OFlag::O_RDONLY, Mode::empty(), @@ -53,11 +55,8 @@ fn test_openat() { .unwrap(); let mut buf = [0u8; 1024]; - assert_eq!(4, read(fd, &mut buf).unwrap()); + assert_eq!(4, read(fd.as_raw_fd(), &mut buf).unwrap()); assert_eq!(CONTENTS, &buf[0..4]); - - close(fd).unwrap(); - close(dirfd).unwrap(); } #[test] @@ -66,6 +65,8 @@ fn test_openat() { // https://gitlab.com/qemu-project/qemu/-/issues/829 #[cfg_attr(qemu, ignore)] fn test_openat2() { + use std::os::fd::AsRawFd; + const CONTENTS: &[u8] = b"abcd"; let mut tmp = NamedTempFile::new().unwrap(); tmp.write_all(CONTENTS).unwrap(); @@ -85,11 +86,8 @@ fn test_openat2() { .unwrap(); let mut buf = [0u8; 1024]; - assert_eq!(4, read(fd, &mut buf).unwrap()); + assert_eq!(4, read(fd.as_raw_fd(), &mut buf).unwrap()); assert_eq!(CONTENTS, &buf[0..4]); - - close(fd).unwrap(); - close(dirfd).unwrap(); } #[test] @@ -115,7 +113,7 @@ fn test_openat2_forbidden() { .flags(OFlag::O_RDONLY) .resolve(ResolveFlag::RESOLVE_BENEATH), ); - assert_eq!(Err(Errno::EXDEV), res); + assert_eq!(res.unwrap_err(), Errno::EXDEV); } #[test] @@ -129,13 +127,11 @@ fn test_renameat() { let new_dir = tempfile::tempdir().unwrap(); let new_dirfd = open(new_dir.path(), OFlag::empty(), Mode::empty()).unwrap(); - renameat(Some(old_dirfd), "old", Some(new_dirfd), "new").unwrap(); + renameat(&old_dirfd, "old", &new_dirfd, "new").unwrap(); assert_eq!( - renameat(Some(old_dirfd), "old", Some(new_dirfd), "new").unwrap_err(), + renameat(&old_dirfd, "old", &new_dirfd, "new").unwrap_err(), Errno::ENOENT ); - close(old_dirfd).unwrap(); - close(new_dirfd).unwrap(); assert!(new_dir.path().join("new").exists()); } @@ -159,26 +155,24 @@ fn test_renameat2_behaves_like_renameat_with_no_flags() { let new_dirfd = open(new_dir.path(), OFlag::empty(), Mode::empty()).unwrap(); renameat2( - Some(old_dirfd), + &old_dirfd, "old", - Some(new_dirfd), + &new_dirfd, "new", RenameFlags::empty(), ) .unwrap(); assert_eq!( renameat2( - Some(old_dirfd), + &old_dirfd, "old", - Some(new_dirfd), + &new_dirfd, "new", RenameFlags::empty() ) .unwrap_err(), Errno::ENOENT ); - close(old_dirfd).unwrap(); - close(new_dirfd).unwrap(); assert!(new_dir.path().join("new").exists()); } @@ -210,9 +204,9 @@ fn test_renameat2_exchange() { new_f.write_all(b"new").unwrap(); } renameat2( - Some(old_dirfd), + &old_dirfd, "old", - Some(new_dirfd), + &new_dirfd, "new", RenameFlags::RENAME_EXCHANGE, ) @@ -225,8 +219,6 @@ fn test_renameat2_exchange() { let mut old_f = File::open(&old_path).unwrap(); old_f.read_to_string(&mut buf).unwrap(); assert_eq!(buf, "new"); - close(old_dirfd).unwrap(); - close(new_dirfd).unwrap(); } #[test] @@ -252,17 +244,15 @@ fn test_renameat2_noreplace() { File::create(new_path).unwrap(); assert_eq!( renameat2( - Some(old_dirfd), + &old_dirfd, "old", - Some(new_dirfd), + &new_dirfd, "new", RenameFlags::RENAME_NOREPLACE ) .unwrap_err(), Errno::EEXIST ); - close(old_dirfd).unwrap(); - close(new_dirfd).unwrap(); assert!(new_dir.path().join("new").exists()); assert!(old_dir.path().join("old").exists()); } @@ -280,7 +270,7 @@ fn test_readlink() { assert_eq!(readlink(&dst).unwrap().to_str().unwrap(), expected_dir); assert_eq!( - readlinkat(Some(dirfd), "b").unwrap().to_str().unwrap(), + readlinkat(dirfd, "b").unwrap().to_str().unwrap(), expected_dir ); } @@ -301,7 +291,6 @@ fn test_readlink() { #[cfg_attr(qemu, ignore)] fn test_copy_file_range() { use nix::fcntl::copy_file_range; - use std::os::unix::io::AsFd; const CONTENTS: &[u8] = b"foobarbaz"; @@ -313,9 +302,9 @@ fn test_copy_file_range() { let mut from_offset: i64 = 3; copy_file_range( - tmp1.as_fd(), + &tmp1, Some(&mut from_offset), - tmp2.as_fd(), + &tmp2, None, 3, ) @@ -334,7 +323,6 @@ mod linux_android { use libc::loff_t; use std::io::prelude::*; use std::io::IoSlice; - use std::os::unix::prelude::*; use nix::fcntl::*; use nix::unistd::{pipe, read, write}; @@ -347,6 +335,8 @@ mod linux_android { #[test] fn test_splice() { + use std::os::fd::AsRawFd; + const CONTENTS: &[u8] = b"abcdef123456"; let mut tmp = tempfile().unwrap(); tmp.write_all(CONTENTS).unwrap(); @@ -367,6 +357,8 @@ mod linux_android { #[test] fn test_tee() { + use std::os::fd::AsRawFd; + let (rd1, wr1) = pipe().unwrap(); let (rd2, wr2) = pipe().unwrap(); @@ -389,6 +381,8 @@ mod linux_android { #[test] fn test_vmsplice() { + use std::os::fd::AsRawFd; + let (rd, wr) = pipe().unwrap(); let buf1 = b"abcdef"; @@ -408,14 +402,15 @@ mod linux_android { #[cfg(target_os = "linux")] #[test] fn test_fallocate() { + use std::os::fd::AsRawFd; + let tmp = NamedTempFile::new().unwrap(); - let fd = tmp.as_raw_fd(); - fallocate(fd, FallocateFlags::empty(), 0, 100).unwrap(); + fallocate(&tmp, FallocateFlags::empty(), 0, 100).unwrap(); // Check if we read exactly 100 bytes let mut buf = [0u8; 200]; - assert_eq!(100, read(fd, &mut buf).unwrap()); + assert_eq!(100, read(tmp.as_raw_fd(), &mut buf).unwrap()); } // The tests below are disabled for the listed targets @@ -429,10 +424,10 @@ mod linux_android { fn test_ofd_write_lock() { use nix::sys::stat::fstat; use std::mem; + use std::os::fd::AsRawFd; let tmp = NamedTempFile::new().unwrap(); - let fd = tmp.as_raw_fd(); let statfs = nix::sys::statfs::fstatfs(tmp.as_file()).unwrap(); if statfs.filesystem_type() == nix::sys::statfs::OVERLAYFS_SUPER_MAGIC { // OverlayFS is a union file system. It returns one inode value in @@ -440,7 +435,7 @@ mod linux_android { // skip the test. skip!("/proc/locks does not work on overlayfs"); } - let inode = fstat(fd).expect("fstat failed").st_ino as usize; + let inode = fstat(tmp.as_raw_fd()).expect("fstat failed").st_ino as usize; let mut flock: libc::flock = unsafe { mem::zeroed() // required for Linux/mips @@ -450,14 +445,14 @@ mod linux_android { flock.l_start = 0; flock.l_len = 0; flock.l_pid = 0; - fcntl(fd, FcntlArg::F_OFD_SETLKW(&flock)).expect("write lock failed"); + fcntl(&tmp, FcntlArg::F_OFD_SETLKW(&flock)).expect("write lock failed"); assert_eq!( Some(("OFDLCK".to_string(), "WRITE".to_string())), lock_info(inode) ); flock.l_type = libc::F_UNLCK as libc::c_short; - fcntl(fd, FcntlArg::F_OFD_SETLKW(&flock)).expect("write unlock failed"); + fcntl(&tmp, FcntlArg::F_OFD_SETLKW(&flock)).expect("write unlock failed"); assert_eq!(None, lock_info(inode)); } @@ -467,10 +462,10 @@ mod linux_android { fn test_ofd_read_lock() { use nix::sys::stat::fstat; use std::mem; + use std::os::fd::AsRawFd; let tmp = NamedTempFile::new().unwrap(); - let fd = tmp.as_raw_fd(); let statfs = nix::sys::statfs::fstatfs(tmp.as_file()).unwrap(); if statfs.filesystem_type() == nix::sys::statfs::OVERLAYFS_SUPER_MAGIC { // OverlayFS is a union file system. It returns one inode value in @@ -478,7 +473,7 @@ mod linux_android { // skip the test. skip!("/proc/locks does not work on overlayfs"); } - let inode = fstat(fd).expect("fstat failed").st_ino as usize; + let inode = fstat(tmp.as_raw_fd()).expect("fstat failed").st_ino as usize; let mut flock: libc::flock = unsafe { mem::zeroed() // required for Linux/mips @@ -488,14 +483,14 @@ mod linux_android { flock.l_start = 0; flock.l_len = 0; flock.l_pid = 0; - fcntl(fd, FcntlArg::F_OFD_SETLKW(&flock)).expect("read lock failed"); + fcntl(&tmp, FcntlArg::F_OFD_SETLKW(&flock)).expect("read lock failed"); assert_eq!( Some(("OFDLCK".to_string(), "READ".to_string())), lock_info(inode) ); flock.l_type = libc::F_UNLCK as libc::c_short; - fcntl(fd, FcntlArg::F_OFD_SETLKW(&flock)).expect("read unlock failed"); + fcntl(&tmp, FcntlArg::F_OFD_SETLKW(&flock)).expect("read unlock failed"); assert_eq!(None, lock_info(inode)); } @@ -530,18 +525,15 @@ mod linux_android { target_os = "freebsd" ))] mod test_posix_fadvise { - use nix::errno::Errno; use nix::fcntl::*; use nix::unistd::pipe; - use std::os::unix::io::AsRawFd; use tempfile::NamedTempFile; #[test] fn test_success() { let tmp = NamedTempFile::new().unwrap(); - let fd = tmp.as_raw_fd(); - posix_fadvise(fd, 0, 100, PosixFadviseAdvice::POSIX_FADV_WILLNEED) + posix_fadvise(&tmp, 0, 100, PosixFadviseAdvice::POSIX_FADV_WILLNEED) .expect("posix_fadvise failed"); } @@ -549,7 +541,7 @@ mod test_posix_fadvise { fn test_errno() { let (rd, _wr) = pipe().unwrap(); let res = posix_fadvise( - rd.as_raw_fd(), + &rd, 0, 100, PosixFadviseAdvice::POSIX_FADV_WILLNEED, @@ -570,15 +562,14 @@ mod test_posix_fallocate { use nix::errno::Errno; use nix::fcntl::*; use nix::unistd::pipe; - use std::{io::Read, os::unix::io::AsRawFd}; + use std::io::Read; use tempfile::NamedTempFile; #[test] fn success() { const LEN: usize = 100; let mut tmp = NamedTempFile::new().unwrap(); - let fd = tmp.as_raw_fd(); - let res = posix_fallocate(fd, 0, LEN as libc::off_t); + let res = posix_fallocate(&tmp, 0, LEN as libc::off_t); match res { Ok(_) => { let mut data = [1u8; LEN]; @@ -600,7 +591,7 @@ mod test_posix_fallocate { #[test] fn errno() { let (rd, _wr) = pipe().unwrap(); - let err = posix_fallocate(rd.as_raw_fd(), 0, 100).unwrap_err(); + let err = posix_fallocate(&rd, 0, 100).unwrap_err(); match err { Errno::EINVAL | Errno::ENODEV | Errno::ESPIPE | Errno::EBADF => (), errno => panic!("unexpected errno {errno}",), @@ -615,10 +606,9 @@ fn test_f_get_path() { use std::{os::unix::io::AsRawFd, path::PathBuf}; let tmp = NamedTempFile::new().unwrap(); - let fd = tmp.as_raw_fd(); let mut path = PathBuf::new(); let res = - fcntl(fd, FcntlArg::F_GETPATH(&mut path)).expect("get path failed"); + fcntl(&tmp, FcntlArg::F_GETPATH(&mut path)).expect("get path failed"); assert_ne!(res, -1); assert_eq!( path.as_path().canonicalize().unwrap(), @@ -633,9 +623,8 @@ fn test_f_get_path_nofirmlink() { use std::{os::unix::io::AsRawFd, path::PathBuf}; let tmp = NamedTempFile::new().unwrap(); - let fd = tmp.as_raw_fd(); let mut path = PathBuf::new(); - let res = fcntl(fd, FcntlArg::F_GETPATH_NOFIRMLINK(&mut path)) + let res = fcntl(&tmp, FcntlArg::F_GETPATH_NOFIRMLINK(&mut path)) .expect("get path failed"); let mut tmpstr = String::from("/System/Volumes/Data"); tmpstr.push_str( @@ -670,9 +659,8 @@ fn test_f_kinfo() { // Therefore, we reopen the tempfile a second time for the test // to pass. let tmp2 = File::open(tmp.path()).unwrap(); - let fd = tmp2.as_raw_fd(); let mut path = PathBuf::new(); - let res = fcntl(fd, FcntlArg::F_KINFO(&mut path)).expect("get path failed"); + let res = fcntl(&tmp2, FcntlArg::F_KINFO(&mut path)).expect("get path failed"); assert_ne!(res, -1); assert_eq!(path, tmp.path()); } diff --git a/test/test_pty.rs b/test/test_pty.rs index bc618e0bd8..3f5137cb0d 100644 --- a/test/test_pty.rs +++ b/test/test_pty.rs @@ -121,7 +121,7 @@ fn open_ptty_pair() -> (PtyMaster, File) { } } - let slave = unsafe { File::from_raw_fd(slave_fd) }; + let slave = File::from(slave_fd); (master, slave) } diff --git a/test/test_stat.rs b/test/test_stat.rs index 386f1084cc..a09aeffec0 100644 --- a/test/test_stat.rs +++ b/test/test_stat.rs @@ -109,14 +109,16 @@ fn test_stat_and_fstat() { #[test] #[cfg(not(any(target_os = "netbsd", target_os = "redox")))] fn test_fstatat() { + use std::os::fd::AsRawFd; + let tempdir = tempfile::tempdir().unwrap(); let filename = tempdir.path().join("foo.txt"); File::create(&filename).unwrap(); let dirfd = - fcntl::open(tempdir.path(), fcntl::OFlag::empty(), stat::Mode::empty()); + fcntl::open(tempdir.path(), fcntl::OFlag::empty(), stat::Mode::empty()).unwrap(); let result = - stat::fstatat(Some(dirfd.unwrap()), &filename, fcntl::AtFlags::empty()); + stat::fstatat(Some(dirfd.as_raw_fd()), &filename, fcntl::AtFlags::empty()); assert_stat_results(result); } @@ -170,6 +172,8 @@ fn test_fchmod() { #[test] #[cfg(not(target_os = "redox"))] fn test_fchmodat() { + use std::os::fd::AsRawFd; + let _dr = crate::DirRestore::new(); let tempdir = tempfile::tempdir().unwrap(); let filename = "foo.txt"; @@ -183,7 +187,7 @@ fn test_fchmodat() { let mut mode1 = Mode::empty(); mode1.insert(Mode::S_IRUSR); mode1.insert(Mode::S_IWUSR); - fchmodat(Some(dirfd), filename, mode1, FchmodatFlags::FollowSymlink) + fchmodat(Some(dirfd.as_raw_fd()), filename, mode1, FchmodatFlags::FollowSymlink) .unwrap(); let file_stat1 = stat(&fullpath).unwrap(); @@ -266,6 +270,8 @@ fn test_lutimes() { #[test] #[cfg(not(any(target_os = "redox", target_os = "haiku")))] fn test_futimens() { + use std::os::fd::AsRawFd; + let tempdir = tempfile::tempdir().unwrap(); let fullpath = tempdir.path().join("file"); drop(File::create(&fullpath).unwrap()); @@ -273,13 +279,15 @@ fn test_futimens() { let fd = fcntl::open(&fullpath, fcntl::OFlag::empty(), stat::Mode::empty()) .unwrap(); - futimens(fd, &TimeSpec::seconds(10), &TimeSpec::seconds(20)).unwrap(); + futimens(fd.as_raw_fd(), &TimeSpec::seconds(10), &TimeSpec::seconds(20)).unwrap(); assert_times_eq(10, 20, &fs::metadata(&fullpath).unwrap()); } #[test] #[cfg(not(any(target_os = "redox", target_os = "haiku")))] fn test_utimensat() { + use std::os::fd::AsRawFd; + let _dr = crate::DirRestore::new(); let tempdir = tempfile::tempdir().unwrap(); let filename = "foo.txt"; @@ -291,7 +299,7 @@ fn test_utimensat() { .unwrap(); utimensat( - Some(dirfd), + Some(dirfd.as_raw_fd()), filename, &TimeSpec::seconds(12345), &TimeSpec::seconds(678), @@ -321,13 +329,15 @@ fn test_mkdirat_success_path() { let dirfd = fcntl::open(tempdir.path(), fcntl::OFlag::empty(), stat::Mode::empty()) .unwrap(); - mkdirat(Some(dirfd), filename, Mode::S_IRWXU).expect("mkdirat failed"); + mkdirat(Some(dirfd.as_raw_fd()), filename, Mode::S_IRWXU).expect("mkdirat failed"); assert!(Path::exists(&tempdir.path().join(filename))); } #[test] #[cfg(not(any(target_os = "redox", target_os = "haiku")))] fn test_mkdirat_success_mode() { + use std::os::fd::AsRawFd; + let expected_bits = stat::SFlag::S_IFDIR.bits() | stat::Mode::S_IRWXU.bits(); let tempdir = tempfile::tempdir().unwrap(); @@ -335,7 +345,7 @@ fn test_mkdirat_success_mode() { let dirfd = fcntl::open(tempdir.path(), fcntl::OFlag::empty(), stat::Mode::empty()) .unwrap(); - mkdirat(Some(dirfd), filename, Mode::S_IRWXU).expect("mkdirat failed"); + mkdirat(Some(dirfd.as_raw_fd()), filename, Mode::S_IRWXU).expect("mkdirat failed"); let permissions = fs::metadata(tempdir.path().join(filename)) .unwrap() .permissions(); @@ -346,6 +356,8 @@ fn test_mkdirat_success_mode() { #[test] #[cfg(not(target_os = "redox"))] fn test_mkdirat_fail() { + use std::os::fd::AsRawFd; + let tempdir = tempfile::tempdir().unwrap(); let not_dir_filename = "example_not_dir"; let filename = "example_subdir_dir"; @@ -355,7 +367,7 @@ fn test_mkdirat_fail() { stat::Mode::empty(), ) .unwrap(); - let result = mkdirat(Some(dirfd), filename, Mode::S_IRWXU).unwrap_err(); + let result = mkdirat(Some(dirfd.as_raw_fd()), filename, Mode::S_IRWXU).unwrap_err(); assert_eq!(result, Errno::ENOTDIR); } @@ -417,6 +429,8 @@ fn test_mknodat() { #[test] #[cfg(not(any(target_os = "redox", target_os = "haiku")))] fn test_futimens_unchanged() { + use std::os::fd::AsRawFd; + let tempdir = tempfile::tempdir().unwrap(); let fullpath = tempdir.path().join("file"); drop(File::create(&fullpath).unwrap()); @@ -432,7 +446,7 @@ fn test_futimens_unchanged() { .modified() .unwrap(); - futimens(fd, &TimeSpec::UTIME_OMIT, &TimeSpec::UTIME_OMIT).unwrap(); + futimens(fd.as_raw_fd(), &TimeSpec::UTIME_OMIT, &TimeSpec::UTIME_OMIT).unwrap(); let new_atime = fs::metadata(fullpath.as_path()) .unwrap() @@ -449,6 +463,8 @@ fn test_futimens_unchanged() { #[test] #[cfg(not(any(target_os = "redox", target_os = "haiku")))] fn test_utimensat_unchanged() { + use std::os::fd::AsRawFd; + let _dr = crate::DirRestore::new(); let tempdir = tempfile::tempdir().unwrap(); let filename = "foo.txt"; @@ -467,7 +483,7 @@ fn test_utimensat_unchanged() { .modified() .unwrap(); utimensat( - Some(dirfd), + Some(dirfd.as_raw_fd()), filename, &TimeSpec::UTIME_OMIT, &TimeSpec::UTIME_OMIT, diff --git a/test/test_unistd.rs b/test/test_unistd.rs index 6ccf59fb05..70397e56e8 100644 --- a/test/test_unistd.rs +++ b/test/test_unistd.rs @@ -184,15 +184,16 @@ fn test_mkfifoat_none() { )))] fn test_mkfifoat() { use nix::fcntl; + use std::os::fd::AsRawFd; let tempdir = tempdir().unwrap(); let dirfd = open(tempdir.path(), OFlag::empty(), Mode::empty()).unwrap(); let mkfifoat_name = "mkfifoat_name"; - mkfifoat(Some(dirfd), mkfifoat_name, Mode::S_IRUSR).unwrap(); + mkfifoat(Some(dirfd.as_raw_fd()), mkfifoat_name, Mode::S_IRUSR).unwrap(); let stats = - stat::fstatat(Some(dirfd), mkfifoat_name, fcntl::AtFlags::empty()) + stat::fstatat(Some(dirfd.as_raw_fd()), mkfifoat_name, fcntl::AtFlags::empty()) .unwrap(); let typ = stat::SFlag::from_bits_truncate(stats.st_mode); assert_eq!(typ, SFlag::S_IFIFO); @@ -221,13 +222,15 @@ fn test_mkfifoat_directory_none() { target_os = "haiku" )))] fn test_mkfifoat_directory() { + use std::os::fd::AsRawFd; + // mkfifoat should fail if a directory is given let tempdir = tempdir().unwrap(); let dirfd = open(tempdir.path(), OFlag::empty(), Mode::empty()).unwrap(); let mkfifoat_dir = "mkfifoat_dir"; - stat::mkdirat(Some(dirfd), mkfifoat_dir, Mode::S_IRUSR).unwrap(); + stat::mkdirat(Some(dirfd.as_raw_fd()), mkfifoat_dir, Mode::S_IRUSR).unwrap(); - mkfifoat(Some(dirfd), mkfifoat_dir, Mode::S_IRUSR) + mkfifoat(Some(dirfd.as_raw_fd()), mkfifoat_dir, Mode::S_IRUSR) .expect_err("assertion failed"); } @@ -555,6 +558,7 @@ fn test_fchown() { #[cfg(not(target_os = "redox"))] fn test_fchownat() { use nix::fcntl::AtFlags; + use std::os::fd::AsRawFd; let _dr = crate::DirRestore::new(); // Testing for anything other than our own UID/GID is hard. @@ -569,7 +573,7 @@ fn test_fchownat() { let dirfd = open(tempdir.path(), OFlag::empty(), Mode::empty()).unwrap(); - fchownat(Some(dirfd), "file", uid, gid, AtFlags::empty()).unwrap(); + fchownat(Some(dirfd.as_raw_fd()), "file", uid, gid, AtFlags::empty()).unwrap(); chdir(tempdir.path()).unwrap(); fchownat(None, "file", uid, gid, AtFlags::empty()).unwrap(); @@ -759,11 +763,11 @@ fn test_pipe2() { let (fd0, fd1) = pipe2(OFlag::O_CLOEXEC).unwrap(); let f0 = FdFlag::from_bits_truncate( - fcntl(fd0.as_raw_fd(), FcntlArg::F_GETFD).unwrap(), + fcntl(&fd0, FcntlArg::F_GETFD).unwrap(), ); assert!(f0.contains(FdFlag::FD_CLOEXEC)); let f1 = FdFlag::from_bits_truncate( - fcntl(fd1.as_raw_fd(), FcntlArg::F_GETFD).unwrap(), + fcntl(&fd1, FcntlArg::F_GETFD).unwrap(), ); assert!(f1.contains(FdFlag::FD_CLOEXEC)); } @@ -872,6 +876,8 @@ fn test_canceling_alarm() { #[test] #[cfg(not(any(target_os = "redox", target_os = "haiku")))] fn test_symlinkat() { + use std::os::fd::AsRawFd; + let _m = crate::CWD_LOCK.read(); let tempdir = tempdir().unwrap(); @@ -887,7 +893,7 @@ fn test_symlinkat() { let dirfd = open(tempdir.path(), OFlag::empty(), Mode::empty()).unwrap(); let target = "c"; let linkpath = "d"; - symlinkat(target, Some(dirfd), linkpath).unwrap(); + symlinkat(target, Some(dirfd.as_raw_fd()), linkpath).unwrap(); assert_eq!( readlink(&tempdir.path().join(linkpath)) .unwrap() @@ -901,6 +907,7 @@ fn test_symlinkat() { #[cfg(not(any(target_os = "redox", target_os = "haiku")))] fn test_linkat_file() { use nix::fcntl::AtFlags; + use std::os::fd::AsRawFd; let tempdir = tempdir().unwrap(); let oldfilename = "foo.txt"; @@ -919,9 +926,9 @@ fn test_linkat_file() { // Attempt hard link file at relative path linkat( - Some(dirfd), + Some(dirfd.as_raw_fd()), oldfilename, - Some(dirfd), + Some(dirfd.as_raw_fd()), newfilename, AtFlags::AT_SYMLINK_FOLLOW, ) @@ -933,6 +940,7 @@ fn test_linkat_file() { #[cfg(not(any(target_os = "redox", target_os = "haiku")))] fn test_linkat_olddirfd_none() { use nix::fcntl::AtFlags; + use std::os::fd::AsRawFd; let _dr = crate::DirRestore::new(); @@ -960,7 +968,7 @@ fn test_linkat_olddirfd_none() { linkat( None, oldfilename, - Some(dirfd), + Some(dirfd.as_raw_fd()), newfilename, AtFlags::AT_SYMLINK_FOLLOW, ) @@ -972,6 +980,7 @@ fn test_linkat_olddirfd_none() { #[cfg(not(any(target_os = "redox", target_os = "haiku")))] fn test_linkat_newdirfd_none() { use nix::fcntl::AtFlags; + use std::os::fd::AsRawFd; let _dr = crate::DirRestore::new(); @@ -997,7 +1006,7 @@ fn test_linkat_newdirfd_none() { // Attempt hard link file using current working directory as relative path for new file path chdir(tempdir_newfile.path()).unwrap(); linkat( - Some(dirfd), + Some(dirfd.as_raw_fd()), oldfilename, None, newfilename, @@ -1011,6 +1020,7 @@ fn test_linkat_newdirfd_none() { #[cfg(not(any(apple_targets, target_os = "redox", target_os = "haiku")))] fn test_linkat_no_follow_symlink() { use nix::fcntl::AtFlags; + use std::os::fd::AsRawFd; let _m = crate::CWD_LOCK.read(); @@ -1037,9 +1047,9 @@ fn test_linkat_no_follow_symlink() { // Attempt link symlink of file at relative path linkat( - Some(dirfd), + Some(dirfd.as_raw_fd()), symoldfilename, - Some(dirfd), + Some(dirfd.as_raw_fd()), newfilename, AtFlags::empty(), ) @@ -1082,9 +1092,9 @@ fn test_linkat_follow_symlink() { // Attempt link target of symlink of file at relative path linkat( - Some(dirfd), + Some(dirfd.as_raw_fd()), symoldfilename, - Some(dirfd), + Some(dirfd.as_raw_fd()), newfilename, AtFlags::AT_SYMLINK_FOLLOW, ) @@ -1106,6 +1116,8 @@ fn test_linkat_follow_symlink() { #[test] #[cfg(not(target_os = "redox"))] fn test_unlinkat_dir_noremovedir() { + use std::os::fd::AsRawFd; + let tempdir = tempdir().unwrap(); let dirname = "foo_dir"; let dirpath = tempdir.path().join(dirname); @@ -1120,13 +1132,15 @@ fn test_unlinkat_dir_noremovedir() { // Attempt unlink dir at relative path without proper flag let err_result = - unlinkat(Some(dirfd), dirname, UnlinkatFlags::NoRemoveDir).unwrap_err(); + unlinkat(Some(dirfd.as_raw_fd()), dirname, UnlinkatFlags::NoRemoveDir).unwrap_err(); assert!(err_result == Errno::EISDIR || err_result == Errno::EPERM); } #[test] #[cfg(not(target_os = "redox"))] fn test_unlinkat_dir_removedir() { + use std::os::fd::AsRawFd; + let tempdir = tempdir().unwrap(); let dirname = "foo_dir"; let dirpath = tempdir.path().join(dirname); @@ -1140,13 +1154,15 @@ fn test_unlinkat_dir_removedir() { .unwrap(); // Attempt unlink dir at relative path with proper flag - unlinkat(Some(dirfd), dirname, UnlinkatFlags::RemoveDir).unwrap(); + unlinkat(Some(dirfd.as_raw_fd()), dirname, UnlinkatFlags::RemoveDir).unwrap(); assert!(!dirpath.exists()); } #[test] #[cfg(not(target_os = "redox"))] fn test_unlinkat_file() { + use std::os::fd::AsRawFd; + let tempdir = tempdir().unwrap(); let filename = "foo.txt"; let filepath = tempdir.path().join(filename); @@ -1160,7 +1176,7 @@ fn test_unlinkat_file() { .unwrap(); // Attempt unlink file at relative path - unlinkat(Some(dirfd), filename, UnlinkatFlags::NoRemoveDir).unwrap(); + unlinkat(Some(dirfd.as_raw_fd()), filename, UnlinkatFlags::NoRemoveDir).unwrap(); assert!(!filepath.exists()); } @@ -1305,12 +1321,14 @@ fn test_faccessat_none_not_existing() { #[cfg(not(target_os = "redox"))] fn test_faccessat_not_existing() { use nix::fcntl::AtFlags; + use std::os::fd::AsRawFd; + let tempdir = tempfile::tempdir().unwrap(); let dirfd = open(tempdir.path(), OFlag::empty(), Mode::empty()).unwrap(); let not_exist_file = "does_not_exist.txt"; assert_eq!( faccessat( - Some(dirfd), + Some(dirfd.as_raw_fd()), not_exist_file, AccessFlags::F_OK, AtFlags::empty(), @@ -1341,13 +1359,15 @@ fn test_faccessat_none_file_exists() { #[cfg(not(target_os = "redox"))] fn test_faccessat_file_exists() { use nix::fcntl::AtFlags; + use std::os::fd::AsRawFd; + let tempdir = tempfile::tempdir().unwrap(); let dirfd = open(tempdir.path(), OFlag::empty(), Mode::empty()).unwrap(); let exist_file = "does_exist.txt"; let path = tempdir.path().join(exist_file); let _file = File::create(path.clone()).unwrap(); assert!(faccessat( - Some(dirfd), + Some(dirfd.as_raw_fd()), &path, AccessFlags::R_OK | AccessFlags::W_OK, AtFlags::empty(), From 437b9fd7585ed85d37ba52b2cc38c20dd0058d07 Mon Sep 17 00:00:00 2001 From: Steve Lau Date: Sun, 9 Jun 2024 11:03:50 +0800 Subject: [PATCH 02/10] fix: move the ownership from OwnedFd to Dir --- src/dir.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/dir.rs b/src/dir.rs index f04a77cba8..81d9fef42f 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -74,9 +74,9 @@ impl Dir { /// Converts from a file descriptor, closing it on failure. #[doc(alias("fdopendir"))] pub fn from_fd(fd: std::os::fd::OwnedFd) -> Result { - use std::os::fd::AsRawFd; - - let d = ptr::NonNull::new(unsafe { libc::fdopendir(fd.as_raw_fd()) }).ok_or(Errno::last())?; + // take the ownership as the constructed `Dir` is now the owner + let raw_fd = fd.into_raw_fd(); + let d = ptr::NonNull::new(unsafe { libc::fdopendir(raw_fd) }).ok_or(Errno::last())?; Ok(Dir(d)) } From c1af6d704bc3275683ab44892fb285c302ba938a Mon Sep 17 00:00:00 2001 From: Steve Lau Date: Sun, 9 Jun 2024 11:05:29 +0800 Subject: [PATCH 03/10] style: format code --- src/dir.rs | 12 +++++++----- src/fcntl.rs | 7 ++++--- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/dir.rs b/src/dir.rs index 81d9fef42f..c3078b2d75 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -59,7 +59,10 @@ impl Dir { /// /// It is only safe if `fd` is an owned file descriptor. #[inline] - #[deprecated(since = "0.30.0", note = "Deprecate this since it is not I/O-safe, use from_fd instead.")] + #[deprecated( + since = "0.30.0", + note = "Deprecate this since it is not I/O-safe, use from_fd instead." + )] pub unsafe fn from(fd: F) -> Result { use std::os::fd::FromRawFd; use std::os::fd::OwnedFd; @@ -76,7 +79,8 @@ impl Dir { pub fn from_fd(fd: std::os::fd::OwnedFd) -> Result { // take the ownership as the constructed `Dir` is now the owner let raw_fd = fd.into_raw_fd(); - let d = ptr::NonNull::new(unsafe { libc::fdopendir(raw_fd) }).ok_or(Errno::last())?; + let d = ptr::NonNull::new(unsafe { libc::fdopendir(raw_fd) }) + .ok_or(Errno::last())?; Ok(Dir(d)) } @@ -102,9 +106,7 @@ impl std::os::fd::AsFd for Dir { // // `raw_fd` should be open and valid for the lifetime of the returned // `BorrowedFd` as it is extracted from `&self`. - unsafe { - std::os::fd::BorrowedFd::borrow_raw(raw_fd) - } + unsafe { std::os::fd::BorrowedFd::borrow_raw(raw_fd) } } } diff --git a/src/fcntl.rs b/src/fcntl.rs index 9e717570e4..fc6934e627 100644 --- a/src/fcntl.rs +++ b/src/fcntl.rs @@ -13,12 +13,13 @@ use std::ffi::CStr; use std::ffi::OsString; #[cfg(not(any(target_os = "redox", target_os = "solaris")))] use std::ops::{Deref, DerefMut}; +use std::os::fd::BorrowedFd; #[cfg(not(target_os = "redox"))] use std::os::raw; use std::os::unix::ffi::OsStringExt; -use std::os::unix::io::RawFd; #[cfg(not(any(target_os = "redox", target_os = "solaris")))] use std::os::unix::io::OwnedFd; +use std::os::unix::io::RawFd; #[cfg(any( target_os = "netbsd", apple_targets, @@ -28,7 +29,6 @@ use std::os::unix::io::OwnedFd; use std::path::PathBuf; #[cfg(any(linux_android, target_os = "freebsd"))] use std::ptr; -use std::os::fd::BorrowedFd; #[cfg(feature = "fs")] use std::os::fd::FromRawFd; @@ -53,7 +53,8 @@ pub use self::posix_fadvise::{posix_fadvise, PosixFadviseAdvice}; // 1. `AT_FDCWD` is usable for the whole process life, so it is `'static`. // 2. It is not a valid file descriptor, but OS will handle it for us when passed // to `xxat(2)` calls. -pub const AT_FDCWD: BorrowedFd<'static> = unsafe { BorrowedFd::borrow_raw(libc::AT_FDCWD) }; +pub const AT_FDCWD: BorrowedFd<'static> = + unsafe { BorrowedFd::borrow_raw(libc::AT_FDCWD) }; #[cfg(not(target_os = "redox"))] #[cfg(any(feature = "fs", feature = "process", feature = "user"))] From 1bf136c78cd8b111686bba952319bb9b191fa883 Mon Sep 17 00:00:00 2001 From: Steve Lau Date: Sun, 9 Jun 2024 11:06:24 +0800 Subject: [PATCH 04/10] style: format code --- test/sys/test_socket.rs | 3 ++- test/test_fcntl.rs | 52 ++++++++++++++--------------------------- test/test_stat.rs | 48 +++++++++++++++++++++++++------------ test/test_unistd.rs | 50 +++++++++++++++++++++++---------------- 4 files changed, 82 insertions(+), 71 deletions(-) diff --git a/test/sys/test_socket.rs b/test/sys/test_socket.rs index b1a8646fa0..3bb882d10c 100644 --- a/test/sys/test_socket.rs +++ b/test/sys/test_socket.rs @@ -1127,7 +1127,8 @@ pub fn test_af_alg_aead() { // // `session_socket` will be valid for the lifetime of this test // TODO: remove this workaround when accept(2) becomes I/O-safe. - let borrowed_fd = unsafe { std::os::fd::BorrowedFd::borrow_raw(session_socket) }; + let borrowed_fd = + unsafe { std::os::fd::BorrowedFd::borrow_raw(session_socket) }; fcntl(borrowed_fd, FcntlArg::F_SETFL(OFlag::O_NONBLOCK)) .expect("fcntl non_blocking"); let num_bytes = diff --git a/test/test_fcntl.rs b/test/test_fcntl.rs index 31cce0a9d6..8c24d53942 100644 --- a/test/test_fcntl.rs +++ b/test/test_fcntl.rs @@ -154,23 +154,11 @@ fn test_renameat2_behaves_like_renameat_with_no_flags() { let new_dir = tempfile::tempdir().unwrap(); let new_dirfd = open(new_dir.path(), OFlag::empty(), Mode::empty()).unwrap(); - renameat2( - &old_dirfd, - "old", - &new_dirfd, - "new", - RenameFlags::empty(), - ) - .unwrap(); + renameat2(&old_dirfd, "old", &new_dirfd, "new", RenameFlags::empty()) + .unwrap(); assert_eq!( - renameat2( - &old_dirfd, - "old", - &new_dirfd, - "new", - RenameFlags::empty() - ) - .unwrap_err(), + renameat2(&old_dirfd, "old", &new_dirfd, "new", RenameFlags::empty()) + .unwrap_err(), Errno::ENOENT ); assert!(new_dir.path().join("new").exists()); @@ -301,14 +289,7 @@ fn test_copy_file_range() { tmp1.flush().unwrap(); let mut from_offset: i64 = 3; - copy_file_range( - &tmp1, - Some(&mut from_offset), - &tmp2, - None, - 3, - ) - .unwrap(); + copy_file_range(&tmp1, Some(&mut from_offset), &tmp2, None, 3).unwrap(); let mut res: String = String::new(); tmp2.rewind().unwrap(); @@ -435,7 +416,8 @@ mod linux_android { // skip the test. skip!("/proc/locks does not work on overlayfs"); } - let inode = fstat(tmp.as_raw_fd()).expect("fstat failed").st_ino as usize; + let inode = + fstat(tmp.as_raw_fd()).expect("fstat failed").st_ino as usize; let mut flock: libc::flock = unsafe { mem::zeroed() // required for Linux/mips @@ -452,7 +434,8 @@ mod linux_android { ); flock.l_type = libc::F_UNLCK as libc::c_short; - fcntl(&tmp, FcntlArg::F_OFD_SETLKW(&flock)).expect("write unlock failed"); + fcntl(&tmp, FcntlArg::F_OFD_SETLKW(&flock)) + .expect("write unlock failed"); assert_eq!(None, lock_info(inode)); } @@ -473,7 +456,8 @@ mod linux_android { // skip the test. skip!("/proc/locks does not work on overlayfs"); } - let inode = fstat(tmp.as_raw_fd()).expect("fstat failed").st_ino as usize; + let inode = + fstat(tmp.as_raw_fd()).expect("fstat failed").st_ino as usize; let mut flock: libc::flock = unsafe { mem::zeroed() // required for Linux/mips @@ -490,7 +474,8 @@ mod linux_android { ); flock.l_type = libc::F_UNLCK as libc::c_short; - fcntl(&tmp, FcntlArg::F_OFD_SETLKW(&flock)).expect("read unlock failed"); + fcntl(&tmp, FcntlArg::F_OFD_SETLKW(&flock)) + .expect("read unlock failed"); assert_eq!(None, lock_info(inode)); } @@ -540,12 +525,8 @@ mod test_posix_fadvise { #[test] fn test_errno() { let (rd, _wr) = pipe().unwrap(); - let res = posix_fadvise( - &rd, - 0, - 100, - PosixFadviseAdvice::POSIX_FADV_WILLNEED, - ); + let res = + posix_fadvise(&rd, 0, 100, PosixFadviseAdvice::POSIX_FADV_WILLNEED); assert_eq!(res, Err(Errno::ESPIPE)); } } @@ -660,7 +641,8 @@ fn test_f_kinfo() { // to pass. let tmp2 = File::open(tmp.path()).unwrap(); let mut path = PathBuf::new(); - let res = fcntl(&tmp2, FcntlArg::F_KINFO(&mut path)).expect("get path failed"); + let res = + fcntl(&tmp2, FcntlArg::F_KINFO(&mut path)).expect("get path failed"); assert_ne!(res, -1); assert_eq!(path, tmp.path()); } diff --git a/test/test_stat.rs b/test/test_stat.rs index a09aeffec0..d60f30434b 100644 --- a/test/test_stat.rs +++ b/test/test_stat.rs @@ -110,15 +110,19 @@ fn test_stat_and_fstat() { #[cfg(not(any(target_os = "netbsd", target_os = "redox")))] fn test_fstatat() { use std::os::fd::AsRawFd; - + let tempdir = tempfile::tempdir().unwrap(); let filename = tempdir.path().join("foo.txt"); File::create(&filename).unwrap(); let dirfd = - fcntl::open(tempdir.path(), fcntl::OFlag::empty(), stat::Mode::empty()).unwrap(); + fcntl::open(tempdir.path(), fcntl::OFlag::empty(), stat::Mode::empty()) + .unwrap(); - let result = - stat::fstatat(Some(dirfd.as_raw_fd()), &filename, fcntl::AtFlags::empty()); + let result = stat::fstatat( + Some(dirfd.as_raw_fd()), + &filename, + fcntl::AtFlags::empty(), + ); assert_stat_results(result); } @@ -173,7 +177,7 @@ fn test_fchmod() { #[cfg(not(target_os = "redox"))] fn test_fchmodat() { use std::os::fd::AsRawFd; - + let _dr = crate::DirRestore::new(); let tempdir = tempfile::tempdir().unwrap(); let filename = "foo.txt"; @@ -187,8 +191,13 @@ fn test_fchmodat() { let mut mode1 = Mode::empty(); mode1.insert(Mode::S_IRUSR); mode1.insert(Mode::S_IWUSR); - fchmodat(Some(dirfd.as_raw_fd()), filename, mode1, FchmodatFlags::FollowSymlink) - .unwrap(); + fchmodat( + Some(dirfd.as_raw_fd()), + filename, + mode1, + FchmodatFlags::FollowSymlink, + ) + .unwrap(); let file_stat1 = stat(&fullpath).unwrap(); assert_eq!(file_stat1.st_mode as mode_t & 0o7777, mode1.bits()); @@ -271,7 +280,7 @@ fn test_lutimes() { #[cfg(not(any(target_os = "redox", target_os = "haiku")))] fn test_futimens() { use std::os::fd::AsRawFd; - + let tempdir = tempfile::tempdir().unwrap(); let fullpath = tempdir.path().join("file"); drop(File::create(&fullpath).unwrap()); @@ -279,7 +288,12 @@ fn test_futimens() { let fd = fcntl::open(&fullpath, fcntl::OFlag::empty(), stat::Mode::empty()) .unwrap(); - futimens(fd.as_raw_fd(), &TimeSpec::seconds(10), &TimeSpec::seconds(20)).unwrap(); + futimens( + fd.as_raw_fd(), + &TimeSpec::seconds(10), + &TimeSpec::seconds(20), + ) + .unwrap(); assert_times_eq(10, 20, &fs::metadata(&fullpath).unwrap()); } @@ -287,7 +301,7 @@ fn test_futimens() { #[cfg(not(any(target_os = "redox", target_os = "haiku")))] fn test_utimensat() { use std::os::fd::AsRawFd; - + let _dr = crate::DirRestore::new(); let tempdir = tempfile::tempdir().unwrap(); let filename = "foo.txt"; @@ -329,7 +343,8 @@ fn test_mkdirat_success_path() { let dirfd = fcntl::open(tempdir.path(), fcntl::OFlag::empty(), stat::Mode::empty()) .unwrap(); - mkdirat(Some(dirfd.as_raw_fd()), filename, Mode::S_IRWXU).expect("mkdirat failed"); + mkdirat(Some(dirfd.as_raw_fd()), filename, Mode::S_IRWXU) + .expect("mkdirat failed"); assert!(Path::exists(&tempdir.path().join(filename))); } @@ -337,7 +352,7 @@ fn test_mkdirat_success_path() { #[cfg(not(any(target_os = "redox", target_os = "haiku")))] fn test_mkdirat_success_mode() { use std::os::fd::AsRawFd; - + let expected_bits = stat::SFlag::S_IFDIR.bits() | stat::Mode::S_IRWXU.bits(); let tempdir = tempfile::tempdir().unwrap(); @@ -345,7 +360,8 @@ fn test_mkdirat_success_mode() { let dirfd = fcntl::open(tempdir.path(), fcntl::OFlag::empty(), stat::Mode::empty()) .unwrap(); - mkdirat(Some(dirfd.as_raw_fd()), filename, Mode::S_IRWXU).expect("mkdirat failed"); + mkdirat(Some(dirfd.as_raw_fd()), filename, Mode::S_IRWXU) + .expect("mkdirat failed"); let permissions = fs::metadata(tempdir.path().join(filename)) .unwrap() .permissions(); @@ -367,7 +383,8 @@ fn test_mkdirat_fail() { stat::Mode::empty(), ) .unwrap(); - let result = mkdirat(Some(dirfd.as_raw_fd()), filename, Mode::S_IRWXU).unwrap_err(); + let result = + mkdirat(Some(dirfd.as_raw_fd()), filename, Mode::S_IRWXU).unwrap_err(); assert_eq!(result, Errno::ENOTDIR); } @@ -446,7 +463,8 @@ fn test_futimens_unchanged() { .modified() .unwrap(); - futimens(fd.as_raw_fd(), &TimeSpec::UTIME_OMIT, &TimeSpec::UTIME_OMIT).unwrap(); + futimens(fd.as_raw_fd(), &TimeSpec::UTIME_OMIT, &TimeSpec::UTIME_OMIT) + .unwrap(); let new_atime = fs::metadata(fullpath.as_path()) .unwrap() diff --git a/test/test_unistd.rs b/test/test_unistd.rs index 70397e56e8..a86a170bac 100644 --- a/test/test_unistd.rs +++ b/test/test_unistd.rs @@ -192,9 +192,12 @@ fn test_mkfifoat() { mkfifoat(Some(dirfd.as_raw_fd()), mkfifoat_name, Mode::S_IRUSR).unwrap(); - let stats = - stat::fstatat(Some(dirfd.as_raw_fd()), mkfifoat_name, fcntl::AtFlags::empty()) - .unwrap(); + let stats = stat::fstatat( + Some(dirfd.as_raw_fd()), + mkfifoat_name, + fcntl::AtFlags::empty(), + ) + .unwrap(); let typ = stat::SFlag::from_bits_truncate(stats.st_mode); assert_eq!(typ, SFlag::S_IFIFO); } @@ -228,7 +231,8 @@ fn test_mkfifoat_directory() { let tempdir = tempdir().unwrap(); let dirfd = open(tempdir.path(), OFlag::empty(), Mode::empty()).unwrap(); let mkfifoat_dir = "mkfifoat_dir"; - stat::mkdirat(Some(dirfd.as_raw_fd()), mkfifoat_dir, Mode::S_IRUSR).unwrap(); + stat::mkdirat(Some(dirfd.as_raw_fd()), mkfifoat_dir, Mode::S_IRUSR) + .unwrap(); mkfifoat(Some(dirfd.as_raw_fd()), mkfifoat_dir, Mode::S_IRUSR) .expect_err("assertion failed"); @@ -573,7 +577,8 @@ fn test_fchownat() { let dirfd = open(tempdir.path(), OFlag::empty(), Mode::empty()).unwrap(); - fchownat(Some(dirfd.as_raw_fd()), "file", uid, gid, AtFlags::empty()).unwrap(); + fchownat(Some(dirfd.as_raw_fd()), "file", uid, gid, AtFlags::empty()) + .unwrap(); chdir(tempdir.path()).unwrap(); fchownat(None, "file", uid, gid, AtFlags::empty()).unwrap(); @@ -762,13 +767,11 @@ fn test_pipe2() { use nix::fcntl::{fcntl, FcntlArg, FdFlag}; let (fd0, fd1) = pipe2(OFlag::O_CLOEXEC).unwrap(); - let f0 = FdFlag::from_bits_truncate( - fcntl(&fd0, FcntlArg::F_GETFD).unwrap(), - ); + let f0 = + FdFlag::from_bits_truncate(fcntl(&fd0, FcntlArg::F_GETFD).unwrap()); assert!(f0.contains(FdFlag::FD_CLOEXEC)); - let f1 = FdFlag::from_bits_truncate( - fcntl(&fd1, FcntlArg::F_GETFD).unwrap(), - ); + let f1 = + FdFlag::from_bits_truncate(fcntl(&fd1, FcntlArg::F_GETFD).unwrap()); assert!(f1.contains(FdFlag::FD_CLOEXEC)); } @@ -877,7 +880,7 @@ fn test_canceling_alarm() { #[cfg(not(any(target_os = "redox", target_os = "haiku")))] fn test_symlinkat() { use std::os::fd::AsRawFd; - + let _m = crate::CWD_LOCK.read(); let tempdir = tempdir().unwrap(); @@ -1117,7 +1120,7 @@ fn test_linkat_follow_symlink() { #[cfg(not(target_os = "redox"))] fn test_unlinkat_dir_noremovedir() { use std::os::fd::AsRawFd; - + let tempdir = tempdir().unwrap(); let dirname = "foo_dir"; let dirpath = tempdir.path().join(dirname); @@ -1132,7 +1135,8 @@ fn test_unlinkat_dir_noremovedir() { // Attempt unlink dir at relative path without proper flag let err_result = - unlinkat(Some(dirfd.as_raw_fd()), dirname, UnlinkatFlags::NoRemoveDir).unwrap_err(); + unlinkat(Some(dirfd.as_raw_fd()), dirname, UnlinkatFlags::NoRemoveDir) + .unwrap_err(); assert!(err_result == Errno::EISDIR || err_result == Errno::EPERM); } @@ -1140,7 +1144,7 @@ fn test_unlinkat_dir_noremovedir() { #[cfg(not(target_os = "redox"))] fn test_unlinkat_dir_removedir() { use std::os::fd::AsRawFd; - + let tempdir = tempdir().unwrap(); let dirname = "foo_dir"; let dirpath = tempdir.path().join(dirname); @@ -1154,7 +1158,8 @@ fn test_unlinkat_dir_removedir() { .unwrap(); // Attempt unlink dir at relative path with proper flag - unlinkat(Some(dirfd.as_raw_fd()), dirname, UnlinkatFlags::RemoveDir).unwrap(); + unlinkat(Some(dirfd.as_raw_fd()), dirname, UnlinkatFlags::RemoveDir) + .unwrap(); assert!(!dirpath.exists()); } @@ -1162,7 +1167,7 @@ fn test_unlinkat_dir_removedir() { #[cfg(not(target_os = "redox"))] fn test_unlinkat_file() { use std::os::fd::AsRawFd; - + let tempdir = tempdir().unwrap(); let filename = "foo.txt"; let filepath = tempdir.path().join(filename); @@ -1176,7 +1181,12 @@ fn test_unlinkat_file() { .unwrap(); // Attempt unlink file at relative path - unlinkat(Some(dirfd.as_raw_fd()), filename, UnlinkatFlags::NoRemoveDir).unwrap(); + unlinkat( + Some(dirfd.as_raw_fd()), + filename, + UnlinkatFlags::NoRemoveDir, + ) + .unwrap(); assert!(!filepath.exists()); } @@ -1322,7 +1332,7 @@ fn test_faccessat_none_not_existing() { fn test_faccessat_not_existing() { use nix::fcntl::AtFlags; use std::os::fd::AsRawFd; - + let tempdir = tempfile::tempdir().unwrap(); let dirfd = open(tempdir.path(), OFlag::empty(), Mode::empty()).unwrap(); let not_exist_file = "does_not_exist.txt"; @@ -1360,7 +1370,7 @@ fn test_faccessat_none_file_exists() { fn test_faccessat_file_exists() { use nix::fcntl::AtFlags; use std::os::fd::AsRawFd; - + let tempdir = tempfile::tempdir().unwrap(); let dirfd = open(tempdir.path(), OFlag::empty(), Mode::empty()).unwrap(); let exist_file = "does_exist.txt"; From 83e03685d178ae59c1e80a6b3038577fc07bdeca Mon Sep 17 00:00:00 2001 From: Steve Lau Date: Sun, 9 Jun 2024 11:16:52 +0800 Subject: [PATCH 05/10] chore: fix imports --- src/fcntl.rs | 2 ++ test/test_fcntl.rs | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/fcntl.rs b/src/fcntl.rs index fc6934e627..80c94fc13d 100644 --- a/src/fcntl.rs +++ b/src/fcntl.rs @@ -1358,6 +1358,8 @@ impl SpacectlRange { #[cfg(target_os = "freebsd")] #[inline] // Delays codegen, preventing linker errors with dylibs and --no-allow-shlib-undefined pub fn fspacectl(fd: Fd, range: SpacectlRange) -> Result { + use std::os::fd::AsRawFd; + let mut rqsr = libc::spacectl_range { r_offset: range.0, r_len: range.1, diff --git a/test/test_fcntl.rs b/test/test_fcntl.rs index 8c24d53942..1db8b44886 100644 --- a/test/test_fcntl.rs +++ b/test/test_fcntl.rs @@ -584,7 +584,7 @@ mod test_posix_fallocate { #[test] fn test_f_get_path() { use nix::fcntl::*; - use std::{os::unix::io::AsRawFd, path::PathBuf}; + use std::path::PathBuf; let tmp = NamedTempFile::new().unwrap(); let mut path = PathBuf::new(); @@ -601,7 +601,7 @@ fn test_f_get_path() { #[test] fn test_f_get_path_nofirmlink() { use nix::fcntl::*; - use std::{os::unix::io::AsRawFd, path::PathBuf}; + use std::path::PathBuf; let tmp = NamedTempFile::new().unwrap(); let mut path = PathBuf::new(); From a5180ef3a9165bf1e1c9f41f7f53a25a89e74a3a Mon Sep 17 00:00:00 2001 From: Steve Lau Date: Sun, 9 Jun 2024 11:26:17 +0800 Subject: [PATCH 06/10] chore: fix imports --- src/fcntl.rs | 1 + test/test_fcntl.rs | 2 +- test/test_pty.rs | 10 +++++++--- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/fcntl.rs b/src/fcntl.rs index 80c94fc13d..c86f22f49b 100644 --- a/src/fcntl.rs +++ b/src/fcntl.rs @@ -53,6 +53,7 @@ pub use self::posix_fadvise::{posix_fadvise, PosixFadviseAdvice}; // 1. `AT_FDCWD` is usable for the whole process life, so it is `'static`. // 2. It is not a valid file descriptor, but OS will handle it for us when passed // to `xxat(2)` calls. +#[cfg(not(target_os = "redox"))] // Redox does not have this pub const AT_FDCWD: BorrowedFd<'static> = unsafe { BorrowedFd::borrow_raw(libc::AT_FDCWD) }; diff --git a/test/test_fcntl.rs b/test/test_fcntl.rs index 1db8b44886..046058496b 100644 --- a/test/test_fcntl.rs +++ b/test/test_fcntl.rs @@ -632,7 +632,7 @@ fn test_f_get_path_nofirmlink() { #[test] fn test_f_kinfo() { use nix::fcntl::*; - use std::{os::unix::io::AsRawFd, path::PathBuf}; + use std::path::PathBuf; let tmp = NamedTempFile::new().unwrap(); // With TMPDIR set with UFS, the vnode name is not entered diff --git a/test/test_pty.rs b/test/test_pty.rs index 3f5137cb0d..b2ff730cf8 100644 --- a/test/test_pty.rs +++ b/test/test_pty.rs @@ -103,19 +103,23 @@ fn open_ptty_pair() -> (PtyMaster, File) { #[allow(clippy::comparison_chain)] { use libc::{ioctl, I_FIND, I_PUSH}; + use std::os::fd::AsRawFd; // On illumos systems, as per pts(7D), one must push STREAMS modules // after opening a device path returned from ptsname(). let ptem = b"ptem\0"; let ldterm = b"ldterm\0"; - let r = unsafe { ioctl(slave_fd, I_FIND, ldterm.as_ptr()) }; + let r = unsafe { ioctl(slave_fd.as_raw_fd(), I_FIND, ldterm.as_ptr()) }; if r < 0 { panic!("I_FIND failure"); } else if r == 0 { - if unsafe { ioctl(slave_fd, I_PUSH, ptem.as_ptr()) } < 0 { + if unsafe { ioctl(slave_fd.as_raw_fd(), I_PUSH, ptem.as_ptr()) } < 0 + { panic!("I_PUSH ptem failure"); } - if unsafe { ioctl(slave_fd, I_PUSH, ldterm.as_ptr()) } < 0 { + if unsafe { ioctl(slave_fd.as_raw_fd(), I_PUSH, ldterm.as_ptr()) } + < 0 + { panic!("I_PUSH ldterm failure"); } } From 0651ea45fe532df97158cdb3b43bddf90c491d50 Mon Sep 17 00:00:00 2001 From: Steve Lau Date: Sun, 9 Jun 2024 11:37:59 +0800 Subject: [PATCH 07/10] chore: fix imports --- src/fcntl.rs | 19 +++++++++---------- test/test_pty.rs | 1 - 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/fcntl.rs b/src/fcntl.rs index c86f22f49b..bc8328e239 100644 --- a/src/fcntl.rs +++ b/src/fcntl.rs @@ -13,7 +13,6 @@ use std::ffi::CStr; use std::ffi::OsString; #[cfg(not(any(target_os = "redox", target_os = "solaris")))] use std::ops::{Deref, DerefMut}; -use std::os::fd::BorrowedFd; #[cfg(not(target_os = "redox"))] use std::os::raw; use std::os::unix::ffi::OsStringExt; @@ -30,9 +29,6 @@ use std::path::PathBuf; #[cfg(any(linux_android, target_os = "freebsd"))] use std::ptr; -#[cfg(feature = "fs")] -use std::os::fd::FromRawFd; - #[cfg(feature = "fs")] use crate::{sys::stat::Mode, NixPath, Result}; @@ -54,8 +50,8 @@ pub use self::posix_fadvise::{posix_fadvise, PosixFadviseAdvice}; // 2. It is not a valid file descriptor, but OS will handle it for us when passed // to `xxat(2)` calls. #[cfg(not(target_os = "redox"))] // Redox does not have this -pub const AT_FDCWD: BorrowedFd<'static> = - unsafe { BorrowedFd::borrow_raw(libc::AT_FDCWD) }; +pub const AT_FDCWD: std::os::fd::BorrowedFd<'static> = + unsafe { std::os::fd::BorrowedFd::borrow_raw(libc::AT_FDCWD) }; #[cfg(not(target_os = "redox"))] #[cfg(any(feature = "fs", feature = "process", feature = "user"))] @@ -235,7 +231,9 @@ pub fn open( path: &P, oflag: OFlag, mode: Mode, -) -> Result { +) -> Result { + use std::os::fd::FromRawFd; + let fd = path.with_nix_path(|cstr| unsafe { libc::open(cstr.as_ptr(), oflag.bits(), mode.bits() as c_uint) })?; @@ -244,7 +242,7 @@ pub fn open( // SAFETY: // // `open(2)` should return a valid owned fd on success - Ok( unsafe { OwnedFd::from_raw_fd(fd) } ) + Ok( unsafe { std::os::fd::OwnedFd::from_raw_fd(fd) } ) } /// open or create a file for reading, writing or executing @@ -265,6 +263,7 @@ pub fn openat( mode: Mode, ) -> Result { use std::os::fd::AsRawFd; + use std::os::fd::FromRawFd; let fd = path.with_nix_path(|cstr| unsafe { libc::openat(dirfd.as_fd().as_raw_fd(), cstr.as_ptr(), oflag.bits(), mode.bits() as c_uint) @@ -1350,7 +1349,7 @@ impl SpacectlRange { /// f.write_all(INITIAL).unwrap(); /// let mut range = SpacectlRange(3, 6); /// while (!range.is_empty()) { -/// range = fspacectl(f.as_raw_fd(), range).unwrap(); +/// range = fspacectl(&f, range).unwrap(); /// } /// let mut buf = vec![0; INITIAL.len()]; /// f.read_exact_at(&mut buf, 0).unwrap(); @@ -1402,7 +1401,7 @@ pub fn fspacectl(fd: Fd, range: SpacectlRange) -> Result< /// const INITIAL: &[u8] = b"0123456789abcdef"; /// let mut f = tempfile().unwrap(); /// f.write_all(INITIAL).unwrap(); -/// fspacectl_all(f.as_raw_fd(), 3, 6).unwrap(); +/// fspacectl_all(&f., 3, 6).unwrap(); /// let mut buf = vec![0; INITIAL.len()]; /// f.read_exact_at(&mut buf, 0).unwrap(); /// assert_eq!(buf, b"012\0\0\0\0\0\09abcdef"); diff --git a/test/test_pty.rs b/test/test_pty.rs index b2ff730cf8..fcbb452d80 100644 --- a/test/test_pty.rs +++ b/test/test_pty.rs @@ -103,7 +103,6 @@ fn open_ptty_pair() -> (PtyMaster, File) { #[allow(clippy::comparison_chain)] { use libc::{ioctl, I_FIND, I_PUSH}; - use std::os::fd::AsRawFd; // On illumos systems, as per pts(7D), one must push STREAMS modules // after opening a device path returned from ptsname(). From 5bf789ceac8a6e1884011f5ad33844ed5c2d4194 Mon Sep 17 00:00:00 2001 From: Steve Lau Date: Sun, 9 Jun 2024 11:42:55 +0800 Subject: [PATCH 08/10] chore: remove a redundant dot from FreeBSD example --- src/fcntl.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fcntl.rs b/src/fcntl.rs index bc8328e239..bfc30c9bb3 100644 --- a/src/fcntl.rs +++ b/src/fcntl.rs @@ -1401,7 +1401,7 @@ pub fn fspacectl(fd: Fd, range: SpacectlRange) -> Result< /// const INITIAL: &[u8] = b"0123456789abcdef"; /// let mut f = tempfile().unwrap(); /// f.write_all(INITIAL).unwrap(); -/// fspacectl_all(&f., 3, 6).unwrap(); +/// fspacectl_all(&f, 3, 6).unwrap(); /// let mut buf = vec![0; INITIAL.len()]; /// f.read_exact_at(&mut buf, 0).unwrap(); /// assert_eq!(buf, b"012\0\0\0\0\0\09abcdef"); From ec5444f05c1ebd93e255593f7f16d24e214fd16b Mon Sep 17 00:00:00 2001 From: Steve Lau Date: Sun, 9 Jun 2024 14:25:46 +0800 Subject: [PATCH 09/10] docs: more examples --- src/dir.rs | 48 +++++++++++++++++++++++++++++++++++++++++------- src/fcntl.rs | 26 ++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 7 deletions(-) diff --git a/src/dir.rs b/src/dir.rs index c3078b2d75..eebb92d725 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -17,17 +17,38 @@ use libc::{dirent, readdir_r}; /// An open directory. /// -/// This is a lower-level interface than `std::fs::ReadDir`. Notable differences: -/// * can be opened from a file descriptor (as returned by `openat`, perhaps before knowing -/// if the path represents a file or directory). -/// * implements `AsRawFd`, so it can be passed to `fstat`, `openat`, etc. -/// The file descriptor continues to be owned by the `Dir`, so callers must not keep a `RawFd` -/// after the `Dir` is dropped. +/// This is a lower-level interface than [`std::fs::ReadDir`]. Notable differences: +/// * can be opened from a file descriptor (as returned by [`openat`][openat], +/// perhaps before knowing if the path represents a file or directory). +/// * implements [`AsFd`][AsFd], so it can be passed to [`fstat`][fstat], +/// [`openat`][openat], etc. The file descriptor continues to be owned by the +/// `Dir`, so callers must not keep a `RawFd` after the `Dir` is dropped. /// * can be iterated through multiple times without closing and reopening the file /// descriptor. Each iteration rewinds when finished. /// * returns entries for `.` (current directory) and `..` (parent directory). -/// * returns entries' names as a `CStr` (no allocation or conversion beyond whatever libc +/// * returns entries' names as a [`CStr`][cstr] (no allocation or conversion beyond whatever libc /// does). +/// +/// [AsFd]: std::os::fd::AsFd +/// [fstat]: crate::sys::stat::fstat +/// [openat]: crate::fcntl::openat +/// [cstr]: std::ffi::CStr +/// +/// # Examples +/// +/// Traverse the current directory, and print entries' names: +/// +/// ``` +/// use nix::dir::Dir; +/// use nix::fcntl::OFlag; +/// use nix::sys::stat::Mode; +/// +/// let mut cwd = Dir::open(".", OFlag::O_RDONLY | OFlag::O_CLOEXEC, Mode::empty()).unwrap(); +/// for res_entry in cwd.iter() { +/// let entry = res_entry.unwrap(); +/// println!("File name: {}", entry.file_name().to_str().unwrap()); +/// } +/// ``` #[derive(Debug, Eq, Hash, PartialEq)] pub struct Dir(ptr::NonNull); @@ -75,6 +96,19 @@ impl Dir { } /// Converts from a file descriptor, closing it on failure. + /// + /// # Examples + /// + /// `ENOTDIR` would be returned if `fd` does not refer to a directory: + /// + /// ```should_panic + /// use std::os::fd::OwnedFd; + /// use nix::dir::Dir; + /// + /// let temp_file = tempfile::tempfile().unwrap(); + /// let temp_file_fd: OwnedFd = temp_file.into(); + /// let never = Dir::from_fd(temp_file_fd).unwrap(); + /// ``` #[doc(alias("fdopendir"))] pub fn from_fd(fd: std::os::fd::OwnedFd) -> Result { // take the ownership as the constructed `Dir` is now the owner diff --git a/src/fcntl.rs b/src/fcntl.rs index bfc30c9bb3..ab82e4a73d 100644 --- a/src/fcntl.rs +++ b/src/fcntl.rs @@ -44,6 +44,32 @@ use crate::{sys::stat::Mode, NixPath, Result}; pub use self::posix_fadvise::{posix_fadvise, PosixFadviseAdvice}; /// A file descriptor referring to the working directory of the current process. +/// +/// # Examples +/// +/// Use it in [`openat()`]: +/// +/// ```no_run +/// use nix::fcntl::AT_FDCWD; +/// use nix::fcntl::openat; +/// use nix::fcntl::OFlag; +/// use nix::sys::stat::Mode; +/// +/// let fd = openat(AT_FDCWD, "foo", OFlag::O_RDONLY | OFlag::O_CLOEXEC, Mode::empty()).unwrap(); +/// ``` +/// +/// # WARNING +/// +/// Do NOT pass this symbol to non-`xxat()` functions, it won't work: +/// +/// ```should_panic +/// use nix::errno::Errno; +/// use nix::fcntl::AT_FDCWD; +/// use nix::sys::stat::fstat; +/// use std::os::fd::AsRawFd; +/// +/// let never = fstat(AT_FDCWD.as_raw_fd()).unwrap(); +/// ``` // // SAFETY: // 1. `AT_FDCWD` is usable for the whole process life, so it is `'static`. From d6d7f40dfd68a1b6b01cdcf14d21fe20fab682a9 Mon Sep 17 00:00:00 2001 From: Steve Lau Date: Sun, 9 Jun 2024 14:27:47 +0800 Subject: [PATCH 10/10] chore: changelog entry --- changelog/2434.changed.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/2434.changed.md diff --git a/changelog/2434.changed.md b/changelog/2434.changed.md new file mode 100644 index 0000000000..a182edd50f --- /dev/null +++ b/changelog/2434.changed.md @@ -0,0 +1 @@ +Public interfaces in `fcntl.rs` and `dir.rs` now use I/O-safe types.