>From c4baa1e21a88e44af8c0592fdb1beccb5c338a7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= Date: Wed, 29 Jan 2014 04:42:56 +0000 Subject: [PATCH] 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. --- src/head.c | 85 +++++++++++++++++----------------------- src/tail.c | 20 ++++++--- tests/local.mk | 1 + tests/misc/head-write-error.sh | 47 ++++++++++++++++++++++ 4 files changed, 97 insertions(+), 56 deletions(-) create mode 100755 tests/misc/head-write-error.sh diff --git a/src/head.c b/src/head.c index ef368d7..3145fa7 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,24 @@ 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, _("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 +198,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 +290,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 +355,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 +366,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 +390,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 +449,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 +495,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 +534,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 +563,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 +579,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 +675,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 +684,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 +778,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 +818,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..c421d81 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,19 @@ 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, _("write error")); + } +} + /* 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 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 . + +. "${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