From: Pádraig Brady <P@draigBrady.com>
Date: Sat, 15 Jul 2023 19:41:44 +0000 (+0100)
Subject: all: avoid repeated diagnostic upon write error
X-Git-Tag: v9.4~77
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0b2ff7637f1eabba48e461f9f6e3aec956bed7d1;p=thirdparty%2Fcoreutils.git

all: avoid repeated diagnostic upon write error

* cfg.mk (sc_some_programs_must_avoid_exit_failure): Adjust to
avoid false positive.
(sc_prohibit_exit_write_error): A new syntax check to prohibit
open coding error(..., "write error"); instead directing to use...
* src/system.h (write_error): ... a new function to clear stdout errors
before we explicitly diagnose a write error and exit.
* src/basenc.c: Use write_error() to ensure no repeated diagnostics.
* src/cat.c: Likewise.
* src/expand.c: Likewise.
* src/factor.c: Likewise.
* src/paste.c: Likewise.
* src/seq.c: Likewise.
* src/shuf.c: Likewise.
* src/split.c: Likewise.
* src/tail.c: Likewise.
* src/tr.c: Likewise.
* src/unexpand.c: Likewise.
* tests/misc/write-errors.sh: Remove TODOs for the fixed utilities:
expand, factor, paste, shuf, tr, unexpand.
---

diff --git a/cfg.mk b/cfg.mk
index e7b0519c40..2d6856e153 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -503,6 +503,12 @@ sc_prohibit_man_see_also_period:
 	  { echo '$(ME): do not end "SEE ALSO" section with a period'	\
 	      1>&2; exit 1; } || :
 
+sc_prohibit_exit_write_error:
+	@prohibit='error.*EXIT_FAILURE.*write error' \
+	in_vc_files='\.c$$' \
+	halt='Use write_error() instead' \
+	  $(_sc_search_regexp)
+
 # Don't use "indent-tabs-mode: nil" anymore.  No longer needed.
 sc_prohibit_emacs__indent_tabs_mode__setting:
 	@prohibit='^( *[*#] *)?indent-tabs-mode:'			\
@@ -620,7 +626,8 @@ sc_prohibit_test_empty:
 sc_some_programs_must_avoid_exit_failure:
 	@cd $(srcdir)							\
 	&& grep -nw EXIT_FAILURE					\
-	    $$(git grep -El '[^T]_FAILURE|EXIT_CANCELED' $(srcdir)/src)	\
+	    $$(git grep -El '[^T]_FAILURE|EXIT_CANCELED' src/)		\
+	  | grep -v '^src/system\.h:'					\
 	  | grep -vE '= EXIT_FAILURE|return .* \?' | grep .		\
 	    && { echo '$(ME): do not use EXIT_FAILURE in the above'	\
 		  1>&2; exit 1; } || :
diff --git a/src/basenc.c b/src/basenc.c
index 9152ad0deb..be7a616413 100644
--- a/src/basenc.c
+++ b/src/basenc.c
@@ -924,7 +924,7 @@ wrap_write (char const *buffer, idx_t len,
     {
       /* Simple write. */
       if (fwrite (buffer, 1, len, stdout) < len)
-        error (EXIT_FAILURE, errno, _("write error"));
+        write_error ();
     }
   else
     for (idx_t written = 0; written < len; )
@@ -934,13 +934,13 @@ wrap_write (char const *buffer, idx_t len,
         if (to_write == 0)
           {
             if (fputc ('\n', out) == EOF)
-              error (EXIT_FAILURE, errno, _("write error"));
+              write_error ();
             *current_column = 0;
           }
         else
           {
             if (fwrite (buffer + written, 1, to_write, stdout) < to_write)
-              error (EXIT_FAILURE, errno, _("write error"));
+              write_error ();
             *current_column += to_write;
             written += to_write;
           }
@@ -997,7 +997,7 @@ do_encode (FILE *in, char const *infile, FILE *out, idx_t wrap_column)
 
   /* When wrapping, terminate last line. */
   if (wrap_column && current_column > 0 && fputc ('\n', out) == EOF)
-    error (EXIT_FAILURE, errno, _("write error"));
+    write_error ();
 
   if (ferror (in))
     error (EXIT_FAILURE, errno, _("read error"));
@@ -1060,7 +1060,7 @@ do_decode (FILE *in, char const *infile, FILE *out, bool ignore_garbage)
           ok = base_decode_ctx (&ctx, inbuf, (k == 0 ? sum : 0), outbuf, &n);
 
           if (fwrite (outbuf, 1, n, out) < n)
-            error (EXIT_FAILURE, errno, _("write error"));
+            write_error ();
 
           if (!ok)
             error (EXIT_FAILURE, 0, _("invalid input"));
diff --git a/src/cat.c b/src/cat.c
index 880fcf2e7f..b109998bfa 100644
--- a/src/cat.c
+++ b/src/cat.c
@@ -178,7 +178,7 @@ simple_cat (char *buf, idx_t bufsize)
       /* Write this block out.  */
 
       if (full_write (STDOUT_FILENO, buf, n_read) != n_read)
-        error (EXIT_FAILURE, errno, _("write error"));
+        write_error ();
     }
 }
 
