Subject: Re: bug#64540: [PATCH] od: exit out on failure to write to stdout

From: Bernhard Voelker <[email-masked]>
Date: Mon, 17 Jul 2023 08:35:49 +0200

On 7/15/23 23:08, Pádraig Brady wrote:
> The attached patch-set addresses two classes of issue:
> 1. Doubled error messages upon write errors
> 2. Continued processing upon write errors  (the orig problem reported).

Nice work!

Patch 1:

 > --- /dev/null
 > +++ b/tests/misc/write-errors.sh
 > @@ -0,0 +1,70 @@

 > +if ! test -w /dev/full || ! test -c /dev/full; then
 > +  skip_ 'Need /dev/full'

Other tests use a slightly different skip message:
   skip_ '/dev/full is required'

I'd favor the latter, so that we don't miss it once we'd like
to factor it out to a require_dev_full in init.cfg.

Re. patch 002:

 >     all: avoid repeated diagnostic upon write error
 >
 >     * cfg.mk (sc_some_programs_must_avoid_exit_failure): Adjust to
 >     avoid false positive.
 >     (sc_prohibit_exit_write_error): A new syntax check to prohibit
 >     open coding error(..., "write error"); instead directiing to use...

s/ii/i/

 > --- a/src/system.h
 > +++ b/src/system.h
 > @@ -762,6 +762,14 @@ The following directory is part of the cycle:\n  %s\n"), \
 >      }					\
 >    while (0)
 >
 > +static inline void
 > +write_error (void)
 > +{
 > +  fflush (stdout);    /* Ensure nothing buffered that might induce an error. */
 > +  clearerr (stdout);  /* To avoid extraneous diagnostic from close_stdout.  */
 > +  error (EXIT_FAILURE, errno, _("write error"));
 > +}

Hmm, fflush could theoretically change errno, couldn't it?
In that case, error() would give a wrong diagnostic.
FWIW: `man clearerr` states that this function doesn't touch errno.

Thanks & have a nice day,
Berny