From 4dd8e58c5b55b273c213ab4bd37924eb5356b8ca Mon Sep 17 00:00:00 2001
From: Michael Aaron Murphy <mmstickman@gmail.com>
Date: Tue, 7 Nov 2017 12:00:54 -0500
Subject: [PATCH] Enhance Fork API & Simplify Public API

---
 Cargo.toml                 |   4 ++
 src/lib.rs                 |   7 ++-
 src/shell/binary/prompt.rs |   8 +--
 src/shell/fork.rs          | 109 +++++++++++++++++++++++++++++++++++++
 src/shell/mod.rs           |  96 +++++++++-----------------------
 5 files changed, 147 insertions(+), 77 deletions(-)
 create mode 100644 src/shell/fork.rs

diff --git a/Cargo.toml b/Cargo.toml
index cf7614be..47cb4bcf 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -39,12 +39,16 @@ unicode-segmentation = "1.2"
 
 [dependencies.app_dirs]
 git = "https://github.com/redox-os/app-dirs-rs.git"
+
 [profile.release]
+lto = true
 panic = "abort"
+
 [target."cfg(all(unix, not(target_os = \"redox\")))".dependencies]
 errno = "0.2.3"
 libc = "0.2"
 libloading = "0.4"
 users = "0.5.1"
+
 [target."cfg(target_os = \"redox\")".dependencies]
 redox_syscall = "0.1"
diff --git a/src/lib.rs b/src/lib.rs
index de92f835..54b29368 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -43,8 +43,9 @@ mod types;
 #[macro_use]
 pub mod parser;
 mod builtins;
-pub mod shell;
+mod shell;
 mod ascii_helpers;
 
-pub use builtins::Builtin;
-pub use shell::Shell;
+pub use shell::{Capture, Fork, IonError, IonResult, Shell};
+pub use shell::flags;
+pub use shell::status;
diff --git a/src/shell/binary/prompt.rs b/src/shell/binary/prompt.rs
index 0e86f154..f0c058bd 100644
--- a/src/shell/binary/prompt.rs
+++ b/src/shell/binary/prompt.rs
@@ -1,4 +1,4 @@
-use super::super::{Function, Shell};
+use super::super::{Capture, Function, Shell};
 use parser::shell_expand::expand_string;
 use std::io::Read;
 use std::process;
