diff options
| -rw-r--r-- | CHANGELOG.md | 3 | ||||
| -rw-r--r-- | Cargo.toml | 4 | ||||
| -rw-r--r-- | src/kmod.rs | 2 | ||||
| -rw-r--r-- | src/pty.rs | 62 | ||||
| -rw-r--r-- | src/sched.rs | 6 | ||||
| -rw-r--r-- | src/sys/inotify.rs | 28 | ||||
| -rw-r--r-- | src/sys/mman.rs | 2 | ||||
| -rw-r--r-- | src/sys/statfs.rs | 2 | ||||
| -rw-r--r-- | src/sys/statvfs.rs | 6 | ||||
| -rw-r--r-- | src/sys/termios.rs | 49 | ||||
| -rw-r--r-- | src/sys/timerfd.rs | 44 | ||||
| -rw-r--r-- | src/sys/uio.rs | 28 | ||||
| -rw-r--r-- | test/sys/test_termios.rs | 59 | ||||
| -rw-r--r-- | test/sys/test_uio.rs | 29 | ||||
| -rw-r--r-- | test/test.rs | 8 | ||||
| -rw-r--r-- | test/test_pty.rs | 77 | ||||
| -rw-r--r-- | test/test_ptymaster_drop.rs | 20 | ||||
| -rw-r--r-- | test/test_unistd.rs | 14 |
18 files changed, 174 insertions, 269 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index f6ee0fc5..c75e875e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,9 @@ This project adheres to [Semantic Versioning](https://semver.org/). ([#1862](https://github.com/nix-rust/nix/pull/1862)) - The epoll interface now uses a type. ([#1882](https://github.com/nix-rust/nix/pull/1882)) +- With I/O-safe type applied in `pty::OpenptyResult` and `pty::ForkptyResult`, + users no longer need to manually close the file descriptors in these types. + ([#1921](https://github.com/nix-rust/nix/pull/1921)) ### Fixed ### Removed @@ -108,7 +108,3 @@ path = "test/test_clearenv.rs" name = "test-mount" path = "test/test_mount.rs" harness = false - -[[test]] -name = "test-ptymaster-drop" -path = "test/test_ptymaster_drop.rs" diff --git a/src/kmod.rs b/src/kmod.rs index d7146612..d3725c3f 100644 --- a/src/kmod.rs +++ b/src/kmod.rs @@ -80,7 +80,7 @@ libc_bitflags!( /// /// See [`man init_module(2)`](https://man7.org/linux/man-pages/man2/init_module.2.html) for more information. pub fn finit_module<Fd: AsFd>( - fd: &Fd, + fd: Fd, param_values: &CStr, flags: ModuleInitFlags, ) -> Result<()> { @@ -16,26 +16,24 @@ use crate::{fcntl, unistd, Result}; /// Representation of a master/slave pty pair /// -/// This is returned by `openpty`. Note that this type does *not* implement `Drop`, so the user -/// must manually close the file descriptors. -#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] +/// This is returned by [`openpty`]. +#[derive(Debug)] pub struct OpenptyResult { /// The master port in a virtual pty pair - pub master: RawFd, + pub master: OwnedFd, /// The slave port in a virtual pty pair - pub slave: RawFd, + pub slave: OwnedFd, } feature! { #![feature = "process"] /// Representation of a master with a forked pty /// -/// This is returned by `forkpty`. Note that this type does *not* implement `Drop`, so the user -/// must manually close the file descriptors. -#[derive(Clone, Copy, Debug)] +/// This is returned by [`forkpty`]. +#[derive(Debug)] pub struct ForkptyResult { /// The master port in a virtual pty pair - pub master: RawFd, + pub master: OwnedFd, /// Metadata about forked process pub fork_result: ForkResult, } @@ -43,51 +41,33 @@ pub struct ForkptyResult { /// Representation of the Master device in a master/slave pty pair /// -/// While this datatype is a thin wrapper around `RawFd`, it enforces that the available PTY -/// functions are given the correct file descriptor. Additionally this type implements `Drop`, -/// so that when it's consumed or goes out of scope, it's automatically cleaned-up. -#[derive(Debug, Eq, Hash, PartialEq)] -pub struct PtyMaster(RawFd); +/// While this datatype is a thin wrapper around `OwnedFd`, it enforces that the available PTY +/// functions are given the correct file descriptor. +#[derive(Debug)] +pub struct PtyMaster(OwnedFd); impl AsRawFd for PtyMaster { fn as_raw_fd(&self) -> RawFd { - self.0 + self.0.as_raw_fd() } } impl IntoRawFd for PtyMaster { fn into_raw_fd(self) -> RawFd { let fd = self.0; - mem::forget(self); - fd - } -} - -impl Drop for PtyMaster { - fn drop(&mut self) { - // On drop, we ignore errors like EINTR and EIO because there's no clear - // way to handle them, we can't return anything, and (on FreeBSD at - // least) the file descriptor is deallocated in these cases. However, - // we must panic on EBADF, because it is always an error to close an - // invalid file descriptor. That frequently indicates a double-close - // condition, which can cause confusing errors for future I/O - // operations. - let e = unistd::close(self.0); - if e == Err(Errno::EBADF) { - panic!("Closing an invalid file descriptor!"); - }; + fd.into_raw_fd() } } impl io::Read for PtyMaster { fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> { - unistd::read(self.0, buf).map_err(io::Error::from) + unistd::read(self.0.as_raw_fd(), buf).map_err(io::Error::from) } } impl io::Write for PtyMaster { fn write(&mut self, buf: &[u8]) -> io::Result<usize> { - unistd::write(self.0, buf).map_err(io::Error::from) + unistd::write(self.0.as_raw_fd(), buf).map_err(io::Error::from) } fn flush(&mut self) -> io::Result<()> { Ok(()) @@ -96,13 +76,13 @@ impl io::Write for PtyMaster { impl io::Read for &PtyMaster { fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> { - unistd::read(self.0, buf).map_err(io::Error::from) + unistd::read(self.0.as_raw_fd(), buf).map_err(io::Error::from) } } impl io::Write for &PtyMaster { fn write(&mut self, buf: &[u8]) -> io::Result<usize> { - unistd::write(self.0, buf).map_err(io::Error::from) + unistd::write(self.0.as_raw_fd(), buf).map_err(io::Error::from) } fn flush(&mut self) -> io::Result<()> { Ok(()) @@ -164,7 +144,7 @@ pub fn posix_openpt(flags: fcntl::OFlag) -> Result<PtyMaster> { return Err(Errno::last()); } - Ok(PtyMaster(fd)) + Ok(PtyMaster(unsafe { OwnedFd::from_raw_fd(fd) })) } /// Get the name of the slave pseudoterminal (see @@ -308,8 +288,8 @@ pub fn openpty< unsafe { Ok(OpenptyResult { - master: master.assume_init(), - slave: slave.assume_init(), + master: OwnedFd::from_raw_fd(master.assume_init()), + slave: OwnedFd::from_raw_fd(slave.assume_init()), }) } } @@ -364,7 +344,7 @@ pub unsafe fn forkpty<'a, 'b, T: Into<Option<&'a Winsize>>, U: Into<Option<&'b T })?; Ok(ForkptyResult { - master: master.assume_init(), + master: OwnedFd::from_raw_fd(master.assume_init()), fork_result, }) } diff --git a/src/sched.rs b/src/sched.rs index d5b1233c..0515e30f 100644 --- a/src/sched.rs +++ b/src/sched.rs @@ -16,7 +16,7 @@ mod sched_linux_like { use libc::{self, c_int, c_void}; use std::mem; use std::option::Option; - use std::os::unix::io::RawFd; + use std::os::unix::io::{AsFd, AsRawFd}; // For some functions taking with a parameter of type CloneFlags, // only a subset of these flags have an effect. @@ -136,8 +136,8 @@ mod sched_linux_like { /// reassociate thread with a namespace /// /// See also [setns(2)](https://man7.org/linux/man-pages/man2/setns.2.html) - pub fn setns(fd: RawFd, nstype: CloneFlags) -> Result<()> { - let res = unsafe { libc::setns(fd, nstype.bits()) }; + pub fn setns<Fd: AsFd>(fd: Fd, nstype: CloneFlags) -> Result<()> { + let res = unsafe { libc::setns(fd.as_fd().as_raw_fd(), nstype.bits()) }; Errno::result(res).map(drop) } diff --git a/src/sys/inotify.rs b/src/sys/inotify.rs index 84356ec7..2398c16e 100644 --- a/src/sys/inotify.rs +++ b/src/sys/inotify.rs @@ -32,7 +32,7 @@ use libc::{c_char, c_int}; use std::ffi::{CStr, OsStr, OsString}; use std::mem::{size_of, MaybeUninit}; use std::os::unix::ffi::OsStrExt; -use std::os::unix::io::{AsRawFd, FromRawFd, RawFd}; +use std::os::unix::io::{AsRawFd, FromRawFd, OwnedFd, RawFd}; use std::ptr; libc_bitflags! { @@ -101,9 +101,9 @@ libc_bitflags! { /// An inotify instance. This is also a file descriptor, you can feed it to /// other interfaces consuming file descriptors, epoll for example. -#[derive(Debug, Clone, Copy)] +#[derive(Debug)] pub struct Inotify { - fd: RawFd, + fd: OwnedFd, } /// This object is returned when you create a new watch on an inotify instance. @@ -143,7 +143,7 @@ impl Inotify { pub fn init(flags: InitFlags) -> Result<Inotify> { let res = Errno::result(unsafe { libc::inotify_init1(flags.bits()) }); - res.map(|fd| Inotify { fd }) + res.map(|fd| Inotify { fd: unsafe { OwnedFd::from_raw_fd(fd) } }) } /// Adds a new watch on the target file or directory. @@ -152,12 +152,12 @@ impl Inotify { /// /// For more information see, [inotify_add_watch(2)](https://man7.org/linux/man-pages/man2/inotify_add_watch.2.html). pub fn add_watch<P: ?Sized + NixPath>( - self, + &self, path: &P, mask: AddWatchFlags, ) -> Result<WatchDescriptor> { let res = path.with_nix_path(|cstr| unsafe { - libc::inotify_add_watch(self.fd, cstr.as_ptr(), mask.bits()) + libc::inotify_add_watch(self.fd.as_raw_fd(), cstr.as_ptr(), mask.bits()) })?; Errno::result(res).map(|wd| WatchDescriptor { wd }) @@ -169,7 +169,7 @@ impl Inotify { /// Returns an EINVAL error if the watch descriptor is invalid. /// /// For more information see, [inotify_rm_watch(2)](https://man7.org/linux/man-pages/man2/inotify_rm_watch.2.html). - pub fn rm_watch(self, wd: WatchDescriptor) -> Result<()> { + pub fn rm_watch(&self, wd: WatchDescriptor) -> Result<()> { cfg_if! { if #[cfg(target_os = "linux")] { let arg = wd.wd; @@ -177,7 +177,7 @@ impl Inotify { let arg = wd.wd as u32; } } - let res = unsafe { libc::inotify_rm_watch(self.fd, arg) }; + let res = unsafe { libc::inotify_rm_watch(self.fd.as_raw_fd(), arg) }; Errno::result(res).map(drop) } @@ -188,14 +188,14 @@ impl Inotify { /// /// Returns as many events as available. If the call was non blocking and no /// events could be read then the EAGAIN error is returned. - pub fn read_events(self) -> Result<Vec<InotifyEvent>> { + pub fn read_events(&self) -> Result<Vec<InotifyEvent>> { let header_size = size_of::<libc::inotify_event>(); const BUFSIZ: usize = 4096; let mut buffer = [0u8; BUFSIZ]; let mut events = Vec::new(); let mut offset = 0; - let nread = read(self.fd, &mut buffer)?; + let nread = read(self.fd.as_raw_fd(), &mut buffer)?; while (nread - offset) >= header_size { let event = unsafe { @@ -235,14 +235,8 @@ impl Inotify { } } -impl AsRawFd for Inotify { - fn as_raw_fd(&self) -> RawFd { - self.fd - } -} - impl FromRawFd for Inotify { unsafe fn from_raw_fd(fd: RawFd) -> Self { - Inotify { fd } + Inotify { fd: OwnedFd::from_raw_fd(fd) } } } diff --git a/src/sys/mman.rs b/src/sys/mman.rs index deef7005..e689e06e 100644 --- a/src/sys/mman.rs +++ b/src/sys/mman.rs @@ -421,7 +421,7 @@ pub unsafe fn mmap<F: AsFd>( length: NonZeroUsize, prot: ProtFlags, flags: MapFlags, - f: Option<&F>, + f: Option<F>, offset: off_t, ) -> Result<*mut c_void> { let ptr = diff --git a/src/sys/statfs.rs b/src/sys/statfs.rs index 721d45cb..5111df2e 100644 --- a/src/sys/statfs.rs +++ b/src/sys/statfs.rs @@ -740,7 +740,7 @@ pub fn statfs<P: ?Sized + NixPath>(path: &P) -> Result<Statfs> { /// # Arguments /// /// `fd` - File descriptor of any open file within the file system to describe -pub fn fstatfs<Fd: AsFd>(fd: &Fd) -> Result<Statfs> { +pub fn fstatfs<Fd: AsFd>(fd: Fd) -> Result<Statfs> { unsafe { let mut stat = mem::MaybeUninit::<type_of_statfs>::uninit(); Errno::result(LIBC_FSTATFS(fd.as_fd().as_raw_fd(), stat.as_mut_ptr())) diff --git a/src/sys/statvfs.rs b/src/sys/statvfs.rs index 8de369f4..c2c86624 100644 --- a/src/sys/statvfs.rs +++ b/src/sys/statvfs.rs @@ -3,7 +3,7 @@ //! See [the man pages](https://pubs.opengroup.org/onlinepubs/9699919799/functions/fstatvfs.html) //! for more details. use std::mem; -use std::os::unix::io::AsRawFd; +use std::os::unix::io::{AsFd, AsRawFd}; use libc::{self, c_ulong}; @@ -146,11 +146,11 @@ pub fn statvfs<P: ?Sized + NixPath>(path: &P) -> Result<Statvfs> { } /// Return a `Statvfs` object with information about `fd` -pub fn fstatvfs<T: AsRawFd>(fd: &T) -> Result<Statvfs> { +pub fn fstatvfs<Fd: AsFd>(fd: Fd) -> Result<Statvfs> { unsafe { Errno::clear(); let mut stat = mem::MaybeUninit::<libc::statvfs>::uninit(); - Errno::result(libc::fstatvfs(fd.as_raw_fd(), stat.as_mut_ptr())) + Errno::result(libc::fstatvfs(fd.as_fd().as_raw_fd(), stat.as_mut_ptr())) .map(|_| Statvfs(stat.assume_init())) } } diff --git a/src/sys/termios.rs b/src/sys/termios.rs index fba2cd82..b0286f51 100644 --- a/src/sys/termios.rs +++ b/src/sys/termios.rs @@ -222,7 +222,7 @@ use libc::{self, c_int, tcflag_t}; use std::cell::{Ref, RefCell}; use std::convert::From; use std::mem; -use std::os::unix::io::RawFd; +use std::os::unix::io::{AsFd, AsRawFd}; #[cfg(feature = "process")] use crate::unistd::Pid; @@ -1143,10 +1143,12 @@ pub fn cfmakesane(termios: &mut Termios) { /// `tcgetattr()` returns a `Termios` structure with the current configuration for a port. Modifying /// this structure *will not* reconfigure the port, instead the modifications should be done to /// the `Termios` structure and then the port should be reconfigured using `tcsetattr()`. -pub fn tcgetattr(fd: RawFd) -> Result<Termios> { +pub fn tcgetattr<Fd: AsFd>(fd: Fd) -> Result<Termios> { let mut termios = mem::MaybeUninit::uninit(); - let res = unsafe { libc::tcgetattr(fd, termios.as_mut_ptr()) }; + let res = unsafe { + libc::tcgetattr(fd.as_fd().as_raw_fd(), termios.as_mut_ptr()) + }; Errno::result(res)?; @@ -1159,18 +1161,26 @@ pub fn tcgetattr(fd: RawFd) -> Result<Termios> { /// `tcsetattr()` reconfigures the given port based on a given `Termios` structure. This change /// takes affect at a time specified by `actions`. Note that this function may return success if /// *any* of the parameters were successfully set, not only if all were set successfully. -pub fn tcsetattr(fd: RawFd, actions: SetArg, termios: &Termios) -> Result<()> { +pub fn tcsetattr<Fd: AsFd>( + fd: Fd, + actions: SetArg, + termios: &Termios, +) -> Result<()> { let inner_termios = termios.get_libc_termios(); Errno::result(unsafe { - libc::tcsetattr(fd, actions as c_int, &*inner_termios) + libc::tcsetattr( + fd.as_fd().as_raw_fd(), + actions as c_int, + &*inner_termios, + ) }) .map(drop) } /// Block until all output data is written (see /// [tcdrain(3p)](https://pubs.opengroup.org/onlinepubs/9699919799/functions/tcdrain.html)). -pub fn tcdrain(fd: RawFd) -> Result<()> { - Errno::result(unsafe { libc::tcdrain(fd) }).map(drop) +pub fn tcdrain<Fd: AsFd>(fd: Fd) -> Result<()> { + Errno::result(unsafe { libc::tcdrain(fd.as_fd().as_raw_fd()) }).map(drop) } /// Suspend or resume the transmission or reception of data (see @@ -1178,8 +1188,11 @@ pub fn tcdrain(fd: RawFd) -> Result<()> { /// /// `tcflow()` suspends of resumes the transmission or reception of data for the given port /// depending on the value of `action`. -pub fn tcflow(fd: RawFd, action: FlowArg) -> Result<()> { - Errno::result(unsafe { libc::tcflow(fd, action as c_int) }).map(drop) +pub fn tcflow<Fd: AsFd>(fd: Fd, action: FlowArg) -> Result<()> { + Errno::result(unsafe { + libc::tcflow(fd.as_fd().as_raw_fd(), action as c_int) + }) + .map(drop) } /// Discard data in the output or input queue (see @@ -1187,8 +1200,11 @@ pub fn tcflow(fd: RawFd, action: FlowArg) -> Result<()> { /// /// `tcflush()` will discard data for a terminal port in the input queue, output queue, or both /// depending on the value of `action`. -pub fn tcflush(fd: RawFd, action: FlushArg) -> Result<()> { - Errno::result(unsafe { libc::tcflush(fd, action as c_int) }).map(drop) +pub fn tcflush<Fd: AsFd>(fd: Fd, action: FlushArg) -> Result<()> { + Errno::result(unsafe { + libc::tcflush(fd.as_fd().as_raw_fd(), action as c_int) + }) + .map(drop) } /// Send a break for a specific duration (see @@ -1196,16 +1212,19 @@ pub fn tcflush(fd: RawFd, action: FlushArg) -> Result<()> { /// /// When using asynchronous data transmission `tcsendbreak()` will transmit a continuous stream /// of zero-valued bits for an implementation-defined duration. -pub fn tcsendbreak(fd: RawFd, duration: c_int) -> Result<()> { - Errno::result(unsafe { libc::tcsendbreak(fd, duration) }).map(drop) +pub fn tcsendbreak<Fd: AsFd>(fd: Fd, duration: c_int) -> Result<()> { + Errno::result(unsafe { + libc::tcsendbreak(fd.as_fd().as_raw_fd(), duration) + }) + .map(drop) } feature! { #![feature = "process"] /// Get the session controlled by the given terminal (see /// [tcgetsid(3)](https://pubs.opengroup.org/onlinepubs/9699919799/functions/tcgetsid.html)). -pub fn tcgetsid(fd: RawFd) -> Result<Pid> { - let res = unsafe { libc::tcgetsid(fd) }; +pub fn tcgetsid<Fd: AsFd>(fd: Fd) -> Result<Pid> { + let res = unsafe { libc::tcgetsid(fd.as_fd().as_raw_fd()) }; Errno::result(res).map(Pid::from_raw) } diff --git a/src/sys/timerfd.rs b/src/sys/timerfd.rs index a35fc927..90a05a8c 100644 --- a/src/sys/timerfd.rs +++ b/src/sys/timerfd.rs @@ -33,24 +33,28 @@ pub use crate::sys::time::timer::{Expiration, TimerSetTimeFlags}; use crate::unistd::read; use crate::{errno::Errno, Result}; use libc::c_int; -use std::os::unix::io::{AsRawFd, FromRawFd, RawFd}; +use std::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, OwnedFd, RawFd}; /// A timerfd instance. This is also a file descriptor, you can feed it to -/// other interfaces consuming file descriptors, epoll for example. +/// other interfaces taking file descriptors as arguments, [`epoll`] for example. +/// +/// [`epoll`]: crate::sys::epoll #[derive(Debug)] pub struct TimerFd { - fd: RawFd, + fd: OwnedFd, } -impl AsRawFd for TimerFd { - fn as_raw_fd(&self) -> RawFd { - self.fd +impl AsFd for TimerFd { + fn as_fd(&self) -> BorrowedFd<'_> { + self.fd.as_fd() } } impl FromRawFd for TimerFd { unsafe fn from_raw_fd(fd: RawFd) -> Self { - TimerFd { fd } + TimerFd { + fd: OwnedFd::from_raw_fd(fd), + } } } @@ -97,7 +101,9 @@ impl TimerFd { Errno::result(unsafe { libc::timerfd_create(clockid as i32, flags.bits()) }) - .map(|fd| Self { fd }) + .map(|fd| Self { + fd: unsafe { OwnedFd::from_raw_fd(fd) }, + }) } /// Sets a new alarm on the timer. @@ -145,7 +151,7 @@ impl TimerFd { let timerspec: TimerSpec = expiration.into(); Errno::result(unsafe { libc::timerfd_settime( - self.fd, + self.fd.as_fd().as_raw_fd(), flags.bits(), timerspec.as_ref(), std::ptr::null_mut(), @@ -159,7 +165,10 @@ impl TimerFd { pub fn get(&self) -> Result<Option<Expiration>> { let mut timerspec = TimerSpec::none(); Errno::result(unsafe { - libc::timerfd_gettime(self.fd, timerspec.as_mut()) + libc::timerfd_gettime( + self.fd.as_fd().as_raw_fd(), + timerspec.as_mut(), + ) }) .map(|_| { if timerspec.as_ref().it_interval.tv_sec == 0 @@ -179,7 +188,7 @@ impl TimerFd { pub fn unset(&self) -> Result<()> { Errno::result(unsafe { libc::timerfd_settime( - self.fd, + self.fd.as_fd().as_raw_fd(), TimerSetTimeFlags::empty().bits(), TimerSpec::none().as_ref(), std::ptr::null_mut(), @@ -192,7 +201,7 @@ impl TimerFd { /// /// Note: If the alarm is unset, then you will wait forever. pub fn wait(&self) -> Result<()> { - while let Err(e) = read(self.fd, &mut [0u8; 8]) { + while let Err(e) = read(self.fd.as_fd().as_raw_fd(), &mut [0u8; 8]) { if e != Errno::EINTR { return Err(e); } @@ -201,14 +210,3 @@ impl TimerFd { Ok(()) } } - -impl Drop for TimerFd { - fn drop(&mut self) { - if !std::thread::panicking() { - let result = Errno::result(unsafe { libc::close(self.fd) }); - if let Err(Errno::EBADF) = result { - panic!("close of TimerFd encountered EBADF"); - } - } - } -} diff --git a/src/sys/uio.rs b/src/sys/uio.rs index 7248bd0c..ce0fb54d 100644 --- a/src/sys/uio.rs +++ b/src/sys/uio.rs @@ -4,12 +4,12 @@ use crate::errno::Errno; use crate::Result; use libc::{self, c_int, c_void, off_t, size_t}; use std::io::{IoSlice, IoSliceMut}; -use std::os::unix::io::RawFd; +use std::os::unix::io::{AsFd, AsRawFd}; /// Low-level vectored write to a raw file descriptor /// /// See also [writev(2)](https://pubs.opengroup.org/onlinepubs/9699919799/functions/writev.html) -pub fn writev(fd: RawFd, iov: &[IoSlice<'_>]) -> Result<usize> { +pub fn writev<Fd: AsFd>(fd: Fd, iov: &[IoSlice<'_>]) -> Result<usize> { // SAFETY: to quote the documentation for `IoSlice`: // // [IoSlice] is semantically a wrapper around a &[u8], but is @@ -18,7 +18,7 @@ pub fn writev(fd: RawFd, iov: &[IoSlice<'_>]) -> Result<usize> { // // Because it is ABI compatible, a pointer cast here is valid let res = unsafe { - libc::writev(fd, iov.as_ptr() as *const libc::iovec, iov.len() as c_int) + libc::writev(fd.as_fd().as_raw_fd(), iov.as_ptr() as *const libc::iovec, iov.len() as c_int) }; Errno::result(res).map(|r| r as usize) @@ -27,10 +27,10 @@ pub fn writev(fd: RawFd, iov: &[IoSlice<'_>]) -> Result<usize> { /// Low-level vectored read from a raw file descriptor /// /// See also [readv(2)](https://pubs.opengroup.org/onlinepubs/9699919799/functions/readv.html) -pub fn readv(fd: RawFd, iov: &mut [IoSliceMut<'_>]) -> Result<usize> { +pub fn readv<Fd: AsFd>(fd: Fd, iov: &mut [IoSliceMut<'_>]) -> Result<usize> { // SAFETY: same as in writev(), IoSliceMut is ABI-compatible with iovec let res = unsafe { - libc::readv(fd, iov.as_ptr() as *const libc::iovec, iov.len() as c_int) + libc::readv(fd.as_fd().as_raw_fd(), iov.as_ptr() as *const libc::iovec, iov.len() as c_int) }; Errno::result(res).map(|r| r as usize) @@ -44,14 +44,14 @@ pub fn readv(fd: RawFd, iov: &mut [IoSliceMut<'_>]) -> Result<usize> { /// See also: [`writev`](fn.writev.html) and [`pwrite`](fn.pwrite.html) #[cfg(not(any(target_os = "redox", target_os = "haiku")))] #[cfg_attr(docsrs, doc(cfg(all())))] -pub fn pwritev(fd: RawFd, iov: &[IoSlice<'_>], offset: off_t) -> Result<usize> { +pub fn pwritev<Fd: AsFd>(fd: Fd, iov: &[IoSlice<'_>], offset: off_t) -> Result<usize> { #[cfg(target_env = "uclibc")] let offset = offset as libc::off64_t; // uclibc doesn't use off_t // SAFETY: same as in writev() let res = unsafe { libc::pwritev( - fd, + fd.as_fd().as_raw_fd(), iov.as_ptr() as *const libc::iovec, iov.len() as c_int, offset, @@ -70,8 +70,8 @@ pub fn pwritev(fd: RawFd, iov: &[IoSlice<'_>], offset: off_t) -> Result<usize> { /// See also: [`readv`](fn.readv.html) and [`pread`](fn.pread.html) #[cfg(not(any(target_os = "redox", target_os = "haiku")))] #[cfg_attr(docsrs, doc(cfg(all())))] -pub fn preadv( - fd: RawFd, +pub fn preadv<Fd: AsFd>( + fd: Fd, iov: &mut [IoSliceMut<'_>], offset: off_t, ) -> Result<usize> { @@ -81,7 +81,7 @@ pub fn preadv( // SAFETY: same as in readv() let res = unsafe { libc::preadv( - fd, + fd.as_fd().as_raw_fd(), iov.as_ptr() as *const libc::iovec, iov.len() as c_int, offset, @@ -95,10 +95,10 @@ pub fn preadv( /// /// See also [pwrite(2)](https://pubs.opengroup.org/onlinepubs/9699919799/functions/pwrite.html) // TODO: move to unistd -pub fn pwrite(fd: RawFd, buf: &[u8], offset: off_t) -> Result<usize> { +pub fn pwrite<Fd: AsFd>(fd: Fd, buf: &[u8], offset: off_t) -> Result<usize> { let res = unsafe { libc::pwrite( - fd, + fd.as_fd().as_raw_fd(), buf.as_ptr() as *const c_void, buf.len() as size_t, offset, @@ -112,10 +112,10 @@ pub fn pwrite(fd: RawFd, buf: &[u8], offset: off_t) -> Result<usize> { /// /// See also [pread(2)](https://pubs.opengroup.org/onlinepubs/9699919799/functions/pread.html) // TODO: move to unistd -pub fn pread(fd: RawFd, buf: &mut [u8], offset: off_t) -> Result<usize> { +pub fn pread<Fd: AsFd>(fd: Fd, buf: &mut [u8], offset: off_t) -> Result<usize> { let res = unsafe { libc::pread( - fd, + fd.as_fd().as_raw_fd(), buf.as_mut_ptr() as *mut c_void, buf.len() as size_t, offset, diff --git a/test/sys/test_termios.rs b/test/sys/test_termios.rs index aaf00084..83919378 100644 --- a/test/sys/test_termios.rs +++ b/test/sys/test_termios.rs @@ -1,17 +1,17 @@ -use std::os::unix::prelude::*; +use std::os::unix::io::{AsFd, AsRawFd}; use tempfile::tempfile; use nix::errno::Errno; use nix::fcntl; use nix::pty::openpty; use nix::sys::termios::{self, tcgetattr, LocalFlags, OutputFlags}; -use nix::unistd::{close, read, write}; +use nix::unistd::{read, write}; -/// Helper function analogous to `std::io::Write::write_all`, but for `RawFd`s -fn write_all(f: RawFd, buf: &[u8]) { +/// Helper function analogous to `std::io::Write::write_all`, but for `Fd`s +fn write_all<Fd: AsFd>(f: Fd, buf: &[u8]) { let mut len = 0; while len < buf.len() { - len += write(f, &buf[len..]).unwrap(); + len += write(f.as_fd().as_raw_fd(), &buf[len..]).unwrap(); } } @@ -22,25 +22,14 @@ fn test_tcgetattr_pty() { let _m = crate::PTSNAME_MTX.lock(); let pty = openpty(None, None).expect("openpty failed"); - termios::tcgetattr(pty.slave).unwrap(); - close(pty.master).expect("closing the master failed"); - close(pty.slave).expect("closing the slave failed"); + termios::tcgetattr(&pty.slave).unwrap(); } // Test tcgetattr on something that isn't a terminal #[test] fn test_tcgetattr_enotty() { let file = tempfile().unwrap(); - assert_eq!( - termios::tcgetattr(file.as_raw_fd()).err(), - Some(Errno::ENOTTY) - ); -} - -// Test tcgetattr on an invalid file descriptor -#[test] -fn test_tcgetattr_ebadf() { - assert_eq!(termios::tcgetattr(-1).err(), Some(Errno::EBADF)); + assert_eq!(termios::tcgetattr(&file).err(), Some(Errno::ENOTTY)); } // Test modifying output flags @@ -52,12 +41,7 @@ fn test_output_flags() { // Open one pty to get attributes for the second one let mut termios = { let pty = openpty(None, None).expect("openpty failed"); - assert!(pty.master > 0); - assert!(pty.slave > 0); - let termios = tcgetattr(pty.slave).expect("tcgetattr failed"); - close(pty.master).unwrap(); - close(pty.slave).unwrap(); - termios + tcgetattr(&pty.slave).expect("tcgetattr failed") }; // Make sure postprocessing '\r' isn't specified by default or this test is useless. @@ -73,19 +57,15 @@ fn test_output_flags() { // Open a pty let pty = openpty(None, &termios).unwrap(); - assert!(pty.master > 0); - assert!(pty.slave > 0); // Write into the master let string = "foofoofoo\r"; - write_all(pty.master, string.as_bytes()); + write_all(&pty.master, string.as_bytes()); // Read from the slave verifying that the output has been properly transformed let mut buf = [0u8; 10]; - crate::read_exact(pty.slave, &mut buf); + crate::read_exact(&pty.slave, &mut buf); let transformed_string = "foofoofoo\n"; - close(pty.master).unwrap(); - close(pty.slave).unwrap(); assert_eq!(&buf, transformed_string.as_bytes()); } @@ -98,12 +78,7 @@ fn test_local_flags() { // Open one pty to get attributes for the second one let mut termios = { let pty = openpty(None, None).unwrap(); - assert!(pty.master > 0); - assert!(pty.slave > 0); - let termios = tcgetattr(pty.slave).unwrap(); - close(pty.master).unwrap(); - close(pty.slave).unwrap(); - termios + tcgetattr(&pty.slave).unwrap() }; // Make sure echo is specified by default or this test is useless. @@ -114,23 +89,19 @@ fn test_local_flags() { // Open a new pty with our modified termios settings let pty = openpty(None, &termios).unwrap(); - assert!(pty.master > 0); - assert!(pty.slave > 0); // Set the master is in nonblocking mode or reading will never return. - let flags = fcntl::fcntl(pty.master, fcntl::F_GETFL).unwrap(); + let flags = fcntl::fcntl(pty.master.as_raw_fd(), fcntl::F_GETFL).unwrap(); let new_flags = fcntl::OFlag::from_bits_truncate(flags) | fcntl::OFlag::O_NONBLOCK; - fcntl::fcntl(pty.master, fcntl::F_SETFL(new_flags)).unwrap(); + fcntl::fcntl(pty.master.as_raw_fd(), fcntl::F_SETFL(new_flags)).unwrap(); // Write into the master let string = "foofoofoo\r"; - write_all(pty.master, string.as_bytes()); + write_all(&pty.master, string.as_bytes()); // Try to read from the master, which should not have anything as echoing was disabled. let mut buf = [0u8; 10]; - let read = read(pty.master, &mut buf).unwrap_err(); - close(pty.master).unwrap(); - close(pty.slave).unwrap(); + let read = read(pty.master.as_raw_fd(), &mut buf).unwrap_err(); assert_eq!(read, Errno::EAGAIN); } diff --git a/test/sys/test_uio.rs b/test/sys/test_uio.rs index 0f4b8a65..fc09465f 100644 --- a/test/sys/test_uio.rs +++ b/test/sys/test_uio.rs @@ -4,7 +4,7 @@ use rand::distributions::Alphanumeric; use rand::{thread_rng, Rng}; use std::fs::OpenOptions; use std::io::IoSlice; -use std::os::unix::io::AsRawFd; +use std::os::unix::io::{FromRawFd, OwnedFd}; use std::{cmp, iter}; #[cfg(not(target_os = "redox"))] @@ -40,12 +40,16 @@ fn test_writev() { iovecs.push(IoSlice::new(b)); consumed += slice_len; } - let pipe_res = pipe(); - let (reader, writer) = pipe_res.expect("Couldn't create pipe"); + let (reader, writer) = pipe().expect("Couldn't create pipe"); // FileDesc will close its filedesc (reader). let mut read_buf: Vec<u8> = iter::repeat(0u8).take(128 * 16).collect(); + + // Temporary workaround to cope with the existing RawFd pipe(2), should be + // removed when pipe(2) becomes I/O-safe. + let writer = unsafe { OwnedFd::from_raw_fd(writer) }; + // Blocking io, should write all data. - let write_res = writev(writer, &iovecs); + let write_res = writev(&writer, &iovecs); let written = write_res.expect("couldn't write"); // Check whether we written all data assert_eq!(to_write.len(), written); @@ -55,7 +59,6 @@ fn test_writev() { assert_eq!(read, written); // Check equality of written and read data assert_eq!(&to_write, &read_buf); - close(writer).expect("closed writer"); close(reader).expect("closed reader"); } @@ -88,7 +91,12 @@ fn test_readv() { let (reader, writer) = pipe().expect("couldn't create pipe"); // Blocking io, should write all data. write(writer, &to_write).expect("write failed"); - let read = readv(reader, &mut iovecs[..]).expect("read failed"); + + // Temporary workaround to cope with the existing RawFd pipe(2), should be + // removed when pipe(2) becomes I/O-safe. + let reader = unsafe { OwnedFd::from_raw_fd(reader) }; + + let read = readv(&reader, &mut iovecs[..]).expect("read failed"); // Check whether we've read all data assert_eq!(to_write.len(), read); // Cccumulate data from iovecs @@ -100,7 +108,6 @@ fn test_readv() { assert_eq!(read_buf.len(), to_write.len()); // Check equality of written and read data assert_eq!(&read_buf, &to_write); - close(reader).expect("couldn't close reader"); close(writer).expect("couldn't close writer"); } @@ -111,7 +118,7 @@ fn test_pwrite() { let mut file = tempfile().unwrap(); let buf = [1u8; 8]; - assert_eq!(Ok(8), pwrite(file.as_raw_fd(), &buf, 8)); + assert_eq!(Ok(8), pwrite(&file, &buf, 8)); let mut file_content = Vec::new(); file.read_to_end(&mut file_content).unwrap(); let mut expected = vec![0u8; 8]; @@ -137,7 +144,7 @@ fn test_pread() { file.write_all(&file_content).unwrap(); let mut buf = [0u8; 16]; - assert_eq!(Ok(16), pread(file.as_raw_fd(), &mut buf, 16)); + assert_eq!(Ok(16), pread(&file, &mut buf, 16)); let expected: Vec<_> = (16..32).collect(); assert_eq!(&buf[..], &expected[..]); } @@ -168,7 +175,7 @@ fn test_pwritev() { .open(path) .unwrap(); - let written = pwritev(file.as_raw_fd(), &iovecs, 100).ok().unwrap(); + let written = pwritev(&file, &iovecs, 100).ok().unwrap(); assert_eq!(written, to_write.len()); // Read the data back and make sure it matches @@ -206,7 +213,7 @@ fn test_preadv() { .iter_mut() .map(|buf| IoSliceMut::new(&mut buf[..])) .collect(); - assert_eq!(Ok(100), preadv(file.as_raw_fd(), &mut iovecs, 100)); + assert_eq!(Ok(100), preadv(&file, &mut iovecs, 100)); } let all = buffers.concat(); diff --git a/test/test.rs b/test/test.rs index 6b42aad9..efd5fd2a 100644 --- a/test/test.rs +++ b/test/test.rs @@ -66,16 +66,16 @@ mod test_unistd; use nix::unistd::{chdir, getcwd, read}; use parking_lot::{Mutex, RwLock, RwLockWriteGuard}; -use std::os::unix::io::RawFd; +use std::os::unix::io::{AsFd, AsRawFd}; use std::path::PathBuf; -/// Helper function analogous to `std::io::Read::read_exact`, but for `RawFD`s -fn read_exact(f: RawFd, buf: &mut [u8]) { +/// Helper function analogous to `std::io::Read::read_exact`, but for `Fd`s +fn read_exact<Fd: AsFd>(f: Fd, buf: &mut [u8]) { let mut len = 0; while len < buf.len() { // get_mut would be better than split_at_mut, but it requires nightly let (_, remaining) = buf.split_at_mut(len); - len += read(f, remaining).unwrap(); + len += read(f.as_fd().as_raw_fd(), remaining).unwrap(); } } diff --git a/test/test_pty.rs b/test/test_pty.rs index 5c27e2d6..4cc6620c 100644 --- a/test/test_pty.rs +++ b/test/test_pty.rs @@ -2,28 +2,13 @@ use std::fs::File; use std::io::{Read, Write}; use std::os::unix::prelude::*; use std::path::Path; -use tempfile::tempfile; use libc::{_exit, STDOUT_FILENO}; use nix::fcntl::{open, OFlag}; use nix::pty::*; use nix::sys::stat; use nix::sys::termios::*; -use nix::unistd::{close, pause, write}; - -/// Regression test for Issue #659 -/// This is the correct way to explicitly close a `PtyMaster` -#[test] -fn test_explicit_close() { - let mut f = { - let m = posix_openpt(OFlag::O_RDWR).unwrap(); - close(m.into_raw_fd()).unwrap(); - tempfile().unwrap() - }; - // This should work. But if there's been a double close, then it will - // return EBADF - f.write_all(b"whatever").unwrap(); -} +use nix::unistd::{pause, write}; /// Test equivalence of `ptsname` and `ptsname_r` #[test] @@ -50,7 +35,6 @@ fn test_ptsname_copy() { // Open a new PTTY master let master_fd = posix_openpt(OFlag::O_RDWR).unwrap(); - assert!(master_fd.as_raw_fd() > 0); // Get the name of the slave let slave_name1 = unsafe { ptsname(&master_fd) }.unwrap(); @@ -67,7 +51,6 @@ fn test_ptsname_copy() { fn test_ptsname_r_copy() { // Open a new PTTY master let master_fd = posix_openpt(OFlag::O_RDWR).unwrap(); - assert!(master_fd.as_raw_fd() > 0); // Get the name of the slave let slave_name1 = ptsname_r(&master_fd).unwrap(); @@ -84,11 +67,9 @@ fn test_ptsname_unique() { // Open a new PTTY master let master1_fd = posix_openpt(OFlag::O_RDWR).unwrap(); - assert!(master1_fd.as_raw_fd() > 0); // Open a second PTTY master let master2_fd = posix_openpt(OFlag::O_RDWR).unwrap(); - assert!(master2_fd.as_raw_fd() > 0); // Get the name of the slave let slave_name1 = unsafe { ptsname(&master1_fd) }.unwrap(); @@ -147,26 +128,24 @@ fn open_ptty_pair() -> (PtyMaster, File) { /// /// This uses a common `open_ptty_pair` because much of these functions aren't useful by /// themselves. So for this test we perform the basic act of getting a file handle for a -/// master/slave PTTY pair, then just sanity-check the raw values. +/// master/slave PTTY pair. #[test] fn test_open_ptty_pair() { - let (master, slave) = open_ptty_pair(); - assert!(master.as_raw_fd() > 0); - assert!(slave.as_raw_fd() > 0); + let (_, _) = open_ptty_pair(); } /// Put the terminal in raw mode. -fn make_raw(fd: RawFd) { - let mut termios = tcgetattr(fd).unwrap(); +fn make_raw<Fd: AsFd>(fd: Fd) { + let mut termios = tcgetattr(&fd).unwrap(); cfmakeraw(&mut termios); - tcsetattr(fd, SetArg::TCSANOW, &termios).unwrap(); + tcsetattr(&fd, SetArg::TCSANOW, &termios).unwrap(); } /// Test `io::Read` on the PTTY master #[test] fn test_read_ptty_pair() { let (mut master, mut slave) = open_ptty_pair(); - make_raw(slave.as_raw_fd()); + make_raw(&slave); let mut buf = [0u8; 5]; slave.write_all(b"hello").unwrap(); @@ -183,7 +162,7 @@ fn test_read_ptty_pair() { #[test] fn test_write_ptty_pair() { let (mut master, mut slave) = open_ptty_pair(); - make_raw(slave.as_raw_fd()); + make_raw(&slave); let mut buf = [0u8; 5]; master.write_all(b"adios").unwrap(); @@ -202,33 +181,28 @@ fn test_openpty() { let _m = crate::PTSNAME_MTX.lock(); let pty = openpty(None, None).unwrap(); - assert!(pty.master > 0); - assert!(pty.slave > 0); // Writing to one should be readable on the other one let string = "foofoofoo\n"; let mut buf = [0u8; 10]; - write(pty.master, string.as_bytes()).unwrap(); - crate::read_exact(pty.slave, &mut buf); + write(pty.master.as_raw_fd(), string.as_bytes()).unwrap(); + crate::read_exact(&pty.slave, &mut buf); assert_eq!(&buf, string.as_bytes()); // Read the echo as well let echoed_string = "foofoofoo\r\n"; let mut buf = [0u8; 11]; - crate::read_exact(pty.master, &mut buf); + crate::read_exact(&pty.master, &mut buf); assert_eq!(&buf, echoed_string.as_bytes()); let string2 = "barbarbarbar\n"; let echoed_string2 = "barbarbarbar\r\n"; let mut buf = [0u8; 14]; - write(pty.slave, string2.as_bytes()).unwrap(); - crate::read_exact(pty.master, &mut buf); + write(pty.slave.as_raw_fd(), string2.as_bytes()).unwrap(); + crate::read_exact(&pty.master, &mut buf); assert_eq!(&buf, echoed_string2.as_bytes()); - - close(pty.master).unwrap(); - close(pty.slave).unwrap(); } #[test] @@ -239,44 +213,34 @@ fn test_openpty_with_termios() { // Open one pty to get attributes for the second one let mut termios = { let pty = openpty(None, None).unwrap(); - assert!(pty.master > 0); - assert!(pty.slave > 0); - let termios = tcgetattr(pty.slave).unwrap(); - close(pty.master).unwrap(); - close(pty.slave).unwrap(); - termios + tcgetattr(&pty.slave).unwrap() }; // Make sure newlines are not transformed so the data is preserved when sent. termios.output_flags.remove(OutputFlags::ONLCR); let pty = openpty(None, &termios).unwrap(); // Must be valid file descriptors - assert!(pty.master > 0); - assert!(pty.slave > 0); // Writing to one should be readable on the other one let string = "foofoofoo\n"; let mut buf = [0u8; 10]; - write(pty.master, string.as_bytes()).unwrap(); - crate::read_exact(pty.slave, &mut buf); + write(pty.master.as_raw_fd(), string.as_bytes()).unwrap(); + crate::read_exact(&pty.slave, &mut buf); assert_eq!(&buf, string.as_bytes()); // read the echo as well let echoed_string = "foofoofoo\n"; - crate::read_exact(pty.master, &mut buf); + crate::read_exact(&pty.master, &mut buf); assert_eq!(&buf, echoed_string.as_bytes()); let string2 = "barbarbarbar\n"; let echoed_string2 = "barbarbarbar\n"; let mut buf = [0u8; 13]; - write(pty.slave, string2.as_bytes()).unwrap(); - crate::read_exact(pty.master, &mut buf); + write(pty.slave.as_raw_fd(), string2.as_bytes()).unwrap(); + crate::read_exact(&pty.master, &mut buf); assert_eq!(&buf, echoed_string2.as_bytes()); - - close(pty.master).unwrap(); - close(pty.slave).unwrap(); } #[test] @@ -303,11 +267,10 @@ fn test_forkpty() { Parent { child } => { let mut buf = [0u8; 10]; assert!(child.as_raw() > 0); - crate::read_exact(pty.master, &mut buf); + crate::read_exact(&pty.master, &mut buf); kill(child, SIGTERM).unwrap(); wait().unwrap(); // keep other tests using generic wait from getting our child assert_eq!(&buf, echoed_string.as_bytes()); - close(pty.master).unwrap(); } } } diff --git a/test/test_ptymaster_drop.rs b/test/test_ptymaster_drop.rs deleted file mode 100644 index ffbaa569..00000000 --- a/test/test_ptymaster_drop.rs +++ /dev/null @@ -1,20 +0,0 @@ -#[cfg(not(any(target_os = "redox", target_os = "fuchsia")))] -mod t { - use nix::fcntl::OFlag; - use nix::pty::*; - use nix::unistd::close; - use std::os::unix::io::AsRawFd; - - /// Regression test for Issue #659 - /// - /// `PtyMaster` should panic rather than double close the file descriptor - /// This must run in its own test process because it deliberately creates a - /// race condition. - #[test] - #[should_panic(expected = "Closing an invalid file descriptor!")] - fn test_double_close() { - let m = posix_openpt(OFlag::O_RDWR).unwrap(); - close(m.as_raw_fd()).unwrap(); - drop(m); // should panic here - } -} diff --git a/test/test_unistd.rs b/test/test_unistd.rs index 6619262e..1d50c5fa 100644 --- a/test/test_unistd.rs +++ b/test/test_unistd.rs @@ -555,16 +555,13 @@ fn test_lseek() { const CONTENTS: &[u8] = b"abcdef123456"; let mut tmp = tempfile().unwrap(); tmp.write_all(CONTENTS).unwrap(); - let tmpfd = tmp.into_raw_fd(); let offset: off_t = 5; - lseek(tmpfd, offset, Whence::SeekSet).unwrap(); + lseek(tmp.as_raw_fd(), offset, Whence::SeekSet).unwrap(); let mut buf = [0u8; 7]; - crate::read_exact(tmpfd, &mut buf); + crate::read_exact(&tmp, &mut buf); assert_eq!(b"f123456", &buf); - - close(tmpfd).unwrap(); } #[cfg(any(target_os = "linux", target_os = "android"))] @@ -573,15 +570,12 @@ fn test_lseek64() { const CONTENTS: &[u8] = b"abcdef123456"; let mut tmp = tempfile().unwrap(); tmp.write_all(CONTENTS).unwrap(); - let tmpfd = tmp.into_raw_fd(); - lseek64(tmpfd, 5, Whence::SeekSet).unwrap(); + lseek64(tmp.as_raw_fd(), 5, Whence::SeekSet).unwrap(); let mut buf = [0u8; 7]; - crate::read_exact(tmpfd, &mut buf); + crate::read_exact(&tmp, &mut buf); assert_eq!(b"f123456", &buf); - - close(tmpfd).unwrap(); } cfg_if! { |