Split out io_buffer_t, make input_redirect in exec() a raw pointer

exec.cpp

@@ -538,7 +538,7 @@ void exec(parser_t &parser, job_t *j)
     int mypipe[2];
     sigset_t chldset;
 
-    shared_ptr<io_data_t> io_buffer;
+    shared_ptr<io_buffer_t> io_buffer;
 
     /*
       Set to true if something goes wrong while exec:ing the job, in
@@ -566,24 +566,24 @@ void exec(parser_t &parser, job_t *j)
         io_duplicate_prepend(parser.block_io, j->io);
     }
 
-    shared_ptr<const io_data_t> input_redirect;
+    const io_buffer_t *input_redirect = 0;
     for (size_t idx = 0; idx < j->io.size(); idx++)
     {
-        input_redirect = j->io.at(idx);
+        shared_ptr<io_data_t> &io = j->io.at(idx);
 
-        if ((input_redirect->io_mode == IO_BUFFER) &&
+        if ((io->io_mode == IO_BUFFER) && io->is_input)
-                input_redirect->is_input)
         {
             /*
               Input redirection - create a new gobetween process to take
-              care of buffering
+              care of buffering, save the redirection in input_redirect
             */
             process_t *fake = new process_t();
             fake->type  = INTERNAL_BUFFER;
             fake->pipe_write_fd = 1;
-            j->first_process->pipe_read_fd = input_redirect->fd;
+            j->first_process->pipe_read_fd = io->fd;
             fake->next = j->first_process;
             j->first_process = fake;
+            input_redirect = static_cast<const io_buffer_t *>(io.get());
             break;
         }
     }
@@ -1135,8 +1135,9 @@ void exec(parser_t &parser, job_t *j)
                         (! get_stdout_buffer().empty()) &&
                         (buffer_stdout))
                 {
+                    CAST_INIT(io_buffer_t *, io_buffer, io.get());
                     const std::string res = wcs2string(get_stdout_buffer());
-                    io->out_buffer_append(res.c_str(), res.size());
+                    io_buffer->out_buffer_append(res.c_str(), res.size());
                     skip_fork = true;
                 }
 
@@ -1410,7 +1411,7 @@ static int exec_subshell_internal(const wcstring &cmd, wcstring_list_t *lst)
 
     is_subshell=1;
 
-    const shared_ptr<io_data_t> io_buffer(io_buffer_create(0));
+    const shared_ptr<io_buffer_t> io_buffer(io_buffer_create(0));
 
     prev_status = proc_get_last_status();
 

io.cpp

@@ -58,9 +58,6 @@ void io_data_t::print() const
         case IO_PIPE:
             fprintf(stderr, "pipe {%d, %d}\n", param1.pipe_fd[0], param1.pipe_fd[1]);
             break;
-        case IO_BUFFER:
-            fprintf(stderr, "buffer %p (size %lu)\n", out_buffer_ptr(), out_buffer_size());
-            break;
     }
 }
 
@@ -79,7 +76,12 @@ void io_file_t::print() const
     fprintf(stderr, "file (%s)\n", filename_cstr);
 }
 
-void io_buffer_read(io_data_t *d)
+void io_buffer_t::print() const
+{
+    fprintf(stderr, "buffer %p (size %lu)\n", out_buffer_ptr(), out_buffer_size());
+}
+
+void io_buffer_read(io_buffer_t *d)
 {
     exec_close(d->param1.pipe_fd[1]);
 
@@ -128,14 +130,12 @@ void io_buffer_read(io_data_t *d)
 }
 
 
-io_data_t *io_buffer_create(bool is_input)
+io_buffer_t *io_buffer_create(bool is_input)
 {
     bool success = true;
-    io_data_t *buffer_redirect = new io_data_t;
+    io_buffer_t *buffer_redirect = new io_buffer_t(is_input ? 0 : 1);
     buffer_redirect->out_buffer_create();
-    buffer_redirect->io_mode = IO_BUFFER;
     buffer_redirect->is_input = is_input ? true : false;
-    buffer_redirect->fd=is_input?0:1;
 
     if (exec_pipe(buffer_redirect->param1.pipe_fd) == -1)
     {
@@ -161,7 +161,7 @@ io_data_t *io_buffer_create(bool is_input)
     return buffer_redirect;
 }
 
-void io_buffer_destroy(const shared_ptr<io_data_t> &io_buffer)
+void io_buffer_destroy(const shared_ptr<io_buffer_t> &io_buffer)
 {
 
     /**

io.h

@@ -17,9 +17,6 @@ enum io_mode_t
 class io_data_t
 {
 private:
-    /** buffer to save output in for IO_BUFFER. Note that in the original fish, the buffer was a pointer to a buffer_t stored in the param2 union down below, and when an io_data_t was duplicated the pointer was copied so that two io_data_ts referenced the same buffer. It's not clear to me how this was ever cleaned up correctly. But it's important that they share the same buffer for reasons I don't yet understand either. We can get correct sharing and cleanup with shared_ptr. */
-    shared_ptr<std::vector<char> > out_buffer;
-
     /* No assignment or copying allowed */
     io_data_t(const io_data_t &rhs);
     void operator=(const io_data_t &rhs);
@@ -39,46 +36,12 @@ public:
         int pipe_fd[2];
     } param1;
 
-    /** Function to create the output buffer */
-    void out_buffer_create()
-    {
-        out_buffer.reset(new std::vector<char>);
-    }
-
-    /** Function to append to the buffer */
-    void out_buffer_append(const char *ptr, size_t count)
-    {
-        assert(out_buffer.get() != NULL);
-        out_buffer->insert(out_buffer->end(), ptr, ptr + count);
-    }
-
-    /** Function to get a pointer to the buffer */
-    char *out_buffer_ptr(void)
-    {
-        assert(out_buffer.get() != NULL);
-        return out_buffer->empty() ? NULL : &out_buffer->at(0);
-    }
-
-    const char *out_buffer_ptr(void) const
-    {
-        assert(out_buffer.get() != NULL);
-        return out_buffer->empty() ? NULL : &out_buffer->at(0);
-    }
-
-    /** Function to get the size of the buffer */
-    size_t out_buffer_size(void) const
-    {
-        assert(out_buffer.get() != NULL);
-        return out_buffer->size();
-    }
-
     virtual void print() const;
 
     /** Set to true if this is an input io redirection */
     bool is_input;
 
     io_data_t(io_mode_t m = IO_INVALID, int f=0) :
-        out_buffer(),
         io_mode(m),
         fd(f),
         param1(),
@@ -150,6 +113,55 @@ public:
     }
 };
 
