GNU bug report logs - #64540
[PATCH] od: exit out on failure to write to stdout

Previous Next

Package: coreutils;

Reported by: Josef Bacik <josef <at> toxicpanda.com>

Date: Sun, 9 Jul 2023 09:09:02 UTC

Severity: normal

Tags: patch

Done: Pádraig Brady <P <at> draigBrady.com>

Bug is archived. No further changes may be made.

Full log


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

From: Bernhard Voelker <mail <at> bernhard-voelker.de>
To: Pádraig Brady <P <at> draigBrady.com>,
 Paul Eggert <eggert <at> cs.ucla.edu>, Josef Bacik <josef <at> toxicpanda.com>,
 64540 <at> debbugs.gnu.org
Subject: Re: bug#64540: [PATCH] od: exit out on failure to write to stdout
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





This bug report was last modified 1 year and 304 days ago.

Previous Next


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