diff --git a/exec.cpp b/exec.cpp index f501157bb..840aa7ab1 100644 --- a/exec.cpp +++ b/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 &tmp_io = *iter; + const shared_ptr &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_buffer(io_buffer_create(0)); + const shared_ptr io_buffer(io_buffer_create(0)); prev_status = proc_get_last_status(); diff --git a/io.cpp b/io.cpp index 85408405c..e5d5a0c10 100644 --- a/io.cpp +++ b/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_buffer) +void io_buffer_destroy(const shared_ptr &io_buffer) { /** @@ -153,7 +153,7 @@ void io_buffer_destroy(shared_ptr io_buffer) */ } -void io_chain_t::remove(shared_ptr element) +void io_chain_t::remove(const shared_ptr &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 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()); - 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 element) +void io_remove(io_chain_t &list, const shared_ptr &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 io = chain.at(i); + const shared_ptr &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 io_chain_t::get_io_for_fd(int fd) const size_t idx = this->size(); while (idx--) { - shared_ptr data = this->at(idx); + const shared_ptr &data = this->at(idx); if (data->fd == fd) { return data; @@ -270,7 +257,7 @@ shared_ptr io_chain_t::get_io_for_fd(int fd) size_t idx = this->size(); while (idx--) { - shared_ptr data = this->at(idx); + const shared_ptr &data = this->at(idx); if (data->fd == fd) { return data; @@ -290,7 +277,7 @@ shared_ptr io_chain_get(io_chain_t &src, int fd) return src.get_io_for_fd(fd); } -io_chain_t::io_chain_t(shared_ptr data) : +io_chain_t::io_chain_t(const shared_ptr &data) : std::vector >(1, data) { diff --git a/io.h b/io.h index af6f71bba..1d31a14ff 100644 --- a/io.h +++ b/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 > 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 > { public: io_chain_t(); - io_chain_t(shared_ptr ); + io_chain_t(const shared_ptr &); - void remove(shared_ptr element); + void remove(const shared_ptr &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 element); +void io_remove(io_chain_t &list, const shared_ptr &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_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_buffer); +void io_buffer_destroy(const shared_ptr &io_buffer); /** Create a IO_BUFFER type io redirection, complete with a pipe and a diff --git a/postfork.cpp b/postfork.cpp index 8dc978d1e..ed8dfb9a4 100644 --- a/postfork.cpp +++ b/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 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 = 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)