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

@@ -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::{FromRawFd, OwnedFd};
+use std::os::unix::io::{AsRawFd, FromRawFd, OwnedFd};
 use std::{cmp, iter};
 
 #[cfg(not(target_os = "redox"))]
@@ -241,10 +241,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 +263,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]
@@ -359,9 +351,6 @@ mod linux_android {
         let mut buf = [0u8; 32];
         assert_eq!(6, read(rd, &mut buf).unwrap());
         assert_eq!(b"abcdef", &buf[0..6]);
-
-        close(rd).unwrap();
-        close(wr).unwrap();
     }
 
     #[cfg(target_os = "linux")]

test/test_poll.rs

@@ -28,13 +28,12 @@ fn test_poll() {
     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.
@@ -63,13 +62,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]

test/test_pty.rs

@@ -1,9 +1,9 @@
 use std::fs::File;
-use std::io::{Read, Write};
+use std::io::{stdout, Read, Write};
 use std::os::unix::prelude::*;
 use std::path::Path;
 
-use libc::{_exit, STDOUT_FILENO};
+use libc::_exit;
 use nix::fcntl::{open, OFlag};
 use nix::pty::*;
 use nix::sys::stat;
@@ -185,7 +185,7 @@ fn test_openpty() {
     // Writing to one should be readable on the other one
     let string = "foofoofoo\n";
     let mut buf = [0u8; 10];
-    write(pty.master.as_raw_fd(), string.as_bytes()).unwrap();
+    write(&pty.master, string.as_bytes()).unwrap();
     crate::read_exact(&pty.slave, &mut buf);
 
     assert_eq!(&buf, string.as_bytes());
@@ -199,7 +199,7 @@ fn test_openpty() {
     let string2 = "barbarbarbar\n";
     let echoed_string2 = "barbarbarbar\r\n";
     let mut buf = [0u8; 14];
-    write(pty.slave.as_raw_fd(), string2.as_bytes()).unwrap();
+    write(&pty.slave, string2.as_bytes()).unwrap();
     crate::read_exact(&pty.master, &mut buf);
 
     assert_eq!(&buf, echoed_string2.as_bytes());
@@ -224,7 +224,7 @@ fn test_openpty_with_termios() {
     // Writing to one should be readable on the other one
     let string = "foofoofoo\n";
     let mut buf = [0u8; 10];
-    write(pty.master.as_raw_fd(), string.as_bytes()).unwrap();
+    write(&pty.master, string.as_bytes()).unwrap();
     crate::read_exact(&pty.slave, &mut buf);
 
     assert_eq!(&buf, string.as_bytes());
@@ -237,7 +237,7 @@ fn test_openpty_with_termios() {
     let string2 = "barbarbarbar\n";
     let echoed_string2 = "barbarbarbar\n";
     let mut buf = [0u8; 13];
-    write(pty.slave.as_raw_fd(), string2.as_bytes()).unwrap();
+    write(&pty.slave, string2.as_bytes()).unwrap();
     crate::read_exact(&pty.master, &mut buf);
 
     assert_eq!(&buf, echoed_string2.as_bytes());
@@ -258,7 +258,7 @@ fn test_forkpty() {
     let pty = unsafe { forkpty(None, None).unwrap() };
     match pty.fork_result {
         Child => {
-            write(STDOUT_FILENO, string.as_bytes()).unwrap();
+            write(stdout(), string.as_bytes()).unwrap();
             pause(); // we need the child to stay alive until the parent calls read
             unsafe {
                 _exit(0);