On 03/16/2016 09:14 AM, Assaf Gordon wrote:
Attached is a patch that adds errno information to 'write error' messages
Thanks, I installed the attached somewhat-tweaked version of that. Among other things I renamed the new static functions to avoid confusion with existing "safer" functions.
This doesn't address the SIGPIPE mystery so I'll leave the bug report open, but my guess is that this part isn't really a grep bug.
From 1cec27a7f0e5c7bdc44a88ef51af09f87a8cbc24 Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Thu, 17 Mar 2016 11:44:46 -0700 Subject: [PATCH] grep: use errno consistently in write diagnostics Feature request and initial version reported by Assaf Gordon in: http://bugs.gnu.org/23031 * NEWS: Document this. * src/grep.c: Include <stdarg.h>. (stdout_errno): New static var. (write_error_seen): Remove; superseded by stdout_errno. All uses changed. (putchar_errno, fputs_errno, printf_errno, fwrite_errno) (fflush_errno): New static functions. (print_filename, print_sep, print_offset, print_line_head) (print_line_middle, print_line_tail, prline, prtext, grep) (grepdesc): Use them. * tests/write-error-msg: New file. * tests/Makefile.am (TESTS): Add it. --- NEWS | 6 ++++ src/grep.c | 92 ++++++++++++++++++++++++++++++++++++--------------- tests/Makefile.am | 1 + tests/write-error-msg | 55 ++++++++++++++++++++++++++++++ 4 files changed, 127 insertions(+), 27 deletions(-) create mode 100755 tests/write-error-msg diff --git a/NEWS b/NEWS index 26168fb..9de07ab 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,12 @@ GNU grep NEWS -*- outline -*- * Noteworthy changes in release 2.24 (2016-03-10) [stable] +** Improvements + + grep now outputs details more consistently when reporting a write error. + E.g., "grep: write error: No space left on device" rather than just + "grep: write error". + ** Bug fixes grep -z would match strings it should not. To trigger the bug, you'd diff --git a/src/grep.c b/src/grep.c index 73c3651..94ddbef 100644 --- a/src/grep.c +++ b/src/grep.c @@ -25,6 +25,7 @@ #include <wctype.h> #include <fcntl.h> #include <inttypes.h> +#include <stdarg.h> #include <stdio.h> #include "system.h" @@ -296,6 +297,47 @@ static const struct color_cap color_dict[] = { NULL, NULL, NULL } }; +/* Saved errno value from failed output functions on stdout. */ +static int stdout_errno; + +static void +putchar_errno (int c) +{ + if (putchar (c) < 0) + stdout_errno = errno; +} + +static void +fputs_errno (char const *s) +{ + if (fputs (s, stdout) < 0) + stdout_errno = errno; +} + +static void _GL_ATTRIBUTE_FORMAT_PRINTF (1, 2) +printf_errno (char const *format, ...) +{ + va_list ap; + va_start (ap, format); + if (vfprintf (stdout, format, ap) < 0) + stdout_errno = errno; + va_end (ap); +} + +static void +fwrite_errno (void const *ptr, size_t size, size_t nmemb) +{ + if (fwrite (ptr, size, nmemb, stdout) != nmemb) + stdout_errno = errno; +} + +static void +fflush_errno (void) +{ + if (fflush (stdout) != 0) + stdout_errno = errno; +} + static struct exclude *excluded_patterns[2]; static struct exclude *excluded_directory_patterns[2]; /* Short options. */ @@ -386,7 +428,6 @@ static char const *filename; /* Omit leading "./" from file names in diagnostics. */ static bool omit_dot_slash; static bool errseen; -static bool write_error_seen; /* True if output from the current input file has been suppressed because an output line had an encoding error. */ @@ -480,7 +521,7 @@ suppressible_error (char const *mesg, int errnum) static void clean_up_stdout (void) { - if (! write_error_seen) + if (! stdout_errno) close_stdout (); } @@ -940,7 +981,7 @@ static void print_filename (void) { pr_sgr_start_if (filename_color); - fputs (filename, stdout); + fputs_errno (filename); pr_sgr_end_if (filename_color); } @@ -949,7 +990,7 @@ static void print_sep (char sep) { pr_sgr_start_if (sep_color); - fputc (sep, stdout); + putchar_errno (sep); pr_sgr_end_if (sep_color); } @@ -976,7 +1017,7 @@ print_offset (uintmax_t pos, int min_width, const char *color) *--p = ' '; pr_sgr_start_if (color); - fwrite (p, 1, buf + sizeof buf - p, stdout); + fwrite_errno (p, 1, buf + sizeof buf - p); pr_sgr_end_if (color); } @@ -1013,7 +1054,7 @@ print_line_head (char *beg, size_t len, char const *lim, char sep) if (filename_mask) pending_sep = true; else - fputc (0, stdout); + putchar_errno (0); } if (out_line) @@ -1047,7 +1088,7 @@ print_line_head (char *beg, size_t len, char const *lim, char sep) (and its combining and wide characters) filenames and you're wasting your efforts. */ if (align_tabs) - fputs ("\t\b", stdout); + fputs_errno ("\t\b"); print_sep (sep); } @@ -1104,14 +1145,14 @@ print_line_middle (char *beg, char *lim, cur = mid; mid = NULL; } - fwrite (cur, sizeof (char), b - cur, stdout); + fwrite_errno (cur, 1, b - cur); } pr_sgr_start_if (match_color); - fwrite (b, sizeof (char), match_size, stdout); + fwrite_errno (b, 1, match_size); pr_sgr_end_if (match_color); if (only_matching) - fputs ("\n", stdout); + putchar_errno ('\n'); } } @@ -1136,7 +1177,7 @@ print_line_tail (char *beg, const char *lim, const char *line_color) if (tail_size > 0) { pr_sgr_start (line_color); - fwrite (beg, 1, tail_size, stdout); + fwrite_errno (beg, 1, tail_size); beg += tail_size; pr_sgr_end (line_color); } @@ -1188,16 +1229,13 @@ prline (char *beg, char *lim, char sep) } if (!only_matching && lim > beg) - fwrite (beg, 1, lim - beg, stdout); + fwrite_errno (beg, 1, lim - beg); if (line_buffered) - fflush (stdout); + fflush_errno (); - if (ferror (stdout)) - { - write_error_seen = true; - error (EXIT_TROUBLE, 0, _("write error")); - } + if (stdout_errno) + error (EXIT_TROUBLE, stdout_errno, _("write error")); lastout = lim; } @@ -1253,9 +1291,9 @@ prtext (char *beg, char *lim) && p != lastout && group_separator) { pr_sgr_start_if (sep_color); - fputs (group_separator, stdout); + fputs_errno (group_separator); pr_sgr_end_if (sep_color); - fputc ('\n', stdout); + putchar_errno ('\n'); } while (p < beg) @@ -1498,9 +1536,9 @@ grep (int fd, struct stat const *st) if (!out_quiet && (encoding_error_output || (0 <= nlines_first_null && nlines_first_null < nlines))) { - printf (_("Binary file %s matches\n"), filename); + printf_errno (_("Binary file %s matches\n"), filename); if (line_buffered) - fflush (stdout); + fflush_errno (); } return nlines; } @@ -1742,20 +1780,20 @@ grepdesc (int desc, bool command_line) if (filename_mask) print_sep (SEP_CHAR_SELECTED); else - fputc (0, stdout); + putchar_errno (0); } - printf ("%" PRIdMAX "\n", count); + printf_errno ("%" PRIdMAX "\n", count); if (line_buffered) - fflush (stdout); + fflush_errno (); } status = !count; if (list_files == 1 - 2 * status) { print_filename (); - fputc ('\n' & filename_mask, stdout); + putchar_errno ('\n' & filename_mask); if (line_buffered) - fflush (stdout); + fflush_errno (); } if (desc == STDIN_FILENO) diff --git a/tests/Makefile.am b/tests/Makefile.am index 5a2c0f0..0326ff4 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -141,6 +141,7 @@ TESTS = \ word-delim-multibyte \ word-multi-file \ word-multibyte \ + write-error-msg \ yesno \ z-anchor-newline diff --git a/tests/write-error-msg b/tests/write-error-msg new file mode 100755 index 0000000..130648b --- /dev/null +++ b/tests/write-error-msg @@ -0,0 +1,55 @@ +#!/bin/sh +# Ensure that output errors are reported with errno information. + +# Copyright 2016 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=.}/init.sh"; path_prepend_ ../src +test -e /dev/full || skip_ your system lacks /dev/full + +export LC_ALL=C + +# generate large input, filling the libc stdio buffers +# and triggering a write(2) even without line buffering. +yes 12345 | head -n 50000 > in || framework_failure_ + +fail=0 + +# disk-full error, line buffered +# (note: GNU grep returns 2 on error) +returns_ 2 grep --line-buffered -v '^$' <in >/dev/full 2>err1 \ + || framework_failure_ + +# disk-full error, unbuffered +# (note: GNU grep returns 2 on error) +returns_ 2 grep -v '^$' <in >/dev/full 2>err2 \ + || framework_failure_ + +# ensure each error message file contains a 'write error' with additional text +for f in err1 err2 ; +do + grep -Eiq '^grep: write error: [a-z]+' $f \ + || { + warn_ "incorrect/missing error message in file $f" + compare /dev/null $f # print the content in the logs + fail=1 + } +done + +# These messages should be identical +compare err1 err2 \ + || { warn_ "err1,err2 contain different error messages" ; fail=1 ; } + +Exit $fail -- 2.5.0