@@ -24,12 +24,12 @@ pub(crate) fn prompt_fn(shell: &mut Shell) -> Option<String> {
 
     let mut output = None;
 
-    match shell.fork(|child| unsafe {
+    match shell.fork(Capture::Stdout, |child| unsafe {
         let _ = function.read().execute(child, &["ion"]);
     }) {
-        Ok(mut result) => {
+        Ok(result) => {
             let mut string = String::new();
-            match result.stdout.read_to_string(&mut string) {
+            match result.stdout.unwrap().read_to_string(&mut string) {
                 Ok(_) => output = Some(string),
                 Err(why) => {
                     eprintln!("ion: error reading stdout of child: {}", why);
diff --git a/src/shell/fork.rs b/src/shell/fork.rs
new file mode 100644
index 00000000..9db0bb8f
--- /dev/null
+++ b/src/shell/fork.rs
@@ -0,0 +1,109 @@
+use super::{IonError, Shell};
+use std::fs::File;
+use std::os::unix::io::{AsRawFd, FromRawFd};
+use std::process::exit;
+use sys;
+
+#[repr(u8)]
+#[derive(Copy, Clone, Debug)]
+/// Instructs whether or not to capture the standard output and error streams.
+///
+/// A type that is utilized by the `Fork` structure.
+pub enum Capture {
+    /// Don't capture any streams at all.
+    None = 0,
+    /// Capture just the standard output stream.
+    Stdout = 1,
+    /// Capture just the standard error stream.
+    Stderr = 2,
+    /// Capture both the standard output and error streams.
+    Both = 3,
+}
+
+/// Utilized by the shell for performing forks and capturing streams.
+///
+/// Using this structure directly is equivalent to using `Shell`'s fork method.
+pub struct Fork<'a> {
+    shell:   &'a Shell,
+    capture: Capture,
+}
+
+#[derive(Debug)]
+/// The result returned by a fork, which optionally contains file handles to the standard
+/// output and error streams, as well as the PID of the child. This structure is subject to change
+/// in the future, once there's a better means of obtaining the exit status without having to
+/// wait on the PID.
+pub struct IonResult {
+    pub pid:    u32,
+    pub stdout: Option<File>,
+    pub stderr: Option<File>,
+}
+
+impl<'a> Fork<'a> {
+    /// Creates a new `Fork` state from an existing shell.
+    pub fn new(shell: &'a Shell, capture: Capture) -> Fork<'a> { Fork { shell, capture } }
+
+    /// Executes a closure within the child of the fork, and returning an `IonResult` in a
+    /// non-blocking fashion.
+    pub fn exec<F: FnMut(&mut Shell)>(&self, mut child_func: F) -> Result<IonResult, IonError> {
+        sys::signals::block();
+
+        // If we are to capture stdout, create a pipe for capturing outputs.
+        let outs = if self.capture as u8 & Capture::Stdout as u8 != 0 {
+            Some(sys::pipe2(sys::O_CLOEXEC)
+                .map(|fds| unsafe { (File::from_raw_fd(fds.0), File::from_raw_fd(fds.1)) })
+                .map_err(IonError::Fork)?)
+        } else {
+            None
+        };
+
+        // And if we are to capture stderr, create a pipe for that as well.
+        let errs = if self.capture as u8 & Capture::Stderr as u8 != 0 {
+            Some(sys::pipe2(sys::O_CLOEXEC)
+                .map(|fds| unsafe { (File::from_raw_fd(fds.0), File::from_raw_fd(fds.1)) })
+                .map_err(IonError::Fork)?)
+        } else {
+            None
+        };
+
+        match unsafe { sys::fork() } {
+            Ok(0) => {
+                // Allow the child to handle it's own signal handling.
+                sys::signals::unblock();
+
+                // Redirect standard output to a pipe, if needed.
+                if let Some((read, write)) = outs {
+                    let _ = sys::dup2(write.as_raw_fd(), sys::STDOUT_FILENO);
+                    drop(write);
+                    drop(read);
+                }
+
+                // Redirect standard error to a pipe, if needed.
+                if let Some((read, write)) = errs {
+                    let _ = sys::dup2(write.as_raw_fd(), sys::STDERR_FILENO);
+                    drop(write);
+                    drop(read);
+                }
+
+                // Execute the given closure within the child's shell.
+                let mut shell: Shell = unsafe { (self.shell as *const Shell).read() };
+                child_func(&mut shell);
+
+                // Reap the child, enabling the parent to get EOF from the read end of the pipe.
+                exit(shell.previous_status);
+            }
+            Ok(pid) => Ok(IonResult {
+                pid,
+                stdout: outs.map(|(read, write)| {
+                    drop(write);
+                    read
+                }),
+                stderr: errs.map(|(read, write)| {
+                    drop(write);
+                    read
+                }),
+            }),
+            Err(why) => Err(IonError::Fork(why)),
+        }
+    }
+}
diff --git a/src/shell/mod.rs b/src/shell/mod.rs
index 605fe2b6..1b438d21 100644
--- a/src/shell/mod.rs
+++ b/src/shell/mod.rs
@@ -2,6 +2,7 @@ mod assignments;
 mod binary;
 mod completer;
 mod flow;
+mod fork;
 mod history;
 mod job;
 mod pipe_exec;
@@ -16,6 +17,7 @@ pub mod variables;
 
 pub(crate) use self::binary::Binary;
 pub(crate) use self::flow::FlowLogic;
+pub use self::fork::{Capture, Fork, IonResult};
 pub(crate) use self::history::{IgnoreSetting, ShellHistory};
 pub(crate) use self::job::{Job, JobKind};
 pub(crate) use self::pipe_exec::{foreground, job_control};
@@ -70,13 +72,6 @@ impl Display for IonError {
     }
 }
 
-#[allow(dead_code)]
-pub struct IonResult {
-    pub pid:    u32,
-    pub stdout: File,
-    pub stderr: File,
-}
-
 /// The shell structure is a megastructure that manages all of the state of the shell throughout
 /// the entirety of the
 /// program. It is initialized at the beginning of the program, and lives until the end of the
@@ -145,6 +140,7 @@ impl<'a> Shell {
     }
 
     #[allow(dead_code)]
+    /// Creates a new shell within memory.
     pub fn new() -> Shell {
         Shell {
             builtins:            BUILTINS,
@@ -332,10 +328,10 @@ impl<'a> Shell {
         exit_status
     }
 
-    /// Sets a variable.
+    /// Sets a variable of `name` with the given `value` in the shell's variable map.
     pub fn set_var(&mut self, name: &str, value: &str) { self.variables.set_var(name, value); }
 
-    /// Gets a variable, if it exists.
+    /// Gets a string variable, if it exists within the shell's variable map.
     pub fn get_var(&self, name: &str) -> Option<String> { self.variables.get_var(name) }
 
     /// Obtains a variable, returning an empty string if it does not exist.
@@ -343,17 +339,19 @@ impl<'a> Shell {
         self.variables.get_var_or_empty(name)
     }
 
-    /// Gets an array, if it exists.
+    /// Gets an array variable, if it exists within the shell's array map.
     pub fn get_array(&self, name: &str) -> Option<&[String]> {
         self.variables.get_array(name).map(SmallVec::as_ref)
     }
 
     #[allow(dead_code)]
-    /// A method for executing commands in the Ion shell without capturing.
-    ///
-    /// Takes command(s) as a string argument, parses them, and executes them the same as it
-    /// would if you had executed the command(s) in the command line REPL interface for Ion.
-    /// If the supplied command is not terminated, then an error will be returned.
+    /// A method for executing commands in the Ion shell without capturing. It takes command(s)
+    /// as
+    /// a string argument, parses them, and executes them the same as it would if you had
+    /// executed
+    /// the command(s) in the command line REPL interface for Ion. If the supplied command is
+    /// not
+    /// terminated, then an error will be returned.
     pub fn execute_command<CMD>(&mut self, command: CMD) -> Result<i32, IonError>
         where CMD: Into<Terminator>
     {
@@ -367,10 +365,9 @@ impl<'a> Shell {
     }
 
     #[allow(dead_code)]
-    /// A method for executing scripts in the Ion shell without capturing.
-    ///
-    /// Given a `Path`, this method will attempt to execute that file as a script, and then
-    /// returns the final exit status of the evaluated script.
+    /// A method for executing scripts in the Ion shell without capturing. Given a `Path`, this
+    /// method will attempt to execute that file as a script, and then returns the final exit
+    /// status of the evaluated script.
     pub fn execute_script<SCRIPT: AsRef<Path>>(&mut self, script: SCRIPT) -> io::Result<i32> {
         let mut script = File::open(script.as_ref())?;
         let capacity = script.metadata().ok().map_or(0, |x| x.len());
@@ -400,58 +397,17 @@ impl<'a> Shell {
 
     /// A method for capturing the output of the shell, and performing actions without modifying
     /// the state of the original shell. This performs a fork, taking a closure that controls
-    /// the
-    /// shell in the child of the fork.
+    /// the shell in the child of the fork.
     ///
     /// The method is non-blocking, and therefore will immediately return file handles to the
     /// stdout and stderr of the child. The PID of the child is returned, which may be used to
     /// wait for and obtain the exit status.
-    pub fn fork<F: FnMut(&mut Shell)>(&self, mut child_func: F) -> Result<IonResult, IonError> {
-        use std::os::unix::io::{AsRawFd, FromRawFd};
-        use std::process::exit;
-        use sys;
-
-        sys::signals::block();
-
-        let (stdout_read, stdout_write) = sys::pipe2(sys::O_CLOEXEC)
-            .map(|fds| unsafe { (File::from_raw_fd(fds.0), File::from_raw_fd(fds.1)) })
-            .map_err(IonError::Fork)?;
-
-        let (stderr_read, stderr_write) = sys::pipe2(sys::O_CLOEXEC)
-            .map(|fds| unsafe { (File::from_raw_fd(fds.0), File::from_raw_fd(fds.1)) })
-            .map_err(IonError::Fork)?;
-
-        match unsafe { sys::fork() } {
-            Ok(0) => {
-                sys::signals::unblock();
-
-                let _ = sys::dup2(stdout_write.as_raw_fd(), sys::STDOUT_FILENO);
-                let _ = sys::dup2(stderr_write.as_raw_fd(), sys::STDERR_FILENO);
-
-                drop(stdout_write);
-                drop(stdout_read);
-                drop(stderr_write);
-                drop(stderr_read);
-
-                let mut shell: Shell = unsafe { (self as *const Shell).read() };
-                child_func(&mut shell);
-
-                // Reap the child, enabling the parent to get EOF from the read end of the pipe.
-                exit(shell.previous_status);
-            }
-            Ok(pid) => {
-                // Drop the write end of the pipe, because the parent will not use it.
-                drop(stdout_write);
-                drop(stderr_write);
-
-                Ok(IonResult {
-                    pid,
-                    stdout: stdout_read,
-                    stderr: stderr_read,
-                })
-            }
-            Err(why) => Err(IonError::Fork(why)),
-        }
+    pub fn fork<F: FnMut(&mut Shell)>(
+        &self,
+        capture: Capture,
+        child_func: F,
+    ) -> Result<IonResult, IonError> {
+        Fork::new(self, capture).exec(child_func)
     }
 }
 
@@ -522,10 +478,10 @@ impl<'a> Expander for Shell {
     /// Uses a subshell to expand a given command.
     fn command(&self, command: &str) -> Option<Value> {
         let mut output = None;
-        match self.fork(move |shell| shell.on_command(command)) {
-            Ok(mut result) => {
+        match self.fork(Capture::Stdout, move |shell| shell.on_command(command)) {
+            Ok(result) => {
                 let mut string = String::new();
-                match result.stdout.read_to_string(&mut string) {
+                match result.stdout.unwrap().read_to_string(&mut string) {
                     Ok(_) => output = Some(string),
                     Err(why) => {
                         eprintln!("ion: error reading stdout of child: {}", why);
-- 
GitLab