diff --git a/brush-core/src/commands.rs b/brush-core/src/commands.rs index e1aef8f1..8bc82795 100644 --- a/brush-core/src/commands.rs +++ b/brush-core/src/commands.rs @@ -131,11 +131,7 @@ impl ExecutionContext<'_> { /// Returns the file descriptor with the given number. #[allow(clippy::unwrap_in_result)] pub fn fd(&self, fd: u32) -> Option { - self.params - .open_files - .files - .get(&fd) - .map(|f| f.try_dup().unwrap()) + self.params.open_files.files.get(&fd).cloned() } pub(crate) fn should_cmd_lead_own_process_group(&self) -> bool { @@ -562,7 +558,7 @@ pub(crate) async fn invoke_command_in_subshell_and_get_output( subshell .open_files .files - .insert(1, openfiles::OpenFile::PipeWriter(writer)); + .insert(1, openfiles::OpenFile::PipeWriter(Arc::new(writer))); let mut params = subshell.default_exec_params(); params.process_group_policy = ProcessGroupPolicy::SameProcessGroup; diff --git a/brush-core/src/interp.rs b/brush-core/src/interp.rs index 5be6a3e2..3481c484 100644 --- a/brush-core/src/interp.rs +++ b/brush-core/src/interp.rs @@ -1254,7 +1254,7 @@ fn setup_pipeline_redirection( // Set up stdin of this process to take stdout of the preceding process. open_files .files - .insert(0, OpenFile::PipeReader(preceding_output_reader)); + .insert(0, OpenFile::PipeReader(Arc::new(preceding_output_reader))); } else { open_files.files.insert(0, OpenFile::Null); } @@ -1266,7 +1266,9 @@ fn setup_pipeline_redirection( // Set up stdout of this process to go to stdin of the succeeding process. let (reader, writer) = sys::pipes::pipe()?; context.output_pipes.push(reader); - open_files.files.insert(1, OpenFile::PipeWriter(writer)); + open_files + .files + .insert(1, OpenFile::PipeWriter(Arc::new(writer))); } Ok(()) @@ -1301,8 +1303,8 @@ pub(crate) async fn setup_redirect( ) })?; - let stdout_file = OpenFile::File(opened_file); - let stderr_file = stdout_file.try_dup()?; + let stdout_file = OpenFile::File(Arc::new(opened_file)); + let stderr_file = stdout_file.clone(); open_files.files.insert(1, stdout_file); open_files.files.insert(2, stderr_file); @@ -1382,7 +1384,7 @@ pub(crate) async fn setup_redirect( err, ) })?; - target_file = OpenFile::File(opened_file); + target_file = OpenFile::File(Arc::new(opened_file)); } ast::IoFileRedirectTarget::Fd(fd) => { let default_fd_if_unspecified = match kind { @@ -1396,7 +1398,7 @@ pub(crate) async fn setup_redirect( fd_num = specified_fd_num.unwrap_or(default_fd_if_unspecified); if let Some(f) = open_files.files.get(fd) { - target_file = f.try_dup()?; + target_file = f.clone(); } else { tracing::error!("{}: Bad file descriptor", fd); return Ok(None); @@ -1416,7 +1418,7 @@ pub(crate) async fn setup_redirect( subshell_cmd, )?; - target_file = substitution_file.try_dup()?; + target_file = substitution_file.clone(); open_files.files.insert(substitution_fd, substitution_file); fd_num = specified_fd_num @@ -1491,15 +1493,15 @@ fn setup_process_substitution( subshell .open_files .files - .insert(1, openfiles::OpenFile::PipeWriter(writer)); - OpenFile::PipeReader(reader) + .insert(1, openfiles::OpenFile::PipeWriter(Arc::new(writer))); + OpenFile::PipeReader(Arc::new(reader)) } ast::ProcessSubstitutionKind::Write => { subshell .open_files .files - .insert(0, openfiles::OpenFile::PipeReader(reader)); - OpenFile::PipeWriter(writer) + .insert(0, openfiles::OpenFile::PipeReader(Arc::new(reader))); + OpenFile::PipeWriter(Arc::new(writer)) } }; @@ -1545,5 +1547,5 @@ fn setup_open_file_with_contents(contents: &str) -> Result), /// A read end of a pipe. - PipeReader(sys::pipes::PipeReader), + PipeReader(Arc), /// A write end of a pipe. - PipeWriter(sys::pipes::PipeWriter), -} - -impl Clone for OpenFile { - fn clone(&self) -> Self { - self.try_dup().unwrap() - } + PipeWriter(Arc), } impl OpenFile { - /// Tries to duplicate the open file. - pub fn try_dup(&self) -> Result { - let result = match self { - OpenFile::Stdin => OpenFile::Stdin, - OpenFile::Stdout => OpenFile::Stdout, - OpenFile::Stderr => OpenFile::Stderr, - OpenFile::Null => OpenFile::Null, - OpenFile::File(f) => OpenFile::File(f.try_clone()?), - OpenFile::PipeReader(f) => OpenFile::PipeReader(f.try_clone()?), - OpenFile::PipeWriter(f) => OpenFile::PipeWriter(f.try_clone()?), - }; - - Ok(result) - } - /// Converts the open file into an `OwnedFd`. #[cfg(unix)] pub(crate) fn into_owned_fd(self) -> Result { @@ -58,9 +39,9 @@ impl OpenFile { OpenFile::Stdout => Ok(std::io::stdout().as_fd().try_clone_to_owned()?), OpenFile::Stderr => Ok(std::io::stderr().as_fd().try_clone_to_owned()?), OpenFile::Null => error::unimp("to_owned_fd for null open file"), - OpenFile::File(f) => Ok(f.into()), - OpenFile::PipeReader(r) => Ok(OwnedFd::from(r)), - OpenFile::PipeWriter(w) => Ok(OwnedFd::from(w)), + OpenFile::File(f) => Ok((*f).try_clone()?.into()), + OpenFile::PipeReader(r) => Ok(OwnedFd::from((*r).try_clone()?)), + OpenFile::PipeWriter(w) => Ok(OwnedFd::from((*w).try_clone()?)), } } @@ -137,7 +118,7 @@ impl OpenFile { impl From for OpenFile { fn from(file: std::fs::File) -> Self { - OpenFile::File(file) + OpenFile::File(Arc::new(file)) } } @@ -148,9 +129,9 @@ impl From for Stdio { OpenFile::Stdout => Stdio::inherit(), OpenFile::Stderr => Stdio::inherit(), OpenFile::Null => Stdio::null(), - OpenFile::File(f) => f.into(), - OpenFile::PipeReader(f) => f.into(), - OpenFile::PipeWriter(f) => f.into(), + OpenFile::File(f) => (*f).try_clone().unwrap().into(), + OpenFile::PipeReader(f) => (*f).try_clone().unwrap().into(), + OpenFile::PipeWriter(f) => (*f).try_clone().unwrap().into(), } } } @@ -169,7 +150,7 @@ impl std::io::Read for OpenFile { )), OpenFile::Null => Ok(0), OpenFile::File(f) => f.read(buf), - OpenFile::PipeReader(reader) => reader.read(buf), + OpenFile::PipeReader(reader) => reader.as_ref().read(buf), OpenFile::PipeWriter(_) => Err(std::io::Error::new( std::io::ErrorKind::Other, error::Error::OpenFileNotReadable("pipe writer"), @@ -193,7 +174,7 @@ impl std::io::Write for OpenFile { std::io::ErrorKind::Other, error::Error::OpenFileNotWritable("pipe reader"), )), - OpenFile::PipeWriter(writer) => writer.write(buf), + OpenFile::PipeWriter(writer) => writer.as_ref().write(buf), } } @@ -205,7 +186,7 @@ impl std::io::Write for OpenFile { OpenFile::Null => Ok(()), OpenFile::File(f) => f.flush(), OpenFile::PipeReader(_) => Ok(()), - OpenFile::PipeWriter(writer) => writer.flush(), + OpenFile::PipeWriter(writer) => writer.as_ref().flush(), } } } @@ -230,16 +211,6 @@ impl Default for OpenFiles { } impl OpenFiles { - /// Tries to clone the open files. - pub fn try_clone(&self) -> Result { - let mut files = im::HashMap::new(); - for (fd, file) in &self.files { - files.insert(*fd, file.try_dup()?); - } - - Ok(OpenFiles { files }) - } - /// Retrieves the file backing standard input in this context. pub fn stdin(&self) -> Option<&OpenFile> { self.files.get(&0) diff --git a/brush-core/src/shell.rs b/brush-core/src/shell.rs index 64e62983..b057a83d 100644 --- a/brush-core/src/shell.rs +++ b/brush-core/src/shell.rs @@ -1060,7 +1060,7 @@ impl Shell { if let Some(filename) = path_to_open.file_name() { if let Ok(fd_num) = filename.to_string_lossy().to_string().parse::() { if let Some(open_file) = params.open_files.files.get(&fd_num) { - return open_file.try_dup(); + return Ok(open_file.clone()); } } } @@ -1151,13 +1151,13 @@ impl Shell { /// Returns a value that can be used to write to the shell's currently configured /// standard output stream using `write!` at al. pub fn stdout(&self) -> openfiles::OpenFile { - self.open_files.files.get(&1).unwrap().try_dup().unwrap() + self.open_files.files.get(&1).unwrap().clone() } /// Returns a value that can be used to write to the shell's currently configured /// standard error stream using `write!` et al. pub fn stderr(&self) -> openfiles::OpenFile { - self.open_files.files.get(&2).unwrap().try_dup().unwrap() + self.open_files.files.get(&2).unwrap().clone() } /// Outputs `set -x` style trace output for a command.