From cc1c973025094736036c4c91ec83978576438fd3 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Tue, 15 Oct 2019 14:37:10 -0700 Subject: [PATCH] Remove job_flags as an enum, just use a struct This removes an over-complicated flag implementation, replacing it with just a plain struct. --- src/builtin_bg.cpp | 2 +- src/builtin_disown.cpp | 2 +- src/builtin_fg.cpp | 2 +- src/exec.cpp | 14 ++++----- src/parse_execution.cpp | 7 +++-- src/proc.cpp | 16 ++++------- src/proc.h | 64 +++++++++++++++++++---------------------- 7 files changed, 50 insertions(+), 57 deletions(-) diff --git a/src/builtin_bg.cpp b/src/builtin_bg.cpp index 60bc11bb4..2e9b4e5ea 100644 --- a/src/builtin_bg.cpp +++ b/src/builtin_bg.cpp @@ -31,7 +31,7 @@ static int send_to_bg(parser_t &parser, io_streams_t &streams, job_t *j) { streams.err.append_format(_(L"Send job %d '%ls' to background\n"), j->job_id, j->command_wcstr()); parser.job_promote(j); - j->set_flag(job_flag_t::FOREGROUND, false); + j->mut_flags().foreground = false; j->continue_job(parser, true, j->is_stopped()); return STATUS_CMD_OK; } diff --git a/src/builtin_disown.cpp b/src/builtin_disown.cpp index d8e568ec9..f6c5965a3 100644 --- a/src/builtin_disown.cpp +++ b/src/builtin_disown.cpp @@ -35,7 +35,7 @@ static int disown_job(const wchar_t *cmd, parser_t &parser, io_streams_t &stream // We cannot directly remove the job from the jobs() list as `disown` might be called // within the context of a subjob which will cause the parent job to crash in exec_job(). // Instead, we set a flag and the parser removes the job from the jobs list later. - j->set_flag(job_flag_t::DISOWN_REQUESTED, true); + j->mut_flags().disown_requested = true; add_disowned_pgid(j->pgid); return STATUS_CMD_OK; diff --git a/src/builtin_fg.cpp b/src/builtin_fg.cpp index 072e038a2..c06d2a1a7 100644 --- a/src/builtin_fg.cpp +++ b/src/builtin_fg.cpp @@ -105,7 +105,7 @@ int builtin_fg(parser_t &parser, io_streams_t &streams, wchar_t **argv) { reader_write_title(job->command(), parser); parser.job_promote(job); - job->set_flag(job_flag_t::FOREGROUND, true); + job->mut_flags().foreground = true; job->continue_job(parser, true, job->is_stopped()); return STATUS_CMD_OK; diff --git a/src/exec.cpp b/src/exec.cpp index 2a9957e33..f3403c9ce 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -316,7 +316,7 @@ void internal_exec(env_stack_t &vars, job_t *j, const io_chain_t &all_ios) { // launch_process _never_ returns. launch_process_nofork(vars, j->processes.front().get()); } else { - j->set_flag(job_flag_t::CONSTRUCTED, true); + j->mut_flags().constructed = true; j->processes.front()->completed = 1; return; } @@ -563,15 +563,15 @@ static bool exec_internal_builtin_proc(parser_t &parser, const std::shared_ptris_foreground(); - j->set_flag(job_flag_t::FOREGROUND, false); + const bool fg = j->is_foreground(); + j->mut_flags().foreground = false; // Note this call may block for a long time, while the builtin performs I/O. p->status = builtin_run(parser, j->pgid, p->get_argv(), streams); // Restore the fg flag, which is temporarily set to false during builtin // execution so as not to confuse some job-handling builtins. - j->set_flag(job_flag_t::FOREGROUND, fg); + j->mut_flags().foreground = fg; return true; // "success" } @@ -1062,7 +1062,7 @@ bool exec_job(parser_t &parser, shared_ptr j) { // Perhaps inherit our parent's pgid and job control flag. if (parent_job && parent_job->pgid != INVALID_PID) { j->pgid = parent_job->pgid; - j->set_flag(job_flag_t::JOB_CONTROL, true); + j->mut_flags().job_control = true; } if (j->pgid == INVALID_PID && should_claim_process_group_for_job(j)) { @@ -1085,7 +1085,7 @@ bool exec_job(parser_t &parser, shared_ptr j) { internal_exec(parser.vars(), j.get(), all_ios); // internal_exec only returns if it failed to set up redirections. // In case of an successful exec, this code is not reached. - bool status = j->get_flag(job_flag_t::NEGATE) ? 0 : 1; + bool status = j->flags().negate ? 0 : 1; parser.set_last_statuses(statuses_t::just(status)); return false; } @@ -1149,7 +1149,7 @@ bool exec_job(parser_t &parser, shared_ptr j) { FLOGF(exec_job_exec, L"Executed job %d from command '%ls' with pgrp %d", j->job_id, j->command_wcstr(), j->pgid); - j->set_flag(job_flag_t::CONSTRUCTED, true); + j->mut_flags().constructed = true; if (!j->is_foreground()) { parser.vars().set_one(L"last_pid", ENV_GLOBAL, to_string(j->pgid)); } diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index badec1d8e..cbce87bb0 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -1029,7 +1029,8 @@ bool parse_execution_context_t::determine_io_chain(tnode_t not_statement) { - job->set_flag(job_flag_t::NEGATE, !job->get_flag(job_flag_t::NEGATE)); + auto &flags = job->mut_flags(); + flags.negate = !flags.negate; return this->populate_job_process(job, proc, not_statement.require_get_child()); } @@ -1263,8 +1264,8 @@ parse_execution_result_t parse_execution_context_t::run_1_job(tnode_t jo shared_ptr job = std::make_shared(acquire_job_id(), props, block_io, parent_job); job->tmodes = tmodes; - job->set_flag(job_flag_t::FOREGROUND, !job_node_is_background(job_node)); - job->set_flag(job_flag_t::JOB_CONTROL, wants_job_control); + job->mut_flags().foreground = !job_node_is_background(job_node); + job->mut_flags().job_control = wants_job_control; // We are about to populate a job. One possible argument to the job is a command substitution // which may be interested in the job that's populating it, via '--on-job-exit caller'. Record diff --git a/src/proc.cpp b/src/proc.cpp index d6f216c8b..b269934c8 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -177,10 +177,6 @@ bool job_t::job_chain_is_fully_constructed() const { return true; } -void job_t::set_flag(job_flag_t flag, bool set) { this->flags.set(flag, set); } - -bool job_t::get_flag(job_flag_t flag) const { return this->flags.get(flag); } - bool job_t::signal(int signal) { // Presumably we are distinguishing between the two cases below because we do // not want to send ourselves the signal in question in case the job shares @@ -211,7 +207,7 @@ statuses_t job_t::get_statuses() const { st.pipestatus.push_back(p->status.status_value()); } int laststatus = st.pipestatus.back(); - st.status = (get_flag(job_flag_t::NEGATE) ? !laststatus : laststatus); + st.status = flags().negate ? !laststatus : laststatus; return st; } @@ -453,7 +449,7 @@ void remove_disowned_jobs(job_list_t &jobs) { auto iter = jobs.begin(); while (iter != jobs.end()) { const auto &j = *iter; - if (j->get_flag(job_flag_t::DISOWN_REQUESTED) && j->job_chain_is_fully_constructed()) { + if (j->flags().disown_requested && j->job_chain_is_fully_constructed()) { iter = jobs.erase(iter); } else { ++iter; @@ -484,7 +480,7 @@ static bool try_clean_process_in_job(process_t *p, job_t *j, std::vectoris_first_in_job && p->is_last_in_job); - if (proc_is_job) j->set_flag(job_flag_t::NOTIFIED, true); + if (proc_is_job) j->mut_flags().notified = true; // Handle signals other than SIGPIPE. // Always report crashes. @@ -535,7 +531,7 @@ static bool try_clean_process_in_job(process_t *p, job_t *j, std::vector &j) { // Did we already print a status message? - if (j->get_flag(job_flag_t::NOTIFIED)) return false; + if (j->flags().notified) return false; // Do we just skip notifications? if (j->skip_notification()) return false; @@ -598,7 +594,7 @@ static bool process_clean_after_marking(parser_t &parser, bool allow_interactive // Print the message if we need to. if (job_wants_message(j) && (j->is_completed() || j->is_stopped())) { print_job_status(j.get(), j->is_completed() ? JOB_ENDED : JOB_STOPPED); - j->set_flag(job_flag_t::NOTIFIED, true); + j->mut_flags().notified = true; printed = true; } @@ -864,7 +860,7 @@ static bool terminal_return_from_job(job_t *j, int restore_attrs) { void job_t::continue_job(parser_t &parser, bool reclaim_foreground_pgrp, bool send_sigcont) { // Put job first in the job list. parser.job_promote(this); - set_flag(job_flag_t::NOTIFIED, false); + mut_flags().notified = false; FLOGF(proc_job_run, L"%ls job %d, gid %d (%ls), %ls, %ls", send_sigcont ? L"Continue" : L"Start", job_id, pgid, command_wcstr(), diff --git a/src/proc.h b/src/proc.h index 166085acb..c093e5c1d 100644 --- a/src/proc.h +++ b/src/proc.h @@ -17,7 +17,6 @@ #include #include "common.h" -#include "enum_set.h" #include "event.h" #include "io.h" #include "parse_tree.h" @@ -252,30 +251,6 @@ class process_t { typedef std::unique_ptr process_ptr_t; typedef std::vector process_list_t; -/// Constants for the flag variable in the job struct. -enum class job_flag_t { - /// Whether the user has been told about stopped job. - NOTIFIED, - /// Whether this job is in the foreground. - FOREGROUND, - /// Whether the specified job is completely constructed, i.e. completely parsed, and every - /// process in the job has been forked, etc. - CONSTRUCTED, - /// Whether the exit status should be negated. This flag can only be set by the not builtin. - NEGATE, - /// Whether the job is under job control, i.e. has its own pgrp. - JOB_CONTROL, - /// This job is disowned, and should be removed from the active jobs list. - DISOWN_REQUESTED, - - JOB_FLAG_COUNT -}; - -template <> -struct enum_info_t { - static constexpr auto count = job_flag_t::JOB_FLAG_COUNT; -}; - typedef int job_id_t; job_id_t acquire_job_id(void); void release_job_id(job_id_t jobid); @@ -384,15 +359,36 @@ class job_t { /// stops. struct termios tmodes {}; - /// Bitset containing information about the job. A combination of the JOB_* constants. - enum_set_t flags{}; + /// Flags associated with the job. + struct flags_t { + /// Whether the user has been told about stopped job. + bool notified{false}; - // Get and set flags - bool get_flag(job_flag_t flag) const; - void set_flag(job_flag_t flag, bool set); + /// Whether this job is in the foreground. + bool foreground{false}; + + /// Whether the specified job is completely constructed, i.e. completely parsed, and every + /// process in the job has been forked, etc. + bool constructed{false}; + + /// Whether the exit status should be negated. This flag can only be set by the not builtin. + bool negate{false}; + + /// Whether the job is under job control, i.e. has its own pgrp. + bool job_control{false}; + + /// This job is disowned, and should be removed from the active jobs list. + bool disown_requested{false}; + } job_flags{}; + + /// Access the job flags. + const flags_t &flags() const { return job_flags; } + + /// Access mutable job flags. + flags_t &mut_flags() { return job_flags; } /// \return if we want job control. - bool wants_job_control() const { return get_flag(job_flag_t::JOB_CONTROL); } + bool wants_job_control() const { return flags().job_control; } /// \return if this job should own the terminal when it runs. bool should_claim_terminal() const { return properties.wants_terminal && is_foreground(); } @@ -406,16 +402,16 @@ class job_t { // Helper functions to check presence of flags on instances of jobs /// The job has been fully constructed, i.e. all its member processes have been launched - bool is_constructed() const { return get_flag(job_flag_t::CONSTRUCTED); } + bool is_constructed() const { return flags().constructed; } /// The job was launched in the foreground and has control of the terminal - bool is_foreground() const { return get_flag(job_flag_t::FOREGROUND); } + bool is_foreground() const { return flags().foreground; } /// The job is complete, i.e. all its member processes have been reaped bool is_completed() const; /// The job is in a stopped state bool is_stopped() const; /// The job is OK to be externally visible, e.g. to the user via `jobs` bool is_visible() const { - return !is_completed() && is_constructed() && !get_flag(job_flag_t::DISOWN_REQUESTED); + return !is_completed() && is_constructed() && !flags().disown_requested; }; bool skip_notification() const { return properties.skip_notification; } bool from_event_handler() const { return properties.from_event_handler; }