On 01/30/2014 01:23 AM, Pádraig Brady wrote: >> Unfortunately, the atexit code now produces a second error diagnostic. >> It doesn't hurt, but it looks a bit ugly. > > We discussed that foible previously, and thought it not worth > adding the extra complexity to avoid in general. > > http://lists.gnu.org/archive/html/coreutils/2011-05/msg00061.html > > Though in this specific case we're using wrapper functions > instead of fwrite() so we can easily add the clearerr() there. > > Updated patch attached.
Thanks ... also for the link to the other thread. The changes in the sources look rather good to me - a minor nit follows below (regarding the diagnostic message). I'm not sure about the test: > diff --git a/tests/misc/head-write-error.sh b/tests/misc/head-write-error.sh > new file mode 100755 > index 0000000..b749760 > --- /dev/null > +++ b/tests/misc/head-write-error.sh > @@ -0,0 +1,47 @@ > +#!/bin/sh > +# Ensure we diagnose and not continue writing to > +# the output if we get a write error. > + If I read it correctly, the test claims to verify that head exits early on write errors ... > +# Memory is bounded in these cases > +for item in lines bytes; do > + for N in 0 1; do > + # pipe case > + yes | timeout 10s head --$item=-$N > /dev/full 2> err && fail=1 > + test $? = 124 && fail=1 > + test -s err || fail=1 > + rm err > + > + # seekable case > + timeout 10s head --$item=-$N bigseek > /dev/full 2> err && fail=1 > + test $? = 124 && fail=1 > + test -s err || fail=1 > + done > +done ... while the above just seems to verify that head exits non-Zero on a write error - well, head already did before. The difference with the patch is that for the pipe case head now prints the detailed diagnostic - /usr/bin/head: write error + src/head: write error: No space left on device and in the seekable case only outputs 1 line instead of 2: - /usr/bin/head: error writing ‘standard input’: No space left on device - /usr/bin/head: write error + src/head: write error: No space left on device (Interestingly, in the latter case there was an error in the old message: we don't write standard input.) So to say, the test doesn't do exactly what it says. It would kind of do if it would verify that the output only contains the message from xwrite_stdout and not that from the atexit code, something like: echo 'head: write error: No space left on device' > exp compare_ exp err || fail=1 Finally, looking at the changes in the error messages above: I'm missing the file name in the new error message, i.e. stdout. The user might be confused and wonder *where* writing failed. I'd add this to the message again. Have a nice day, Berny BTW: /usr/bin/head above is v8.21.
