[PATCH] head,tail: consistently diagnose write errors

[Pádraig Brady]

If we can't output more data, we should immediately
diagnose the issue and exit rather than consuming all
of input (in some cases).

* src/tail.c (xwrite_stdout): Also diagnose the case where
only some data is written.
* src/head.c (xwrite_stdout): Copy this new function from tail,
and use it to write all output.
* tests/misc/head-write-error.sh: A new test to ensure we
exit immediately on write error.
* tests/local.mk: Reference the new test.
---
 NEWS                           |    4 ++
 src/head.c                     |   83 ++++++++++++++++-----------------------
 src/tail.c                     |    2 +-
 tests/local.mk                 |    1 +
 tests/misc/head-write-error.sh |   47 ++++++++++++++++++++++
 5 files changed, 87 insertions(+), 50 deletions(-)
 create mode 100755 tests/misc/head-write-error.sh

diff --git a/NEWS b/NEWS
index f86e589..24f41fb 100644
--- a/NEWS
+++ b/NEWS
@@ -22,6 +22,10 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   it would display an error, requiring --no-dereference to avoid the issue.
   [bug introduced in coreutils-5.3.0]
 
+  tail now diagnoses all failures when writing to stdout.  Previously write
+  errors could have been silently ignored if some data was output.
+  [This bug was present in "the beginning".]
+
 ** Improvements
 
   stat and tail work better with HFS+ and HFSX.  stat -f --format=%T now 
reports
diff --git a/src/head.c b/src/head.c
index ef368d7..98e7bcf 100644
--- a/src/head.c
+++ b/src/head.c
@@ -70,7 +70,6 @@ enum Copy_fd_status
   {
     COPY_FD_OK = 0,
     COPY_FD_READ_ERROR,
-    COPY_FD_WRITE_ERROR,
     COPY_FD_UNEXPECTED_EOF
   };
 
@@ -147,9 +146,6 @@ diagnose_copy_fd_failure (enum Copy_fd_status err, char 
const *filename)
     case COPY_FD_READ_ERROR:
       error (0, errno, _("error reading %s"), quote (filename));
       break;
-    case COPY_FD_WRITE_ERROR:
-      error (0, errno, _("error writing %s"), quote (filename));
-      break;
     case COPY_FD_UNEXPECTED_EOF:
       error (0, errno, _("%s: file has shrunk too much"), quote (filename));
       break;
@@ -167,11 +163,22 @@ write_header (const char *filename)
   first_file = false;
 }
 
-/* Copy no more than N_BYTES from file descriptor SRC_FD to O_STREAM.
-   Return an appropriate indication of success or failure. */
+/* Write N_BYTES from BUFFER to stdout.
+   Exit immediately on error.  */
+
+static void
+xwrite_stdout (char const *buffer, size_t n_bytes)
+{
+  if (n_bytes > 0 && fwrite (buffer, 1, n_bytes, stdout) < n_bytes)
+    error (EXIT_FAILURE, errno, _("write error"));
+}
+
+
+/* Copy no more than N_BYTES from file descriptor SRC_FD to stdout.
+   Return an appropriate indication of success or read failure.  */
 
 static enum Copy_fd_status
-copy_fd (int src_fd, FILE *o_stream, uintmax_t n_bytes)
+copy_fd (int src_fd, uintmax_t n_bytes)
 {
   char buf[BUFSIZ];
   const size_t buf_size = sizeof (buf);
@@ -189,8 +196,7 @@ copy_fd (int src_fd, FILE *o_stream, uintmax_t n_bytes)
       if (n_read == 0 && n_bytes != 0)
         return COPY_FD_UNEXPECTED_EOF;
 
-      if (fwrite (buf, 1, n_read, o_stream) < n_read)
-        return COPY_FD_WRITE_ERROR;
+      xwrite_stdout (buf, n_read);
     }
 
   return COPY_FD_OK;
