From edfffc8c490b9d179a62901dfb874a85ab0984e3 Mon Sep 17 00:00:00 2001 From: Jesse Luehrs Date: Mon, 22 Feb 2021 03:43:49 -0500 Subject: refactor --- examples/basic.rs | 2 +- src/command.rs | 156 +++++++++++++++++++++++++---------------------------- src/command/std.rs | 37 +++++++++++++ 3 files changed, 111 insertions(+), 84 deletions(-) create mode 100644 src/command/std.rs diff --git a/examples/basic.rs b/examples/basic.rs index c0c1236..d7f99d0 100644 --- a/examples/basic.rs +++ b/examples/basic.rs @@ -35,7 +35,7 @@ impl Drop for RawGuard { } } -fn run(child: &pty_process::Child) { +fn run(child: &pty_process::Child) { let _raw = RawGuard::new(); let mut buf = [0_u8; 4096]; let pty = child.pty().as_raw_fd(); diff --git a/src/command.rs b/src/command.rs index 4f8f808..ba7d764 100644 --- a/src/command.rs +++ b/src/command.rs @@ -1,87 +1,23 @@ use crate::error::*; -use std::os::unix::io::{AsRawFd as _, FromRawFd as _}; -use std::os::unix::process::CommandExt as _; +use ::std::os::unix::io::AsRawFd as _; -pub trait Command { - fn spawn_pty(&mut self, size: Option<&crate::pty::Size>) - -> Result; -} +mod std; -impl Command for std::process::Command { +pub trait Command { fn spawn_pty( &mut self, size: Option<&crate::pty::Size>, - ) -> Result { - let pty = crate::pty::Pty::new()?; - if let Some(size) = size { - pty.resize(size)?; - } - - let pts = pty.pts()?; - - let pt_fd = pty.pt().as_raw_fd(); - let pts_fd = pts.as_raw_fd(); - - let stdin = nix::unistd::dup(pts_fd).map_err(Error::SpawnNix)?; - let stdout = nix::unistd::dup(pts_fd).map_err(Error::SpawnNix)?; - let stderr = nix::unistd::dup(pts_fd).map_err(Error::SpawnNix)?; - - // 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 - // the file descriptor), allowing from_raw_fd to take ownership of it. - 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) }); - - // 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 || { - nix::unistd::setsid().map_err(|e| e.as_errno().unwrap())?; - set_controlling_terminal(&pts) - .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().map_err(Error::Spawn)?; - - Ok(Child { child, pty }) - } + ) -> Result>; } -pub struct Child { - child: std::process::Child, +pub struct Child { + child: T, pty: crate::pty::Pty, } -impl Child { - pub fn pty(&self) -> &std::fs::File { +impl Child { + pub fn pty(&self) -> &::std::fs::File { self.pty.pt() } @@ -90,30 +26,84 @@ impl Child { } } -impl std::ops::Deref for Child { - type Target = std::process::Child; +impl ::std::ops::Deref for Child { + type Target = T; fn deref(&self) -> &Self::Target { &self.child } } -impl std::ops::DerefMut for Child { +impl ::std::ops::DerefMut for Child { fn deref_mut(&mut self) -> &mut Self::Target { &mut self.child } } -nix::ioctl_write_ptr_bad!( - set_controlling_terminal_unsafe, - libc::TIOCSCTTY, - libc::c_int -); +fn setup_pty( + size: Option<&crate::pty::Size>, +) -> Result<( + crate::pty::Pty, + ::std::fs::File, + ::std::os::unix::io::RawFd, + ::std::os::unix::io::RawFd, + ::std::os::unix::io::RawFd, +)> { + let pty = crate::pty::Pty::new()?; + if let Some(size) = size { + pty.resize(size)?; + } + + let pts = pty.pts()?; + let pts_fd = pts.as_raw_fd(); + + let stdin = nix::unistd::dup(pts_fd).map_err(Error::SpawnNix)?; + let stdout = nix::unistd::dup(pts_fd).map_err(Error::SpawnNix)?; + let stderr = nix::unistd::dup(pts_fd).map_err(Error::SpawnNix)?; -fn set_controlling_terminal(file: &std::fs::File) -> nix::Result<()> { - let fd = file.as_raw_fd(); + 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<()> { // safe because std::fs::File is required to contain a valid file // descriptor - unsafe { set_controlling_terminal_unsafe(fd, std::ptr::null()) } + unsafe { set_controlling_terminal_unsafe(fd, ::std::ptr::null()) } .map(|_| ()) } + +nix::ioctl_write_ptr_bad!( + set_controlling_terminal_unsafe, + libc::TIOCSCTTY, + libc::c_int +); diff --git a/src/command/std.rs b/src/command/std.rs new file mode 100644 index 0000000..d544b83 --- /dev/null +++ b/src/command/std.rs @@ -0,0 +1,37 @@ +use crate::error::*; + +use std::os::unix::io::{AsRawFd as _, FromRawFd as _}; +use std::os::unix::process::CommandExt as _; + +impl super::Command for std::process::Command { + fn spawn_pty( + &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(); + + // 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 + // the file descriptor), allowing from_raw_fd to take ownership of it. + 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) }); + + // 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) + }); + } + + let child = self.spawn().map_err(Error::Spawn)?; + + Ok(super::Child { child, pty }) + } +} -- cgit v1.2.3-54-g00ecf