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

From: Pádraig Brady <[email-masked]>
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