feat: I/O safety pipe & pipe2

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/test_unistd.rs

@@ -356,7 +356,7 @@ macro_rules! execve_test_factory (
         match unsafe{fork()}.unwrap() {
             Child => {
                 // Make `writer` be the stdout of the new process.
-                dup2(writer, 1).unwrap();
+                dup2(writer.as_raw_fd(), 1).unwrap();
                 let r = syscall();
                 let _ = std::io::stderr()
                     .write_all(format!("{:?}", r).as_bytes());
@@ -370,7 +370,7 @@ macro_rules! execve_test_factory (
                 assert_eq!(ws, Ok(WaitStatus::Exited(child, 0)));
                 // Read 1024 bytes.
                 let mut buf = [0u8; 1024];
-                read(reader, &mut buf).unwrap();
+                read(reader.as_raw_fd(), &mut buf).unwrap();
                 // It should contain the things we printed using `/bin/sh`.
                 let string = String::from_utf8_lossy(&buf);
                 assert!(string.contains("nix!!!"));
@@ -714,12 +714,12 @@ fn test_getresgid() {
 fn test_pipe() {
     let (fd0, fd1) = pipe().unwrap();
     let m0 = stat::SFlag::from_bits_truncate(
-        stat::fstat(fd0).unwrap().st_mode as mode_t,
+        stat::fstat(fd0.as_raw_fd()).unwrap().st_mode as mode_t,
     );
     // S_IFIFO means it's a pipe
     assert_eq!(m0, SFlag::S_IFIFO);
     let m1 = stat::SFlag::from_bits_truncate(
-        stat::fstat(fd1).unwrap().st_mode as mode_t,
+        stat::fstat(fd1.as_raw_fd()).unwrap().st_mode as mode_t,
     );
     assert_eq!(m1, SFlag::S_IFIFO);
 }
@@ -743,9 +743,13 @@ 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.as_raw_fd(), 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.as_raw_fd(), FcntlArg::F_GETFD).unwrap(),
+    );
     assert!(f1.contains(FdFlag::FD_CLOEXEC));
 }