Replace some shared_ptrs with const shared_ptr &. Don't use shared_ptr in postfork.cpp. Eliminate copy constructor in io_data_t. Simplify some other uses of shared_ptr<io_data_t>

exec.cpp

@@ -1183,7 +1183,7 @@ void exec(parser_t &parser, job_t *j)
 
                 for (io_chain_t::iterator iter = j->io.begin(); iter != j->io.end(); iter++)
                 {
-                    shared_ptr<io_data_t> &tmp_io = *iter;
+                    const shared_ptr<io_data_t> &tmp_io = *iter;
                     if (tmp_io->io_mode == IO_FILE && strcmp(tmp_io->filename_cstr, "/dev/null") != 0)
                     {
                         skip_fork = false;
@@ -1439,7 +1439,7 @@ static int exec_subshell_internal(const wcstring &cmd, wcstring_list_t *lst)
 
     is_subshell=1;
 
-    shared_ptr<io_data_t> io_buffer(io_buffer_create(0));
+    const shared_ptr<io_data_t> io_buffer(io_buffer_create(0));
 
     prev_status = proc_get_last_status();
 

io.cpp

@@ -133,7 +133,7 @@ io_data_t *io_buffer_create(bool is_input)
     return buffer_redirect;
 }
 
-void io_buffer_destroy(shared_ptr<io_data_t> io_buffer)
+void io_buffer_destroy(const shared_ptr<io_data_t> &io_buffer)
 {
 
     /**
@@ -153,7 +153,7 @@ void io_buffer_destroy(shared_ptr<io_data_t> io_buffer)
     */
 }
 
-void io_chain_t::remove(shared_ptr<const io_data_t> element)
+void io_chain_t::remove(const shared_ptr<const io_data_t> &element)
 {
     // See if you can guess why std::find doesn't work here
     for (io_chain_t::iterator iter = this->begin(); iter != this->end(); ++iter)
@@ -168,35 +168,22 @@ void io_chain_t::remove(shared_ptr<const io_data_t> element)
 
 io_chain_t io_chain_t::duplicate() const
 {
-    io_chain_t result;
-    result.reserve(this->size());
-    for (io_chain_t::const_iterator iter = this->begin(); iter != this->end(); iter++)
-    {
-        result.push_back(*iter);
-    }
+    io_chain_t result = *this;
     return result;
 }
 
 void io_chain_t::duplicate_prepend(const io_chain_t &src)
 {
-    /* Prepend a duplicate of src before this. Start by inserting a bunch of empty shared_ptr's (so we only have to reallocate once) and then replace them. */
-    this->insert(this->begin(), src.size(), shared_ptr<io_data_t>());
-    for (size_t idx = 0; idx < src.size(); idx++)
-    {
-        this->at(idx) = src.at(idx);
-    }
+    /* Prepend a duplicate of src before this. */
+    this->insert(this->begin(), src.begin(), src.end());
 }
 
 void io_chain_t::destroy()
 {
-    for (size_t idx = 0; idx < this->size(); idx++)
-    {
-        this->at(idx).reset();
-    }
     this->clear();
 }
 
-void io_remove(io_chain_t &list, shared_ptr<const io_data_t> element)
+void io_remove(io_chain_t &list, const shared_ptr<const io_data_t> &element)
 {
     list.remove(element);
 }
@@ -217,7 +204,7 @@ void io_print(const io_chain_t &chain)
     fprintf(stderr, "Chain %p (%ld items):\n", &chain, (long)chain.size());
     for (size_t i=0; i < chain.size(); i++)
     {
-        shared_ptr<const io_data_t> io = chain.at(i);
+        const shared_ptr<const io_data_t> &io = chain.at(i);
         fprintf(stderr, "\t%lu: fd:%d, input:%s, ", (unsigned long)i, io->fd, io->is_input ? "yes" : "no");
         switch (io->io_mode)
         {
@@ -256,7 +243,7 @@ shared_ptr<const io_data_t> io_chain_t::get_io_for_fd(int fd) const
     size_t idx = this->size();
     while (idx--)
     {
-        shared_ptr<const io_data_t> data = this->at(idx);
+        const shared_ptr<const io_data_t> &data = this->at(idx);
         if (data->fd == fd)
         {
             return data;
@@ -270,7 +257,7 @@ shared_ptr<io_data_t> io_chain_t::get_io_for_fd(int fd)
     size_t idx = this->size();
     while (idx--)
     {
-        shared_ptr<io_data_t> data = this->at(idx);
+        const shared_ptr<io_data_t> &data = this->at(idx);
         if (data->fd == fd)
         {
             return data;
@@ -290,7 +277,7 @@ shared_ptr<io_data_t> io_chain_get(io_chain_t &src, int fd)
     return src.get_io_for_fd(fd);
 }
 
-io_chain_t::io_chain_t(shared_ptr<io_data_t> data) :
+io_chain_t::io_chain_t(const shared_ptr<io_data_t> &data) :
     std::vector<shared_ptr<io_data_t> >(1, data)
 {
 

io.h

@@ -20,9 +20,10 @@ 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 allowed */
+    /* No assignment or copying allowed */
+    io_data_t(const io_data_t &rhs);
     void operator=(const io_data_t &rhs);
-
+    
 public:
     /** Type of redirect */
     int io_mode;
@@ -117,9 +118,9 @@ class io_chain_t : public std::vector<shared_ptr<io_data_t> >
 {
 public:
     io_chain_t();
-    io_chain_t(shared_ptr<io_data_t> );
+    io_chain_t(const shared_ptr<io_data_t> &);
 
-    void remove(shared_ptr<const io_data_t> element);
+    void remove(const shared_ptr<const io_data_t> &element);
     io_chain_t duplicate() const;
     void duplicate_prepend(const io_chain_t &src);
     void destroy();
@@ -132,7 +133,7 @@ public:
 /**
    Remove the specified io redirection from the chain
 */
-void io_remove(io_chain_t &list, shared_ptr<const io_data_t> element);
+void io_remove(io_chain_t &list, const shared_ptr<const io_data_t> &element);
 
 /** Make a copy of the specified chain of redirections. Uses operator new. */
 io_chain_t io_duplicate(const io_chain_t &chain);
@@ -156,7 +157,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(shared_ptr<io_data_t> io_buffer);
+void io_buffer_destroy(const shared_ptr<io_data_t> &io_buffer);
 
 /**
    Create a IO_BUFFER type io redirection, complete with a pipe and a

postfork.cpp

@@ -117,7 +117,7 @@ static void free_redirected_fds_from_pipes(io_chain_t &io_chain)
         for (size_t j = 0; j < max; j++)
         {
             /* We're only interested in pipes */
-            shared_ptr<io_data_t> possible_conflict = io_chain.at(j);
+            io_data_t *possible_conflict = io_chain.at(j).get();
             if (possible_conflict->io_mode != IO_PIPE && possible_conflict->io_mode != IO_BUFFER)
                 continue;
 
@@ -166,7 +166,7 @@ static int handle_child_io(io_chain_t &io_chain)
     free_redirected_fds_from_pipes(io_chain);
     for (size_t idx = 0; idx < io_chain.size(); idx++)
     {
-        shared_ptr<io_data_t> io = io_chain.at(idx);
+        io_data_t *io = io_chain.at(idx).get();
         int tmp;
 
         if (io->io_mode == IO_FD && io->fd == io->param1.old_fd)