feat: I/O safety pipe, pipe2 & write

Author: JonathanWoollett-Light

src/pty.rs

@@ -71,7 +71,7 @@ impl io::Read for PtyMaster {
 
 impl io::Write for PtyMaster {
     fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
-        unistd::write(self.0.as_raw_fd(), buf).map_err(io::Error::from)
+        unistd::write(&self.0, buf).map_err(io::Error::from)
     }
     fn flush(&mut self) -> io::Result<()> {
         Ok(())
@@ -86,7 +86,7 @@ impl io::Read for &PtyMaster {
 
 impl io::Write for &PtyMaster {
     fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
-        unistd::write(self.0.as_raw_fd(), buf).map_err(io::Error::from)
+        unistd::write(&self.0, buf).map_err(io::Error::from)
     }
     fn flush(&mut self) -> io::Result<()> {
         Ok(())

src/sys/select.rs

@@ -322,8 +322,8 @@ where
 mod tests {
     use super::*;
     use crate::sys::time::{TimeVal, TimeValLike};
-    use crate::unistd::{close, pipe, write};
-    use std::os::unix::io::{FromRawFd, OwnedFd, RawFd};
+    use crate::unistd::{pipe, write};
+    use std::os::unix::io::RawFd;
 
     #[test]
     fn fdset_insert() {
@@ -466,12 +466,9 @@ mod tests {
     #[test]
     fn test_select() {
         let (r1, w1) = pipe().unwrap();
-        let r1 = unsafe { OwnedFd::from_raw_fd(r1) };
-        let w1 = unsafe { OwnedFd::from_raw_fd(w1) };
         let (r2, _w2) = pipe().unwrap();
-        let r2 = unsafe { OwnedFd::from_raw_fd(r2) };
 
-        write(w1.as_raw_fd(), b"hi!").unwrap();
+        write(&w1, b"hi!").unwrap();
         let mut fd_set = FdSet::new();
         fd_set.insert(&r1);
         fd_set.insert(&r2);
@@ -483,18 +480,14 @@ mod tests {
         );
         assert!(fd_set.contains(&r1));
         assert!(!fd_set.contains(&r2));
-        close(_w2).unwrap();
     }
 
     #[test]
     fn test_select_nfds() {
         let (r1, w1) = pipe().unwrap();
         let (r2, _w2) = pipe().unwrap();
-        let r1 = unsafe { OwnedFd::from_raw_fd(r1) };
-        let w1 = unsafe { OwnedFd::from_raw_fd(w1) };
-        let r2 = unsafe { OwnedFd::from_raw_fd(r2) };
 
-        write(w1.as_raw_fd(), b"hi!").unwrap();
+        write(&w1, b"hi!").unwrap();
         let mut fd_set = FdSet::new();
         fd_set.insert(&r1);
         fd_set.insert(&r2);
@@ -521,16 +514,13 @@ mod tests {
         }
         assert!(fd_set.contains(&r1));
         assert!(!fd_set.contains(&r2));
-        close(_w2).unwrap();
     }
 
     #[test]
     fn test_select_nfds2() {
         let (r1, w1) = pipe().unwrap();
-        write(w1, b"hi!").unwrap();
+        write(&w1, b"hi!").unwrap();
         let (r2, _w2) = pipe().unwrap();
-        let r1 = unsafe { OwnedFd::from_raw_fd(r1) };
-        let r2 = unsafe { OwnedFd::from_raw_fd(r2) };
         let mut fd_set = FdSet::new();
         fd_set.insert(&r1);
         fd_set.insert(&r2);
@@ -549,6 +539,5 @@ mod tests {
         );
         assert!(fd_set.contains(&r1));
         assert!(!fd_set.contains(&r2));
-        close(_w2).unwrap();
     }
 }

src/unistd.rs

@@ -35,7 +35,7 @@ use std::ffi::{CString, OsStr};
 use std::os::unix::ffi::OsStrExt;
 use std::os::unix::ffi::OsStringExt;
 use std::os::unix::io::RawFd;
-use std::os::unix::io::{AsFd, AsRawFd};
+use std::os::unix::io::{AsFd, AsRawFd, OwnedFd};
 use std::path::PathBuf;
 use std::{fmt, mem, ptr};
 
@@ -1115,9 +1115,13 @@ pub fn read(fd: RawFd, buf: &mut [u8]) -> Result<usize> {
 /// Write to a raw file descriptor.
 ///
 /// See also [write(2)](https://pubs.opengroup.org/onlinepubs/9699919799/functions/write.html)
-pub fn write(fd: RawFd, buf: &[u8]) -> Result<usize> {
+pub fn write<Fd: AsFd>(fd: Fd, buf: &[u8]) -> Result<usize> {
     let res = unsafe {
-        libc::write(fd, buf.as_ptr() as *const c_void, buf.len() as size_t)
+        libc::write(
+            fd.as_fd().as_raw_fd(),
+            buf.as_ptr() as *const c_void,
+            buf.len() as size_t,
+        )
     };
 
     Errno::result(res).map(|r| r as usize)
@@ -1189,14 +1193,15 @@ pub fn lseek64(
 /// Create an interprocess channel.
 ///
 /// See also [pipe(2)](https://pubs.opengroup.org/onlinepubs/9699919799/functions/pipe.html)
-pub fn pipe() -> std::result::Result<(RawFd, RawFd), Error> {
-    let mut fds = mem::MaybeUninit::<[c_int; 2]>::uninit();
+pub fn pipe() -> std::result::Result<(OwnedFd, OwnedFd), Error> {
+    let mut fds = mem::MaybeUninit::<[OwnedFd; 2]>::uninit();
 
     let res = unsafe { libc::pipe(fds.as_mut_ptr() as *mut c_int) };
 
     Error::result(res)?;
 
-    unsafe { Ok((fds.assume_init()[0], fds.assume_init()[1])) }
+    let [read, write] = unsafe { fds.assume_init() };
+    Ok((read, write))
 }
 
 feature! {
@@ -1230,15 +1235,16 @@ feature! {
     target_os = "openbsd",
     target_os = "solaris"
 ))]
-pub fn pipe2(flags: OFlag) -> Result<(RawFd, RawFd)> {
-    let mut fds = mem::MaybeUninit::<[c_int; 2]>::uninit();
+pub fn pipe2(flags: OFlag) -> Result<(OwnedFd, OwnedFd)> {
+    let mut fds = mem::MaybeUninit::<[OwnedFd; 2]>::uninit();
 
     let res =
         unsafe { libc::pipe2(fds.as_mut_ptr() as *mut c_int, flags.bits()) };
 
     Errno::result(res)?;
 
-    unsafe { Ok((fds.assume_init()[0], fds.assume_init()[1])) }
+    let [read, write] = unsafe { fds.assume_init() };
+    Ok((read, write))
 }
 
 /// Truncate a file to a specified length

test/sys/test_select.rs

@@ -9,10 +9,8 @@ pub fn test_pselect() {
     let _mtx = crate::SIGNAL_MTX.lock();
 
     let (r1, w1) = pipe().unwrap();
-    write(w1, b"hi!").unwrap();
-    let r1 = unsafe { OwnedFd::from_raw_fd(r1) };
+    write(&w1, b"hi!").unwrap();
     let (r2, _w2) = pipe().unwrap();
-    let r2 = unsafe { OwnedFd::from_raw_fd(r2) };
 
     let mut fd_set = FdSet::new();
     fd_set.insert(&r1);
@@ -31,10 +29,8 @@ pub fn test_pselect() {
 #[test]
 pub fn test_pselect_nfds2() {
     let (r1, w1) = pipe().unwrap();
-    write(w1, b"hi!").unwrap();
-    let r1 = unsafe { OwnedFd::from_raw_fd(r1) };
+    write(&w1, b"hi!").unwrap();
     let (r2, _w2) = pipe().unwrap();
-    let r2 = unsafe { OwnedFd::from_raw_fd(r2) };
 
     let mut fd_set = FdSet::new();
     fd_set.insert(&r1);

test/sys/test_socket.rs

@@ -770,7 +770,6 @@ pub fn test_scm_rights() {
             .unwrap(),
             5
         );
-        close(r).unwrap();
     }
 
     {
@@ -803,12 +802,11 @@ pub fn test_scm_rights() {
 
     let received_r = received_r.expect("Did not receive passed fd");
     // Ensure that the received file descriptor works
-    write(w.as_raw_fd(), b"world").unwrap();
+    write(&w, b"world").unwrap();
     let mut buf = [0u8; 5];
     read(received_r.as_raw_fd(), &mut buf).unwrap();
     assert_eq!(&buf[..], b"world");
     close(received_r).unwrap();
-    close(w).unwrap();
 }
 
 // Disable the test on emulated platforms due to not enabled support of AF_ALG in QEMU from rust cross
@@ -1467,7 +1465,6 @@ fn test_impl_scm_credentials_and_rights(mut space: Vec<u8>) {
             .unwrap(),
             5
         );
-        close(r).unwrap();
     }
 
     {
@@ -1510,12 +1507,11 @@ fn test_impl_scm_credentials_and_rights(mut space: Vec<u8>) {
 
     let received_r = received_r.expect("Did not receive passed fd");
     // Ensure that the received file descriptor works
-    write(w.as_raw_fd(), b"world").unwrap();
+    write(&w, b"world").unwrap();
     let mut buf = [0u8; 5];
     read(received_r.as_raw_fd(), &mut buf).unwrap();
     assert_eq!(&buf[..], b"world");
     close(received_r).unwrap();
-    close(w).unwrap();
 }
 
 // Test creating and using named unix domain sockets

test/sys/test_uio.rs

@@ -230,6 +230,7 @@ fn test_process_vm_readv() {
     use nix::sys::signal::*;
     use nix::sys::wait::*;
     use nix::unistd::ForkResult::*;
+    use std::os::unix::io::AsRawFd;
 
     require_capability!("test_process_vm_readv", CAP_SYS_PTRACE);
     let _m = crate::FORK_MTX.lock();
@@ -241,10 +242,10 @@ fn test_process_vm_readv() {
     let (r, w) = pipe().unwrap();
     match unsafe { fork() }.expect("Error: Fork Failed") {
         Parent { child } => {
-            close(w).unwrap();
+            drop(w);
             // wait for child
-            read(r, &mut [0u8]).unwrap();
-            close(r).unwrap();
+            read(r.as_raw_fd(), &mut [0u8]).unwrap();
+            drop(r);
 
             let ptr = vector.as_ptr() as usize;
             let remote_iov = RemoteIoVec { base: ptr, len: 5 };
@@ -263,12 +264,12 @@ fn test_process_vm_readv() {
             assert_eq!(20u8, buf.iter().sum());
         }
         Child => {
-            let _ = close(r);
+            drop(r);
             for i in &mut vector {
                 *i += 1;
             }
             let _ = write(w, b"\0");
-            let _ = close(w);
+            drop(w);
             loop {
                 pause();
             }

test/test_fcntl.rs

@@ -309,12 +309,9 @@ mod linux_android {
         assert_eq!(2, res);
 
         let mut buf = [0u8; 1024];
-        assert_eq!(2, read(rd, &mut buf).unwrap());
+        assert_eq!(2, read(rd.as_raw_fd(), &mut buf).unwrap());
         assert_eq!(b"f1", &buf[0..2]);
         assert_eq!(7, offset);
-
-        close(rd).unwrap();
-        close(wr).unwrap();
     }
 
     #[test]
@@ -336,11 +333,6 @@ mod linux_android {
         // Check all the bytes are still at rd1.
         assert_eq!(3, read(rd1, &mut buf).unwrap());
         assert_eq!(b"abc", &buf[0..3]);
-
-        close(rd1).unwrap();
-        close(wr1).unwrap();
-        close(rd2).unwrap();
-        close(wr2).unwrap();
     }
 
     #[test]
@@ -357,11 +349,8 @@ mod linux_android {
 
         // Check the bytes can be read at rd.
         let mut buf = [0u8; 32];
-        assert_eq!(6, read(rd, &mut buf).unwrap());
+        assert_eq!(6, read(rd.as_raw_fd(), &mut buf).unwrap());
         assert_eq!(b"abcdef", &buf[0..6]);
-
-        close(rd).unwrap();
-        close(wr).unwrap();
     }
 
     #[cfg(target_os = "linux")]
@@ -509,7 +498,7 @@ mod test_posix_fadvise {
     fn test_errno() {
         let (rd, _wr) = pipe().unwrap();
         let res = posix_fadvise(
-            rd as RawFd,
+            rd.as_raw_fd(),
             0,
             100,
             PosixFadviseAdvice::POSIX_FADV_WILLNEED,
@@ -565,7 +554,7 @@ mod test_posix_fallocate {
     #[test]
     fn errno() {
         let (rd, _wr) = pipe().unwrap();
-        let err = posix_fallocate(rd as RawFd, 0, 100).unwrap_err();
+        let err = posix_fallocate(rd.as_raw_fd(), 0, 100).unwrap_err();
         match err {
             Errno::EINVAL | Errno::ENODEV | Errno::ESPIPE | Errno::EBADF => (),
             errno => panic!("unexpected errno {errno}",),

test/test_poll.rs

@@ -20,21 +20,19 @@ macro_rules! loop_while_eintr {
 #[test]
 fn test_poll() {
     let (r, w) = pipe().unwrap();
-    let r = unsafe { OwnedFd::from_raw_fd(r) };
     let mut fds = [PollFd::new(&r, PollFlags::POLLIN)];
 
     // Poll an idle pipe.  Should timeout
     let nfds = loop_while_eintr!(poll(&mut fds, 100));
     assert_eq!(nfds, 0);
     assert!(!fds[0].revents().unwrap().contains(PollFlags::POLLIN));
 
-    write(w, b".").unwrap();
+    write(&w, b".").unwrap();
 
     // Poll a readable pipe.  Should return an event.
     let nfds = poll(&mut fds, 100).unwrap();
     assert_eq!(nfds, 1);
     assert!(fds[0].revents().unwrap().contains(PollFlags::POLLIN));
-    close(w).unwrap();
 }
 
 // ppoll(2) is the same as poll except for how it handles timeouts and signals.
@@ -54,7 +52,6 @@ fn test_ppoll() {
 
     let timeout = TimeSpec::milliseconds(1);
     let (r, w) = pipe().unwrap();
-    let r = unsafe { OwnedFd::from_raw_fd(r) };
     let mut fds = [PollFd::new(&r, PollFlags::POLLIN)];
 
     // Poll an idle pipe.  Should timeout
@@ -63,13 +60,12 @@ fn test_ppoll() {
     assert_eq!(nfds, 0);
     assert!(!fds[0].revents().unwrap().contains(PollFlags::POLLIN));
 
-    write(w, b".").unwrap();
+    write(&w, b".").unwrap();
 
     // Poll a readable pipe.  Should return an event.
     let nfds = ppoll(&mut fds, Some(timeout), None).unwrap();
     assert_eq!(nfds, 1);
     assert!(fds[0].revents().unwrap().contains(PollFlags::POLLIN));
-    close(w).unwrap();
 }
 
 #[test]