From cc8c374ccb3a22bb55d33154664d2b81feb35a61 Mon Sep 17 00:00:00 2001 From: Christian Persch Date: Sat, 23 Oct 2021 20:55:15 +0200 Subject: spawn: FD reassignment code tweaks Made these while researching whether vte suffers from glib#2503. --- src/spawn.cc | 27 ++++++++++++++++----------- 1 file 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 */ -- cgit v1.2.1