Re: [PATCH 3/3] pager: properly log pager exit code when signalled
From: Junio C Hamano <hidden>
Date: 2021-02-01 18:08:46
Ævar Arnfjörð Bjarmason [off-list ref] writes:
quoted hunk
When git invokes a pager that exits with non-zero the common case is that we'll already return the correct SIGPIPE failure from git itself, but the exit code logged in trace2 has always been incorrectly reported[1]. Fix that and log the correct exit code in the logs. Since this gives us something to test outside of our recently-added tests needing a !MINGW prerequisite, let's refactor the test to run on MINGW and actually check for SIGPIPE outside of MINGW. The wait_or_whine() is only called with a true "in_signal" from from finish_command_in_signal(), which in turn is only used in pager.c. I'm not quite sure about that BUG() case. Can we have a true in_signal and not have a true WIFEXITED(status)? I haven't been able to think of a test case for it. 1. The incorrect logging of the exit code in was seemingly copy/pasted into finish_command_in_signal() in ee4512ed481 (trace2: create new combined trace facility, 2019-02-22) Signed-off-by: Ævar Arnfjörð Bjarmason <redacted> --- run-command.c | 8 +++++-- t/t7006-pager.sh | 61 +++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 58 insertions(+), 11 deletions(-)diff --git a/run-command.c b/run-command.c index ea4d0fb4b15..10e1c96c2bd 100644 --- a/run-command.c +++ b/run-command.c@@ -551,8 +551,12 @@ static int wait_or_whine(pid_t pid, const char *argv0, int in_signal) while ((waiting = waitpid(pid, &status, 0)) < 0 && errno == EINTR) ; /* nothing */ - if (in_signal) - return 0; + if (in_signal && WIFEXITED(status)) + return WEXITSTATUS(status); + if (in_signal) { + BUG("was not expecting waitpid() status %d", status); + return -1; + }
Doesn't BUG die, never to return control back to us? How about "warning()" or "error()"?