Auto merge of #26159 - alexcrichton:tweak-process-lowering-raising, r=brson

* Slate these features to be stable in 1.2 instead of 1.1 (not being backported)
* Have the `FromRawFd` implementations follow the contract of the `FromRawFd`
  trait by taking ownership of the primitive specified.
* Refactor the implementations slightly to remove the `unreachable!` blocks as
  well as separating the stdio representation of `std::process` from
  `std::sys::process`.

cc #25494 

7 files changed, 32 insertions, 88 deletions

diff --git a/src/libstd/sys/unix/ext/process.rs b/src/libstd/sys/unix/ext/process.rs
index 2d30b016a2d..cfe7a1f2dda 100644
--- a/src/libstd/sys/unix/ext/process.rs
+++ b/src/libstd/sys/unix/ext/process.rs
@@ -65,46 +65,28 @@ impl ExitStatusExt for process::ExitStatus {
     }
 }
 
-#[stable(feature = "from_raw_os", since = "1.1.0")]
+#[stable(feature = "process_extensions", since = "1.2.0")]
 impl FromRawFd for process::Stdio {
-    /// Creates a new instance of `Stdio` from the raw underlying file
-    /// descriptor.
-    ///
-    /// When this `Stdio` is used as an I/O handle for a child process the given
-    /// file descriptor will be `dup`d into the destination file descriptor in
-    /// the child process.
-    ///
-    /// Note that this function **does not** take ownership of the file
-    /// descriptor provided and it will **not** be closed when `Stdio` goes out
-    /// of scope. As a result this method is unsafe because due to the lack of
-    /// knowledge about the lifetime of the provided file descriptor, this could
-    /// cause another I/O primitive's ownership property of its file descriptor
-    /// to be violated.
-    ///
-    /// Also note that this file descriptor may be used multiple times to spawn
-    /// processes. For example the `Command::spawn` function could be called
-    /// more than once to spawn more than one process sharing this file
-    /// descriptor.
     unsafe fn from_raw_fd(fd: RawFd) -> process::Stdio {
-        process::Stdio::from_inner(sys::process::Stdio::Fd(fd))
+        process::Stdio::from_inner(sys::fd::FileDesc::new(fd))
     }
 }
 
-#[stable(feature = "from_raw_os", since = "1.1.0")]
+#[stable(feature = "process_extensions", since = "1.2.0")]
 impl AsRawFd for process::ChildStdin {
     fn as_raw_fd(&self) -> RawFd {
         self.as_inner().fd().raw()
     }
 }
 
-#[stable(feature = "from_raw_os", since = "1.1.0")]
+#[stable(feature = "process_extensions", since = "1.2.0")]
 impl AsRawFd for process::ChildStdout {
     fn as_raw_fd(&self) -> RawFd {
         self.as_inner().fd().raw()
     }
 }
 
