diff --git a/src/exec.cpp b/src/exec.cpp index 84c4ff416..5f4d2ec10 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -292,7 +292,9 @@ void internal_exec(env_stack_t &vars, job_t *j, const io_chain_t &block_io) { // Do a regular launch - but without forking first... process_t *p = j->processes.front().get(); io_chain_t all_ios = block_io; - all_ios.append(p->io_chain()); + if (!all_ios.append_from_specs(p->redirection_specs())) { + return; + } // child_setup_process makes sure signals are properly set up. auto redirs = dup2_list_t::resolve_chain(all_ios); @@ -532,15 +534,19 @@ static bool exec_internal_builtin_proc(parser_t &parser, const std::shared_ptris_first_in_job) { // We must have a pipe stdin_is_directly_redirected = true; } else { // We are not a pipe. Check if there is a redirection local to the process // that's not io_mode_t::close. - const shared_ptr stdin_io = p->io_chain().io_for_fd(STDIN_FILENO); - stdin_is_directly_redirected = stdin_io && stdin_io->io_mode != io_mode_t::close; + for (const auto &redir : p->redirection_specs()) { + if (redir.fd == STDIN_FILENO && !redir.is_close()) { + stdin_is_directly_redirected = true; + break; + } + } } streams.stdin_fd = local_builtin_stdin; @@ -975,13 +981,18 @@ static bool exec_process_in_job(parser_t &parser, process_t *p, std::shared_ptr< // The IO chain for this process. io_chain_t process_net_io_chain = block_io; + if (pipes.write.valid()) { process_net_io_chain.push_back(std::make_shared( p->pipe_write_fd, false /* not input */, std::move(pipes.write))); } - // The explicit IO redirections associated with the process. - process_net_io_chain.append(p->io_chain()); + // Append IOs from the process's redirection specs. + // This may fail. + if (!process_net_io_chain.append_from_specs(p->redirection_specs())) { + // Error. + return false; + } // Read pipe goes last. shared_ptr pipe_read{}; @@ -1149,8 +1160,8 @@ bool exec_job(parser_t &parser, shared_ptr j, const job_lineage_t &lineag // Get the list of all FDs so we can ensure our pipes do not conflict. fd_set_t conflicts = lineage.block_io.fd_set(); for (const auto &p : j->processes) { - for (const auto &io : p->io_chain()) { - conflicts.add(io->fd); + for (const auto &spec : p->redirection_specs()) { + conflicts.add(spec.fd); } } diff --git a/src/io.cpp b/src/io.cpp index 3fcdd9070..5b59105a9 100644 --- a/src/io.cpp +++ b/src/io.cpp @@ -222,6 +222,30 @@ void io_chain_t::append(const io_chain_t &chain) { this->insert(this->end(), chain.begin(), chain.end()); } +bool io_chain_t::append_from_specs(const redirection_spec_list_t &specs) { + for (const auto &spec : specs) { + switch (spec.mode) { + case redirection_mode_t::fd: { + if (spec.is_close()) { + this->push_back(make_unique(spec.fd)); + } else { + auto target_fd = spec.get_target_as_fd(); + assert(target_fd.has_value() && + "fd redirection should have been validated already"); + this->push_back( + make_unique(spec.fd, *target_fd, true /* user supplied */)); + } + break; + } + default: { + this->push_back(make_unique(spec.fd, spec.target, spec.oflags())); + break; + } + } + } + return true; +} + void io_chain_t::print() const { if (this->empty()) { std::fwprintf(stderr, L"Empty chain %p\n", this); diff --git a/src/io.h b/src/io.h index 406bd8e84..d16cf2625 100644 --- a/src/io.h +++ b/src/io.h @@ -16,6 +16,7 @@ #include "env.h" #include "global_safety.h" #include "maybe.h" +#include "redirection.h" using std::shared_ptr; @@ -359,6 +360,10 @@ class io_chain_t : public std::vector { /// if none. io_data_ref_t io_for_fd(int fd) const; + /// Attempt to resolve a list of redirection specs to IOs, appending to 'this'. + /// \return true on success, false on error, in which case an error will have been printed. + bool append_from_specs(const redirection_spec_list_t &specs); + /// Output debugging information to stderr. void print() const; diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index f972fa422..d707c4164 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -82,8 +82,9 @@ static wcstring profiling_cmd_name_for_redirectable_block(const parse_node_t &no } /// Get a redirection from stderr to stdout (i.e. 2>&1). -static std::shared_ptr get_stderr_merge() { - return std::make_shared(STDERR_FILENO, STDOUT_FILENO, true /* user_supplied */); +static redirection_spec_t get_stderr_merge() { + const wchar_t *stdout_fileno_str = L"1"; + return redirection_spec_t{STDERR_FILENO, redirection_mode_t::fd, stdout_fileno_str}; } parse_execution_context_t::parse_execution_context_t(parsed_source_ref_t pstree, parser_t *p, @@ -832,7 +833,7 @@ parse_execution_result_t parse_execution_context_t::populate_plain_process( // Produce the full argument list and the set of IO redirections. wcstring_list_t cmd_args; - io_chain_t process_io_chain; + redirection_spec_list_t redirections; if (use_implicit_cd) { // Implicit cd is simple. cmd_args = {L"cd", cmd}; @@ -858,7 +859,7 @@ parse_execution_result_t parse_execution_context_t::populate_plain_process( } // The set of IO redirections that we construct for the process. - if (!this->determine_io_chain(statement.child<1>(), &process_io_chain)) { + if (!this->determine_redirections(statement.child<1>(), &redirections)) { return parse_execution_errored; } @@ -869,7 +870,7 @@ parse_execution_result_t parse_execution_context_t::populate_plain_process( // Populate the process. proc->type = process_type; proc->set_argv(cmd_args); - proc->set_io_chain(process_io_chain); + proc->set_redirection_specs(std::move(redirections)); proc->actual_cmd = std::move(path_to_external_command); return parse_execution_success; } @@ -928,18 +929,15 @@ parse_execution_result_t parse_execution_context_t::expand_arguments_from_nodes( return parse_execution_success; } -bool parse_execution_context_t::determine_io_chain(tnode_t node, - io_chain_t *out_chain) { - io_chain_t result; - bool errored = false; - +bool parse_execution_context_t::determine_redirections( + tnode_t node, redirection_spec_list_t *out_redirections) { // Get all redirection nodes underneath the statement. while (auto redirect_node = node.next_in_list()) { wcstring target; // file path or target fd auto redirect = redirection_for_node(redirect_node, pstree->src, &target); if (!redirect || !redirect->is_valid()) { - // TODO: improve this error message. + // TODO: figure out if this can ever happen. If so, improve this error message. report_error(redirect_node, _(L"Invalid redirection: %ls"), redirect_node.get_source(pstree->src).c_str()); return false; @@ -955,50 +953,27 @@ bool parse_execution_context_t::determine_io_chain(tnode_t new_io; + // Make a redirection spec from the redirect token. assert(redirect && redirect->is_valid() && "expected to have a valid redirection"); - switch (redirect->mode) { - case redirection_mode_t::fd: { - if (target == L"-") { - new_io.reset(new io_close_t(redirect->fd)); - } else { - int old_fd = fish_wcstoi(target.c_str()); - if (errno || old_fd < 0) { - const wchar_t *fmt = - _(L"Requested redirection to '%ls', " - L"which is not a valid file descriptor"); - errored = report_error(redirect_node, fmt, target.c_str()); - } else { - new_io.reset(new io_fd_t(redirect->fd, old_fd, true)); - } - } - break; - } - default: { - int oflags = redirect->oflags(); - io_file_t *new_io_file = new io_file_t(redirect->fd, target, oflags); - new_io.reset(new_io_file); - break; - } - } - // Append the new_io if we got one. - if (new_io.get() != nullptr) { - result.push_back(new_io); + redirection_spec_t spec{redirect->fd, redirect->mode, std::move(target)}; + + // Validate this spec. + if (spec.mode == redirection_mode_t::fd && !spec.is_close() && !spec.get_target_as_fd()) { + const wchar_t *fmt = + _(L"Requested redirection to '%ls', which is not a valid file descriptor"); + report_error(redirect_node, fmt, spec.target.c_str()); + return false; } + out_redirections->push_back(std::move(spec)); if (redirect->stderr_merge) { // This was a redirect like &> which also modifies stderr. // Also redirect stderr to stdout. - result.push_back(get_stderr_merge()); + out_redirections->push_back(get_stderr_merge()); } } - - if (out_chain && !errored) { - *out_chain = std::move(result); - } - return !errored; + return true; } parse_execution_result_t parse_execution_context_t::populate_not_process( @@ -1026,14 +1001,15 @@ parse_execution_result_t parse_execution_context_t::populate_block_process( // The set of IO redirections that we construct for the process. // TODO: fix this ugly find_child. auto arguments = specific_statement.template find_child(); - io_chain_t process_io_chain; - bool errored = !this->determine_io_chain(arguments, &process_io_chain); - if (errored) return parse_execution_errored; + redirection_spec_list_t redirections; + if (!this->determine_redirections(arguments, &redirections)) { + return parse_execution_errored; + } proc->type = process_type_t::block_node; proc->block_node_source = pstree; proc->internal_block_node = statement; - proc->set_io_chain(process_io_chain); + proc->set_redirection_specs(std::move(redirections)); return parse_execution_success; } @@ -1172,9 +1148,9 @@ parse_execution_result_t parse_execution_context_t::populate_job_from_job_node( if (parsed_pipe->stderr_merge) { // This was a pipe like &| which redirects both stdout and stderr. // Also redirect stderr to stdout. - auto ios = processes.back()->io_chain(); - ios.push_back(get_stderr_merge()); - processes.back()->set_io_chain(std::move(ios)); + auto specs = processes.back()->redirection_specs(); + specs.push_back(get_stderr_merge()); + processes.back()->set_redirection_specs(std::move(specs)); } // Store the new process (and maybe with an error). diff --git a/src/parse_execution.h b/src/parse_execution.h index 5c8d2f7dd..4edf7dc76 100644 --- a/src/parse_execution.h +++ b/src/parse_execution.h @@ -125,9 +125,10 @@ class parse_execution_context_t { wcstring_list_t *out_arguments, globspec_t glob_behavior); - // Determines the IO chain. Returns true on success, false on error. - bool determine_io_chain(tnode_t node, - io_chain_t *out_chain); + // Determines the list of redirections for a node. Returns none() on failure, for example, an + // invalid fd. + bool determine_redirections(tnode_t node, + redirection_spec_list_t *out_redirs); parse_execution_result_t run_1_job(tnode_t job, const block_t *associated_block); parse_execution_result_t run_job_conjunction(tnode_t job_expr, diff --git a/src/proc.h b/src/proc.h index 4cd507745..0118d560a 100644 --- a/src/proc.h +++ b/src/proc.h @@ -172,7 +172,7 @@ class process_t { private: null_terminated_array_t argv_array; - io_chain_t process_io_chain; + redirection_spec_list_t proc_redirection_specs; // No copying. process_t(const process_t &rhs) = delete; @@ -221,10 +221,11 @@ class process_t { return argv ? argv[0] : nullptr; } - /// IO chain getter and setter. - const io_chain_t &io_chain() const { return process_io_chain; } - - void set_io_chain(io_chain_t chain) { this->process_io_chain = std::move(chain); } + /// Redirection list getter and setter. + const redirection_spec_list_t &redirection_specs() const { return proc_redirection_specs; } + void set_redirection_specs(redirection_spec_list_t specs) { + this->proc_redirection_specs = std::move(specs); + } /// Store the current topic generations. That is, right before the process is launched, record /// the generations of all topics; then we can tell which generation values have changed after diff --git a/src/redirection.cpp b/src/redirection.cpp index e6395b40a..8c65ea4c1 100644 --- a/src/redirection.cpp +++ b/src/redirection.cpp @@ -4,6 +4,7 @@ #include +#include "io.h" #include "wutil.h" #define NOCLOB_ERROR _(L"The file '%ls' already exists") @@ -15,6 +16,28 @@ dup2_list_t::~dup2_list_t() = default; +maybe_t redirection_spec_t::get_target_as_fd() const { + errno = 0; + int result = fish_wcstoi(target.c_str()); + if (errno || result < 0) return none(); + return result; +} + +int redirection_spec_t::oflags() const { + switch (mode) { + case redirection_mode_t::append: + return O_CREAT | O_APPEND | O_WRONLY; + case redirection_mode_t::overwrite: + return O_CREAT | O_WRONLY | O_TRUNC; + case redirection_mode_t::noclob: + return O_CREAT | O_EXCL | O_WRONLY; + case redirection_mode_t::input: + return O_RDONLY; + case redirection_mode_t::fd: + DIE("Not a file redirection"); + } +} + maybe_t dup2_list_t::resolve_chain(const io_chain_t &io_chain) { ASSERT_IS_NOT_FORKED_CHILD(); dup2_list_t result; diff --git a/src/redirection.h b/src/redirection.h index 9d5861f1d..0d43e0882 100644 --- a/src/redirection.h +++ b/src/redirection.h @@ -4,10 +4,49 @@ #include #include "common.h" -#include "io.h" #include "maybe.h" -/// This file supports "applying" redirections. +/// This file supports specifying and applying redirections. + +enum class redirection_mode_t { + overwrite, // normal redirection: > file.txt + append, // appending redirection: >> file.txt + input, // input redirection: < file.txt + fd, // fd redirection: 2>&1 + noclob // noclobber redirection: >? file.txt +}; + +class io_chain_t; + +/// A struct which represents a redirection specification from the user. +/// Here the file descriptors don't represent open files - it's purely textual. +struct redirection_spec_t { + /// The redirected fd, or -1 on overflow. + /// In the common case of a pipe, this is 1 (STDOUT_FILENO). + /// For example, in the case of "3>&1" this will be 3. + int fd{-1}; + + /// The redirection mode. + redirection_mode_t mode{redirection_mode_t::overwrite}; + + /// The target of the redirection. + /// For example in "3>&1", this will be "1". + /// In "< file.txt" this will be "file.txt". + wcstring target{}; + + /// \return if this is a close-type redirection. + bool is_close() const { return mode == redirection_mode_t::fd && target == L"-"; } + + /// Attempt to parse target as an fd. Return the fd, or none() if none. + maybe_t get_target_as_fd() const; + + /// \return the open flags for this redirection. + int oflags() const; + + redirection_spec_t(int fd, redirection_mode_t mode, wcstring target) + : fd(fd), mode(mode), target(std::move(target)) {} +}; +using redirection_spec_list_t = std::vector; /// A class representing a sequence of basic redirections. class dup2_list_t { diff --git a/src/tokenizer.h b/src/tokenizer.h index 5e6e3a12b..90563b701 100644 --- a/src/tokenizer.h +++ b/src/tokenizer.h @@ -8,6 +8,7 @@ #include "common.h" #include "maybe.h" #include "parse_constants.h" +#include "redirection.h" /// Token types. enum class token_type_t { @@ -22,14 +23,6 @@ enum class token_type_t { comment, /// comment token }; -enum class redirection_mode_t { - overwrite, // normal redirection: > file.txt - append, // appending redirection: >> file.txt - input, // input redirection: < file.txt - fd, // fd redirection: 2>&1 - noclob // noclobber redirection: >? file.txt -}; - /// Flag telling the tokenizer to accept incomplete parameters, i.e. parameters with mismatching /// parenthesis, etc. This is useful for tab-completion. #define TOK_ACCEPT_UNFINISHED 1