From 60d75e9aa05aa9c155c155ed898d22bbab0a6644 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Mon, 17 May 2021 15:33:33 -0700 Subject: [PATCH] Remove proc_create_event Switch to a set of factory functions inside event_t. No user-visible change here. --- src/event.cpp | 35 +++++++++++++++++++++++++++++++++++ src/event.h | 13 ++++++++++++- src/fish.cpp | 4 +--- src/proc.cpp | 20 +++----------------- src/proc.h | 3 --- 5 files changed, 51 insertions(+), 24 deletions(-) diff --git a/src/event.cpp b/src/event.cpp index 040a5c3b2..eed1cac33 100644 --- a/src/event.cpp +++ b/src/event.cpp @@ -22,6 +22,7 @@ #include "proc.h" #include "signal.h" #include "termsize.h" +#include "wcstringutil.h" #include "wutil.h" // IWYU pragma: keep class pending_signals_t { @@ -486,9 +487,43 @@ event_description_t event_description_t::generic(wcstring str) { return event; } +// static event_t event_t::variable(wcstring name, wcstring_list_t args) { event_t evt{event_type_t::variable}; evt.desc.str_param1 = std::move(name); evt.arguments = std::move(args); return evt; } + +// static +event_t event_t::process_exit(pid_t pid, int status) { + event_t evt{event_type_t::exit}; + evt.desc.param1.pid = pid; + evt.arguments.reserve(3); + evt.arguments.push_back(L"PROCESS_EXIT"); + evt.arguments.push_back(to_string(pid)); + evt.arguments.push_back(to_string(status)); + return evt; +} + +// static +event_t event_t::job_exit(pid_t pgid) { + event_t evt{event_type_t::exit}; + evt.desc.param1.pid = pgid; + evt.arguments.reserve(3); + evt.arguments.push_back(L"JOB_EXIT"); + evt.arguments.push_back(to_string(pgid)); + evt.arguments.push_back(L"0"); // historical + return evt; +} + +// static +event_t event_t::caller_exit(uint64_t caller_id, int job_id) { + event_t evt{event_type_t::caller_exit}; + evt.desc.param1.caller_id = caller_id; + evt.arguments.reserve(3); + evt.arguments.push_back(L"JOB_EXIT"); + evt.arguments.push_back(to_string(job_id)); + evt.arguments.push_back(L"0"); // historical + return evt; +} diff --git a/src/event.h b/src/event.h index 9e8e57636..b9533bba7 100644 --- a/src/event.h +++ b/src/event.h @@ -86,9 +86,20 @@ struct event_t { /// Arguments to any handler. wcstring_list_t arguments{}; - event_t(event_type_t t) : desc(t) {} + explicit event_t(event_type_t t) : desc(t) {} + /// Create an event_type_t::variable event. static event_t variable(wcstring name, wcstring_list_t args); + + /// Create a PROCESS_EXIT event. + static event_t process_exit(pid_t pid, int status); + + /// Create a JOB_EXIT event. The pgid should be negative. + /// The reported status is always 0 for historical reasons. + static event_t job_exit(pid_t pgid); + + /// Create a caller_exit event. + static event_t caller_exit(uint64_t internal_job_id, int job_id); }; class parser_t; diff --git a/src/fish.cpp b/src/fish.cpp index 43621fca1..57d7a42c9 100644 --- a/src/fish.cpp +++ b/src/fish.cpp @@ -577,9 +577,7 @@ int main(int argc, char **argv) { } int exit_status = res ? STATUS_CMD_UNKNOWN : parser.get_last_status(); - - event_fire(parser, - proc_create_event(L"PROCESS_EXIT", event_type_t::exit, getpid(), exit_status)); + event_fire(parser, event_t::process_exit(getpid(), exit_status)); // Trigger any exit handlers. wcstring_list_t event_args = {to_string(exit_status)}; diff --git a/src/proc.cpp b/src/proc.cpp index c167b7e31..cc97f8e1b 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -516,17 +516,6 @@ static void print_job_status(parser_t &parser, const job_t *j, job_status_t stat call_job_summary(parser, args); } -event_t proc_create_event(const wchar_t *msg, event_type_t type, pid_t pid, int status) { - event_t event{type}; - event.desc.param1.pid = pid; - - event.arguments.reserve(3); - event.arguments.push_back(msg); - event.arguments.push_back(to_string(pid)); - event.arguments.push_back(to_string(status)); - return event; -} - /// Remove all disowned jobs whose job chain is fully constructed (that is, do not erase disowned /// jobs that still have an in-flight parent job). Note we never print statuses for such jobs. static void remove_disowned_jobs(job_list_t &jobs) { @@ -554,8 +543,7 @@ static bool try_clean_process_in_job(parser_t &parser, process_t *p, job_t *j, // Add an exit event if the process did not come from a job handler. if (!j->from_event_handler()) { - exit_events->push_back( - proc_create_event(L"PROCESS_EXIT", event_type_t::exit, p->pid, s.status_value())); + exit_events->push_back(event_t::process_exit(p->pid, s.status_value())); } // Ignore SIGPIPE. We issue it ourselves to the pipe writer when the pipe reader dies. @@ -685,14 +673,12 @@ static bool process_clean_after_marking(parser_t &parser, bool allow_interactive // don't create an event or it's easy to get an infinite loop. if (!j->from_event_handler() && j->should_report_process_exits()) { pid_t pgid = *j->get_pgid(); - exit_events.push_back(proc_create_event(L"JOB_EXIT", event_type_t::exit, -pgid, 0)); + exit_events.push_back(event_t::job_exit(-pgid)); } // Caller exit events we still create, which anecdotally fixes `source (thing | psub)` // inside event handlers. This seems benign since this event is barely used (basically // only psub), and it seems hard to construct an infinite loop with it. - exit_events.push_back( - proc_create_event(L"JOB_EXIT", event_type_t::caller_exit, j->job_id(), 0)); - exit_events.back().desc.param1.caller_id = j->internal_job_id; + exit_events.push_back(event_t::caller_exit(j->internal_job_id, j->job_id())); } } diff --git a/src/proc.h b/src/proc.h index 2e7aae43b..625e85bbf 100644 --- a/src/proc.h +++ b/src/proc.h @@ -530,9 +530,6 @@ void proc_update_jiffies(parser_t &parser); /// job is in the foreground, that every process is in a valid state, etc. void proc_sanity_check(const parser_t &parser); -/// Create a process/job exit event notification. -event_t proc_create_event(const wchar_t *msg, event_type_t type, pid_t pid, int status); - /// Initializations. void proc_init();