spawn: FD reassignment code tweaks

Made these while researching whether vte suffers from glib#2503.

1 files changed, 16 insertions, 11 deletions

diff --git a/src/spawn.cc b/src/spawn.cc
index 69c85704..a0d706cd 100644
--- a/src/spawn.cc
+++ b/src/spawn.cc
@@ -405,7 +405,7 @@ SpawnContext::exec(vte::libc::FD& child_report_error_pipe_write,
         /* Assign FDs */
         auto const n_fd_map = m_fd_map.size();
         for (auto i = size_t{0}; i < n_fd_map; ++i) {
-                auto [source_fd, target_fd] = m_fd_map[i];
+                auto const [source_fd, target_fd] = m_fd_map[i];
 
                 /* -1 means the source_fd is only in the map so that it can
                  * be checked for conflicts with other target FDs. It may be
@@ -427,7 +427,10 @@ SpawnContext::exec(vte::libc::FD& child_report_error_pipe_write,
                                 if (from_fd != target_fd)
                                         continue;
 
-                                auto new_from_fd = vte::libc::fd_dup_cloexec(from_fd, target_fd + 1);
+                                /* Duplicate from_fd to any free FD number, which will
+                                 * be != from_fd/target_fd.
+                                 */
+                                auto new_from_fd = vte::libc::fd_dup_cloexec(from_fd, 3);
                                 if (new_from_fd == -1)
                                         return ExecError::DUP;
 
@@ -448,23 +451,25 @@ SpawnContext::exec(vte::libc::FD& child_report_error_pipe_write,
                                         (void)close(from_fd);
                                 }
 
+                                /* We have replaced *all* instances of target_fd as a
+                                 * source with new_from_fd, so we don't need to continue
+                                 * with the loop.
+                                 */
                                 break;
                         }
-                }
-
-                /* source_fd may have been changed by the loop above */
-                source_fd = m_fd_map[i].first;
 
-                if (target_fd == source_fd) {
+                        /* Now we know that target_fd can be safely overwritten. */
+                        if (vte::libc::fd_dup2(source_fd, target_fd) == -1)
+                                return ExecError::DUP2;
+                } else {
                         /* Already assigned correctly, but need to remove FD_CLOEXEC */
                         if (vte::libc::fd_unset_cloexec(target_fd) == -1)
                                 return ExecError::UNSET_CLOEXEC;
 
-                } else {
-                        /* Now we know that target_fd can be safely overwritten. */
-                        if (vte::libc::fd_dup2(source_fd, target_fd) == -1)
-                                return ExecError::DUP2;
                 }
+
+                /* Mark source in the map as done */
+                m_fd_map[i].first = -1;
         }
 
         /* Finally call an extra child setup */