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


View this message in rfc822 format

From: Pádraig Brady <P <at> draigBrady.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>, Josef Bacik <josef <at> toxicpanda.com>, 64540 <at> debbugs.gnu.org
Subject: bug#64540: [PATCH] od: exit out on failure to write to stdout
Date: Sun, 9 Jul 2023 15:11:47 +0100
On 09/07/2023 10:29, Paul Eggert wrote:
> On 2023-07-08 15:43, Josef Bacik wrote:
>> A very weird bug was uncovered when using fstests with github actions.
>> In fstests we are doing something like this
>>
>> od /dev/urandom | dd of=somefile bs=1M count=10
>>
>> The above works fine, except in the case of github actions which runs
>> this script remotely, capturing the output in a pipe to print on their
>> console.  Somehow od ends up writing forever into the stdout pipe
>> without exiting in this scenario, normally it doesn't do this.
> 
> Do you see the same problem if you use 'cat' rather than 'od'? If so,
> the problem isn't with 'od'; it's with the environment, which is somehow
> set up to ignore SIGPIPE. It shouldn't do that, as ignoring SIGPIPE
> breaks a lot of programs, and 'od' would be just one of them.
> 
> Although the patch you sent is probably harmless, it's not clear that we
> should go through all the GNU utilities and make them check for I/O
> errors on every output, as that'd be a maintenance hassle and in some
> cases would hurt performance significantly.

Note the patch looks wrong as it would close the input always.
We can fix that up easily enough anyway.

RE the general point about handling write errors in coreutils,
many other cases are handled implicitly or explicitly,
but some are not. For example these will inf loop:

  pr /dev/urandom > /dev/full
  od /dev/urandom > /dev/full

It shouldn't be too onerous to handle these cases explicitly.
An example where we explicitly handled it for seq was:
https://github.com/coreutils/coreutils/commit/c92585b10
I'll create patches to handle the above, and any others I notice.
For my reference a short list of utils to check (that might inf loop) are:
  cat comm cp cut dd dirname expand factor fmt fold head join ls nl numfmt
  od paste pr printf ptx readlink realpath seq shuf sort stat tac tail
  tee tr tsort unexpand uniq wc yes

Note we had a similar issue recently on the read side,
where all read errors were not diagnosed, which we checked with:
https://github.com/coreutils/coreutils/commit/7ea7c020e
I'll come up with a similar test to diagnose write errors
for the shortlist of utils above.

cheers,
Pádraig




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.