Re: [PATCH] pager: exit without error on SIGPIPE
From: Johannes Sixt <hidden>
Date: 2021-01-30 09:29:36
Am 30.01.21 um 00:48 schrieb Denton Liu:
If the pager closes before the git command feeding the pager finishes, git is killed by a SIGPIPE and the corresponding exit code is 141. Since the pipe is just an implementation detail, it does not make sense for this error code to be user-facing. Handle SIGPIPEs by simply calling exit(0) in wait_for_pager_signal(). Introduce `test-tool pager` which infinitely prints `y` to the pager in order to test the new behavior. This cannot be tested with any existing git command because there are no other commands which produce infinite output. Without the change to pager.c, the newly introduced test fails. Reported-by: Vincent Lefevre <redacted> Signed-off-by: Denton Liu <redacted>
...
quoted hunk ↗ jump to hunk
diff --git a/pager.c b/pager.c index ee435de675..5922d99dc8 100644 --- a/pager.c +++ b/pager.c@@ -34,6 +34,8 @@ static void wait_for_pager_atexit(void) static void wait_for_pager_signal(int signo) { wait_for_pager(1); + if (signo == SIGPIPE) + exit(0); sigchain_pop(signo); raise(signo); }diff --git a/t/helper/test-pager.c b/t/helper/test-pager.c new file mode 100644 index 0000000000..feb68b8643 --- /dev/null +++ b/t/helper/test-pager.c@@ -0,0 +1,12 @@ +#include "test-tool.h" +#include "cache.h" + +int cmd__pager(int argc, const char **argv) +{ + if (argc > 1) + usage("\ttest-tool pager"); + + setup_pager(); + for (;;) + puts("y"); +}
My gut feeling tells that this will end in an infinite loop on Windows. There are no signals on Windows that would kill the upstream of a pipe. This call site will only notice that the downstream of the pipe was closed, when it checks for write errors. Let me test it. -- Hannes