std: Clean up process spawn impl on unix

* De-indent quite a bit by removing usage of FnOnce closures
* Clearly separate code for the parent/child after the fork
* Use `fs2::{File, OpenOptions}` instead of calling `open` manually
* Use RAII to close I/O objects wherever possible
* Remove loop for closing all file descriptors, all our own ones are now
  `CLOEXEC` by default so they cannot be inherited

5 files changed, 213 insertions, 249 deletions

diff --git a/src/libstd/sys/unix/c.rs b/src/libstd/sys/unix/c.rs
index 282e5668e6e..aa4bf821207 100644
--- a/src/libstd/sys/unix/c.rs
+++ b/src/libstd/sys/unix/c.rs
@@ -159,6 +159,8 @@ extern {
     pub fn utimes(filename: *const libc::c_char,
                   times: *const libc::timeval) -> libc::c_int;
     pub fn gai_strerror(errcode: libc::c_int) -> *const libc::c_char;
+    pub fn setgroups(ngroups: libc::c_int,
+                     ptr: *const libc::c_void) -> libc::c_int;
 }
 
 #[cfg(any(target_os = "macos", target_os = "ios"))]
diff --git a/src/libstd/sys/unix/fs2.rs b/src/libstd/sys/unix/fs2.rs
index 20b1aac8f45..ac121f1c82e 100644
--- a/src/libstd/sys/unix/fs2.rs
+++ b/src/libstd/sys/unix/fs2.rs
@@ -205,13 +205,17 @@ impl OpenOptions {
 
 impl File {
     pub fn open(path: &Path, opts: &OpenOptions) -> io::Result<File> {
+        let path = try!(cstr(path));
+        File::open_c(&path, opts)
+    }
+
+    pub fn open_c(path: &CStr, opts: &OpenOptions) -> io::Result<File> {
         let flags = opts.flags | match (opts.read, opts.write) {
             (true, true) => libc::O_RDWR,
             (false, true) => libc::O_WRONLY,
             (true, false) |
             (false, false) => libc::O_RDONLY,
         };
-        let path = try!(cstr(path));
         let fd = try!(cvt_r(|| unsafe {
             libc::open(path.as_ptr(), flags, opts.mode)
         }));
@@ -220,6 +224,8 @@ impl File {
         Ok(File(fd))
     }
 
+    pub fn into_fd(self) -> FileDesc { self.0 }
+
     pub fn file_attr(&self) -> io::Result<FileAttr> {
         let mut stat: libc::stat = unsafe { mem::zeroed() };
         try!(cvt(unsafe { libc::fstat(self.0.raw(), &mut stat) }));
diff --git a/src/libstd/sys/unix/pipe2.rs b/src/libstd/sys/unix/pipe2.rs
index 2fd4d6dd311..e9d8c69fefb 100644
--- a/src/libstd/sys/unix/pipe2.rs
+++ b/src/libstd/sys/unix/pipe2.rs
@@ -20,11 +20,10 @@ use libc;
 
 pub struct AnonPipe(FileDesc);
 
-pub unsafe fn anon_pipe() -> io::Result<(AnonPipe, AnonPipe)> {
+pub fn anon_pipe() -> io::Result<(AnonPipe, AnonPipe)> {
     let mut fds = [0; 2];
-    if libc::pipe(fds.as_mut_ptr()) == 0 {
-        Ok((AnonPipe::from_fd(fds[0]),
-            AnonPipe::from_fd(fds[1])))
+    if unsafe { libc::pipe(fds.as_mut_ptr()) == 0 } {
+        Ok((AnonPipe::from_fd(fds[0]), AnonPipe::from_fd(fds[1])))
     } else {
         Err(io::Error::last_os_error())
     }
@@ -45,7 +44,7 @@ impl AnonPipe {
         self.0.write(buf)
     }
 
-    pub fn raw(&self) -> libc::c_int {
-        self.0.raw()
+    pub fn into_fd(self) -> FileDesc {
+        self.0
     }
 }
diff --git a/src/libstd/sys/unix/process2.rs b/src/libstd/sys/unix/process2.rs
index 60f00c80b4a..dc6897fec8e 100644
--- a/src/libstd/sys/unix/process2.rs
+++ b/src/libstd/sys/unix/process2.rs
@@ -13,14 +13,14 @@ use os::unix::prelude::*;
 
 use collections::HashMap;
 use env;
-use ffi::{OsString, OsStr, CString};
+use ffi::{OsString, OsStr, CString, CStr};
 use fmt;
 use io::{self, Error, ErrorKind};
 use libc::{self, pid_t, c_void, c_int, gid_t, uid_t};
-use mem;
 use ptr;
 use sys::pipe2::AnonPipe;
 use sys::{self, retry, c, cvt};
+use sys::fs2::{File, OpenOptions};
 
 ////////////////////////////////////////////////////////////////////////////////
 // Command
@@ -128,221 +128,178 @@ impl Process {
     }
 
     pub fn spawn(cfg: &Command,
-                 in_fd: Option<AnonPipe>, out_fd: Option<AnonPipe>, err_fd: Option<AnonPipe>)
-                 -> io::Result<Process>
-    {
-        use libc::funcs::posix88::unistd::{fork, dup2, close, chdir, execvp};
-
-        mod rustrt {
-            extern {
-                pub fn rust_unset_sigprocmask();
+                 in_fd: Option<AnonPipe>,
+                 out_fd: Option<AnonPipe>,
+                 err_fd: Option<AnonPipe>) -> io::Result<Process> {
+        let dirp = cfg.cwd.as_ref().map(|c| c.as_ptr()).unwrap_or(ptr::null());
+
+        let (envp, _a, _b) = make_envp(cfg.env.as_ref());
+        let (argv, _a) = make_argv(&cfg.program, &cfg.args);
+        let (input, output) = try!(sys::pipe2::anon_pipe());
+
+        let pid = unsafe {
+            match libc::fork() {
+                0 => {
+                    drop(input);
+                    Process::child_after_fork(cfg, output, argv, envp, dirp,
+                                              in_fd, out_fd, err_fd)
+                }
+                n if n < 0 => return Err(Error::last_os_error()),
+                n => n,
+            }
+        };
+
+        let p = Process{ pid: pid };
+        drop(output);
+        let mut bytes = [0; 8];
+
+        // loop to handle EINTR
+        loop {
+            match input.read(&mut bytes) {
+                Ok(0) => return Ok(p),
+                Ok(8) => {
+                    assert!(combine(CLOEXEC_MSG_FOOTER) == combine(&bytes[4.. 8]),
+                            "Validation on the CLOEXEC pipe failed: {:?}", bytes);
+                    let errno = combine(&bytes[0.. 4]);
+                    assert!(p.wait().is_ok(),
+                            "wait() should either return Ok or panic");
+                    return Err(Error::from_raw_os_error(errno))
+                }
+                Err(ref e) if e.kind() == ErrorKind::Interrupted => {}
+                Err(e) => {
+                    assert!(p.wait().is_ok(),
+                            "wait() should either return Ok or panic");
+                    panic!("the CLOEXEC pipe failed: {:?}", e)
+                },
+                Ok(..) => { // pipe I/O up to PIPE_BUF bytes should be atomic
+                    assert!(p.wait().is_ok(),
+                            "wait() should either return Ok or panic");
+                    panic!("short read on the CLOEXEC pipe")
+                }
             }
         }
 
-        unsafe fn set_cloexec(fd: c_int) {
-            let ret = c::ioctl(fd, c::FIOCLEX);
-            assert_eq!(ret, 0);
-        }
+        fn combine(arr: &[u8]) -> i32 {
+            let a = arr[0] as u32;
+            let b = arr[1] as u32;
+            let c = arr[2] as u32;
+            let d = arr[3] as u32;
 
-        #[cfg(all(target_os = "android", target_arch = "aarch64"))]
-        unsafe fn getdtablesize() -> c_int {
-            libc::sysconf(libc::consts::os::sysconf::_SC_OPEN_MAX) as c_int
+            ((a << 24) | (b << 16) | (c << 8) | (d << 0)) as i32
         }
+    }
 
-        #[cfg(not(all(target_os = "android", target_arch = "aarch64")))]
-        unsafe fn getdtablesize() -> c_int {
-            libc::funcs::bsd44::getdtablesize()
+    // And at this point we've reached a special time in the life of the
+    // child. The child must now be considered hamstrung and unable to
+    // do anything other than syscalls really. Consider the following
+    // scenario:
+    //
+    //      1. Thread A of process 1 grabs the malloc() mutex
+    //      2. Thread B of process 1 forks(), creating thread C
+    //      3. Thread C of process 2 then attempts to malloc()
+    //      4. The memory of process 2 is the same as the memory of
+    //         process 1, so the mutex is locked.
+    //
+    // This situation looks a lot like deadlock, right? It turns out
+    // that this is what pthread_atfork() takes care of, which is
+    // presumably implemented across platforms. The first thing that
+    // threads to *before* forking is to do things like grab the malloc
+    // mutex, and then after the fork they unlock it.
+    //
+    // Despite this information, libnative's spawn has been witnessed to
+    // deadlock on both OSX and FreeBSD. I'm not entirely sure why, but
+    // all collected backtraces point at malloc/free traffic in the
+    // child spawned process.
+    //
+    // For this reason, the block of code below should contain 0
+    // invocations of either malloc of free (or their related friends).
+    //
+    // As an example of not having malloc/free traffic, we don't close
+    // this file descriptor by dropping the FileDesc (which contains an
+    // allocation). Instead we just close it manually. This will never
+    // have the drop glue anyway because this code never returns (the
+    // child will either exec() or invoke libc::exit)
+    unsafe fn child_after_fork(cfg: &Command,
+                               mut output: AnonPipe,
+                               argv: *const *const libc::c_char,
+                               envp: *const libc::c_void,
+                               dirp: *const libc::c_char,
+                               in_fd: Option<AnonPipe>,
+                               out_fd: Option<AnonPipe>,
+                               err_fd: Option<AnonPipe>) -> ! {
+        fn fail(output: &mut AnonPipe) -> ! {
+            let errno = sys::os::errno() as u32;
+            let bytes = [
+                (errno >> 24) as u8,
+                (errno >> 16) as u8,
+                (errno >>  8) as u8,
+                (errno >>  0) as u8,
+                CLOEXEC_MSG_FOOTER[0], CLOEXEC_MSG_FOOTER[1],
+                CLOEXEC_MSG_FOOTER[2], CLOEXEC_MSG_FOOTER[3]
+            ];
+            // pipe I/O up to PIPE_BUF bytes should be atomic, and then we want
+            // to be sure we *don't* run at_exit destructors as we're being torn
+            // down regardless
+            assert!(output.write(&bytes).is_ok());
+            unsafe { libc::_exit(1) }
         }
 
-        let dirp = cfg.cwd.as_ref().map(|c| c.as_ptr()).unwrap_or(ptr::null());
-
-        with_envp(cfg.env.as_ref(), |envp: *const c_void| {
-            with_argv(&cfg.program, &cfg.args, |argv: *const *const libc::c_char| unsafe {
-                let (input, mut output) = try!(sys::pipe2::anon_pipe());
-
-                // We may use this in the child, so perform allocations before the
-                // fork
-                let devnull = b"/dev/null\0";
-
-                set_cloexec(output.raw());
-
-                let pid = fork();
-                if pid < 0 {
-                    return Err(Error::last_os_error())
-                } else if pid > 0 {
-                    #[inline]
-                    fn combine(arr: &[u8]) -> i32 {
-                        let a = arr[0] as u32;
-                        let b = arr[1] as u32;
-                        let c = arr[2] as u32;
-                        let d = arr[3] as u32;
-
-                        ((a << 24) | (b << 16) | (c << 8) | (d << 0)) as i32
-                    }
-
-                    let p = Process{ pid: pid };
-                    drop(output);
-                    let mut bytes = [0; 8];
-
-                    // loop to handle EINTER
-                    loop {
-                        match input.read(&mut bytes) {
-                            Ok(8) => {
-                                assert!(combine(CLOEXEC_MSG_FOOTER) == combine(&bytes[4.. 8]),
-                                        "Validation on the CLOEXEC pipe failed: {:?}", bytes);
-                                let errno = combine(&bytes[0.. 4]);
-                                assert!(p.wait().is_ok(),
-                                        "wait() should either return Ok or panic");
-                                return Err(Error::from_raw_os_error(errno))
-                            }
-                            Ok(0) => return Ok(p),
-                            Err(ref e) if e.kind() == ErrorKind::Interrupted => {}
-                            Err(e) => {
-                                assert!(p.wait().is_ok(),
-                                        "wait() should either return Ok or panic");
-                                panic!("the CLOEXEC pipe failed: {:?}", e)
-                            },
-                            Ok(..) => { // pipe I/O up to PIPE_BUF bytes should be atomic
-                                assert!(p.wait().is_ok(),
-                                        "wait() should either return Ok or panic");
-                                panic!("short read on the CLOEXEC pipe")
-                            }
-                        }
-                    }
-                }
-
-                // And at this point we've reached a special time in the life of the
-                // child. The child must now be considered hamstrung and unable to
-                // do anything other than syscalls really. Consider the following
-                // scenario:
-                //
-                //      1. Thread A of process 1 grabs the malloc() mutex
-                //      2. Thread B of process 1 forks(), creating thread C
-                //      3. Thread C of process 2 then attempts to malloc()
-                //      4. The memory of process 2 is the same as the memory of
-                //         process 1, so the mutex is locked.
-                //
-                // This situation looks a lot like deadlock, right? It turns out
-                // that this is what pthread_atfork() takes care of, which is
-                // presumably implemented across platforms. The first thing that
-                // threads to *before* forking is to do things like grab the malloc
-                // mutex, and then after the fork they unlock it.
-                //
-                // Despite this information, libnative's spawn has been witnessed to
-                // deadlock on both OSX and FreeBSD. I'm not entirely sure why, but
-                // all collected backtraces point at malloc/free traffic in the
-                // child spawned process.
-                //
-                // For this reason, the block of code below should contain 0
-                // invocations of either malloc of free (or their related friends).
-                //
-                // As an example of not having malloc/free traffic, we don't close
-                // this file descriptor by dropping the FileDesc (which contains an
-                // allocation). Instead we just close it manually. This will never
-                // have the drop glue anyway because this code never returns (the
-                // child will either exec() or invoke libc::exit)
-                let _ = libc::close(input.raw());
-
-                fn fail(output: &mut AnonPipe) -> ! {
-                    let errno = sys::os::errno() as u32;
-                    let bytes = [
-                        (errno >> 24) as u8,
-                        (errno >> 16) as u8,
-                        (errno >>  8) as u8,
-                        (errno >>  0) as u8,
-                        CLOEXEC_MSG_FOOTER[0], CLOEXEC_MSG_FOOTER[1],
-                        CLOEXEC_MSG_FOOTER[2], CLOEXEC_MSG_FOOTER[3]
-                    ];
-                    // pipe I/O up to PIPE_BUF bytes should be atomic
-                    assert!(output.write(&bytes).is_ok());
-                    unsafe { libc::_exit(1) }
-                }
-
-                rustrt::rust_unset_sigprocmask();
-
-                // If a stdio file descriptor is set to be ignored, we don't
-                // actually close it, but rather open up /dev/null into that
-                // file descriptor. Otherwise, the first file descriptor opened
-                // up in the child would be numbered as one of the stdio file
-                // descriptors, which is likely to wreak havoc.
-                let setup = |src: Option<AnonPipe>, dst: c_int| {
-                    let src = match src {
-                        None => {
-                            let flags = if dst == libc::STDIN_FILENO {
-                                libc::O_RDONLY
-                            } else {
-                                libc::O_RDWR
-                            };
-                            libc::open(devnull.as_ptr() as *const _, flags, 0)
-                        }
-                        Some(obj) => {
-                            let fd = obj.raw();
-                            // Leak the memory and the file descriptor. We're in the
-                            // child now an all our resources are going to be
-                            // cleaned up very soon
-                            mem::forget(obj);
-                            fd
-                        }
-                    };
-                    src != -1 && retry(|| dup2(src, dst)) != -1
-                };
-
-                if !setup(in_fd, libc::STDIN_FILENO) { fail(&mut output) }
-                if !setup(out_fd, libc::STDOUT_FILENO) { fail(&mut output) }
-                if !setup(err_fd, libc::STDERR_FILENO) { fail(&mut output) }
-
-                // close all other fds
-                for fd in (3..getdtablesize()).rev() {
-                    if fd != output.raw() {
-                        let _ = close(fd as c_int);
-                    }
-                }
-
-                match cfg.gid {
-                    Some(u) => {
-                        if libc::setgid(u as libc::gid_t) != 0 {
-                            fail(&mut output);
-                        }
-                    }
-                    None => {}
-                }
-                match cfg.uid {
-                    Some(u) => {
-                        // When dropping privileges from root, the `setgroups` call
-                        // will remove any extraneous groups. If we don't call this,
-                        // then even though our uid has dropped, we may still have
-                        // groups that enable us to do super-user things. This will
-                        // fail if we aren't root, so don't bother checking the
-                        // return value, this is just done as an optimistic
-                        // privilege dropping function.
-                        extern {
-                            fn setgroups(ngroups: libc::c_int,
-                                         ptr: *const libc::c_void) -> libc::c_int;
-                        }
-                        let _ = setgroups(0, ptr::null());
-
-                        if libc::setuid(u as libc::uid_t) != 0 {
-                            fail(&mut output);
-                        }
-                    }
-                    None => {}
-                }
-                if cfg.detach {
-                    // Don't check the error of setsid because it fails if we're the
-                    // process leader already. We just forked so it shouldn't return
-                    // error, but ignore it anyway.
-                    let _ = libc::setsid();
-                }
-                if !dirp.is_null() && chdir(dirp) == -1 {
-                    fail(&mut output);
-                }
-                if !envp.is_null() {
-                    *sys::os::environ() = envp as *const _;
-                }
-                let _ = execvp(*argv, argv as *mut _);
+        // If a stdio file descriptor is set to be ignored, we don't
+        // actually close it, but rather open up /dev/null into that
+        // file descriptor. Otherwise, the first file descriptor opened
+        // up in the child would be numbered as one of the stdio file
+        // descriptors, which is likely to wreak havoc.
+        let setup = |src: Option<AnonPipe>, dst: c_int| {
+            src.map(|p| p.into_fd()).or_else(|| {
+                let mut opts = OpenOptions::new();
+                opts.read(dst == libc::STDIN_FILENO);
+                opts.write(dst != libc::STDIN_FILENO);
+                let devnull = CStr::from_ptr(b"/dev/null\0".as_ptr()
+                                                as *const _);
+                File::open_c(devnull, &opts).ok().map(|f| f.into_fd())
+            }).map(|fd| {
+                fd.unset_cloexec();
+                retry(|| libc::dup2(fd.raw(), dst)) != -1
+            }).unwrap_or(false)
+        };
+
+        if !setup(in_fd, libc::STDIN_FILENO) { fail(&mut output) }
+        if !setup(out_fd, libc::STDOUT_FILENO) { fail(&mut output) }
+        if !setup(err_fd, libc::STDERR_FILENO) { fail(&mut output) }
+
+        if let Some(u) = cfg.gid {
+            if libc::setgid(u as libc::gid_t) != 0 {
                 fail(&mut output);
-            })
-        })
+            }
+        }
+        if let Some(u) = cfg.uid {
+            // When dropping privileges from root, the `setgroups` call
+            // will remove any extraneous groups. If we don't call this,
+            // then even though our uid has dropped, we may still have
+            // groups that enable us to do super-user things. This will
+            // fail if we aren't root, so don't bother checking the
+            // return value, this is just done as an optimistic
+            // privilege dropping function.
+            let _ = c::setgroups(0, ptr::null());
+
+            if libc::setuid(u as libc::uid_t) != 0 {
+                fail(&mut output);
+            }
+        }
+        if cfg.detach {
+            // Don't check the error of setsid because it fails if we're the
+            // process leader already. We just forked so it shouldn't return
+            // error, but ignore it anyway.
+            let _ = libc::setsid();
+        }
+        if !dirp.is_null() && libc::chdir(dirp) == -1 {
+            fail(&mut output);
+        }
+        if !envp.is_null() {
+            *sys::os::environ() = envp as *const _;
+        }
+        let _ = libc::execvp(*argv, argv as *mut _);
+        fail(&mut output)
     }
 
     pub fn wait(&self) -> io::Result<ExitStatus> {
@@ -364,8 +321,8 @@ impl Process {
     }
 }
 
-fn with_argv<T,F>(prog: &CString, args: &[CString], cb: F) -> T
-    where F : FnOnce(*const *const libc::c_char) -> T
+fn make_argv(prog: &CString, args: &[CString])
+             -> (*const *const libc::c_char, Vec<*const libc::c_char>)
 {
     let mut ptrs: Vec<*const libc::c_char> = Vec::with_capacity(args.len()+1);
 
@@ -380,40 +337,38 @@ fn with_argv<T,F>(prog: &CString, args: &[CString], cb: F) -> T
     // Add a terminating null pointer (required by libc).
     ptrs.push(ptr::null());
 
-    cb(ptrs.as_ptr())
+    (ptrs.as_ptr(), ptrs)
 }
 
-fn with_envp<T, F>(env: Option<&HashMap<OsString, OsString>>, cb: F) -> T
-    where F : FnOnce(*const c_void) -> T
+fn make_envp(env: Option<&HashMap<OsString, OsString>>)
+             -> (*const c_void, Vec<Vec<u8>>, Vec<*const libc::c_char>)
 {
     // On posixy systems we can pass a char** for envp, which is a
     // null-terminated array of "k=v\0" strings. Since we must create
     // these strings locally, yet expose a raw pointer to them, we
     // create a temporary vector to own the CStrings that outlives the
     // call to cb.
-    match env {
-        Some(env) => {
-            let mut tmps = Vec::with_capacity(env.len());
-
-            for pair in env {
-                let mut kv = Vec::new();
-                kv.push_all(pair.0.as_bytes());
-                kv.push('=' as u8);
-                kv.push_all(pair.1.as_bytes());
-                kv.push(0); // terminating null
-                tmps.push(kv);
-            }
+    if let Some(env) = env {
+        let mut tmps = Vec::with_capacity(env.len());
+
+        for pair in env {
+            let mut kv = Vec::new();
+            kv.push_all(pair.0.as_bytes());
+            kv.push('=' as u8);
+            kv.push_all(pair.1.as_bytes());
+            kv.push(0); // terminating null
+            tmps.push(kv);
+        }
 
-            // As with `with_argv`, this is unsafe, since cb could leak the pointers.
-            let mut ptrs: Vec<*const libc::c_char> =
-                tmps.iter()
-                    .map(|tmp| tmp.as_ptr() as *const libc::c_char)
-                    .collect();
-            ptrs.push(ptr::null());
+        let mut ptrs: Vec<*const libc::c_char> =
+            tmps.iter()
+                .map(|tmp| tmp.as_ptr() as *const libc::c_char)
+                .collect();
+        ptrs.push(ptr::null());
 
-            cb(ptrs.as_ptr() as *const c_void)
-        }
-        _ => cb(ptr::null())
+        (ptrs.as_ptr() as *const _, tmps, ptrs)
+    } else {
+        (0 as *const _, Vec::new(), Vec::new())
     }
 }
 
diff --git a/src/libstd/sys/windows/pipe2.rs b/src/libstd/sys/windows/pipe2.rs
index 229481e3d57..ed41c959782 100644
--- a/src/libstd/sys/windows/pipe2.rs
+++ b/src/libstd/sys/windows/pipe2.rs
@@ -22,22 +22,24 @@ pub struct AnonPipe {
     fd: c_int
 }
 
-pub unsafe fn anon_pipe() -> io::Result<(AnonPipe, AnonPipe)> {
+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];
-    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);
+    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])))
+                Ok((AnonPipe::from_fd(fds[0]), AnonPipe::from_fd(fds[1])))
+            }
+            _ => Err(io::Error::last_os_error()),
         }
-        _ => Err(io::Error::last_os_error()),
     }
 }