-#[stable(feature = "from_raw_os", since = "1.1.0")]
+#[stable(feature = "process_extensions", since = "1.2.0")]
 impl AsRawFd for process::ChildStderr {
     fn as_raw_fd(&self) -> RawFd {
         self.as_inner().fd().raw()
diff --git a/src/libstd/sys/unix/fs.rs b/src/libstd/sys/unix/fs.rs
index 51a85a276ed..58e205a01ca 100644
--- a/src/libstd/sys/unix/fs.rs
+++ b/src/libstd/sys/unix/fs.rs
@@ -283,8 +283,6 @@ impl File {
         Ok(File(fd))
     }
 
-    pub fn into_fd(self) -> FileDesc { self.0 }
-
     pub fn file_attr(&self) -> io::Result<FileAttr> {
         let mut stat: raw::stat = unsafe { mem::zeroed() };
         try!(cvt(unsafe {
diff --git a/src/libstd/sys/unix/pipe.rs b/src/libstd/sys/unix/pipe.rs
index 6283a29ae46..946857c05bc 100644
--- a/src/libstd/sys/unix/pipe.rs
+++ b/src/libstd/sys/unix/pipe.rs
@@ -44,6 +44,6 @@ impl AnonPipe {
         self.0.write(buf)
     }
 
+    pub fn raw(&self) -> libc::c_int { self.0.raw() }
     pub fn fd(&self) -> &FileDesc { &self.0 }
-    pub fn into_fd(self) -> FileDesc { self.0 }
 }
diff --git a/src/libstd/sys/unix/process.rs b/src/libstd/sys/unix/process.rs
index 76af42d9348..9178ca7aba4 100644
--- a/src/libstd/sys/unix/process.rs
+++ b/src/libstd/sys/unix/process.rs
@@ -18,9 +18,10 @@ use fmt;
 use io::{self, Error, ErrorKind};
 use libc::{self, pid_t, c_void, c_int, gid_t, uid_t};
 use ptr;
+use sys::fd::FileDesc;
+use sys::fs::{File, OpenOptions};
 use sys::pipe::AnonPipe;
 use sys::{self, c, cvt, cvt_r};
-use sys::fs::{File, OpenOptions};
 
 ////////////////////////////////////////////////////////////////////////////////
 // Command
@@ -121,11 +122,12 @@ pub struct Process {
 
 pub enum Stdio {
     Inherit,
-    Piped(AnonPipe),
     None,
-    Fd(c_int),
+    Raw(c_int),
 }
 
+pub type RawStdio = FileDesc;
+
 const CLOEXEC_MSG_FOOTER: &'static [u8] = b"NOEX";
 
 impl Process {
@@ -252,10 +254,9 @@ impl Process {
         }
 
         let setup = |src: Stdio, dst: c_int| {
-            let fd = match src {
-                Stdio::Inherit => return true,
-                Stdio::Fd(fd) => return cvt_r(|| libc::dup2(fd, dst)).is_ok(),
-                Stdio::Piped(pipe) => pipe.into_fd(),
+            match src {
+                Stdio::Inherit => true,
+                Stdio::Raw(fd) => cvt_r(|| libc::dup2(fd, dst)).is_ok(),
 
                 // If a stdio file descriptor is set to be ignored, we open up
                 // /dev/null into that file descriptor. Otherwise, the first
@@ -269,13 +270,12 @@ impl Process {
                     let devnull = CStr::from_ptr(b"/dev/null\0".as_ptr()
                                                     as *const _);
                     if let Ok(f) = File::open_c(devnull, &opts) {
-                        f.into_fd()
+                        cvt_r(|| libc::dup2(f.fd().raw(), dst)).is_ok()
                     } else {
-                        return false
+                        false
                     }
                 }
-            };
-            cvt_r(|| libc::dup2(fd.raw(), dst)).is_ok()
+            }
         };
 
         if !setup(in_fd, libc::STDIN_FILENO) { fail(&mut output) }
@@ -418,14 +418,3 @@ fn translate_status(status: c_int) -> ExitStatus {
         ExitStatus::Signal(imp::WTERMSIG(status))
     }
 }
-
-impl Stdio {
-    pub fn clone_if_copy(&self) -> Stdio {
-        match *self {
-            Stdio::Inherit => Stdio::Inherit,
-            Stdio::None => Stdio::None,
-            Stdio::Fd(fd) => Stdio::Fd(fd),
-            Stdio::Piped(_) => unreachable!(),
-        }
-    }
-}
diff --git a/src/libstd/sys/windows/ext/process.rs b/src/libstd/sys/windows/ext/process.rs
index 0fd43a450f3..6f59be2687a 100644
--- a/src/libstd/sys/windows/ext/process.rs
+++ b/src/libstd/sys/windows/ext/process.rs
@@ -10,58 +10,43 @@
 
 //! Extensions to `std::process` for Windows.
 
-#![stable(feature = "from_raw_os", since = "1.1.0")]
+#![stable(feature = "process_extensions", since = "1.2.0")]
 
 use os::windows::io::{FromRawHandle, RawHandle, AsRawHandle};
 use process;
 use sys;
 use sys_common::{AsInner, FromInner};
 
-#[stable(feature = "from_raw_os", since = "1.1.0")]
+#[stable(feature = "process_extensions", since = "1.2.0")]
 impl FromRawHandle for process::Stdio {
-    /// Creates a new instance of `Stdio` from the raw underlying handle.
-    ///
-    /// When this `Stdio` is used as an I/O handle for a child process the given
-    /// handle will be duplicated via `DuplicateHandle` to ensure that the
-    /// handle has the correct permissions to cross the process boundary.
-    ///
-    /// Note that this function **does not** take ownership of the handle
-    /// provided and it will **not** be closed when `Stdio` goes out of scope.
-    /// As a result this method is unsafe because due to the lack of knowledge
-    /// about the lifetime of the provided handle, this could cause another I/O
-    /// primitive's ownership property of its handle to be violated.
-    ///
-    /// Also note that this handle may be used multiple times to spawn
-    /// processes. For example the `Command::spawn` function could be called
-    /// more than once to spawn more than one process sharing this handle.
     unsafe fn from_raw_handle(handle: RawHandle) -> process::Stdio {
-        let handle = sys::handle::RawHandle::new(handle as *mut _);
-        process::Stdio::from_inner(sys::process::Stdio::Handle(handle))
+        let handle = sys::handle::Handle::new(handle as *mut _);
+        process::Stdio::from_inner(handle)
     }
 }
 
-#[stable(feature = "from_raw_os", since = "1.1.0")]
+#[stable(feature = "process_extensions", since = "1.2.0")]
 impl AsRawHandle for process::Child {
     fn as_raw_handle(&self) -> RawHandle {
         self.as_inner().handle().raw() as *mut _
     }
 }
 
-#[stable(feature = "from_raw_os", since = "1.1.0")]
+#[stable(feature = "process_extensions", since = "1.2.0")]
 impl AsRawHandle for process::ChildStdin {
     fn as_raw_handle(&self) -> RawHandle {
         self.as_inner().handle().raw() as *mut _
     }
 }
 
-#[stable(feature = "from_raw_os", since = "1.1.0")]
+#[stable(feature = "process_extensions", since = "1.2.0")]
 impl AsRawHandle for process::ChildStdout {
     fn as_raw_handle(&self) -> RawHandle {
         self.as_inner().handle().raw() as *mut _
     }
 }
 
-#[stable(feature = "from_raw_os", since = "1.1.0")]
+#[stable(feature = "process_extensions", since = "1.2.0")]
 impl AsRawHandle for process::ChildStderr {
     fn as_raw_handle(&self) -> RawHandle {
         self.as_inner().handle().raw() as *mut _
diff --git a/src/libstd/sys/windows/pipe.rs b/src/libstd/sys/windows/pipe.rs
index b441d8beedb..b2a6607314a 100644
--- a/src/libstd/sys/windows/pipe.rs
+++ b/src/libstd/sys/windows/pipe.rs
@@ -38,6 +38,8 @@ pub fn anon_pipe() -> io::Result<(AnonPipe, AnonPipe)> {
 impl AnonPipe {
     pub fn handle(&self) -> &Handle { &self.inner }
 
+    pub fn raw(&self) -> libc::HANDLE { self.inner.raw() }
+
     pub fn read(&self, buf: &mut [u8]) -> io::Result<usize> {
         self.inner.read(buf)
     }
diff --git a/src/libstd/sys/windows/process.rs b/src/libstd/sys/windows/process.rs
index 05073e60f5d..0b0268d4746 100644
--- a/src/libstd/sys/windows/process.rs
+++ b/src/libstd/sys/windows/process.rs
@@ -28,7 +28,6 @@ use sync::StaticMutex;
 use sys::c;
 use sys::fs::{OpenOptions, File};
 use sys::handle::{Handle, RawHandle};
-use sys::pipe::AnonPipe;
 use sys::stdio;
 use sys::{self, cvt};
 use sys_common::{AsInner, FromInner};
@@ -107,11 +106,12 @@ pub struct Process {
 
 pub enum Stdio {
     Inherit,
-    Piped(AnonPipe),
     None,
-    Handle(RawHandle),
+    Raw(libc::HANDLE),
 }
 
+pub type RawStdio = Handle;
+
 impl Process {
     pub fn spawn(cfg: &Command,
                  in_handle: Stdio,
@@ -356,15 +356,6 @@ fn make_dirp(d: Option<&OsString>) -> (*const u16, Vec<u16>) {
 }
 
 impl Stdio {
-    pub fn clone_if_copy(&self) -> Stdio {
-        match *self {
-            Stdio::Inherit => Stdio::Inherit,
-            Stdio::None => Stdio::None,
-            Stdio::Handle(handle) => Stdio::Handle(handle),
-            Stdio::Piped(_) => unreachable!(),
-        }
-    }
-
     fn to_handle(&self, stdio_id: libc::DWORD) -> io::Result<Handle> {
         use libc::DUPLICATE_SAME_ACCESS;
 
@@ -374,11 +365,8 @@ impl Stdio {
                     io.handle().duplicate(0, true, DUPLICATE_SAME_ACCESS)
                 })
             }
-            Stdio::Handle(ref handle) => {
-                handle.duplicate(0, true, DUPLICATE_SAME_ACCESS)
-            }
-            Stdio::Piped(ref pipe) => {
-                pipe.handle().duplicate(0, true, DUPLICATE_SAME_ACCESS)
+            Stdio::Raw(handle) => {
+                RawHandle::new(handle).duplicate(0, true, DUPLICATE_SAME_ACCESS)
             }
 
             // Similarly to unix, we don't actually leave holes for the