@@ -193,7 +193,7 @@ write_pending (char *outbuf, char **bpout)
   if (0 < n_write)
     {
       if (full_write (STDOUT_FILENO, outbuf, n_write) != n_write)
-        error (EXIT_FAILURE, errno, _("write error"));
+        write_error ();
       *bpout = outbuf;
     }
 }
@@ -257,7 +257,7 @@ cat (char *inbuf, idx_t insize, char *outbuf, idx_t outsize,
               do
                 {
                   if (full_write (STDOUT_FILENO, wp, outsize) != outsize)
-                    error (EXIT_FAILURE, errno, _("write error"));
+                    write_error ();
                   wp += outsize;
                   remaining_bytes = bpout - wp;
                 }
@@ -794,7 +794,7 @@ main (int argc, char **argv)
   if (pending_cr)
     {
       if (full_write (STDOUT_FILENO, "\r", 1) != 1)
-        error (EXIT_FAILURE, errno, _("write error"));
+        write_error ();
     }
 
   if (have_read_stdin && close (STDIN_FILENO) < 0)
diff --git a/src/expand.c b/src/expand.c
index adad9d9d82..0e74d0cf6e 100644
--- a/src/expand.c
+++ b/src/expand.c
@@ -144,7 +144,7 @@ expand (void)
 
                   while (++column < next_tab_column)
                     if (putchar (' ') < 0)
-                      error (EXIT_FAILURE, errno, _("write error"));
+                      write_error ();
 
                   c = ' ';
                 }
@@ -169,7 +169,7 @@ expand (void)
             return;
 
           if (putchar (c) < 0)
-            error (EXIT_FAILURE, errno, _("write error"));
+            write_error ();
         }
       while (c != '\n');
     }
diff --git a/src/factor.c b/src/factor.c
index 8ea00a13ea..e0a98152cc 100644
--- a/src/factor.c
+++ b/src/factor.c
@@ -2361,7 +2361,7 @@ lbuf_flush (void)
 {
   size_t size = lbuf.end - lbuf.buf;
   if (full_write (STDOUT_FILENO, lbuf.buf, size) != size)
-    error (EXIT_FAILURE, errno, "%s", _("write error"));
+    write_error ();
   lbuf.end = lbuf.buf;
 }
 
diff --git a/src/paste.c b/src/paste.c
index 73ddd1de63..68f8a36efe 100644
--- a/src/paste.c
+++ b/src/paste.c
@@ -152,14 +152,6 @@ collapse_escapes (char const *strptr)
   return backslash_at_end ? 1 : 0;
 }
 
-/* Report a write error and exit.  */
-
-static void
-write_error (void)
-{
-  error (EXIT_FAILURE, errno, _("write error"));
-}
-
 /* Output a single byte, reporting any write errors.  */
 
 static inline void
diff --git a/src/seq.c b/src/seq.c
index 633189c405..977702bb59 100644
--- a/src/seq.c
+++ b/src/seq.c
@@ -285,14 +285,6 @@ long_double_format (char const *fmt, struct layout *layout)
       }
 }
 
-static void
-io_error (void)
-{
-  /* FIXME: consider option to silently ignore errno=EPIPE */
-  clearerr (stdout);
-  error (EXIT_FAILURE, errno, _("write error"));
-}
-
 /* Actually print the sequence of numbers in the specified range, with the
    given or default stepping and format.  */
 
