Thread (2 messages) flat view 2 messages, 2 authors, 2021-02-01

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()"?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help