@@ -282,22 +288,12 @@ elide_tail_bytes_pipe (const char *filename, int fd, 
uintmax_t n_elide_0)
 
           /* Output any (but maybe just part of the) elided data from
              the previous round.  */
-          if ( ! first)
-            {
-              /* Don't bother checking for errors here.
-                 If there's a failure, the test of the following
-                 fwrite or in close_stdout will catch it.  */
-              fwrite (b[!i] + READ_BUFSIZE, 1, n_elide - delta, stdout);
-            }
+          if (! first)
+            xwrite_stdout (b[!i] + READ_BUFSIZE, n_elide - delta);
           first = false;
 
-          if (n_elide < n_read
-              && fwrite (b[i], 1, n_read - n_elide, stdout) < n_read - n_elide)
-            {
-              error (0, errno, _("write error"));
-              ok = false;
-              break;
-            }
+          if (n_elide < n_read)
+            xwrite_stdout (b[i], n_read - n_elide);
         }
 
       free (b[0]);
@@ -357,14 +353,7 @@ elide_tail_bytes_pipe (const char *filename, int fd, 
uintmax_t n_elide_0)
             buffered_enough = true;
 
           if (buffered_enough)
-            {
-              if (fwrite (b[i_next], 1, n_read, stdout) < n_read)
-                {
-                  error (0, errno, _("write error"));
-                  ok = false;
-                  goto free_mem;
-                }
-            }
+            xwrite_stdout (b[i_next], n_read);
         }
 
       /* Output any remainder: rem bytes from b[i] + n_read.  */
@@ -375,12 +364,12 @@ elide_tail_bytes_pipe (const char *filename, int fd, 
uintmax_t n_elide_0)
               size_t n_bytes_left_in_b_i = READ_BUFSIZE - n_read;
               if (rem < n_bytes_left_in_b_i)
                 {
-                  fwrite (b[i] + n_read, 1, rem, stdout);
+                  xwrite_stdout (b[i] + n_read, rem);
                 }
               else
                 {
-                  fwrite (b[i] + n_read, 1, n_bytes_left_in_b_i, stdout);
-                  fwrite (b[i_next], 1, rem - n_bytes_left_in_b_i, stdout);
+                  xwrite_stdout (b[i] + n_read, n_bytes_left_in_b_i);
+                  xwrite_stdout (b[i_next], rem - n_bytes_left_in_b_i);
                 }
             }
           else if (i + 1 == n_bufs)
@@ -399,7 +388,7 @@ elide_tail_bytes_pipe (const char *filename, int fd, 
uintmax_t n_elide_0)
                */
               size_t y = READ_BUFSIZE - rem;
               size_t x = n_read - y;
-              fwrite (b[i_next], 1, x, stdout);
+              xwrite_stdout (b[i_next], x);
             }
         }
 
@@ -458,7 +447,7 @@ elide_tail_bytes_file (const char *filename, int fd, 
uintmax_t n_elide)
           return false;
         }
 
-      err = copy_fd (fd, stdout, bytes_remaining - n_elide);
+      err = copy_fd (fd, bytes_remaining - n_elide);
       if (err == COPY_FD_OK)
         return true;
 
@@ -504,7 +493,7 @@ elide_tail_lines_pipe (const char *filename, int fd, 
uintmax_t n_elide)
 
       if (! n_elide)
         {
-          fwrite (tmp->buffer, 1, n_read, stdout);
+          xwrite_stdout (tmp->buffer, n_read);
           continue;
         }
 
@@ -543,7 +532,7 @@ elide_tail_lines_pipe (const char *filename, int fd, 
uintmax_t n_elide)
           last = last->next = tmp;
           if (n_elide < total_lines - first->nlines)
             {
-              fwrite (first->buffer, 1, first->nbytes, stdout);
+              xwrite_stdout (first->buffer, first->nbytes);
               tmp = first;
               total_lines -= first->nlines;
               first = first->next;
@@ -572,7 +561,7 @@ elide_tail_lines_pipe (const char *filename, int fd, 
uintmax_t n_elide)
 
   for (tmp = first; n_elide < total_lines - tmp->nlines; tmp = tmp->next)
     {
-      fwrite (tmp->buffer, 1, tmp->nbytes, stdout);
+      xwrite_stdout (tmp->buffer, tmp->nbytes);
       total_lines -= tmp->nlines;
     }
 
@@ -588,7 +577,7 @@ elide_tail_lines_pipe (const char *filename, int fd, 
uintmax_t n_elide)
           ++tmp->nlines;
           --n;
         }