@@ -311,7 +303,7 @@ print_numbers (char const *fmt, struct layout layout,
         {
           long double x0 = x;
           if (printf (fmt, x) < 0)
-            io_error ();
+            write_error ();
           if (out_of_range)
             break;
           x = first + i * step;
@@ -358,11 +350,11 @@ print_numbers (char const *fmt, struct layout layout,
             }
 
           if (fputs (separator, stdout) == EOF)
-            io_error ();
+            write_error ();
         }
 
       if (fputs (terminator, stdout) == EOF)
-        io_error ();
+        write_error ();
     }
 }
 
@@ -539,7 +531,7 @@ seq_fast (char const *a, char const *b, uintmax_t step)
           if (buf_end - (p_len + 1) < bufp)
             {
               if (fwrite (buf, bufp - buf, 1, stdout) != 1)
-                io_error ();
+                write_error ();
               bufp = buf;
             }
         }
@@ -547,7 +539,7 @@ seq_fast (char const *a, char const *b, uintmax_t step)
       /* Write any remaining buffered output, and the terminator.  */
       *bufp++ = *terminator;
       if (fwrite (buf, bufp - buf, 1, stdout) != 1)
-        io_error ();
+        write_error ();
     }
 
   if (ok)
diff --git a/src/shuf.c b/src/shuf.c
index 436f327e1b..be07c4a47b 100644
--- a/src/shuf.c
+++ b/src/shuf.c
@@ -597,7 +597,7 @@ main (int argc, char **argv)
     }
 
   if (i != 0)
-    error (EXIT_FAILURE, errno, _("write error"));
+    write_error ();
 
   main_exit (EXIT_SUCCESS);
 }
diff --git a/src/split.c b/src/split.c
index a3ab8d88b7..bfe391728b 100644
--- a/src/split.c
+++ b/src/split.c
@@ -957,7 +957,7 @@ lines_chunk_split (intmax_t k, intmax_t n, char *buf, idx_t bufsize,
                  large chunks from an existing file, so it's more efficient
                  to write out directly.  */
               if (full_write (STDOUT_FILENO, bp, to_write) != to_write)
-                error (EXIT_FAILURE, errno, "%s", _("write error"));
+                write_error ();
             }
           else if (! k)
             cwrite (new_file_flag, bp, to_write);
@@ -1214,12 +1214,11 @@ lines_rr (intmax_t k, intmax_t n, char *buf, idx_t bufsize, of_t **filesp)
               if (line_no == k && unbuffered)
                 {
                   if (full_write (STDOUT_FILENO, bp, to_write) != to_write)
-                    error (EXIT_FAILURE, errno, "%s", _("write error"));
+                    write_error ();
                 }
               else if (line_no == k && fwrite (bp, to_write, 1, stdout) != 1)
                 {
-                  clearerr (stdout); /* To silence close_stdout().  */
-                  error (EXIT_FAILURE, errno, "%s", _("write error"));
+                  write_error ();
                 }
               if (next)
                 line_no = (line_no == n) ? 1 : line_no + 1;
diff --git a/src/system.h b/src/system.h
index c8c8d52f2a..db1a6773b9 100644
--- a/src/system.h
+++ b/src/system.h
@@ -762,6 +762,17 @@ The following directory is part of the cycle:\n  %s\n"), \
     }					\
   while (0)
 
+/* exit with a _single_ "write error" diagnostic.  */
+
+static inline void
+write_error (void)
+{
+  int saved_errno = errno;
+  fflush (stdout);    /* Ensure nothing buffered that might induce an error. */
+  clearerr (stdout);  /* To avoid extraneous diagnostic from close_stdout.  */
+  error (EXIT_FAILURE, saved_errno, _("write error"));
+}
+
 /* Like stpncpy, but do ensure that the result is NUL-terminated,
    and do not NUL-pad out to LEN.  I.e., when strnlen (src, len) == len,
    this function writes a NUL byte into dest[len].  Thus, the length
diff --git a/src/tail.c b/src/tail.c
index db09136525..a7e681f8e2 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -1263,7 +1263,7 @@ tail_forever (struct File_spec *f, size_t n_files, double sleep_interval)
         }
 
       if ((!any_input || blocking) && fflush (stdout) != 0)
-        error (EXIT_FAILURE, errno, _("write error"));
+        write_error ();
 
       check_output_alive ();
 
@@ -1417,7 +1417,7 @@ check_fspec (struct File_spec *fspec, struct File_spec **prev_fspec)
     {
       *prev_fspec = fspec;
       if (fflush (stdout) != 0)
-        error (EXIT_FAILURE, errno, _("write error"));
+        write_error ();
     }
 }
 
@@ -2454,7 +2454,7 @@ main (int argc, char **argv)
                  tail_forever_inotify flushes only after writing,
                  not before reading.  */
               if (fflush (stdout) != 0)
