a bit more logic inversion

4 files changed, 110 insertions, 83 deletions

diff --git a/src/command.rs b/src/command.rs
index a5df5ed..ca4a67b 100644
--- a/src/command.rs
+++ b/src/command.rs
@@ -18,6 +18,52 @@ pub trait Command {
     ) -> Result<Child<Self::Child>>;
 }
 
+impl<T> Command for T
+where
+    T: CommandImpl,
+{
+    type Child = T::Child;
+
+    fn spawn_pty(
+        &mut self,
+        size: Option<&crate::pty::Size>,
+    ) -> Result<Child<Self::Child>> {
+        let (pty, pts, stdin, stdout, stderr) = setup_pty(size)?;
+
+        let pt_fd = pty.pt().as_raw_fd();
+        let pts_fd = pts.as_raw_fd();
+
+        self.std_fds(stdin, stdout, stderr);
+        self.pre_exec_impl(move || {
+            nix::unistd::setsid().map_err(|e| e.as_errno().unwrap())?;
+            set_controlling_terminal(pts_fd)
+                .map_err(|e| e.as_errno().unwrap())?;
+
+            // in the parent, destructors will handle closing these file
+            // descriptors (other than pt, used by the parent to
+            // communicate with the child) when the function ends, but in
+            // the child, we end by calling exec(), which doesn't call
+            // destructors.
+
+            // XXX unwrap
+            nix::unistd::close(pt_fd).map_err(|e| e.as_errno().unwrap())?;
+            nix::unistd::close(pts_fd).map_err(|e| e.as_errno().unwrap())?;
+            // at this point, stdin/stdout/stderr have already been
+            // reopened as fds 0/1/2 in the child, so we can (and should)
+            // close the originals
+            nix::unistd::close(stdin).map_err(|e| e.as_errno().unwrap())?;
+            nix::unistd::close(stdout).map_err(|e| e.as_errno().unwrap())?;
+            nix::unistd::close(stderr).map_err(|e| e.as_errno().unwrap())?;
+
+            Ok(())
+        });
+
+        let child = self.spawn_impl().map_err(Error::Spawn)?;
+
+        Ok(Child { child, pty })
+    }
+}
+
 pub struct Child<T> {
     child: T,
     pty: crate::pty::Pty,
@@ -47,6 +93,22 @@ impl<T> ::std::ops::DerefMut for Child<T> {
     }
 }
 
