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
