From 1634c9df7835dd05046fde50c7140dd9766b1bfb Mon Sep 17 00:00:00 2001
From: ridiculousfish <corydoras@ridiculousfish.com>
Date: Thu, 26 Jan 2017 15:06:58 -0800
Subject: [PATCH] Make job_get_flag and job_set_flag instance methods of jobs

Makes them easier to call when you have a smart pointer
---
 src/builtin.cpp         | 18 +++++++-------
 src/exec.cpp            | 26 ++++++++++-----------
 src/parse_execution.cpp | 13 +++++------
 src/postfork.cpp        |  6 ++---
 src/proc.cpp            | 52 ++++++++++++++++++++---------------------
 src/proc.h              | 10 ++++----
 6 files changed, 61 insertions(+), 64 deletions(-)

diff --git a/src/builtin.cpp b/src/builtin.cpp
index 7ba07970e..b7ece1e9b 100644
--- a/src/builtin.cpp
+++ b/src/builtin.cpp
@@ -2870,9 +2870,9 @@ static int builtin_fg(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
         // the foreground.
         job_iterator_t jobs;
         while ((j = jobs.next())) {
-            if (job_get_flag(j, JOB_CONSTRUCTED) && (!job_is_completed(j)) &&
-                ((job_is_stopped(j) || (!job_get_flag(j, JOB_FOREGROUND))) &&
-                 job_get_flag(j, JOB_CONTROL))) {
+            if (j->get_flag(JOB_CONSTRUCTED) && (!job_is_completed(j)) &&
+                ((job_is_stopped(j) || (!j->get_flag(JOB_FOREGROUND))) &&
+                 j->get_flag(JOB_CONTROL))) {
                 break;
             }
         }
@@ -2909,10 +2909,10 @@ static int builtin_fg(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
             builtin_print_help(parser, streams, argv[0], streams.err);
         } else {
             j = job_get_from_pid(pid);
-            if (!j || !job_get_flag(j, JOB_CONSTRUCTED) || job_is_completed(j)) {
+            if (!j || !j->get_flag(JOB_CONSTRUCTED) || job_is_completed(j)) {
                 streams.err.append_format(_(L"%ls: No suitable job: %d\n"), argv[0], pid);
                 j = 0;
-            } else if (!job_get_flag(j, JOB_CONTROL)) {
+            } else if (!j->get_flag(JOB_CONTROL)) {
                 streams.err.append_format(_(L"%ls: Can't put job %d, '%ls' to foreground because "
                                             L"it is not under job control\n"),
                                           argv[0], pid, j->command_wcstr());
@@ -2938,7 +2938,7 @@ static int builtin_fg(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
     reader_write_title(j->command());
 
     job_promote(j);
-    job_set_flag(j, JOB_FOREGROUND, 1);
+    j->set_flag(JOB_FOREGROUND, true);
 
     job_continue(j, job_is_stopped(j));
     return STATUS_BUILTIN_OK;
@@ -2950,7 +2950,7 @@ static int send_to_bg(parser_t &parser, io_streams_t &streams, job_t *j, const w
         streams.err.append_format(_(L"%ls: Unknown job '%ls'\n"), L"bg", name);
         builtin_print_help(parser, streams, L"bg", streams.err);
         return STATUS_BUILTIN_ERROR;
-    } else if (!job_get_flag(j, JOB_CONTROL)) {
+    } else if (!j->get_flag(JOB_CONTROL)) {
         streams.err.append_format(
             _(L"%ls: Can't put job %d, '%ls' to background because it is not under job control\n"),
             L"bg", j->job_id, j->command_wcstr());
@@ -2961,7 +2961,7 @@ static int send_to_bg(parser_t &parser, io_streams_t &streams, job_t *j, const w
     streams.err.append_format(_(L"Send job %d '%ls' to background\n"), j->job_id,
                               j->command_wcstr());
     job_promote(j);
-    job_set_flag(j, JOB_FOREGROUND, 0);
+    j->set_flag(JOB_FOREGROUND, false);
     job_continue(j, job_is_stopped(j));
     return STATUS_BUILTIN_OK;
 }
@@ -2974,7 +2974,7 @@ static int builtin_bg(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
         job_t *j;
         job_iterator_t jobs;
         while ((j = jobs.next())) {
-            if (job_is_stopped(j) && job_get_flag(j, JOB_CONTROL) && (!job_is_completed(j))) {
+            if (job_is_stopped(j) && j->get_flag(JOB_CONTROL) && (!job_is_completed(j))) {
                 break;
             }
         }
diff --git a/src/exec.cpp b/src/exec.cpp
index 51c8d1f4d..386a534e5 100644
--- a/src/exec.cpp
+++ b/src/exec.cpp
@@ -345,10 +345,10 @@ static void internal_exec_helper(parser_t &parser, const wcstring &def, node_off
 // foreground process group, we don't use posix_spawn if we're going to foreground the process. (If
 // we use fork(), we can call tcsetpgrp after the fork, before the exec, and avoid the race).
 static bool can_use_posix_spawn_for_job(const job_t *job, const process_t *process) {
-    if (job_get_flag(job, JOB_CONTROL)) {  //!OCLINT(collapsible if statements)
+    if (job->get_flag(JOB_CONTROL)) {  //!OCLINT(collapsible if statements)
         // We are going to use job control; therefore when we launch this job it will get its own
         // process group ID. But will it be foregrounded?
-        if (job_get_flag(job, JOB_TERMINAL) && job_get_flag(job, JOB_FOREGROUND)) {
+        if (job->get_flag(JOB_TERMINAL) && job->get_flag(JOB_FOREGROUND)) {
             // It will be foregrounded, so we will call tcsetpgrp(), therefore do not use
             // posix_spawn.
             return false;
@@ -423,7 +423,7 @@ void exec_job(parser_t &parser, job_t *j) {
             // launch_process _never_ returns.
             launch_process_nofork(j->processes.front().get());
         } else {
-            job_set_flag(j, JOB_CONSTRUCTED, 1);
+            j->set_flag(JOB_CONSTRUCTED, true);
             j->processes.front()->completed = 1;
             return;
         }
@@ -452,7 +452,7 @@ void exec_job(parser_t &parser, job_t *j) {
     // sure that the process group doesn't die accidentally, and is often needed when a
     // builtin/block/function is inside a pipeline, since that usually means we have to wait for one
     // program to exit before continuing in the pipeline, causing the group leader to exit.
-    if (job_get_flag(j, JOB_CONTROL) && !exec_error) {
+    if (j->get_flag(JOB_CONTROL) && !exec_error) {
         for (const process_ptr_t &p : j->processes) {
             if (p->type != EXTERNAL) {
                 if (! p->is_last_in_job || ! p->is_first_in_job) {
@@ -792,8 +792,8 @@ void exec_job(parser_t &parser, job_t *j) {
                     // way, the builtin does not need to know what job it is part of. It could
                     // probably figure that out by walking the job list, but it seems more robust to
                     // make exec handle things.
-                    const int fg = job_get_flag(j, JOB_FOREGROUND);
-                    job_set_flag(j, JOB_FOREGROUND, 0);
+                    const int fg = j->get_flag(JOB_FOREGROUND);
+                    j->set_flag(JOB_FOREGROUND, false);
 
                     signal_unblock();
 
@@ -803,7 +803,7 @@ void exec_job(parser_t &parser, job_t *j) {
 
                     // Restore the fg flag, which is temporarily set to false during builtin
                     // execution so as not to confuse some job-handling builtins.
-                    job_set_flag(j, JOB_FOREGROUND, fg);
+                    j->set_flag(JOB_FOREGROUND, fg);
                 }
 
                 // If stdin has been redirected, close the redirection stream.
@@ -839,7 +839,7 @@ void exec_job(parser_t &parser, job_t *j) {
                     // No buffer, so we exit directly. This means we have to manually set the exit
                     // status.
                     if (p->is_last_in_job) {
-                        proc_set_last_status(job_get_flag(j, JOB_NEGATE) ? (!status) : status);
+                        proc_set_last_status(j->get_flag(JOB_NEGATE) ? (!status) : status);
                     }
                     p->completed = 1;
                     break;
@@ -874,7 +874,7 @@ void exec_job(parser_t &parser, job_t *j) {
 
                 } else {
                     if (p->is_last_in_job) {
-                        proc_set_last_status(job_get_flag(j, JOB_NEGATE) ? (!status) : status);
+                        proc_set_last_status(j->get_flag(JOB_NEGATE) ? (!status) : status);
                     }
                     p->completed = 1;
                 }
@@ -951,7 +951,7 @@ void exec_job(parser_t &parser, job_t *j) {
                               p->status);
 
                         int status = p->status;
-                        proc_set_last_status(job_get_flag(j, JOB_NEGATE) ? (!status) : status);
+                        proc_set_last_status(j->get_flag(JOB_NEGATE) ? (!status) : status);
                     }
                 } else {
                     // Ok, unfortunately, we have to do a real fork. Bummer. We work hard to make
@@ -1114,8 +1114,8 @@ void exec_job(parser_t &parser, job_t *j) {
     signal_unblock();
     debug(3, L"Job is constructed");
 
-    job_set_flag(j, JOB_CONSTRUCTED, 1);
-    if (!job_get_flag(j, JOB_FOREGROUND)) {
+    j->set_flag(JOB_CONSTRUCTED, true);
+    if (!j->get_flag(JOB_FOREGROUND)) {
         proc_last_bg_pid = j->pgid;
     }
 
@@ -1124,7 +1124,7 @@ void exec_job(parser_t &parser, job_t *j) {
     } else {
         // Mark the errored job as not in the foreground. I can't fully justify whether this is the
         // right place, but it prevents sanity_lose from complaining.
-        job_set_flag(j, JOB_FOREGROUND, 0);
+        j->set_flag(JOB_FOREGROUND, false);
     }
 }
 
diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp
index 0284a8fc9..252f8d689 100644
--- a/src/parse_execution.cpp
+++ b/src/parse_execution.cpp
@@ -1058,7 +1058,7 @@ parse_execution_result_t parse_execution_context_t::populate_boolean_process(
         }
         case parse_bool_not: {
             // NOT. Negate it.
-            job_set_flag(job, JOB_NEGATE, !job_get_flag(job, JOB_NEGATE));
+            job->set_flag(JOB_NEGATE, !job->get_flag(JOB_NEGATE));
             break;
         }
     }
@@ -1267,16 +1267,15 @@ parse_execution_result_t parse_execution_context_t::run_1_job(const parse_node_t
 
     shared_ptr<job_t> job = std::make_shared<job_t>(acquire_job_id(), block_io);
     job->tmodes = tmodes;
-    job_set_flag(job.get(), JOB_CONTROL,
-                 (job_control_mode == JOB_CONTROL_ALL) ||
+    job->set_flag(JOB_CONTROL,
+                   (job_control_mode == JOB_CONTROL_ALL) ||
                      ((job_control_mode == JOB_CONTROL_INTERACTIVE) && shell_is_interactive()));
 
-    job_set_flag(job.get(), JOB_FOREGROUND, !tree.job_should_be_backgrounded(job_node));
+    job->set_flag(JOB_FOREGROUND, !tree.job_should_be_backgrounded(job_node));
 
-    job_set_flag(job.get(), JOB_TERMINAL, job_get_flag(job.get(), JOB_CONTROL) && !is_subshell && !is_event);
+    job->set_flag(JOB_TERMINAL, job->get_flag(JOB_CONTROL) && !is_subshell && !is_event);
 
-    job_set_flag(job.get(), JOB_SKIP_NOTIFICATION,
-                 is_subshell || is_block || is_event || !shell_is_interactive());
+    job->set_flag(JOB_SKIP_NOTIFICATION, is_subshell || is_block || is_event || !shell_is_interactive());
 
     // Tell the current block what its job is. This has to happen before we populate it (#1394).
     parser->current_block()->job = job;
diff --git a/src/postfork.cpp b/src/postfork.cpp
index f18ba1d7f..8b6c0c24a 100644
--- a/src/postfork.cpp
+++ b/src/postfork.cpp
@@ -69,7 +69,7 @@ static void debug_safe_int(int level, const char *format, int val) {
 bool set_child_group(job_t *j, process_t *p, int print_errors) {
     bool retval = true;
 
-    if (job_get_flag(j, JOB_CONTROL)) {
+    if (j->get_flag(JOB_CONTROL)) {
         if (!j->pgid) {
             j->pgid = p->pid;
         }
@@ -104,7 +104,7 @@ bool set_child_group(job_t *j, process_t *p, int print_errors) {
         j->pgid = getpid();
     }
 
-    if (job_get_flag(j, JOB_TERMINAL) && job_get_flag(j, JOB_FOREGROUND)) {  //!OCLINT(early exit)
+    if (j->get_flag(JOB_TERMINAL) && j->get_flag(JOB_FOREGROUND)) {  //!OCLINT(early exit)
         int result = -1;
         errno = EINTR;
         while (result == -1 && errno == EINTR) {
@@ -317,7 +317,7 @@ bool fork_actions_make_spawn_properties(posix_spawnattr_t *attr,
 
     bool should_set_parent_group_id = false;
     int desired_parent_group_id = 0;
-    if (job_get_flag(j, JOB_CONTROL)) {
+    if (j->get_flag(JOB_CONTROL)) {
         should_set_parent_group_id = true;
 
         // PCA: I'm quite fuzzy on process groups, but I believe that the default value of 0 means
diff --git a/src/proc.cpp b/src/proc.cpp
index 8be007fb6..1a9a61d68 100644
--- a/src/proc.cpp
+++ b/src/proc.cpp
@@ -81,8 +81,8 @@ void print_jobs(void)
     job_t *j;
     while (j = jobs.next()) {
         fwprintf(stdout, L"%p -> %ls -> (foreground %d, complete %d, stopped %d, constructed %d)\n",
-                 j, j->command_wcstr(), job_get_flag(j, JOB_FOREGROUND), job_is_completed(j),
-                 job_is_stopped(j), job_get_flag(j, JOB_CONSTRUCTED));
+                 j, j->command_wcstr(), j->get_flag(JOB_FOREGROUND), job_is_completed(j),
+                 job_is_stopped(j), j->get_flag(JOB_CONSTRUCTED));
     }
 }
 #endif
@@ -231,16 +231,16 @@ bool job_is_completed(const job_t *j) {
     return result;
 }
 
-void job_set_flag(job_t *j, job_flag_t flag, bool set) {
+void job_t::set_flag(job_flag_t flag, bool set) {
     if (set) {
-        j->flags |= flag;
+        this->flags |= flag;
     } else {
-        j->flags &= ~flag;
+        this->flags &= ~flag;
     }
 }
 
-bool job_get_flag(const job_t *j, job_flag_t flag) {
-    return !! (j->flags & flag);
+bool job_t::get_flag(job_flag_t flag) const {
+    return !! (this->flags & flag);
 }
 
 int job_signal(job_t *j, int signal) {
@@ -546,8 +546,8 @@ int job_reap(bool allow_interactive) {
 
         // If we are reaping only jobs who do not need status messages sent to the console, do not
         // consider reaping jobs that need status messages.
-        if ((!job_get_flag(j, JOB_SKIP_NOTIFICATION)) && (!interactive) &&
-            (!job_get_flag(j, JOB_FOREGROUND))) {
+        if ((!j->get_flag(JOB_SKIP_NOTIFICATION)) && (!interactive) &&
+            (!j->get_flag(JOB_FOREGROUND))) {
             continue;
         }
 
@@ -570,8 +570,8 @@ int job_reap(bool allow_interactive) {
 
             // Handle signals other than SIGPIPE.
             int proc_is_job = (p->is_first_in_job && p->is_last_in_job);
-            if (proc_is_job) job_set_flag(j, JOB_NOTIFIED, 1);
-            if (job_get_flag(j, JOB_SKIP_NOTIFICATION)) {
+            if (proc_is_job) j->set_flag(JOB_NOTIFIED, true);
+            if (j->get_flag(JOB_SKIP_NOTIFICATION)) {
                 continue;
             }
 
@@ -584,7 +584,7 @@ int job_reap(bool allow_interactive) {
             // signals. If echoctl is on, then the terminal will have written ^C to the console.
             // If off, it won't have. We don't echo ^C either way, so as to respect the user's
             // preference.
-            if (WTERMSIG(p->status) != SIGINT || !job_get_flag(j, JOB_FOREGROUND)) {
+            if (WTERMSIG(p->status) != SIGINT || !j->get_flag(JOB_FOREGROUND)) {
                 if (proc_is_job) {
                     // We want to report the job number, unless it's the only job, in which case
                     // we don't need to.
@@ -618,8 +618,8 @@ int job_reap(bool allow_interactive) {
         // If all processes have completed, tell the user the job has completed and delete it from
         // the active job list.
         if (job_is_completed(j)) {
-            if (!job_get_flag(j, JOB_FOREGROUND) && !job_get_flag(j, JOB_NOTIFIED) &&
-                !job_get_flag(j, JOB_SKIP_NOTIFICATION)) {
+            if (!j->get_flag(JOB_FOREGROUND) && !j->get_flag(JOB_NOTIFIED) &&
+                !j->get_flag(JOB_SKIP_NOTIFICATION)) {
                 format_job_info(j, _(L"ended"), job_count);
                 found = 1;
             }
@@ -627,13 +627,13 @@ int job_reap(bool allow_interactive) {
             proc_fire_event(L"JOB_EXIT", EVENT_JOB_ID, j->job_id, 0);
 
             job_remove(j);
-        } else if (job_is_stopped(j) && !job_get_flag(j, JOB_NOTIFIED)) {
+        } else if (job_is_stopped(j) && !j->get_flag(JOB_NOTIFIED)) {
             // Notify the user about newly stopped jobs.
-            if (!job_get_flag(j, JOB_SKIP_NOTIFICATION)) {
+            if (!j->get_flag(JOB_SKIP_NOTIFICATION)) {
                 format_job_info(j, _(L"stopped"), job_count);
                 found = 1;
             }
-            job_set_flag(j, JOB_NOTIFIED, 1);
+            j->set_flag(JOB_NOTIFIED, true);
         }
     }
 
@@ -852,7 +852,7 @@ static int terminal_return_from_job(job_t *j) {
 void job_continue(job_t *j, bool cont) {
     // Put job first in the job list.
     job_promote(j);
-    job_set_flag(j, JOB_NOTIFIED, 0);
+    j->set_flag(JOB_NOTIFIED, false);
 
     CHECK_BLOCK();
 
@@ -861,7 +861,7 @@ void job_continue(job_t *j, bool cont) {
           is_interactive ? L"INTERACTIVE" : L"NON-INTERACTIVE");
 
     if (!job_is_completed(j)) {
-        if (job_get_flag(j, JOB_TERMINAL) && job_get_flag(j, JOB_FOREGROUND)) {
+        if (j->get_flag(JOB_TERMINAL) && j->get_flag(JOB_FOREGROUND)) {
             // Put the job into the foreground. Hack: ensure that stdin is marked as blocking first
             // (issue #176).
             make_fd_blocking(STDIN_FILENO);
@@ -879,7 +879,7 @@ void job_continue(job_t *j, bool cont) {
         if (cont) {
             for (process_ptr_t &p : j->processes) p->stopped = false;
 
-            if (job_get_flag(j, JOB_CONTROL)) {
+            if (j->get_flag(JOB_CONTROL)) {
                 if (killpg(j->pgid, SIGCONT)) {
                     wperror(L"killpg (SIGCONT)");
                     return;
@@ -894,7 +894,7 @@ void job_continue(job_t *j, bool cont) {
             }
         }
 
-        if (job_get_flag(j, JOB_FOREGROUND)) {
+        if (j->get_flag(JOB_FOREGROUND)) {
             // Look for finished processes first, to avoid select() if it's already done.
             process_mark_finished_children(false);
 
@@ -931,7 +931,7 @@ void job_continue(job_t *j, bool cont) {
         }
     }
 
-    if (job_get_flag(j, JOB_FOREGROUND)) {
+    if (j->get_flag(JOB_FOREGROUND)) {
         if (job_is_completed(j)) {
             // It's possible that the job will produce output and exit before we've even read from
             // it.
@@ -950,12 +950,12 @@ void job_continue(job_t *j, bool cont) {
                 int status = proc_format_status(p->status);
                 // fwprintf(stdout, L"setting status %d for %ls\n", job_get_flag( j, JOB_NEGATE
                 // )?!status:status, j->command);
-                proc_set_last_status(job_get_flag(j, JOB_NEGATE) ? !status : status);
+                proc_set_last_status(j->get_flag(JOB_NEGATE) ? !status : status);
             }
         }
 
         // Put the shell back in the foreground.
-        if (job_get_flag(j, JOB_TERMINAL) && job_get_flag(j, JOB_FOREGROUND)) {
+        if (j->get_flag(JOB_TERMINAL) && j->get_flag(JOB_FOREGROUND)) {
             int ok;
 
             signal_block();
@@ -983,11 +983,11 @@ void proc_sanity_check() {
 
     job_iterator_t jobs;
     while (const job_t *j = jobs.next()) {
-        if (!job_get_flag(j, JOB_CONSTRUCTED)) continue;
+        if (!j->get_flag(JOB_CONSTRUCTED)) continue;
 
 
         // More than one foreground job?
-        if (job_get_flag(j, JOB_FOREGROUND) && !(job_is_stopped(j) || job_is_completed(j))) {
+        if (j->get_flag(JOB_FOREGROUND) && !(job_is_stopped(j) || job_is_completed(j))) {
             if (fg_job) {
                 debug(0, _(L"More than one job in foreground: job 1: '%ls' job 2: '%ls'"),
                       fg_job->command_wcstr(), j->command_wcstr());
diff --git a/src/proc.h b/src/proc.h
index 1183de0dc..b75d20742 100644
--- a/src/proc.h
+++ b/src/proc.h
@@ -223,6 +223,10 @@ class job_t {
     /// Bitset containing information about the job. A combination of the JOB_* constants.
     unsigned int flags;
 
+    // Get and set flags
+    bool get_flag(job_flag_t flag) const;
+    void set_flag(job_flag_t flag, bool set);
+
     /// Returns the block IO redirections associated with the job. These are things like the IO
     /// redirections associated with the begin...end statement.
     const io_chain_t &block_io_chain() const { return this->block_io; }
@@ -298,12 +302,6 @@ extern int job_control_mode;
 /// anything.
 extern int no_exec;
 
-/// Add the specified flag to the bitset of flags for the specified job.
-void job_set_flag(job_t *j, job_flag_t flag, bool set);
-
-/// Returns one if the specified flag is set in the specified job, 0 otherwise.
-bool job_get_flag(const job_t *j, job_flag_t flag);
-
 /// Sets the status of the last process to exit.
 void proc_set_last_status(int s);