Replace pid/pgid -2 with INVALID_PID

src/exec.cpp

@@ -388,7 +388,7 @@ void internal_exec(job_t *j, const io_chain_t &&all_ios) {
 
 static void on_process_created(job_t *j, pid_t child_pid) {
     // We only need to do this the first time a child is forked/spawned
-    if (j->pgid != -2) {
+    if (j->pgid != INVALID_PID) {
         return;
     }
 
@@ -724,9 +724,9 @@ static bool exec_external_command(job_t *j, process_t *p, const io_chain_t &proc
         }
 #else
         // In do_fork, the pid of the child process is used as the group leader if j->pgid
-        // == 2 above, posix_spawn assigned the new group a pgid equal to its own id if
-        // j->pgid == 2 so this is what we do instead of calling set_child_group:
-        if (j->pgid == -2) {
+        // invalid, posix_spawn assigned the new group a pgid equal to its own id if
+        // j->pgid was invalid, so this is what we do instead of calling set_child_group
+        if (j->pgid == INVALID_PID) {
             j->pgid = pid;
         }
 #endif

src/postfork.cpp

@@ -65,8 +65,7 @@ static void debug_safe_int(int level, const char *format, int val) {
 bool child_set_group(job_t *j, process_t *p) {
     bool retval = true;
     if (j->get_flag(JOB_CONTROL)) {
-        // New jobs have the pgid set to -2
-        if (j->pgid == -2) {
+        if (j->pgid == INVALID_PID) {
             j->pgid = p->pid;
         }
         if (setpgid(p->pid, j->pgid) < 0) {
@@ -106,7 +105,7 @@ bool child_set_group(job_t *j, process_t *p) {
 /// if it's to run in the foreground.
 bool set_child_group(job_t *j, pid_t child_pid) {
     if (j->get_flag(JOB_CONTROL)) {
-        assert (j->pgid != -2
+        assert (j->pgid != INVALID_PID
                 && "set_child_group called with JOB_CONTROL before job pgid determined!");
 
         // The parent sets the child's group. This incurs the well-known unavoidable race with the
@@ -345,7 +344,7 @@ bool fork_actions_make_spawn_properties(posix_spawnattr_t *attr,
         // set_child_group puts each job into its own process group
         // do the same here if there is no PGID yet (i.e. PGID == -2)
         desired_process_group_id = j->pgid;
-        if (desired_process_group_id == -2) {
+        if (desired_process_group_id == INVALID_PID) {
             desired_process_group_id = 0;
         }
     }

src/proc.cpp

@@ -343,13 +343,8 @@ static void handle_child_status(pid_t pid, int status) {
 
 process_t::process_t() {}
 
-/// The constructor sets the pgid to -2 as a sentinel value
-/// 0 should not be used; although it is not a valid PGID in userspace,
-///   the Linux kernel will use it for kernel processes.
-/// -1 should not be used; it is a possible return value of the getpgid()
-///   function
 job_t::job_t(job_id_t jobid, io_chain_t bio)
-    : block_io(std::move(bio)), pgid(-2), tmodes(), job_id(jobid), flags(0) {}
+    : block_io(std::move(bio)), pgid(INVALID_PID), tmodes(), job_id(jobid), flags(0) {}
 
 job_t::~job_t() { release_job_id(job_id); }
 
@@ -416,7 +411,7 @@ static int process_mark_finished_children(bool wants_await) {
             job_t *j; job_iterator_t jobs;
             while ((j = jobs.next())) {
                 any_jobs = true;
-                if (j->pgid == -2 || !j->get_flag(JOB_CONSTRUCTED)) {
+                if (j->pgid == INVALID_PID || !j->get_flag(JOB_CONSTRUCTED)) {
                     // Job has not been fully constructed yet
                     debug(4, "Skipping iteration of not fully constructed job %d", j->pgid);
                     continue;
@@ -627,11 +622,11 @@ static int process_clean_after_marking(bool allow_interactive) {
             }
             // TODO: The generic process-exit event is useless and unused.
             // Remove this in future.
-            // Don't fire the exit-event for jobs with pgid -2.
+            // Don't fire the exit-event for jobs with pgid INVALID_PID.
             // That's our "sentinel" pgid, for jobs that don't (yet) have a pgid,
             // or jobs that consist entirely of builtins (and hence don't have a process).
             // This causes issues if fish is PID 2, which is quite common on WSL. See #4582.
-            if (j->pgid != -2) {
+            if (j->pgid != INVALID_PID) {
                 proc_fire_event(L"JOB_EXIT", EVENT_EXIT, -j->pgid, 0);
             }
             proc_fire_event(L"JOB_EXIT", EVENT_JOB_ID, j->job_id, 0);
@@ -825,7 +820,7 @@ bool terminal_give_to_job(const job_t *j, bool cont) {
     // http://curiousthing.org/sigttin-sigttou-deep-dive-linux In all cases, our goal here was just
     // to hand over control of the terminal to this process group, which is a no-op if it's already
     // been done.
-    if (j->pgid == -2 || tcgetpgrp(STDIN_FILENO) == j->pgid) {
+    if (j->pgid == INVALID_PID || tcgetpgrp(STDIN_FILENO) == j->pgid) {
         debug(4, L"Process group %d already has control of terminal\n", j->pgid);
     } else {
         debug(4,

src/proc.h

@@ -370,10 +370,17 @@ int proc_format_status(int status);
 /// Wait for any process finishing.
 pid_t proc_wait_any();
 
-#endif
-
 bool terminal_give_to_job(const job_t *j, bool cont);
 
 /// Given that we are about to run a builtin, acquire the terminal if it is owned by the given job.
 /// Returns the pid to restore after running the builtin, or -1 if there is no pid to restore.
 pid_t terminal_acquire_before_builtin(int job_pgid);
+
+
+/// 0 should not be used; although it is not a valid PGID in userspace,
+///   the Linux kernel will use it for kernel processes.
+/// -1 should not be used; it is a possible return value of the getpgid()
+///   function
+enum { INVALID_PID  = -2 };
+
+#endif