-      fwrite (tmp->buffer, 1, p - tmp->buffer, stdout);
+      xwrite_stdout (tmp->buffer, p - tmp->buffer);
     }
 
 free_lbuffers:
@@ -684,7 +673,7 @@ elide_tail_lines_seekable (const char *pretty_filename, int 
fd,
                       return false;
                     }
 
-                  err = copy_fd (fd, stdout, pos - start_pos);
+                  err = copy_fd (fd, pos - start_pos);
                   if (err != COPY_FD_OK)
                     {
                       diagnose_copy_fd_failure (err, pretty_filename);
@@ -693,10 +682,8 @@ elide_tail_lines_seekable (const char *pretty_filename, 
int fd,
                 }
 
               /* Output the initial portion of the buffer
-                 in which we found the desired newline byte.
-                 Don't bother testing for failure for such a small amount.
-                 Any failure will be detected upon close.  */
-              fwrite (buffer, 1, n + 1, stdout);
+                 in which we found the desired newline byte.  */
+              xwrite_stdout (buffer, n + 1);
 
               /* Set file pointer to the byte after what we've output.  */
               if (lseek (fd, pos + n + 1, SEEK_SET) < 0)
@@ -789,8 +776,7 @@ head_bytes (const char *filename, int fd, uintmax_t 
bytes_to_write)
         }
       if (bytes_read == 0)
         break;
-      if (fwrite (buffer, 1, bytes_read, stdout) < bytes_read)
-        error (EXIT_FAILURE, errno, _("write error"));
+      xwrite_stdout (buffer, bytes_read);
       bytes_to_write -= bytes_read;
     }
   return true;
@@ -830,8 +816,7 @@ head_lines (const char *filename, int fd, uintmax_t 
lines_to_write)
               }
             break;
           }
-      if (fwrite (buffer, 1, bytes_to_write, stdout) < bytes_to_write)
-        error (EXIT_FAILURE, errno, _("write error"));
+      xwrite_stdout (buffer, bytes_to_write);
     }
   return true;
 }
diff --git a/src/tail.c b/src/tail.c
index a5268c2..9a26443 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -342,7 +342,7 @@ pretty_name (struct File_spec const *f)
 static void
 xwrite_stdout (char const *buffer, size_t n_bytes)
 {
-  if (n_bytes > 0 && fwrite (buffer, 1, n_bytes, stdout) == 0)
+  if (n_bytes > 0 && fwrite (buffer, 1, n_bytes, stdout) < n_bytes)
     error (EXIT_FAILURE, errno, _("write error"));
 }
 
diff --git a/tests/local.mk b/tests/local.mk
index 9d556f6..4011abd 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -276,6 +276,7 @@ all_tests =                                 \
   tests/misc/groups-version.sh                 \
   tests/misc/head-c.sh                         \
   tests/misc/head-pos.sh                       \
+  tests/misc/head-write-error.sh               \
   tests/misc/md5sum.pl                         \
   tests/misc/md5sum-bsd.sh                     \
   tests/misc/md5sum-newline.pl                 \
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.
+
+# Copyright (C) 2014 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
+print_ver_ head
+
+if ! test -w /dev/full || ! test -c /dev/full; then
+  skip_ '/dev/full is required'
+fi
+
+# We can't use /dev/zero as that's bypassed in the --lines case
+# due to lseek() indicating it has a size of zero.
+yes | head -c10M > bigseek || framework_failure_
+
+# 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
+
+Exit $fail
-- 
1.7.7.6