diff options
author | Pádraig Brady <P@draigBrady.com> | 2014-01-29 04:42:56 +0000 |
---|---|---|
committer | Pádraig Brady <P@draigBrady.com> | 2014-02-09 21:37:24 +0000 |
commit | 6e824a66194528696ba265d6111a6bddce4a8ff8 (patch) | |
tree | 13ae36f27040f4db4b220ab753694e00f45aa7d9 | |
parent | 476ce37019df5b1cb917c5b50e71f9bce5911401 (diff) | |
download | coreutils-6e824a66194528696ba265d6111a6bddce4a8ff8.tar.xz |
head,tail: consistently diagnose write errors
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. Also clearerr() to avoid the
redundant less specific error from atexit (close_stdout);
* 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.
-rw-r--r-- | src/head.c | 86 | ||||
-rw-r--r-- | src/tail.c | 21 | ||||
-rw-r--r-- | tests/local.mk | 1 | ||||
-rwxr-xr-x | tests/misc/head-write-error.sh | 52 |
4 files changed, 104 insertions, 56 deletions
diff --git a/src/head.c b/src/head.c index ef368d7cc..b833af675 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,25 @@ 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 with a single diagnostic. */ + +static void +xwrite_stdout (char const *buffer, size_t n_bytes) +{ + if (n_bytes > 0 && fwrite (buffer, 1, n_bytes, stdout) < n_bytes) + { + clearerr (stdout); /* To avoid redundant close_stdout diagnostic. */ + error (EXIT_FAILURE, errno, _("error writing %s"), + quote ("standard output")); + } +} + +/* 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 +199,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 +291,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 +356,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 +367,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 +391,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 +450,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 +496,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 +535,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 +564,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 +580,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 +676,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 +685,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 +779,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 +819,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 a5268c2c6..5ff738df8 100644 --- a/src/tail.c +++ b/src/tail.c @@ -339,13 +339,6 @@ pretty_name (struct File_spec const *f) return (STREQ (f->name, "-") ? _("standard input") : f->name); } -static void -xwrite_stdout (char const *buffer, size_t n_bytes) -{ - if (n_bytes > 0 && fwrite (buffer, 1, n_bytes, stdout) == 0) - error (EXIT_FAILURE, errno, _("write error")); -} - /* Record a file F with descriptor FD, size SIZE, status ST, and blocking status BLOCKING. */ @@ -385,6 +378,20 @@ write_header (const char *pretty_filename) first_file = false; } +/* Write N_BYTES from BUFFER to stdout. + Exit immediately on error with a single diagnostic. */ + +static void +xwrite_stdout (char const *buffer, size_t n_bytes) +{ + if (n_bytes > 0 && fwrite (buffer, 1, n_bytes, stdout) < n_bytes) + { + clearerr (stdout); /* To avoid redundant close_stdout diagnostic. */ + error (EXIT_FAILURE, errno, _("error writing %s"), + quote ("standard output")); + } +} + /* Read and output N_BYTES of file PRETTY_FILENAME starting at the current position in FD. If N_BYTES is COPY_TO_EOF, then copy until end of file. If N_BYTES is COPY_A_BUFFER, then copy at most one buffer's worth. diff --git a/tests/local.mk b/tests/local.mk index 815dc6fdd..26aef504f 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -277,6 +277,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 000000000..22ecf993e --- /dev/null +++ b/tests/misc/head-write-error.sh @@ -0,0 +1,52 @@ +#!/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_ + +# This is the single output diagnostic expected, +# (without the possibly varying :strerror(ENOSPC) suffix). +printf '%s\n' "head: error writing 'standard output'" > exp + +# 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> errt && fail=1 + test $? = 124 && fail=1 + sed 's/\(head:.*\):.*/\1/' errt > err + compare exp err || fail=1 + + # seekable case + timeout 10s head --$item=-$N bigseek > /dev/full 2> errt && fail=1 + test $? = 124 && fail=1 + sed 's/\(head:.*\):.*/\1/' errt > err + compare exp err || fail=1 + done +done + +Exit $fail |