+class io_buffer_t : public io_data_t
+{
+private:
+    /** buffer to save output in */
+    shared_ptr<std::vector<char> > out_buffer;
+
+public:
+    virtual void print() const;
+
+    io_buffer_t(int f):
+        io_data_t(IO_BUFFER, f),
+        out_buffer()
+    {
+    }
+
+    /** Function to create the output buffer */
+    void out_buffer_create()
+    {
+        out_buffer.reset(new std::vector<char>);
+    }
+
+    /** Function to append to the buffer */
+    void out_buffer_append(const char *ptr, size_t count)
+    {
+        assert(out_buffer.get() != NULL);
+        out_buffer->insert(out_buffer->end(), ptr, ptr + count);
+    }
+
+    /** Function to get a pointer to the buffer */
+    char *out_buffer_ptr(void)
+    {
+        assert(out_buffer.get() != NULL);
+        return out_buffer->empty() ? NULL : &out_buffer->at(0);
+    }
+
+    const char *out_buffer_ptr(void) const
+    {
+        assert(out_buffer.get() != NULL);
+        return out_buffer->empty() ? NULL : &out_buffer->at(0);
+    }
+
+    /** Function to get the size of the buffer */
+    size_t out_buffer_size(void) const
+    {
+        assert(out_buffer.get() != NULL);
+        return out_buffer->size();
+    }
+};
+
 class io_chain_t : public std::vector<shared_ptr<io_data_t> >
 {
 public:
@@ -190,7 +202,7 @@ shared_ptr<io_data_t> io_chain_get(io_chain_t &src, int fd);
 /**
    Free all resources used by a IO_BUFFER type io redirection.
 */
-void io_buffer_destroy(const shared_ptr<io_data_t> &io_buffer);
+void io_buffer_destroy(const shared_ptr<io_buffer_t> &io_buffer);
 
 /**
    Create a IO_BUFFER type io redirection, complete with a pipe and a
@@ -201,12 +213,12 @@ void io_buffer_destroy(const shared_ptr<io_data_t> &io_buffer);
    used to buffer the output of a command, or non-zero to buffer the
    input to a command.
 */
-io_data_t *io_buffer_create(bool is_input);
+io_buffer_t *io_buffer_create(bool is_input);
 
 /**
    Close output pipe, and read from input pipe until eof.
 */
-void io_buffer_read(io_data_t *d);
+void io_buffer_read(io_buffer_t *d);
 
 /** Print debug information about the specified IO redirection chain to stderr. */
 void io_print(const io_chain_t &chain);

proc.cpp

@@ -902,7 +902,7 @@ static int select_try(job_t *j)
 */
 static void read_try(job_t *j)
 {
-    io_data_t *buff=NULL;
+    io_buffer_t *buff=NULL;
 
     /*
       Find the last buffer, which is the one we want to read from
@@ -912,7 +912,7 @@ static void read_try(job_t *j)
         io_data_t *d = j->io.at(idx).get();
         if (d->io_mode == IO_BUFFER)
         {
-            buff=d;
+            buff = static_cast<io_buffer_t *>(d);
         }
     }
 

reader.cpp

@@ -1064,7 +1064,7 @@ static void run_pager(const wcstring &prefix, int is_quoted, const std::vector<c
                                  is_quoted?L"-q":L"",
                                  prefix_esc.c_str());
 
-    shared_ptr<io_data_t> in(io_buffer_create(true));
+    shared_ptr<io_buffer_t> in(io_buffer_create(true));
     in->fd = 3;
 
     escaped_separator = escape(COMPLETE_SEP_STR, 1);
@@ -1133,7 +1133,7 @@ static void run_pager(const wcstring &prefix, int is_quoted, const std::vector<c
 
     term_donate();
 
-    shared_ptr<io_data_t> out(io_buffer_create(false));
+    shared_ptr<io_buffer_t> out(io_buffer_create(false));
     out->fd = 4;
 
     parser_t &parser = parser_t::principal_parser();