-                error (EXIT_FAILURE, errno, _("write error"));
+                write_error ();
 
               Hash_table *ht;
               tail_forever_inotify (wd, F, n_files, sleep_interval, &ht);
diff --git a/src/tr.c b/src/tr.c
index af47021dcf..799e1609ec 100644
--- a/src/tr.c
+++ b/src/tr.c
@@ -1571,7 +1571,7 @@ squeeze_filter (char *buf, size_t size, size_t (*reader) (char *, size_t))
             }
           if (out_len > 0
               && fwrite (&buf[begin], 1, out_len, stdout) != out_len)
-            error (EXIT_FAILURE, errno, _("write error"));
+            write_error ();
         }
 
       if (char_to_squeeze != NOT_A_CHAR)
@@ -1797,7 +1797,7 @@ main (int argc, char **argv)
           if (nr == 0)
             break;
           if (fwrite (io_buf, 1, nr, stdout) != nr)
-            error (EXIT_FAILURE, errno, _("write error"));
+            write_error ();
         }
     }
   else if (squeeze_repeats && delete && non_option_args == 2)
@@ -1889,7 +1889,7 @@ main (int argc, char **argv)
               if (bytes_read == 0)
                 break;
               if (fwrite (io_buf, 1, bytes_read, stdout) != bytes_read)
-                error (EXIT_FAILURE, errno, _("write error"));
+                write_error ();
             }
         }
     }
diff --git a/src/unexpand.c b/src/unexpand.c
index 8505a07619..5a2283fdd4 100644
--- a/src/unexpand.c
+++ b/src/unexpand.c
@@ -228,7 +228,7 @@ unexpand (void)
                   if (pending > 1 && one_blank_before_tab_stop)
                     pending_blank[0] = '\t';
                   if (fwrite (pending_blank, 1, pending, stdout) != pending)
-                    error (EXIT_FAILURE, errno, _("write error"));
+                    write_error ();
                   pending = 0;
                   one_blank_before_tab_stop = false;
                 }
@@ -244,7 +244,7 @@ unexpand (void)
             }
 
           if (putchar (c) < 0)
-            error (EXIT_FAILURE, errno, _("write error"));
+            write_error ();
         }
       while (c != '\n');
     }
diff --git a/tests/misc/write-errors.sh b/tests/misc/write-errors.sh
index 8cda6a7691..a1ba2deb14 100755
--- a/tests/misc/write-errors.sh
+++ b/tests/misc/write-errors.sh
@@ -31,9 +31,7 @@ cat /dev/zero
 # TODO: cut -z -c1- /dev/zero
 dd if=/dev/zero
 expand /dev/zero
-# TODO: avoid double error from expand
 factor --version; yes 1 | factor
-# TODO: avoid double error from factor
 # TODO: fmt /dev/zero
 # TODO: fold -b /dev/zero
 head -z -n-1 /dev/zero
@@ -42,16 +40,12 @@ head -z -n-1 /dev/zero
 # TODO: numfmt --version; yes 1 | numfmt
 # TODO: od -v /dev/zero
 paste /dev/zero
-# TODO: avoid double error from paste
 # TODO: pr /dev/zero
 seq inf
-# TODO: avoid double error from shuf
 tail -n+1 -z /dev/zero
 tee < /dev/zero
 tr . . < /dev/zero
-# TODO: avoid double error from tr
 unexpand /dev/zero
-# TODO: avoid double error from unexpand
 # TODO: uniq -z -D /dev/zero
 yes
 " |