From d4daa286909c14f4bba5bc9db287fcea7fa66e5c Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Tue, 17 Dec 2019 18:15:42 -0800 Subject: [PATCH] Correctly set the exit status in block and function processes Previously, if the user control-C'd out of a process, we would set a bogus exit status in the process, but it was difficult to observe this because we would be cancelling anyways. But set it properly. --- src/exec.cpp | 42 ++++++++++++++++++++++++++---------------- src/proc.h | 5 +++++ 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/exec.cpp b/src/exec.cpp index a421c52f4..c872b9226 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -753,11 +753,17 @@ static proc_performer_t get_performer_for_process(process_t *p, const job_t *job tnode_t node = p->internal_block_node; assert(source && node && "Process is missing node info"); return [=](parser_t &parser) { - parser.eval_node(source, node, TOP, lineage); - int status = parser.get_last_status(); - // FIXME: setting the status this way is dangerous nonsense, we need to decode the - // status properly if it was a signal. - return proc_status_t::from_exit_code(status); + eval_result_t res = parser.eval_node(source, node, TOP, lineage); + switch (res) { + case eval_result_t::ok: + case eval_result_t::error: + return proc_status_t::from_exit_code(parser.get_last_status()); + case eval_result_t::cancelled: + // TODO: we should reflect the actual signal which was received. + return proc_status_t::from_signal(SIGINT); + case eval_result_t::control_flow: + DIE("eval_result_t::control_flow should not be returned from eval_node"); + } }; } else { assert(p->type == process_type_t::function); @@ -771,20 +777,24 @@ static proc_performer_t get_performer_for_process(process_t *p, const job_t *job const auto &ld = parser.libdata(); auto saved_exec_count = ld.exec_count; const block_t *fb = function_prepare_environment(parser, *argv, *props); - parser.eval_node(props->parsed_source, props->body_node, TOP, lineage); + auto res = parser.eval_node(props->parsed_source, props->body_node, TOP, lineage); function_restore_environment(parser, fb); - // If the function did not execute anything, treat it as success. - int status; - if (saved_exec_count == ld.exec_count) { - status = 0; - } else { - status = parser.get_last_status(); - } + switch (res) { + case eval_result_t::ok: + // If the function did not execute anything, treat it as success. + return proc_status_t::from_exit_code(saved_exec_count == ld.exec_count + ? EXIT_SUCCESS + : parser.get_last_status()); + case eval_result_t::error: + return proc_status_t::from_exit_code(parser.get_last_status()); - // FIXME: setting the status this way is dangerous nonsense, we need to decode the - // status properly if it was a signal. - return proc_status_t::from_exit_code(status); + case eval_result_t::cancelled: + // TODO: we should reflect the actual signal which was received. + return proc_status_t::from_signal(SIGINT); + case eval_result_t::control_flow: + DIE("eval_result_t::control_flow should not be returned from eval_node"); + } }; } } diff --git a/src/proc.h b/src/proc.h index 0118d560a..c209007b3 100644 --- a/src/proc.h +++ b/src/proc.h @@ -74,6 +74,11 @@ class proc_status_t { return proc_status_t(w_exitcode(ret, 0 /* sig */)); } + /// Construct directly from a signal. + static proc_status_t from_signal(int sig) { + return proc_status_t(w_exitcode(0 /* ret */, sig)); + } + /// \return if we are stopped (as in SIGSTOP). bool stopped() const { return WIFSTOPPED(status_); }