From b1a1b617f15655168709c8a69df663320af5fc75 Mon Sep 17 00:00:00 2001
From: ridiculousfish <corydoras@ridiculousfish.com>
Date: Mon, 1 Jul 2019 10:57:09 -0700
Subject: [PATCH] child_setup_process to accept new termowner directly

Soon we will have more complicated logic around whether to call tcsetpgrp.
Prepare to centralize the logic by passing in the new term owner pgrp,
instead of having child_setup_process perform the decision.
---
 src/exec.cpp     | 6 ++++--
 src/postfork.cpp | 6 +++---
 src/postfork.h   | 6 ++++--
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/src/exec.cpp b/src/exec.cpp
index bd66ff308..dc6ce6089 100644
--- a/src/exec.cpp
+++ b/src/exec.cpp
@@ -300,7 +300,7 @@ void internal_exec(env_stack_t &vars, job_t *j, const io_chain_t &all_ios) {
     // commands in the pipeline will apply to exec. However, using exec in a pipeline doesn't
     // really make sense, so I'm not trying to fix it here.
     auto redirs = dup2_list_t::resolve_chain(all_ios);
-    if (redirs && !child_setup_process(nullptr, false, *redirs)) {
+    if (redirs && !child_setup_process(INVALID_PID, false, *redirs)) {
         // Decrement SHLVL as we're removing ourselves from the shell "stack".
         auto shlvl_var = vars.get(L"SHLVL", ENV_GLOBAL | ENV_EXPORT);
         wcstring shlvl_str = L"0";
@@ -443,9 +443,11 @@ static bool fork_child_for_process(const std::shared_ptr<job_t> &job, process_t
     if (pid == 0) {
         // This is the child process. Setup redirections, print correct output to
         // stdout and stderr, and then exit.
+        maybe_t<pid_t> new_termowner{};
         p->pid = getpid();
         child_set_group(job.get(), p);
-        child_setup_process(job.get(), true, dup2s);
+        child_setup_process(job->wants_terminal() && job->is_foreground() ? job->pgid : INVALID_PID,
+                            true, dup2s);
         child_action();
         DIE("Child process returned control to fork_child lambda!");
     }
diff --git a/src/postfork.cpp b/src/postfork.cpp
index 566c1ed63..0d85275bc 100644
--- a/src/postfork.cpp
+++ b/src/postfork.cpp
@@ -134,7 +134,7 @@ bool set_child_group(job_t *j, pid_t child_pid) {
     return true;
 }
 
-int child_setup_process(const job_t *job, bool is_forked, const dup2_list_t &dup2s) {
+int child_setup_process(pid_t new_termowner, bool is_forked, const dup2_list_t &dup2s) {
     // Note we are called in a forked child.
     for (const auto &act : dup2s.get_actions()) {
         int err = act.target < 0 ? close(act.src) : dup2(act.src, act.target);
@@ -146,7 +146,7 @@ int child_setup_process(const job_t *job, bool is_forked, const dup2_list_t &dup
             return err;
         }
     }
-    if (job != nullptr && job->wants_terminal() && job->is_foreground()) {
+    if (new_termowner != INVALID_PID) {
         // Assign the terminal within the child to avoid the well-known race between tcsetgrp() in
         // the parent and the child executing. We are not interested in error handling here, except
         // we try to avoid this for non-terminals; in particular pipelines often make non-terminal
@@ -155,7 +155,7 @@ int child_setup_process(const job_t *job, bool is_forked, const dup2_list_t &dup
             // Ensure this doesn't send us to the background (see #5963)
             signal(SIGTTIN, SIG_IGN);
             signal(SIGTTOU, SIG_IGN);
-            (void)tcsetpgrp(STDIN_FILENO, job->pgid);
+            (void)tcsetpgrp(STDIN_FILENO, new_termowner);
         }
     }
     // Set the handling for job control signals back to the default.
diff --git a/src/postfork.h b/src/postfork.h
index 997c5f07b..3085fe0ef 100644
--- a/src/postfork.h
+++ b/src/postfork.h
@@ -27,9 +27,11 @@ bool child_set_group(job_t *j, process_t *p);     // called by child
 /// inside the exec function, which blocks all signals), and all IO redirections and other file
 /// descriptor actions are performed.
 ///
+/// Assign the terminal to new_termowner unless it is INVALID_PID.
+///
 /// \return 0 on sucess, -1 on failiure. When this function returns, signals are always unblocked.
-/// On failiure, signal handlers, io redirections and process group of the process is undefined.
-int child_setup_process(const job_t *job, bool is_forked, const dup2_list_t &dup2s);
+/// On failure, signal handlers, io redirections and process group of the process is undefined.
+int child_setup_process(pid_t new_termowner, bool is_forked, const dup2_list_t &dup2s);
 
 /// Call fork(), optionally waiting until we are no longer multithreaded. If the forked child
 /// doesn't do anything that could allocate memory, take a lock, etc. (like call exec), then it's