Make io_data_t::fd const

In exec(), pipe_{write,read} no longer get reused.

exec.cpp

@@ -622,15 +622,6 @@ void exec(parser_t &parser, job_t *j)
 
     }
 
-    // This is a pipe that the "current" process in our loop below reads from
-    // Only pipe_read->pipe_fd[0] is used
-    shared_ptr<io_pipe_t> pipe_read(new io_pipe_t(0, true));
-
-    // This is the pipe that the "current" process in our loop below writes to
-    shared_ptr<io_pipe_t> pipe_write(new io_pipe_t(1, false));
-
-    j->io.push_back(pipe_write);
-
     signal_block();
 
     /*
@@ -714,16 +705,29 @@ void exec(parser_t &parser, job_t *j)
         pipe_current_read = pipe_next_read;
         pipe_next_read = -1;
 
-        /* Record the current read in pipe_read */
-        pipe_read->pipe_fd[0] = pipe_current_read;
-
         /* See if we need a pipe */
         const bool pipes_to_next_command = (p->next != NULL);
 
-        pipe_write->fd = p->pipe_write_fd;
+        /* The pipes the current process write to and read from.
-        pipe_read->fd = p->pipe_read_fd;
+           Unfortunately these can't be just allocated on the stack, since
+           j->io wants shared_ptr. */
+        shared_ptr<io_pipe_t> pipe_write(new io_pipe_t(p->pipe_write_fd, false));
+        shared_ptr<io_pipe_t> pipe_read(new io_pipe_t(p->pipe_read_fd, true));
+
+        /* Record the current read in pipe_read */
+        pipe_read->pipe_fd[0] = pipe_current_read;
+
 //    debug( 0, L"Pipe created from fd %d to fd %d", pipe_write->fd, pipe_read->fd );
 
+        if (p != j->first_process)
+        {
+            j->io.push_back(pipe_read);
+        }
+
+        if (p->next)
+        {
+            j->io.push_back(pipe_write);
+        }
 
         /*
            This call is used so the global environment variable array
@@ -742,12 +746,6 @@ void exec(parser_t &parser, job_t *j)
           Set up fd:s that will be used in the pipe
         */
 
-        if (p == j->first_process->next)
-        {
-            /* We are the first process that could possibly read from a pipe (aka the second process), so add the pipe read redirection */
-            j->io.push_back(pipe_read);
-        }
-
         if (pipes_to_next_command)
         {
 //      debug( 1, L"%ls|%ls" , p->argv[0], p->next->argv[0]);
@@ -772,16 +770,6 @@ void exec(parser_t &parser, job_t *j)
             assert(pipe_next_read == -1);
             pipe_next_read = local_pipe[0];
         }
-        else
-        {
-            /*
-              This is the last element of the pipeline.
-              Remove the io redirection for pipe output.
-            */
-            io_chain_t::iterator where = std::find(j->io.begin(), j->io.end(), pipe_write);
-            if (where != j->io.end())
-                j->io.erase(where);
-        }
 
         switch (p->type)
         {
@@ -1396,6 +1384,9 @@ void exec(parser_t &parser, job_t *j)
             exec_close(pipe_current_write);
             pipe_current_write = -1;
         }
+
+        j->io.remove(pipe_write);
+        j->io.remove(pipe_read);
     }
 
     /* Clean up any file descriptors we left open */
@@ -1416,8 +1407,6 @@ void exec(parser_t &parser, job_t *j)
 
     debug(3, L"Job is constructed");
 
-    io_remove(j->io, pipe_read);
-
     job_set_flag(j, JOB_CONSTRUCTED, 1);
 
     if (!job_get_flag(j, JOB_FOREGROUND))

io.cpp

@@ -131,10 +131,14 @@ void io_buffer_t::read()
 }
 
 
-io_buffer_t *io_buffer_t::create(bool is_input)
+io_buffer_t *io_buffer_t::create(bool is_input, int fd)
 {
     bool success = true;
-    io_buffer_t *buffer_redirect = new io_buffer_t(is_input ? 0 : 1, is_input);
+    if (fd == -1)
+    {
+        fd = is_input ? 0 : 1;
+    }
+    io_buffer_t *buffer_redirect = new io_buffer_t(fd, is_input);
 
     if (exec_pipe(buffer_redirect->pipe_fd) == -1)
     {

io.h

@@ -32,7 +32,7 @@ public:
     /** Type of redirect */
     const io_mode_t io_mode;
     /** FD to redirect */
-    int fd;
+    const int fd;
 
     virtual void print() const = 0;
     virtual ~io_data_t() = 0;
@@ -167,8 +167,10 @@ public:
        \param is_input set this parameter to zero if the buffer should be
        used to buffer the output of a command, or non-zero to buffer the
        input to a command.
+
+       \param fd when -1, determined from is_input.
     */
-    static io_buffer_t *create(bool is_input);
+    static io_buffer_t *create(bool is_input, int fd = -1);
 };
 
 class io_chain_t : public std::vector<shared_ptr<io_data_t> >

reader.cpp

@@ -1113,8 +1113,8 @@ static void run_pager(const wcstring &prefix, int is_quoted, const std::vector<c
     wcstring prefix_esc;
     char *foo;
 
-    shared_ptr<io_buffer_t> in(io_buffer_t::create(true));
+    shared_ptr<io_buffer_t> in(io_buffer_t::create(true, 3));
-    shared_ptr<io_buffer_t> out(io_buffer_t::create(false));
+    shared_ptr<io_buffer_t> out(io_buffer_t::create(false, 4));
 
     // The above may fail e.g. if we have too many open fds
     if (in.get() == NULL || out.get() == NULL)
@@ -1137,8 +1137,6 @@ static void run_pager(const wcstring &prefix, int is_quoted, const std::vector<c
                                  is_quoted?L"-q":L"",
                                  prefix_esc.c_str());
 
-    in->fd = 3;
-
     escaped_separator = escape(COMPLETE_SEP_STR, 1);
 
     for (size_t i=0; i< comp.size(); i++)
@@ -1203,8 +1201,6 @@ static void run_pager(const wcstring &prefix, int is_quoted, const std::vector<c
     in->out_buffer_append(foo, strlen(foo));
     free(foo);
 
-    out->fd = 4;
-
     term_donate();
     parser_t &parser = parser_t::principal_parser();
     io_chain_t io_chain;