+// XXX shouldn't be pub?
+pub trait CommandImpl {
+    type Child;
+
+    fn std_fds(
+        &mut self,
+        stdin: ::std::os::unix::io::RawFd,
+        stdout: ::std::os::unix::io::RawFd,
+        stderr: ::std::os::unix::io::RawFd,
+    );
+    fn pre_exec_impl<F>(&mut self, f: F)
+    where
+        F: FnMut() -> ::std::io::Result<()> + Send + Sync + 'static;
+    fn spawn_impl(&mut self) -> ::std::io::Result<Self::Child>;
+}
+
 fn setup_pty(
     size: Option<&crate::pty::Size>,
 ) -> Result<(
@@ -71,35 +133,6 @@ fn setup_pty(
     Ok((pty, pts, stdin, stdout, stderr))
 }
 
-fn pre_exec(
-    pt_fd: ::std::os::unix::io::RawFd,
-    pts_fd: ::std::os::unix::io::RawFd,
-    stdin: ::std::os::unix::io::RawFd,
-    stdout: ::std::os::unix::io::RawFd,
-    stderr: ::std::os::unix::io::RawFd,
-) -> ::std::io::Result<()> {
-    nix::unistd::setsid().map_err(|e| e.as_errno().unwrap())?;
-    set_controlling_terminal(pts_fd).map_err(|e| e.as_errno().unwrap())?;
-
-    // in the parent, destructors will handle closing these file
-    // descriptors (other than pt, used by the parent to
-    // communicate with the child) when the function ends, but in
-    // the child, we end by calling exec(), which doesn't call
-    // destructors.
-
-    // XXX unwrap
-    nix::unistd::close(pt_fd).map_err(|e| e.as_errno().unwrap())?;
-    nix::unistd::close(pts_fd).map_err(|e| e.as_errno().unwrap())?;
-    // at this point, stdin/stdout/stderr have already been
-    // reopened as fds 0/1/2 in the child, so we can (and should)
-    // close the originals
-    nix::unistd::close(stdin).map_err(|e| e.as_errno().unwrap())?;
-    nix::unistd::close(stdout).map_err(|e| e.as_errno().unwrap())?;
-    nix::unistd::close(stderr).map_err(|e| e.as_errno().unwrap())?;
-
-    Ok(())
-}
-
 fn set_controlling_terminal(
     fd: ::std::os::unix::io::RawFd,
 ) -> nix::Result<()> {
diff --git a/src/command/async_process.rs b/src/command/async_process.rs
index 687fbe4..0b5ceda 100644
--- a/src/command/async_process.rs
+++ b/src/command/async_process.rs
@@ -1,20 +1,15 @@
-use crate::error::*;
-
 use async_process::unix::CommandExt as _;
-use std::os::unix::io::{AsRawFd as _, FromRawFd as _};
+use std::os::unix::io::FromRawFd as _;
 
-impl super::Command for async_process::Command {
+impl super::CommandImpl for async_process::Command {
     type Child = async_process::Child;
 
-    fn spawn_pty(
+    fn std_fds(
         &mut self,
-        size: Option<&crate::pty::Size>,
-    ) -> Result<super::Child<Self::Child>> {
-        let (pty, pts, stdin, stdout, stderr) = super::setup_pty(size)?;
-
-        let pt_fd = pty.pt().as_raw_fd();
-        let pts_fd = pts.as_raw_fd();
-
+        stdin: ::std::os::unix::io::RawFd,
+        stdout: ::std::os::unix::io::RawFd,
+        stderr: ::std::os::unix::io::RawFd,
+    ) {
         // safe because the fds are valid (otherwise pty.pts() or dup() would
         // have returned an Err and we would have exited early) and are not
         // owned by any other structure (since dup() returns a fresh copy of
@@ -22,18 +17,21 @@ impl super::Command for async_process::Command {
         self.stdin(unsafe { std::process::Stdio::from_raw_fd(stdin) })
             .stdout(unsafe { std::process::Stdio::from_raw_fd(stdout) })
             .stderr(unsafe { std::process::Stdio::from_raw_fd(stderr) });
+    }
 
+    fn pre_exec_impl<F>(&mut self, f: F)
+    where
+        F: FnMut() -> ::std::io::Result<()> + Send + Sync + 'static,
+    {
         // safe because setsid() and close() are async-signal-safe functions
         // and ioctl() is a raw syscall (which is inherently
         // async-signal-safe).
         unsafe {
-            self.pre_exec(move || {
-                super::pre_exec(pt_fd, pts_fd, stdin, stdout, stderr)
-            });
+            self.pre_exec(f);
         }
+    }
 
-        let child = self.spawn().map_err(Error::Spawn)?;
-
-        Ok(super::Child { child, pty })
+    fn spawn_impl(&mut self) -> ::std::io::Result<Self::Child> {
+        self.spawn()
     }
 }
diff --git a/src/command/std.rs b/src/command/std.rs
index 0f3bef1..b331fc0 100644
--- a/src/command/std.rs
+++ b/src/command/std.rs
@@ -1,20 +1,15 @@
-use crate::error::*;
-
-use std::os::unix::io::{AsRawFd as _, FromRawFd as _};
+use std::os::unix::io::FromRawFd as _;
 use std::os::unix::process::CommandExt as _;
 
-impl super::Command for std::process::Command {
+impl super::CommandImpl for std::process::Command {
     type Child = std::process::Child;
 
-    fn spawn_pty(
+    fn std_fds(
         &mut self,
-        size: Option<&crate::pty::Size>,
-    ) -> Result<super::Child<Self::Child>> {
-        let (pty, pts, stdin, stdout, stderr) = super::setup_pty(size)?;
-
-        let pt_fd = pty.pt().as_raw_fd();
-        let pts_fd = pts.as_raw_fd();
-
+        stdin: ::std::os::unix::io::RawFd,
+        stdout: ::std::os::unix::io::RawFd,
+        stderr: ::std::os::unix::io::RawFd,
+    ) {
         // safe because the fds are valid (otherwise pty.pts() or dup() would
         // have returned an Err and we would have exited early) and are not
         // owned by any other structure (since dup() returns a fresh copy of
@@ -22,18 +17,21 @@ impl super::Command for std::process::Command {
         self.stdin(unsafe { std::process::Stdio::from_raw_fd(stdin) })
             .stdout(unsafe { std::process::Stdio::from_raw_fd(stdout) })
             .stderr(unsafe { std::process::Stdio::from_raw_fd(stderr) });
+    }
 
+    fn pre_exec_impl<F>(&mut self, f: F)
+    where
+        F: FnMut() -> ::std::io::Result<()> + Send + Sync + 'static,
+    {
         // safe because setsid() and close() are async-signal-safe functions
         // and ioctl() is a raw syscall (which is inherently
         // async-signal-safe).
         unsafe {
-            self.pre_exec(move || {
-                super::pre_exec(pt_fd, pts_fd, stdin, stdout, stderr)
-            });
+            self.pre_exec(f);
         }
+    }
 
-        let child = self.spawn().map_err(Error::Spawn)?;
-
-        Ok(super::Child { child, pty })
+    fn spawn_impl(&mut self) -> ::std::io::Result<Self::Child> {
+        self.spawn()
     }
 }
diff --git a/src/command/tokio.rs b/src/command/tokio.rs
index 54bfefb..338bc08 100644
--- a/src/command/tokio.rs
+++ b/src/command/tokio.rs
@@ -1,19 +1,14 @@
-use crate::error::*;
+use std::os::unix::io::FromRawFd as _;
 
-use std::os::unix::io::{AsRawFd as _, FromRawFd as _};
-
-impl super::Command for tokio::process::Command {
+impl super::CommandImpl for tokio::process::Command {
     type Child = tokio::process::Child;
 
-    fn spawn_pty(
+    fn std_fds(
         &mut self,
-        size: Option<&crate::pty::Size>,
-    ) -> Result<super::Child<Self::Child>> {
-        let (pty, pts, stdin, stdout, stderr) = super::setup_pty(size)?;
-
-        let pt_fd = pty.pt().as_raw_fd();
-        let pts_fd = pts.as_raw_fd();
-
+        stdin: ::std::os::unix::io::RawFd,
+        stdout: ::std::os::unix::io::RawFd,
+        stderr: ::std::os::unix::io::RawFd,
+    ) {
         // safe because the fds are valid (otherwise pty.pts() or dup() would
         // have returned an Err and we would have exited early) and are not
         // owned by any other structure (since dup() returns a fresh copy of
@@ -21,18 +16,21 @@ impl super::Command for tokio::process::Command {
         self.stdin(unsafe { std::process::Stdio::from_raw_fd(stdin) })
             .stdout(unsafe { std::process::Stdio::from_raw_fd(stdout) })
             .stderr(unsafe { std::process::Stdio::from_raw_fd(stderr) });
+    }
 
+    fn pre_exec_impl<F>(&mut self, f: F)
+    where
+        F: FnMut() -> ::std::io::Result<()> + Send + Sync + 'static,
+    {
         // safe because setsid() and close() are async-signal-safe functions
         // and ioctl() is a raw syscall (which is inherently
         // async-signal-safe).
         unsafe {
-            self.pre_exec(move || {
-                super::pre_exec(pt_fd, pts_fd, stdin, stdout, stderr)
-            });
+            self.pre_exec(f);
         }
+    }
 
-        let child = self.spawn().map_err(Error::Spawn)?;
-
-        Ok(super::Child { child, pty })
+    fn spawn_impl(&mut self) -> ::std::io::Result<Self::Child> {
+        self.spawn()
     }
 }