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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:

>> 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/20210201144921.8664-1-avarab@xxxxxxxxx/ 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".

Would it be the matter of propagating the exit status of the pager
noticed by wait_or_white() down thru finish_command_in_signal() and
wait_for_pager(1) to here, so

 - If we know pager exited with non-zero status, we would report,
   perhaps with warning(_("..."));

 - If we notice we got a SIGPIPE, we ignore it---it is nothing of
   interest to the end-user;

 - Otherwise we do not do anything differently.

would be sufficient?  Implementors of "git -p" may know that "git"
happens to implement its paging by piping its output to an external
pager, but the end-users do not care.  Implementors may say they are
giving 'q' to their pager "less", but to the end-users, who report
"I ran 'git log' and after reading a pageful, I told it to 'q'uit",
the distinction does not have any importance.

Or are there more to it, in that the exit status we get from the
pager, combined with the kind of signal we are getting, is not
sufficient for us to tell what is going on?




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux