Eric Blake wrote: > Jim Meyering <jim <at> meyering.net> writes: > >> Regardless of POSIX, what if an application requires that output and >> attempts to parse it? In that case, a successful exit status is defintely >> at odds with empty stderr (which could arise also due to ENOSPC). >> >> So I'd say it's a bug. > > Here's a patch, including a testsuite addition (along with a cleanup to allow > clean compilation on cygwin). OK to apply?
Thanks! The second is fine. Two nits in the first: >>From 8cea0f3d5628ea3e4f74db0a66486cae5d92122a Mon Sep 17 00:00:00 2001 > From: Eric Blake <[email protected]> > Date: Fri, 28 Aug 2009 08:40:06 -0600 > Subject: [PATCH 1/2] dd: detect closed stderr > > * src/dd.c (maybe_close_stdout): Always flush stderr. > * tests/dd/stderr: New test, borrowing from misc/close-stdout. > * tests/Makefile.am (TESTS): Run it. > * NEWS: Mention this. > --- > NEWS | 3 +++ > src/dd.c | 3 +++ > tests/Makefile.am | 1 + > tests/dd/stderr | 47 +++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 54 insertions(+), 0 deletions(-) > create mode 100755 tests/dd/stderr > > diff --git a/NEWS b/NEWS > index c125b31..581620f 100644 > --- a/NEWS > +++ b/NEWS > @@ -8,6 +8,9 @@ GNU coreutils NEWS -*- > outline -*- > due to their running on a kernel older than what was implied by headers > and libraries tested at configure time. > > + dd now returns non-zero status if it encountered a write error while > + printing a summary to stderr. > + > > * Noteworthy changes in release 7.5 (2009-08-20) [stable] > > diff --git a/src/dd.c b/src/dd.c > index dc15cfd..c016d3d 100644 > --- a/src/dd.c > +++ b/src/dd.c > @@ -25,6 +25,7 @@ > #include <getopt.h> > > #include "system.h" > +#include "close-stream.h" > #include "error.h" > #include "fd-reopen.h" > #include "gethrxtime.h" > @@ -450,6 +451,8 @@ maybe_close_stdout (void) > { > if (close_stdout_required) > close_stdout (); > + else if (close_stream (stderr) != 0) > + _exit (exit_failure); Why use exit_failure here? (it works because exitfail.c initializes that global to the value EXIT_FAILURE) Everywhere else in dd.c, we use EXIT_FAILURE. While exit_failure is technically correct, EXIT_FAILURE is more understandable. Certainly more consistent. ... > +. $srcdir/test-lib.sh > + > +p="$abs_top_builddir" For consistency with most of the shell code in coreutils, please elide double quotes in simple shell variable assignments: p=$abs_top_builddir Hmm... I see that close-stdout did it the same way. You're welcome to adjust that one, too.
