Subject: Re: [PATCH] pager: exit without error on SIGPIPE

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Mon, 01 Feb 2021 16:03:55 +0100

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:

> 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.o
> 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);

As shown in
https://lore.kernel.org/git/[email-masked]/ 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/[email-masked]/ I
think this whole "test-tool pager" isn't needed. We can just use git
itself with some trickery.