diff --git a/src/builtins/exists.rs b/src/builtins/exists.rs index 1a16706c49ab22e8db1627b2e32a68da1a6d1dd6..e788897151f9033a17caf476083a5387c12184a3 100644 --- a/src/builtins/exists.rs +++ b/src/builtins/exists.rs @@ -153,7 +153,7 @@ fn binary_is_in_path(binaryname: &str, shell: &Shell) -> bool { // TODO: Right now they use an entirely different logic which means that it *might* be possible // TODO: that `exists` reports a binary to be in the path, while the shell cannot find it or // TODO: vice-versa - if let Some(path) = shell.variables.get_var("PATH") { + if let Some(path) = shell.get_var("PATH") { for dir in path.split(":") { let fname = format!("{}/{}", dir, binaryname); if let Ok(metadata) = fs::metadata(&fname) { @@ -197,7 +197,7 @@ fn array_var_is_not_empty(arrayvar: &str, shell: &Shell) -> bool { /// Returns true if the variable is a string and the string is not empty fn string_var_is_not_empty(stringvar: &str, shell: &Shell) -> bool { - match shell.variables.get_var(stringvar) { + match shell.get_var(stringvar) { Some(string) => !string.is_empty(), None => false, } @@ -250,8 +250,8 @@ fn test_evaluate_arguments() { // TODO: see test_binary_is_in_path() // no argument means we treat it as a string assert_eq!(evaluate_arguments(&["-b"], &mut sink, &shell), Ok(true)); - let oldpath = shell.variables.get_var("PATH").unwrap_or("/usr/bin".to_owned()); - shell.variables.set_var("PATH", "testing/"); + let oldpath = shell.get_var("PATH").unwrap_or("/usr/bin".to_owned()); + shell.set_var("PATH", "testing/"); assert_eq!(evaluate_arguments(&["-b", "executable_file"], &mut sink, &shell), Ok(true)); assert_eq!(evaluate_arguments(&["-b", "empty_file"], &mut sink, &shell), Ok(false)); @@ -259,7 +259,7 @@ fn test_evaluate_arguments() { // restore original PATH. Not necessary for the currently defined test cases but this might // change in the future? Better safe than sorry! - shell.variables.set_var("PATH", &oldpath); + shell.set_var("PATH", &oldpath); // check `exists -d` // no argument means we treat it as a string @@ -278,9 +278,9 @@ fn test_evaluate_arguments() { // check `exists -s` // no argument means we treat it as a string assert_eq!(evaluate_arguments(&["-s"], &mut sink, &shell), Ok(true)); - shell.variables.set_var("emptyvar", ""); + shell.set_var("emptyvar", ""); assert_eq!(evaluate_arguments(&["-s", "emptyvar"], &mut sink, &shell), Ok(false)); - shell.variables.set_var("testvar", "foobar"); + shell.set_var("testvar", "foobar"); assert_eq!(evaluate_arguments(&["-s", "testvar"], &mut sink, &shell), Ok(true)); shell.variables.unset_var("testvar"); assert_eq!(evaluate_arguments(&["-s", "testvar"], &mut sink, &shell), Ok(false)); @@ -362,7 +362,7 @@ fn test_binary_is_in_path() { // TODO: PATH containing directories without read permission (for user) // TODO: PATH containing directories without execute ("enter") permission (for user) // TODO: empty PATH? - shell.variables.set_var("PATH", "testing/"); + shell.set_var("PATH", "testing/"); assert_eq!(binary_is_in_path("executable_file", &shell), true); assert_eq!(binary_is_in_path("empty_file", &shell), false); @@ -400,7 +400,7 @@ fn test_array_var_is_not_empty() { assert_eq!(array_var_is_not_empty("NOT_EMPTY_ARRAY", &shell), false); // array_var_is_not_empty should NOT match for non-array variables with the same name - shell.variables.set_var("VARIABLE", "notempty-variable"); + shell.set_var("VARIABLE", "notempty-variable"); assert_eq!(array_var_is_not_empty("VARIABLE", &shell), false); } @@ -408,10 +408,10 @@ fn test_array_var_is_not_empty() { fn test_string_var_is_not_empty() { let mut shell = Shell::new(); - shell.variables.set_var("EMPTY", ""); + shell.set_var("EMPTY", ""); assert_eq!(string_var_is_not_empty("EMPTY", &shell), false); - shell.variables.set_var("NOT_EMPTY", "notempty"); + shell.set_var("NOT_EMPTY", "notempty"); assert_eq!(string_var_is_not_empty("NOT_EMPTY", &shell), true); // string_var_is_not_empty should NOT match for arrays with the same name diff --git a/src/builtins/ion.rs b/src/builtins/ion.rs index da61866af6e9c3901bd8ab20cccf0d8addd5eea5..bc52c8536396028f4a0405513dede262e8925bef 100644 --- a/src/builtins/ion.rs +++ b/src/builtins/ion.rs @@ -12,7 +12,7 @@ pub(crate) fn ion_docs(_: &[&str], shell: &mut Shell) -> i32 { return FAILURE; } - if let Some(cmd) = shell.variables.get_var("BROWSER".into()) { + if let Some(cmd) = shell.get_var("BROWSER".into()) { if let Ok(_) = Command::new(&cmd).arg(DOCPATH).spawn() { return SUCCESS; } diff --git a/src/shell/assignments.rs b/src/shell/assignments.rs index db5f11640b7f1186234e57b6739d4ed745eeb3a3..d36f86fdbdd6114c3db0522e87144963979009cd 100644 --- a/src/shell/assignments.rs +++ b/src/shell/assignments.rs @@ -96,9 +96,9 @@ impl VariableStore for Shell { match value_check(self, &expression, key.kind) { Ok(ReturnValue::Str(value)) => { - let lhs = self.variables.get_var_or_empty(&key.name); + let lhs = self.get_var_or_empty(&key.name); match math(&lhs, key.kind, operator, &value) { - Ok(value) => self.variables.set_var(&key.name, &value), + Ok(value) => self.set_var(&key.name, &value), Err(why) => { eprintln!("ion: assignment error: {}", why); return FAILURE; @@ -125,7 +125,7 @@ impl VariableStore for Shell { fn export(&mut self, action: ExportAction) -> i32 { let actions = match action { ExportAction::Assign(ref keys, op, ref vals) => AssignmentActions::new(keys, op, vals), - ExportAction::LocalExport(ref key) => match self.variables.get_var(key) { + ExportAction::LocalExport(ref key) => match self.get_var(key) { Some(var) => { env::set_var(key, &var); return SUCCESS; @@ -166,7 +166,7 @@ impl VariableStore for Shell { Ok(Action::UpdateString(key, operator, expression)) => { match value_check(self, &expression, key.kind) { Ok(ReturnValue::Str(value)) => { - let lhs = self.variables.get_var_or_empty(&key.name); + let lhs = self.get_var_or_empty(&key.name); match math(&lhs, key.kind, operator, &value) { Ok(value) => { let value = OsStr::from_bytes(&value.as_bytes()); diff --git a/src/shell/binary/mod.rs b/src/shell/binary/mod.rs index 3584095d5ef90adb7b93955024989cc01ccfea93..a0fd0140179534fc3322103e719db903c431bb15 100644 --- a/src/shell/binary/mod.rs +++ b/src/shell/binary/mod.rs @@ -10,7 +10,6 @@ use self::terminate::{terminate_quotes, terminate_script_quotes}; use super::{FlowLogic, JobControl, Shell, ShellHistory}; use super::flags::*; use super::flow_control::Statement; -use super::library::IonLibrary; use super::status::*; use liner::{Buffer, Context}; use smallvec::SmallVec; @@ -90,8 +89,8 @@ impl Binary for Shell { self.context = Some({ let mut context = Context::new(); context.word_divider_fn = Box::new(word_divide); - if "1" == self.variables.get_var_or_empty("HISTFILE_ENABLED") { - let path = self.variables.get_var("HISTFILE").expect("shell didn't set HISTFILE"); + if "1" == self.get_var_or_empty("HISTFILE_ENABLED") { + let path = self.get_var("HISTFILE").expect("shell didn't set HISTFILE"); context.history.set_file_name(Some(path.clone())); if !Path::new(path.as_str()).exists() { eprintln!("ion: creating history file at \"{}\"", path); @@ -104,7 +103,7 @@ impl Binary for Shell { // pass } Err(ref err) if err.kind() == ErrorKind::NotFound => { - let history_filename = self.variables.get_var_or_empty("HISTFILE"); + let history_filename = self.get_var_or_empty("HISTFILE"); eprintln!("ion: failed to find history file {}: {}", history_filename, err); } Err(err) => { diff --git a/src/shell/binary/prompt.rs b/src/shell/binary/prompt.rs index c227430f36d4ddbed091f461f122b806373d76db..54293de9f6b710102328b4816ca5e528d56e101e 100644 --- a/src/shell/binary/prompt.rs +++ b/src/shell/binary/prompt.rs @@ -1,11 +1,13 @@ use super::super::{Function, Shell}; use parser::shell_expand::expand_string; +use std::io::Read; +use sys; pub(crate) fn prompt(shell: &mut Shell) -> String { if shell.flow_control.level == 0 { let rprompt = match prompt_fn(shell) { Some(prompt) => prompt, - None => shell.variables.get_var_or_empty("PROMPT"), + None => shell.get_var_or_empty("PROMPT"), }; expand_string(&rprompt, shell, false).join(" ") } else { @@ -19,7 +21,26 @@ pub(crate) fn prompt_fn(shell: &mut Shell) -> Option<String> { None => return None, }; - shell.fork_and_output(|child| unsafe { + let mut output = None; + + match shell.fork(|child| unsafe { let _ = function.read().execute(child, &["ion"]); - }) + }) { + Ok(mut result) => { + let mut string = String::new(); + match result.stdout.read_to_string(&mut string) { + Ok(_) => output = Some(string), + Err(why) => { + eprintln!("ion: error reading stdout of child: {}", why); + } + } + } + Err(why) => { + eprintln!("ion: fork error: {}", why); + } + } + + // Ensure that the parent retains ownership of the terminal before exiting. + let _ = sys::tcsetpgrp(sys::STDIN_FILENO, sys::getpid().unwrap()); + output } diff --git a/src/shell/flow.rs b/src/shell/flow.rs index 3a4b6c77cb58c715e7cf4d3598549126769c2fdf..2a62b2e58ac13d9eee51b115ea5a4342acc6b96b 100644 --- a/src/shell/flow.rs +++ b/src/shell/flow.rs @@ -290,9 +290,8 @@ impl FlowLogic for Shell { .map(|x| ReturnValue::Vector(x.clone())); self.variables.set_array(&bind, value.clone()); } else { - previous_bind = - self.variables.get_var(bind).map(|x| ReturnValue::Str(x)); - self.variables.set_var(&bind, &value.join(" ")); + previous_bind = self.get_var(bind).map(|x| ReturnValue::Str(x)); + self.set_var(&bind, &value.join(" ")); } } @@ -308,7 +307,7 @@ impl FlowLogic for Shell { if let Some(ref bind) = case.binding { if let Some(value) = previous_bind { match value { - ReturnValue::Str(value) => self.variables.set_var(bind, &value), + ReturnValue::Str(value) => self.set_var(bind, &value), ReturnValue::Vector(values) => { self.variables.set_array(bind, values) } @@ -327,9 +326,8 @@ impl FlowLogic for Shell { .map(|x| ReturnValue::Vector(x.clone())); self.variables.set_array(&bind, value.clone()); } else { - previous_bind = - self.variables.get_var(bind).map(|x| ReturnValue::Str(x)); - self.variables.set_var(&bind, &value.join(" ")); + previous_bind = self.get_var(bind).map(|x| ReturnValue::Str(x)); + self.set_var(&bind, &value.join(" ")); } } @@ -345,7 +343,7 @@ impl FlowLogic for Shell { if let Some(ref bind) = case.binding { if let Some(value) = previous_bind { match value { - ReturnValue::Str(value) => self.variables.set_var(bind, &value), + ReturnValue::Str(value) => self.set_var(bind, &value), ReturnValue::Vector(values) => { self.variables.set_array(bind, values) } @@ -549,7 +547,7 @@ impl FlowLogic for Shell { } }, ForExpression::Multiple(values) => for value in values.iter() { - self.variables.set_var(variable, &value); + self.set_var(variable, &value); match self.execute_statements(statements.clone()) { Condition::Break => break, Condition::SigInt => return Condition::SigInt, @@ -564,7 +562,7 @@ impl FlowLogic for Shell { } }, ForExpression::Normal(values) => for value in values.lines() { - self.variables.set_var(variable, &value); + self.set_var(variable, &value); match self.execute_statements(statements.clone()) { Condition::Break => break, Condition::SigInt => return Condition::SigInt, @@ -579,7 +577,7 @@ impl FlowLogic for Shell { } }, ForExpression::Range(start, end) => for value in (start..end).map(|x| x.to_string()) { - self.variables.set_var(variable, &value); + self.set_var(variable, &value); match self.execute_statements(statements.clone()) { Condition::Break => break, Condition::SigInt => return Condition::SigInt, diff --git a/src/shell/flow_control.rs b/src/shell/flow_control.rs index 4f6677a81515d4d02491e706855b5a26b9d1adff..8073917593720191804ec56aca704415fc3ace2a 100644 --- a/src/shell/flow_control.rs +++ b/src/shell/flow_control.rs @@ -191,8 +191,8 @@ impl Function { shell.variables.set_array(&type_.name, vector); } ReturnValue::Str(string) => { - variables_backup.insert(&type_.name, shell.variables.get_var(&type_.name)); - shell.variables.set_var(&type_.name, &string); + variables_backup.insert(&type_.name, shell.get_var(&type_.name)); + shell.set_var(&type_.name, &string); } } } @@ -201,7 +201,7 @@ impl Function { for (name, value_option) in &variables_backup { match *value_option { - Some(ref value) => shell.variables.set_var(name, value), + Some(ref value) => shell.set_var(name, value), None => { shell.variables.unset_var(name); } diff --git a/src/shell/history.rs b/src/shell/history.rs index 67708adb5418f26962e8b06636dc5e455bf6a01b..14d52d3a9c337c879f7052d37cc1d057ffef15aa 100644 --- a/src/shell/history.rs +++ b/src/shell/history.rs @@ -85,17 +85,17 @@ impl ShellHistory for Shell { fn set_context_history_from_vars(&mut self) { let context = self.context.as_mut().unwrap(); - let max_history_size = - self.variables.get_var_or_empty("HISTORY_SIZE").parse().unwrap_or(1000); + let variables = &self.variables; + let max_history_size = variables.get_var_or_empty("HISTORY_SIZE").parse().unwrap_or(1000); context.history.set_max_size(max_history_size); - if &*self.variables.get_var_or_empty("HISTFILE_ENABLED") == "1" { - let file_name = self.variables.get_var("HISTFILE"); + if &*variables.get_var_or_empty("HISTFILE_ENABLED") == "1" { + let file_name = variables.get_var("HISTFILE"); context.history.set_file_name(file_name.map(|f| f.into())); let max_histfile_size = - self.variables.get_var_or_empty("HISTFILE_SIZE").parse().unwrap_or(1000); + variables.get_var_or_empty("HISTFILE_SIZE").parse().unwrap_or(1000); context.history.set_max_file_size(max_histfile_size); } else { context.history.set_file_name(None); diff --git a/src/shell/library.rs b/src/shell/library.rs deleted file mode 100644 index eae65d54ade9cff1006df1d3913e96f53ac1ebd9..0000000000000000000000000000000000000000 --- a/src/shell/library.rs +++ /dev/null @@ -1,110 +0,0 @@ -use super::{Binary, FlowLogic, Shell}; -use super::status::*; -use parser::Terminator; -use std::fmt::{self, Display, Formatter}; -use std::fs::File; -use std::io::{self, Read}; -use std::path::Path; - -pub enum IonError { - Fork(io::Error), -} - -impl Display for IonError { - fn fmt(&self, fmt: &mut Formatter) -> fmt::Result { - match *self { - IonError::Fork(ref why) => writeln!(fmt, "failed to fork: {}", why), - } - } -} - -pub struct IonResult { - pub stdout: File, - pub stderr: File, -} - -pub trait IonLibrary { - /// Executes the given command and returns the exit status, or if the supplied command is - /// not - /// terminated, an error will be returned. - fn execute_command<CMD>(&mut self, command: CMD) -> Result<i32, &'static str> - where CMD: Into<Terminator>; - - /// Executes all of the statements contained within a given script, - /// returning the final exit status. - fn execute_script<P: AsRef<Path>>(&mut self, path: P) -> io::Result<i32>; - - /// Performs a fork, taking a closure that controls 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. - fn fork<F>(&self, child_func: F) -> Result<IonResult, IonError> - where F: FnMut(&mut Shell); -} - -impl IonLibrary for Shell { - fn execute_command<CMD>(&mut self, command: CMD) -> Result<i32, &'static str> - where CMD: Into<Terminator> - { - let mut terminator = command.into(); - if terminator.is_terminated() { - self.on_command(&terminator.consume()); - Ok(self.previous_status) - } else { - Err("input is not terminated") - } - } - - fn execute_script<P: AsRef<Path>>(&mut self, path: P) -> io::Result<i32> { - let mut file = File::open(path.as_ref())?; - let capacity = file.metadata().ok().map_or(0, |x| x.len()); - let mut command_list = String::with_capacity(capacity as usize); - let _ = file.read_to_string(&mut command_list)?; - if FAILURE == self.terminate_script_quotes(command_list.lines().map(|x| x.to_owned())) { - self.previous_status = FAILURE; - } - Ok(self.previous_status) - } - - 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; - - 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) => { - 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 { - stdout: stdout_read, - stderr: stderr_read, - }) - } - Err(why) => Err(IonError::Fork(why)), - } - } -} diff --git a/src/shell/mod.rs b/src/shell/mod.rs index 1fe5d1c6cdb9efa206419563fa02e14ed7b58ef4..b5ba7cbaff64d1f74ceaf6bfc93c44d725b0e6fc 100644 --- a/src/shell/mod.rs +++ b/src/shell/mod.rs @@ -13,7 +13,6 @@ pub(crate) mod flow_control; pub(crate) mod signals; pub mod status; pub mod variables; -pub mod library; pub(crate) use self::binary::Binary; pub(crate) use self::flow::FlowLogic; @@ -26,7 +25,6 @@ use self::flags::*; use self::flow_control::{FlowControl, Function, FunctionError}; use self::foreground::ForegroundSignals; use self::job_control::{BackgroundProcess, JobControl}; -use self::library::IonLibrary; use self::pipe_exec::PipelineExecution; use self::status::*; use self::variables::Variables; @@ -35,13 +33,16 @@ use builtins::{BuiltinMap, BUILTINS}; use fnv::FnvHashMap; use liner::Context; use parser::{ArgumentSplitter, Expander, Select}; +use parser::Terminator; use parser::pipelines::Pipeline; use smallvec::SmallVec; use std::env; +use std::fmt::{self, Display, Formatter}; use std::fs::File; -use std::io::{self, Write}; +use std::io::{self, Read, Write}; use std::iter::FromIterator; use std::ops::Deref; +use std::path::Path; use std::process; use std::sync::{Arc, Mutex}; use std::sync::atomic::Ordering; @@ -49,6 +50,27 @@ use std::time::SystemTime; use sys; use types::*; +#[allow(dead_code)] +#[derive(Debug)] +pub enum IonError { + Fork(io::Error), +} + +impl Display for IonError { + fn fmt(&self, fmt: &mut Formatter) -> fmt::Result { + match *self { + IonError::Fork(ref why) => writeln!(fmt, "failed to fork: {}", why), + } + } +} + +#[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 @@ -165,7 +187,7 @@ impl<'a> Shell { env::current_dir().ok().map_or_else( || env::set_var("PWD", "?"), |path| { - let pwd = self.variables.get_var_or_empty("PWD"); + let pwd = self.get_var_or_empty("PWD"); let pwd: &str = &pwd; let current_dir = path.to_str().unwrap_or("?"); if pwd != current_dir { @@ -298,61 +320,106 @@ impl<'a> Shell { // Retrieve the exit_status and set the $? variable and history.previous_status if let Some(code) = exit_status { - self.variables.set_var("?", &code.to_string()); + self.set_var("?", &code.to_string()); self.previous_status = code; } exit_status } - fn fork_and_output<F: FnMut(&mut Shell)>(&self, mut child_func: F) -> Option<String> { - use std::io::Read; + /// Sets a variable. + pub fn set_var(&mut self, name: &str, value: &str) { self.variables.set_var(name, value); } + + /// Gets a variable, if it exists. + 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. + pub(crate) fn get_var_or_empty(&self, name: &str) -> String { + self.variables.get_var_or_empty(name) + } + + #[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. + pub fn execute_command<CMD>(&mut self, command: CMD) -> Result<i32, &'static str> + where CMD: Into<Terminator> + { + let mut terminator = command.into(); + if terminator.is_terminated() { + self.on_command(&terminator.consume()); + Ok(self.previous_status) + } else { + Err("input is not terminated") + } + } + + #[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. + 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()); + let mut command_list = String::with_capacity(capacity as usize); + let _ = script.read_to_string(&mut command_list)?; + if FAILURE == self.terminate_script_quotes(command_list.lines().map(|x| x.to_owned())) { + self.previous_status = FAILURE; + } + Ok(self.previous_status) + } + + /// 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 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; - let (mut out_read, out_write) = match sys::pipe2(sys::O_CLOEXEC) { - Ok(fds) => unsafe { (File::from_raw_fd(fds.0), File::from_raw_fd(fds.1)) }, - Err(why) => { - eprintln!("ion: unable to create pipe: {}", why); - return None; - } - }; + 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) => { - // Redirect stdout in the child to the write end of the pipe. - // Also close the read end of the pipe because we don't need it. - let _ = sys::dup2(out_write.as_raw_fd(), sys::STDOUT_FILENO); - drop(out_write); - drop(out_read); + 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); - // Then execute the required functionality in the child shell. 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(0); + exit(shell.previous_status); } - Ok(_pid) => { + Ok(pid) => { // Drop the write end of the pipe, because the parent will not use it. - drop(out_write); - - // Read from the read end of the pipe into a String. - let mut output = String::new(); - if let Err(why) = out_read.read_to_string(&mut output) { - eprintln!("ion: unable read child's output: {}", why); - return None; - } + drop(stdout_write); + drop(stderr_write); - // Ensure that the parent retains ownership of the terminal before exiting. - let _ = sys::tcsetpgrp(sys::STDIN_FILENO, sys::getpid().unwrap()); - - Some(output) - } - Err(why) => { - eprintln!("ion: fork error: {}", why); - None + Ok(IonResult { + pid, + stdout: stdout_read, + stderr: stderr_read, + }) } + Err(why) => Err(IonError::Fork(why)), } } } @@ -415,16 +482,32 @@ impl<'a> Expander for Shell { fn variable(&self, variable: &str, quoted: bool) -> Option<Value> { use ascii_helpers::AsciiReplace; if quoted { - self.variables.get_var(variable) + self.get_var(variable) } else { - self.variables.get_var(variable).map(|x| x.ascii_replace('\n', ' ').into()) + self.get_var(variable).map(|x| x.ascii_replace('\n', ' ').into()) } } /// Uses a subshell to expand a given command. fn command(&self, command: &str) -> Option<Value> { - self.fork_and_output(move |shell| { - shell.on_command(command); - }) + let mut output = None; + match self.fork(move |shell| shell.on_command(command)) { + Ok(mut result) => { + let mut string = String::new(); + match result.stdout.read_to_string(&mut string) { + Ok(_) => output = Some(string), + Err(why) => { + eprintln!("ion: error reading stdout of child: {}", why); + } + } + } + Err(why) => { + eprintln!("ion: fork error: {}", why); + } + } + + // Ensure that the parent retains ownership of the terminal before exiting. + let _ = sys::tcsetpgrp(sys::STDIN_FILENO, sys::getpid().unwrap()); + output } }