GNU bug report logs - #74159
Unexpected exit code of 0 when -q is set and close_stdout fails

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 74159 in the body.
You can then email your comments to 74159 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jan Černohorský <jan <at> kam.mff.cuni.cz>
To: bug-grep <at> gnu.org
Cc: wizards <at> kam.mff.cuni.cz
Subject: Unexpected exit code of 0 when -q is set and close_stdout fails
Date: Fri, 1 Nov 2024 14:10:43 +0100

Hello,

I've encountered a particularly interesting bug in GNU grep yesterday while 
hunting some issues with LibreOffice (if you're interested in the whole 
post-mortem, it can be read here: 
https://grsc.cz/blog/loffice-linux-issues/, but it's mostly not relevant 
for this bug).

Whenever the `-q` option is used, grep can exit with a 0, even if no 
match has been found and an error has been encountered. In particular, 
when the `close_stdout` function is called and the `close` syscall 
fails, grep exits with 0, even when it's not supposed to.

While reading the source code, I found out that when the `-q` option is 
detected (grep.c:2697), the `exit_failure` variable is set to 0. This 
causes any error, when not specially handled, to exit with 0, even 
before any match is found. This is also true for the `close_stdout` libc 
function, which hard exits using `_exit(exit_failure)` when the syscall 
fails.

This, at first glance, (and pardon me if I'm wrong, I haven't looked 
into the source code that deeply) seems to me as bad design, since it 
makes the code prone to multiple such mistakes – where someone 
inadvertently uses the `exit_failure` variable without realising it 
makes the program return the wrong code. It would make much more sense 
to me (and again, I don't know whether this is possible) to set the 
`exit_failure` variable to 0 only *after* the first match is found, 
preventing such issues altogether. It also makes more sense semantically 
IMO, as the `-q` option states that errors are ignored only when a match 
is found, not always.

As I feel this is more of a design decision than a straightforward fix, 
I'm not sending a patch, but I'll be glad to assist any efforts to 
fix this.

CC'd are my colleagues who helped discover the issue.

Regards,
Jan

Message #10 received at 74159-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Jan Černohorský <jan <at> kam.mff.cuni.cz>
Cc: 74159-done <at> debbugs.gnu.org, wizards <at> kam.mff.cuni.cz
Subject: Re: bug#74159: Unexpected exit code of 0 when -q is set and
 close_stdout fails
Date: Fri, 1 Nov 2024 23:02:11 -0700

[Message part 1 (text/plain, inline)]

On 2024-11-01 06:10, Jan Černohorský wrote:

> when the `-q` option is 
> detected (grep.c:2697), the `exit_failure` variable is set to 0. This 
> causes any error, when not specially handled, to exit with 0, even 
> before any match is found.

Yes, that's a mistake. I can't reproduce your bug but I can see other 
consequences of the mistake. Fixed in the attached patch (with a test 
case that's unlike your problem); please give it a try.

That being said, the lower levels of your OS appear to be buggy, as 
'close' should not fail with EACCES. I wouldn't be surprised if other 
software breaks because of this issue. So I suggest also reporting a bug 
to whoever maintains the NFS client code that you're using.

I'm boldly closing this bug report; we can reopen it if the patch 
doesn't work for you.

[0001-grep-fix-q-suppression-of-diagnostics.patch (text/x-patch, attachment)]

Message #13 received at 74159 <at> debbugs.gnu.org (full text, mbox):

From: Martin Schulte <gnu <at> schrader-schulte.de>
To: Jan Černohorský <jan <at> kam.mff.cuni.cz>
Cc: wizards <at> kam.mff.cuni.cz, 74159 <at> debbugs.gnu.org
Subject: Re: bug#74159: Unexpected exit code of 0 when -q is set and
 close_stdout fails
Date: Sat, 2 Nov 2024 10:28:12 +0100

Hello Jan!

On Fri, 1 Nov 2024 14:10:43 +0100 you wrote:
> https://grsc.cz/blog/loffice-linux-issues/, but it's mostly not relevant 
> for this bug).

While I can't help you with grep/operating system issue I suggest replacing

if echo "$checks" | grep -q "cc" ; then

by

if [ ${#checks} -gt 1 ] ; then

It simply tests if the string inside the variable checks is longer than 1, uses less system resources and all shell features you need have already been part of POSIX 1003.1 Issue 6 from 2004.

Best regards,

Martin

Message #16 received at 74159-done <at> debbugs.gnu.org (full text, mbox):

From: Jan Černohorský <jan <at> kam.mff.cuni.cz>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 74159-done <at> debbugs.gnu.org, wizards <at> kam.mff.cuni.cz
Subject: Re: bug#74159: Unexpected exit code of 0 when -q is set and
 close_stdout fails
Date: Sun, 3 Nov 2024 23:22:10 +0100

Thank you for your very swift reply and fix.

>>when the `-q` option is detected (grep.c:2697), the `exit_failure` 
>>variable is set to 0. This causes any error, when not specially 
>>handled, to exit with 0, even before any match is found.
>
>Yes, that's a mistake. I can't reproduce your bug but I can see other 
>consequences of the mistake. Fixed in the attached patch (with a test 
>case that's unlike your problem); please give it a try.

The provided patch fixes the issue 👍

>That being said, the lower levels of your OS appear to be buggy, as 
>'close' should not fail with EACCES. I wouldn't be surprised if other 
>software breaks because of this issue. So I suggest also reporting a 
>bug to whoever maintains the NFS client code that you're using.

I'm aware. I'll try to look into it, but it seems like a much deeper 
rabbit hole.


Thanks again,
Jan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.