Re: [PATCH] pager: exit without error on SIGPIPE
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-02-01 15:06:03
On Sat, Jan 30 2021, Denton Liu wrote:
[...]
The thread at large has enough about whether this approach even makes sense. I won't repeat that here. Just small notes on the patch itself:
quoted hunk ↗ jump to hunk
diff --git a/Makefile b/Makefile index 4edfda3e00..38a1a20f31 100644 --- a/Makefile +++ b/Makefile@@ -719,6 +719,7 @@ TEST_BUILTINS_OBJS += test-mktemp.o TEST_BUILTINS_OBJS += test-oid-array.o TEST_BUILTINS_OBJS += test-oidmap.o TEST_BUILTINS_OBJS += test-online-cpus.o +TEST_BUILTINS_OBJS += test-pager.o TEST_BUILTINS_OBJS += test-parse-options.o TEST_BUILTINS_OBJS += test-parse-pathspec-file.o TEST_BUILTINS_OBJS += test-path-utils.odiff --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);
As shown in https://lore.kernel.org/git/20210201144921.8664-1-avarab@gmail.com/ (local) this leaves us without guard rails where the pager dies/segfaults or whatever. That's an existing bug, but by not carrying the SIGPIPE forward it changes from "most of the time we'd exit with SIGPIPE anyway" to "we'll never notice".
[...] +test_expect_success TTY 'SIGPIPE from pager returns success' ' + test_terminal env PAGER=true test-tool pager +' + test_done
As noted in https://lore.kernel.org/git/20210201144921.8664-1-avarab@gmail.com/ (local) I think this whole "test-tool pager" isn't needed. We can just use git itself with some trickery.