Auto merge of #24426 - alexcrichton:windows-pipes, r=aturon

This commit removes the last remnants of file descriptors from the Windows
implementation of `std::sys` by using `CreatePipe` to create anonymous pipes
instead of the `pipe` shim provided in msvcrt.

Author: bors

src/libstd/sys/windows/c.rs

@@ -465,6 +465,10 @@ extern "system" {
                            nOutBufferSize: libc::DWORD,
                            lpBytesReturned: libc::LPDWORD,
                            lpOverlapped: libc::LPOVERLAPPED) -> libc::BOOL;
+    pub fn CreatePipe(hReadPipe: libc::LPHANDLE,
+                      hWritePipe: libc::LPHANDLE,
+                      lpPipeAttributes: libc::LPSECURITY_ATTRIBUTES,
+                      nSize: libc::DWORD) -> libc::BOOL;
 }
 
 #[link(name = "userenv")]

src/libstd/sys/windows/handle.rs

@@ -36,11 +36,34 @@ impl Handle {
     }
 
     pub fn read(&self, buf: &mut [u8]) -> io::Result<usize> {
-        read(self.0, buf)
+        let mut read = 0;
+        let res = cvt(unsafe {
+            libc::ReadFile(self.0, buf.as_ptr() as libc::LPVOID,
+                           buf.len() as libc::DWORD, &mut read,
+                           ptr::null_mut())
+        });
+
+        match res {
+            Ok(_) => Ok(read as usize),
+
+            // The special treatment of BrokenPipe is to deal with Windows
+            // pipe semantics, which yields this error when *reading* from
+            // a pipe after the other end has closed; we interpret that as
+            // EOF on the pipe.
+            Err(ref e) if e.kind() == ErrorKind::BrokenPipe => Ok(0),
+
+            Err(e) => Err(e)
+        }
     }
 
     pub fn write(&self, buf: &[u8]) -> io::Result<usize> {
-        write(self.0, buf)
+        let mut amt = 0;
+        try!(cvt(unsafe {
+            libc::WriteFile(self.0, buf.as_ptr() as libc::LPVOID,
+                            buf.len() as libc::DWORD, &mut amt,
+                            ptr::null_mut())
+        }));
+        Ok(amt as usize)
     }
 }
 
@@ -49,35 +72,3 @@ impl Drop for Handle {
         unsafe { let _ = libc::CloseHandle(self.0); }
     }
 }
-
-
-pub fn read(h: HANDLE, buf: &mut [u8]) -> io::Result<usize> {
-    let mut read = 0;
-    let res = cvt(unsafe {
-        libc::ReadFile(h, buf.as_ptr() as libc::LPVOID,
-                       buf.len() as libc::DWORD, &mut read,
-                       ptr::null_mut())
-    });
-
-    match res {
-        Ok(_) => Ok(read as usize),
-
-        // The special treatment of BrokenPipe is to deal with Windows
-        // pipe semantics, which yields this error when *reading* from
-        // a pipe after the other end has closed; we interpret that as
-        // EOF on the pipe.
-        Err(ref e) if e.kind() == ErrorKind::BrokenPipe => Ok(0),
-
-        Err(e) => Err(e)
-    }
-}
-
-pub fn write(h: HANDLE, buf: &[u8]) -> io::Result<usize> {
-    let mut amt = 0;
-    try!(cvt(unsafe {
-        libc::WriteFile(h, buf.as_ptr() as libc::LPVOID,
-                        buf.len() as libc::DWORD, &mut amt,
-                        ptr::null_mut())
-    }));
-    Ok(amt as usize)
-}

src/libstd/sys/windows/pipe2.rs

@@ -10,70 +10,39 @@
 
 use prelude::v1::*;
 
-use sys::handle;
 use io;
-use libc::{self, c_int, HANDLE};
+use libc;
+use sys::cvt;
+use sys::c;
+use sys::handle::Handle;
 
 ////////////////////////////////////////////////////////////////////////////////
 // Anonymous pipes
 ////////////////////////////////////////////////////////////////////////////////
 
 pub struct AnonPipe {
-    fd: c_int
+    inner: Handle,
 }
 
 pub fn anon_pipe() -> io::Result<(AnonPipe, AnonPipe)> {
-    // Windows pipes work subtly differently than unix pipes, and their
-    // inheritance has to be handled in a different way that I do not
-    // fully understand. Here we explicitly make the pipe non-inheritable,
-    // which means to pass it to a subprocess they need to be duplicated
-    // first, as in std::run.
-    let mut fds = [0; 2];
-    unsafe {
-        match libc::pipe(fds.as_mut_ptr(), 1024 as ::libc::c_uint,
-                         (libc::O_BINARY | libc::O_NOINHERIT) as c_int) {
-            0 => {
-                assert!(fds[0] != -1 && fds[0] != 0);
-                assert!(fds[1] != -1 && fds[1] != 0);
-
-                Ok((AnonPipe::from_fd(fds[0]), AnonPipe::from_fd(fds[1])))
-            }
-            _ => Err(io::Error::last_os_error()),
-        }
-    }
+    let mut reader = libc::INVALID_HANDLE_VALUE;
+    let mut writer = libc::INVALID_HANDLE_VALUE;
+    try!(cvt(unsafe {
+        c::CreatePipe(&mut reader, &mut writer, 0 as *mut _, 0)
+    }));
+    let reader = Handle::new(reader);
+    let writer = Handle::new(writer);
+    Ok((AnonPipe { inner: reader }, AnonPipe { inner: writer }))
 }
 
 impl AnonPipe {
-    pub fn from_fd(fd: libc::c_int) -> AnonPipe {
-        AnonPipe { fd: fd }
-    }
-
-    pub fn raw(&self) -> HANDLE {
-        unsafe { libc::get_osfhandle(self.fd) as libc::HANDLE }
-    }
+    pub fn handle(&self) -> &Handle { &self.inner }
 
     pub fn read(&self, buf: &mut [u8]) -> io::Result<usize> {
-        handle::read(self.raw(), buf)
+        self.inner.read(buf)
     }
 
     pub fn write(&self, buf: &[u8]) -> io::Result<usize> {
-        handle::write(self.raw(), buf)
-    }
-}
-
-impl Drop for AnonPipe {
-    fn drop(&mut self) {
-        // closing stdio file handles makes no sense, so never do it. Also, note
-        // that errors are ignored when closing a file descriptor. The reason
-        // for this is that if an error occurs we don't actually know if the
-        // file descriptor was closed or not, and if we retried (for something
-        // like EINTR), we might close another valid file descriptor (opened
-        // after we closed ours.
-        if self.fd > libc::STDERR_FILENO {
-            let n = unsafe { libc::close(self.fd) };
-            if n != 0 {
-                println!("error {} when closing file descriptor {}", n, self.fd);
-            }
-        }
+        self.inner.write(buf)
     }
 }

src/libstd/sys/windows/process2.rs

@@ -199,7 +199,7 @@ impl Process {
                         }
                     }
                     Stdio::Piped(ref pipe) => {
-                        let orig = pipe.raw();
+                        let orig = pipe.handle().raw();
                         if DuplicateHandle(cur_proc, orig, cur_proc, slot,
                                            0, TRUE, DUPLICATE_SAME_ACCESS) == FALSE {
                             return Err(Error::last_os_error())