Make io_chain_t store const io_data_t

This will make it easier to reason about with concurrent execution.

src/exec.cpp

@@ -182,9 +182,6 @@ static void launch_process_nofork(env_stack_t &vars, process_t *p) {
     safe_launch_process(p, actual_cmd, argv_array.get(), envv);
 }
 
-/// Check if the IO redirection chains contains redirections for the specified file descriptor.
-static int has_fd(const io_chain_t &d, int fd) { return io_chain_get(d, fd).get() != NULL; }
-
 /// Make a copy of the specified io redirection chain, but change file redirection into fd
 /// redirection. This makes the redirection chain suitable for use as block-level io, since the file
 /// won't be repeatedly reopened for every command in the block, which would reset the cursor
@@ -211,7 +208,7 @@ static bool resolve_file_redirections_to_fds(const io_chain_t &in_chain, const w
     // Make our chain of redirections.
     io_chain_t result_chain;
 
-    for (const shared_ptr<io_data_t> &in : in_chain) {
+    for (const io_data_ref_t &in : in_chain) {
         switch (in->io_mode) {
             case io_mode_t::pipe:
             case io_mode_t::bufferfill:
@@ -222,7 +219,7 @@ static bool resolve_file_redirections_to_fds(const io_chain_t &in_chain, const w
             }
             case io_mode_t::file: {
                 // We have a path-based redireciton. Resolve it to a file.
-                io_file_t *in_file = static_cast<io_file_t *>(in.get());
+                const io_file_t *in_file = static_cast<const io_file_t *>(in.get());
                 int fd = wopen(path_apply_working_directory(in_file->filename, pwd), in_file->flags,
                                OPEN_MASK);
                 if (fd < 0) {
@@ -487,7 +484,7 @@ static bool exec_internal_builtin_proc(parser_t &parser, const std::shared_ptr<j
     // be reading from.
     if (pipe_read) {
         local_builtin_stdin = pipe_read->pipe_fd();
-    } else if (const auto in = proc_io_chain.get_io_for_fd(STDIN_FILENO)) {
+    } else if (const auto in = proc_io_chain.io_for_fd(STDIN_FILENO)) {
         switch (in->io_mode) {
             case io_mode_t::fd: {
                 const io_fd_t *in_fd = static_cast<const io_fd_t *>(in.get());
@@ -546,13 +543,13 @@ static bool exec_internal_builtin_proc(parser_t &parser, const std::shared_ptr<j
     } 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<const io_data_t> stdin_io = io_chain_get(p->io_chain(), STDIN_FILENO);
+        const shared_ptr<const io_data_t> stdin_io = p->io_chain().io_for_fd(STDIN_FILENO);
         stdin_is_directly_redirected = stdin_io && stdin_io->io_mode != io_mode_t::close;
     }
 
     streams.stdin_fd = local_builtin_stdin;
-    streams.out_is_redirected = has_fd(proc_io_chain, STDOUT_FILENO);
-    streams.err_is_redirected = has_fd(proc_io_chain, STDERR_FILENO);
+    streams.out_is_redirected = proc_io_chain.io_for_fd(STDOUT_FILENO) != nullptr;
+    streams.err_is_redirected = proc_io_chain.io_for_fd(STDERR_FILENO) != nullptr;
     streams.stdin_is_directly_redirected = stdin_is_directly_redirected;
     streams.io_chain = &proc_io_chain;
 
@@ -594,8 +591,8 @@ static bool handle_builtin_output(parser_t &parser, const std::shared_ptr<job_t>
     // We will try to elide constructing an internal process. However if the output is going to a
     // real file, we have to do it. For example in `echo -n > file.txt` we proceed to open file.txt
     // even though there is no output, so that it is properly truncated.
-    const shared_ptr<const io_data_t> stdout_io = io_chain->get_io_for_fd(STDOUT_FILENO);
-    const shared_ptr<const io_data_t> stderr_io = io_chain->get_io_for_fd(STDERR_FILENO);
+    const shared_ptr<const io_data_t> stdout_io = io_chain->io_for_fd(STDOUT_FILENO);
+    const shared_ptr<const io_data_t> stderr_io = io_chain->io_for_fd(STDERR_FILENO);
     bool must_use_process =
         redirection_is_to_real_file(stdout_io) || redirection_is_to_real_file(stderr_io);
 
@@ -1080,7 +1077,7 @@ bool exec_job(parser_t &parser, shared_ptr<job_t> j) {
     // The read limit is dictated by the last bufferfill.
     for (auto &io : all_ios) {
         if ((io->io_mode == io_mode_t::bufferfill)) {
-            const auto *bf = static_cast<io_bufferfill_t *>(io.get());
+            const auto *bf = static_cast<const io_bufferfill_t *>(io.get());
             stdout_read_limit = bf->buffer()->read_limit();
         }
     }

src/io.cpp

@@ -208,10 +208,10 @@ void io_chain_t::remove(const shared_ptr<const io_data_t> &element) {
     }
 }
 
-void io_chain_t::push_back(shared_ptr<io_data_t> element) {
+void io_chain_t::push_back(io_data_ref_t element) {
     // Ensure we never push back NULL.
     assert(element.get() != nullptr);
-    std::vector<shared_ptr<io_data_t> >::push_back(std::move(element));
+    std::vector<io_data_ref_t>::push_back(std::move(element));
 }
 
 void io_chain_t::append(const io_chain_t &chain) {
@@ -248,7 +248,7 @@ void io_print(const io_chain_t &chain)
 #endif
 
 int move_fd_to_unused(int fd, const io_chain_t &io_chain, bool cloexec) {
-    if (fd < 0 || io_chain.get_io_for_fd(fd).get() == NULL) {
+    if (fd < 0 || io_chain.io_for_fd(fd).get() == nullptr) {
         return fd;
     }
 
@@ -325,19 +325,12 @@ maybe_t<autoclose_pipes_t> make_autoclose_pipes(const io_chain_t &ios) {
     return {std::move(result)};
 }
 
-/// Return the last IO for the given fd.
-shared_ptr<const io_data_t> io_chain_t::get_io_for_fd(int fd) const {
-    size_t idx = this->size();
-    while (idx--) {
-        const shared_ptr<io_data_t> &data = this->at(idx);
+shared_ptr<const io_data_t> io_chain_t::io_for_fd(int fd) const {
+    for (auto iter = rbegin(); iter != rend(); ++iter) {
+        const auto &data = *iter;
         if (data->fd == fd) {
             return data;
         }
     }
-    return shared_ptr<const io_data_t>();
-}
-
-/// The old function returned the last match, so we mimic that.
-shared_ptr<const io_data_t> io_chain_get(const io_chain_t &src, int fd) {
-    return src.get_io_for_fd(fd);
+    return nullptr;
 }

src/io.h

@@ -325,22 +325,23 @@ class io_buffer_t {
     void append_from_stream(const output_stream_t &stream);
 };
 
-class io_chain_t : public std::vector<shared_ptr<io_data_t>> {
+using io_data_ref_t = std::shared_ptr<const io_data_t>;
+
+class io_chain_t : public std::vector<io_data_ref_t> {
    public:
-    using std::vector<shared_ptr<io_data_t>>::vector;
+    using std::vector<io_data_ref_t>::vector;
     // user-declared ctor to allow const init. Do not default this, it will break the build.
     io_chain_t() {}
 
-    void remove(const shared_ptr<const io_data_t> &element);
-    void push_back(shared_ptr<io_data_t> element);
+    void remove(const io_data_ref_t &element);
+    void push_back(io_data_ref_t element);
     void append(const io_chain_t &chain);
 
-    shared_ptr<const io_data_t> get_io_for_fd(int fd) const;
+    /// \return the last io redirection in the chain for the specified file descriptor, or nullptr
+    /// if none.
+    io_data_ref_t io_for_fd(int fd) const;
 };
 
-/// Return the last io redirection in the chain for the specified file descriptor.
-shared_ptr<const io_data_t> io_chain_get(const io_chain_t &src, int fd);
-
 /// Helper type returned from making autoclose pipes.
 struct autoclose_pipes_t {
     /// Read end of the pipe.