Hopeful fix to avoid forking for certain builtins like echo when they have an input redirection only

3 files changed, 26 insertions, 14 deletions

diff --git a/exec.cpp b/exec.cpp
index f64f6212..e9135dba 100644
--- a/exec.cpp
+++ b/exec.cpp
@@ -1159,23 +1159,25 @@ void exec(parser_t &parser, job_t *j)
                   performance quite a bit in complex completion code.
                 */
 
-                const shared_ptr<io_data_t> &io = io_chain_get(j->io, 1);
-                bool buffer_stdout = io && io->io_mode == IO_BUFFER;
+                const shared_ptr<io_data_t> &stdout_io = io_chain_get(j->io, STDOUT_FILENO);
+                const shared_ptr<io_data_t> &stderr_io = io_chain_get(j->io, STDERR_FILENO);
+                const bool buffer_stdout = stdout_io && stdout_io->io_mode == IO_BUFFER;
 
                 if ((get_stderr_buffer().empty()) &&
                         (!p->next) &&
                         (! get_stdout_buffer().empty()) &&
                         (buffer_stdout))
                 {
-                    CAST_INIT(io_buffer_t *, io_buffer, io.get());
+                    CAST_INIT(io_buffer_t *, io_buffer, stdout_io.get());
                     const std::string res = wcs2string(get_stdout_buffer());
                     io_buffer->out_buffer_append(res.c_str(), res.size());
                     skip_fork = true;
                 }
-
-                if (! skip_fork && j->io.empty())
+                
+                /* PCA for some reason, fish forks a lot, even for basic builtins like echo just to write out their buffers. I'm certain a lot of this is unnecessary, but I am not sure exactly when. If j->io is NULL, then it means there's no pipes or anything, so we can certainly just write out our data. Beyond that, we may be able to do the same if io_get returns 0 for STDOUT_FILENO and STDERR_FILENO.
+                */
+                if (! skip_fork && stdout_io.get() == NULL && stderr_io.get() == NULL)
                 {
-                    /* PCA for some reason, fish forks a lot, even for basic builtins like echo just to write out their buffers. I'm certain a lot of this is unnecessary, but I am not sure exactly when. If j->io is NULL, then it means there's no pipes or anything, so we can certainly just write out our data. Beyond that, we may be able to do the same if io_get returns 0 for STDOUT_FILENO and STDERR_FILENO. */
                     if (g_log_forks)
                     {
                         printf("fork #-: Skipping fork for internal builtin for '%ls'\n", p->argv0());
@@ -1194,10 +1196,14 @@ void exec(parser_t &parser, job_t *j)
                 for (io_chain_t::iterator iter = j->io.begin(); iter != j->io.end(); ++iter)
                 {
                     const shared_ptr<io_data_t> &tmp_io = *iter;
-                    if (tmp_io->io_mode == IO_FILE && strcmp(static_cast<const io_file_t *>(tmp_io.get())->filename_cstr, "/dev/null") != 0)
+                    if (tmp_io->io_mode == IO_FILE)
                     {
-                        skip_fork = false;
-                        break;
+                        const io_file_t *tmp_file_io = static_cast<const io_file_t *>(tmp_io.get());
+                        if (strcmp(tmp_file_io->filename_cstr, "/dev/null"))
+                        {
+                            skip_fork = false;
+                            break;
+                        }
                     }
                 }
 
@@ -1235,7 +1241,7 @@ void exec(parser_t &parser, job_t *j)
                 if (g_log_forks)
                 {
                     printf("fork #%d: Executing fork for internal builtin for '%ls'\n", g_fork_count, p->argv0());
-                    io_print(io_chain_t(io));
+                    io_print(j->io);
                 }
                 pid = execute_fork(false);
                 if (pid == 0)
diff --git a/io.cpp b/io.cpp
index 669752fd..b7df0416 100644
--- a/io.cpp
+++ b/io.cpp
@@ -209,7 +209,7 @@ void io_chain_t::push_back(const shared_ptr<io_data_t> &element)
 {
     // Ensure we never push back NULL
     assert(element.get() != NULL);
-    std::vector<shared_ptr<io_data_t> >:: push_back(element);
+    std::vector<shared_ptr<io_data_t> >::push_back(element);
 }
 
 void io_remove(io_chain_t &list, const shared_ptr<const io_data_t> &element)
@@ -229,8 +229,15 @@ void io_print(const io_chain_t &chain)
     for (size_t i=0; i < chain.size(); i++)
     {
         const shared_ptr<const io_data_t> &io = chain.at(i);
-        fprintf(stderr, "\t%lu: fd:%d, ", (unsigned long)i, io->fd);
-        io->print();
+        if (io.get() == NULL)
+        {
+            fprintf(stderr, "\t(null)\n");
+        }
+        else
+        {
+            fprintf(stderr, "\t%lu: fd:%d, ", (unsigned long)i, io->fd);
+            io->print();
+        }
     }
 }
 
diff --git a/io.h b/io.h
index 750c3207..263cb738 100644
--- a/io.h
+++ b/io.h
@@ -184,7 +184,6 @@ public:
 
     shared_ptr<const io_data_t> get_io_for_fd(int fd) const;
     shared_ptr<io_data_t> get_io_for_fd(int fd);
-
 };
 
 /**