From 24d2764600c9aee479095a0577ffb8ec69ec7e66 Mon Sep 17 00:00:00 2001 From: Jesse Luehrs Date: Mon, 22 Feb 2021 22:34:16 -0500 Subject: a bit more logic inversion --- src/command.rs | 91 ++++++++++++++++++++++++++++++-------------- src/command/async_process.rs | 34 ++++++++--------- src/command/std.rs | 34 ++++++++--------- src/command/tokio.rs | 34 ++++++++--------- 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>; } +impl Command for T +where + T: CommandImpl, +{ + type Child = T::Child; + + fn spawn_pty( + &mut self, + size: Option<&crate::pty::Size>, + ) -> Result> { + 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 { child: T, pty: crate::pty::Pty, @@ -47,6 +93,22 @@ impl ::std::ops::DerefMut for Child { } } +// 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(&mut self, f: F) + where + F: FnMut() -> ::std::io::Result<()> + Send + Sync + 'static; + fn spawn_impl(&mut self) -> ::std::io::Result; +} + 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> { - 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(&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.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> { - 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(&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.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> { - 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(&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.spawn() } } -- cgit v1.2.3-54-g00ecf