On 09/07/2023 23:54, Pádraig Brady wrote:
On 09/07/2023 20:11, Paul Eggert wrote:
On 2023-07-09 07:11, Pádraig Brady wrote:
Note the patch looks wrong as it would close the input always.
We can fix that up easily enough anyway.
If it's easy and doesn't hurt performance in the usual case, that's of
course fine.
For my reference a short list of utils to check (that might inf loop) are:
cat comm cp cut dd dirname expand factor fmt fold head join ls nl numfmt
od paste pr printf ptx readlink realpath seq shuf sort stat tac tail
tee tr tsort unexpand uniq wc yes
There's no point to complicating programs that have bounded input for
other reasons, as they'll report errors soon enough for those other
reasons. The main problem comes from programs like 'cat' and 'od' that
can read from "infinite" input like /dev/urandom and can write indefinitely.
So I think we needn't worry about dirname, ptx, readlink, realpath,
sort, stat, tac, tsort, wc. For example, we needn't worry about 'sort'
because it needs to read all its input before outputting anything, and
it cannot read all of /dev/urandom. As long as these programs eventually
check for write errors, which I expect they all do, that should be good
enough.
Even for programs like 'od' there is no need to check every output
function; just check occasionally, when it's convenient.
Yes completely agreed.
The shortlist above is for my reference for commands to review,
not for commands to adjust.
The attached patch-set addresses two classes of issue:
1. Doubled error messages upon write errors
2. Continued processing upon write errors (the orig problem reported).
For class 1, we include fixes for:
expand, factor, paste, shuf, tr, unexpand
For class 2, we include fixes for:
od, uniq, cut, comm, join
For class 2, we do _not_ include fixes for these utils
which are more awkward / less important in this context:
fmt, fold, nl, numfmt, pr
I'll update NEWS with this info before pushing.
cheers,
Pádraig
From b09caad2f11dd6b4ee434c70e07c22bb834679ee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Tue, 11 Jul 2023 13:28:53 +0100
Subject: [PATCH 1/7] tests: ensure utilties exit promptly upon write error
* tests/local.mk: Reference the new test.
* tests/misc/write-errors.sh: A new test to ensure utilities
exit promptly upon writing to /dev/full.
---
tests/local.mk | 1 +
tests/misc/write-errors.sh | 70 ++++++++++++++++++++++++++++++++++++++
2 files changed, 71 insertions(+)
create mode 100755 tests/misc/write-errors.sh
diff --git a/tests/local.mk b/tests/local.mk
index cc65fe656..f35728229 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -174,6 +174,7 @@ all_tests = \
tests/cp/no-ctx.sh \
tests/tty/tty-eof.pl \
tests/misc/read-errors.sh \
+ tests/misc/write-errors.sh \
tests/tail/inotify-hash-abuse.sh \
tests/tail/inotify-hash-abuse2.sh \
tests/tail/F-vs-missing.sh \
diff --git a/tests/misc/write-errors.sh b/tests/misc/write-errors.sh
new file mode 100755
index 000000000..f3b2f6c75
--- /dev/null
+++ b/tests/misc/write-errors.sh
@@ -0,0 +1,70 @@
+#!/bin/sh
+# Make sure all of these programs promptly diagnose write errors.
+
+# Copyright (C) 2023 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 <https://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
+print_ver_ timeout
+
+if ! test -w /dev/full || ! test -c /dev/full; then
+ skip_ 'Need /dev/full'
+fi
+
+# Writers that may output data indefinitely
+# First word in command line is checked against built programs
+echo "\
+cat /dev/zero
+# TODO: comm -z /dev/zero /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
+# TODO: join -a 1 -z /dev/zero /dev/null
+# TODO: nl --version; yes | nl
+# 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
+" |
+sort -k 1b,1 > all_writers || framework_failure_
+
+printf '%s\n' $built_programs |
+sort -k 1b,1 > built_programs || framework_failure_
+
+join all_writers built_programs > built_writers || framework_failure_
+
+while read writer; do
+ timeout 10 $SHELL -c "$writer > /dev/full"
+ test $? = 124 && { fail=1; echo "$writer: failed to exit" >&2; }
+done < built_writers
+
+Exit $fail
--
2.41.0
From 8004dcd7adec9938de2dcd9fccbd269dcdd926b1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Sat, 15 Jul 2023 20:41:44 +0100
Subject: [PATCH 2/7] 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 directiing 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.
---
cfg.mk | 9 ++++++++-
src/basenc.c | 10 +++++-----
src/cat.c | 8 ++++----
src/expand.c | 4 ++--
src/factor.c | 2 +-
src/paste.c | 8 --------
src/seq.c | 18 +++++-------------
src/shuf.c | 2 +-
src/split.c | 7 +++----
src/system.h | 8 ++++++++
src/tail.c | 6 +++---
src/tr.c | 6 +++---
src/unexpand.c | 4 ++--
tests/misc/write-errors.sh | 6 ------
14 files changed, 45 insertions(+), 53 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index e7b0519c4..2d6856e15 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 9152ad0de..be7a61641 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 880fcf2e7..b109998bf 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 adad9d9d8..0e74d0cf6 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 8ea00a13e..e0a98152c 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 73ddd1de6..68f8a36ef 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 633189c40..977702bb5 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 436f327e1..be07c4a47 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 a3ab8d88b..bfe391728 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 c8c8d52f2..6c0883f4d 100644
--- a/src/system.h
+++ b/src/system.h
@@ -762,6 +762,14 @@ The following directory is part of the cycle:\n %s\n"), \
} \
while (0)
+static inline void
+write_error (void)
+{
+ fflush (stdout); /* Ensure nothing buffered that might induce an error. */
+ clearerr (stdout); /* To avoid extraneous diagnostic from close_stdout. */
+ error (EXIT_FAILURE, 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 db0913652..a7e681f8e 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 af47021dc..799e1609e 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 8505a0761..5a2283fdd 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 f3b2f6c75..32051508b 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
" |
--
2.41.0
From e4337d9f3dd43fdccd853846357eac57837d0334 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Tue, 11 Jul 2023 12:46:50 +0100
Subject: [PATCH 3/7] od: promptly diagnose write errors
* src/od.c (dump): Check for write errors after each block written,
to exit early even with large / unbounded inputs.
* tests/misc/write-errors.sh: enable od check.
* NEWS: Mention the improvement.
Fixes https://bugs.gnu.org/64540
---
NEWS | 3 +++
src/od.c | 8 ++++++--
tests/misc/write-errors.sh | 2 +-
3 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/NEWS b/NEWS
index 35ae02a8e..892db27b8 100644
--- a/NEWS
+++ b/NEWS
@@ -62,6 +62,9 @@ GNU coreutils NEWS -*- outline -*-
irrespective of which kernel version coreutils is built against,
reinstating that behaviour from coreutils-9.0.
+ od will now exit immediately upon receiving a write error, which is
+ significant when reading large / unbounded inputs.
+
split now uses more tuned access patterns for its potentially large input.
This was seen to improve throughput by 5% when reading from SSD.
diff --git a/src/od.c b/src/od.c
index 6b66ceb4f..cde483c98 100644
--- a/src/od.c
+++ b/src/od.c
@@ -1380,7 +1380,7 @@ dump (void)
if (limit_bytes_to_format)
{
- while (true)
+ while (ok)
{
size_t n_needed;
if (current_offset >= end_offset)
@@ -1396,13 +1396,15 @@ dump (void)
affirm (n_bytes_read == bytes_per_block);
write_block (current_offset, n_bytes_read,
block[!idx], block[idx]);
+ if (ferror (stdout))
+ ok = false;
current_offset += n_bytes_read;
idx = !idx;
}
}
else
{
- while (true)
+ while (ok)
{
ok &= read_block (bytes_per_block, block[idx], &n_bytes_read);
if (n_bytes_read < bytes_per_block)
@@ -1410,6 +1412,8 @@ dump (void)
affirm (n_bytes_read == bytes_per_block);
write_block (current_offset, n_bytes_read,
block[!idx], block[idx]);
+ if (ferror (stdout))
+ ok = false;
current_offset += n_bytes_read;
idx = !idx;
}
diff --git a/tests/misc/write-errors.sh b/tests/misc/write-errors.sh
index 32051508b..8cf9db88d 100755
--- a/tests/misc/write-errors.sh
+++ b/tests/misc/write-errors.sh
@@ -38,7 +38,7 @@ head -z -n-1 /dev/zero
# TODO: join -a 1 -z /dev/zero /dev/null
# TODO: nl --version; yes | nl
# TODO: numfmt --version; yes 1 | numfmt
-# TODO: od -v /dev/zero
+od -v /dev/zero
paste /dev/zero
# TODO: pr /dev/zero
seq inf
--
2.41.0
From 886fa18153977ef34ec06a811f9a517def6d40db Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Sat, 15 Jul 2023 21:10:38 +0100
Subject: [PATCH 4/7] uniq: promptly diagnose write errors
* src/uniq.c (write_line): Check the output from fwrite() immediately.
(check_file): Likewise.
* tests/misc/write-errors.sh: Enable the test case.
---
src/uniq.c | 9 ++++++---
tests/misc/write-errors.sh | 2 +-
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/uniq.c b/src/uniq.c
index 15611540b..2dbec9fcd 100644
--- a/src/uniq.c
+++ b/src/uniq.c
@@ -309,7 +309,9 @@ writeline (struct linebuffer const *line,
if (countmode == count_occurrences)
printf ("%7" PRIuMAX " ", linecount + 1);
- fwrite (line->buffer, sizeof (char), line->length, stdout);
+ if (fwrite (line->buffer, sizeof (char), line->length, stdout)
+ != line->length)
+ write_error ();
}
/* Process input file INFILE with output to OUTFILE.
@@ -378,8 +380,9 @@ check_file (char const *infile, char const *outfile, char delimiter)
if (new_group || grouping != GM_NONE)
{
- fwrite (thisline->buffer, sizeof (char),
- thisline->length, stdout);
+ if (fwrite (thisline->buffer, sizeof (char), thisline->length,
+ stdout) != thisline->length)
+ write_error ();
SWAP_LINES (prevline, thisline);
prevfield = thisfield;
diff --git a/tests/misc/write-errors.sh b/tests/misc/write-errors.sh
index 8cf9db88d..60817bd3d 100755
--- a/tests/misc/write-errors.sh
+++ b/tests/misc/write-errors.sh
@@ -46,7 +46,7 @@ tail -n+1 -z /dev/zero
tee < /dev/zero
tr . . < /dev/zero
unexpand /dev/zero
-# TODO: uniq -z -D /dev/zero
+uniq -z -D /dev/zero
yes
" |
sort -k 1b,1 > all_writers || framework_failure_
--
2.41.0
From d1bac50e2702095813efe324b04890aab1837fdf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Sat, 15 Jul 2023 21:23:55 +0100
Subject: [PATCH 5/7] cut: promptly diagnose write errors
* src/cut.c (cut_bytes): Diagnose errors from fwrite() and putchar().
(cut_fields): Likewise.
* tests/misc/write-errors.sh: Enable the test for cut,
and augment to cover both cut_bytes() and cut_fields().
---
src/cut.c | 32 ++++++++++++++++++++++----------
tests/misc/write-errors.sh | 3 ++-
2 files changed, 24 insertions(+), 11 deletions(-)
diff --git a/src/cut.c b/src/cut.c
index 12dfd2299..476df0943 100644
--- a/src/cut.c
+++ b/src/cut.c
@@ -232,7 +232,8 @@ cut_bytes (FILE *stream)
if (c == line_delim)
{
- putchar (c);
+ if (putchar (c) < 0)
+ write_error ();
byte_idx = 0;
print_delimiter = false;
current_rp = frp;
@@ -252,13 +253,16 @@ cut_bytes (FILE *stream)
{
if (print_delimiter && is_range_start_index (byte_idx))
{
- fwrite (output_delimiter_string, sizeof (char),
- output_delimiter_length, stdout);
+ if (fwrite (output_delimiter_string, sizeof (char),
+ output_delimiter_length, stdout)
+ != output_delimiter_length)
+ write_error ();
}
print_delimiter = true;
}
- putchar (c);
+ if (putchar (c) < 0)
+ write_error ();
}
}
}
@@ -325,7 +329,9 @@ cut_fields (FILE *stream)
}
else
{
- fwrite (field_1_buffer, sizeof (char), n_bytes, stdout);
+ if (fwrite (field_1_buffer, sizeof (char), n_bytes, stdout)
+ != n_bytes)
+ write_error ();
/* Make sure the output line is newline terminated. */
if (field_1_buffer[n_bytes - 1] != line_delim)
putchar (line_delim);
@@ -336,7 +342,9 @@ cut_fields (FILE *stream)
if (print_kth (1))
{
/* Print the field, but not the trailing delimiter. */
- fwrite (field_1_buffer, sizeof (char), n_bytes - 1, stdout);
+ if (fwrite (field_1_buffer, sizeof (char), n_bytes - 1, stdout)
+ != n_bytes - 1)
+ write_error ();
/* With -d$'\n' don't treat the last '\n' as a delimiter. */
if (delim == line_delim)
@@ -360,14 +368,17 @@ cut_fields (FILE *stream)
{
if (found_any_selected_field)
{
- fwrite (output_delimiter_string, sizeof (char),
- output_delimiter_length, stdout);
+ if (fwrite (output_delimiter_string, sizeof (char),
+ output_delimiter_length, stdout)
+ != output_delimiter_length)
+ write_error ();
}
found_any_selected_field = true;
while ((c = getc (stream)) != delim && c != line_delim && c != EOF)
{
- putchar (c);
+ if (putchar (c) < 0)
+ write_error ();
prev_c = c;
}
}
@@ -398,7 +409,8 @@ cut_fields (FILE *stream)
{
if (c == line_delim || prev_c != line_delim
|| delim == line_delim)
- putchar (line_delim);
+ if (putchar (line_delim) < 0)
+ write_error ();
}
if (c == EOF)
break;
diff --git a/tests/misc/write-errors.sh b/tests/misc/write-errors.sh
index 60817bd3d..d27970e2c 100755
--- a/tests/misc/write-errors.sh
+++ b/tests/misc/write-errors.sh
@@ -28,7 +28,8 @@ fi
echo "\
cat /dev/zero
# TODO: comm -z /dev/zero /dev/zero
-# TODO: cut -z -c1- /dev/zero
+cut -z -c1- /dev/zero
+cut -z -f1- /dev/zero
dd if=/dev/zero
expand /dev/zero
factor --version; yes 1 | factor
--
2.41.0
From f67a6770b0c0907efdf5b582adb8c1b2f7c0824a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Sat, 15 Jul 2023 21:43:07 +0100
Subject: [PATCH 6/7] comm: promptly diagnose write errors
* src/comm.c (write_line): Call write_error() upon ferror().
* tests/misc/write-errors.sh: Enable comm test.
---
src/comm.c | 21 ++++++++++++---------
tests/misc/write-errors.sh | 2 +-
2 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/src/comm.c b/src/comm.c
index 12fa9c9ea..5cb2410e5 100644
--- a/src/comm.c
+++ b/src/comm.c
@@ -163,13 +163,13 @@ Examples:\n\
exit (status);
}
-/* Output the line in linebuffer LINE to stream STREAM
+/* Output the line in linebuffer LINE to stdout
provided the switches say it should be output.
CLASS is 1 for a line found only in file 1,
2 for a line only in file 2, 3 for a line in both. */
static void
-writeline (struct linebuffer const *line, FILE *stream, int class)
+writeline (struct linebuffer const *line, int class)
{
switch (class)
{
@@ -182,20 +182,23 @@ writeline (struct linebuffer const *line, FILE *stream, int class)
if (!only_file_2)
return;
if (only_file_1)
- fwrite (col_sep, 1, col_sep_len, stream);
+ fwrite (col_sep, 1, col_sep_len, stdout);
break;
case 3:
if (!both)
return;
if (only_file_1)
- fwrite (col_sep, 1, col_sep_len, stream);
+ fwrite (col_sep, 1, col_sep_len, stdout);
if (only_file_2)
- fwrite (col_sep, 1, col_sep_len, stream);
+ fwrite (col_sep, 1, col_sep_len, stdout);
break;
}
- fwrite (line->buffer, sizeof (char), line->length, stream);
+ fwrite (line->buffer, sizeof (char), line->length, stdout);
+
+ if (ferror (stdout))
+ write_error ();
}
/* Check that successive input lines PREV and CURRENT from input file
@@ -329,7 +332,7 @@ compare_files (char **infiles)
{
/* Line is seen in both files. */
total[2]++;
- writeline (thisline[1], stdout, 3);
+ writeline (thisline[1], 3);
}
else
{
@@ -338,13 +341,13 @@ compare_files (char **infiles)
{
/* Line is seen in file 1 only. */
total[0]++;
- writeline (thisline[0], stdout, 1);
+ writeline (thisline[0], 1);
}
else
{
/* Line is seen in file 2 only. */
total[1]++;
- writeline (thisline[1], stdout, 2);
+ writeline (thisline[1], 2);
}
}
diff --git a/tests/misc/write-errors.sh b/tests/misc/write-errors.sh
index d27970e2c..65c2cec2b 100755
--- a/tests/misc/write-errors.sh
+++ b/tests/misc/write-errors.sh
@@ -27,7 +27,7 @@ fi
# First word in command line is checked against built programs
echo "\
cat /dev/zero
-# TODO: comm -z /dev/zero /dev/zero
+comm -z /dev/zero /dev/zero
cut -z -c1- /dev/zero
cut -z -f1- /dev/zero
dd if=/dev/zero
--
2.41.0
From df1c1e4d863bc02bbe9d3588424b12a0e2fc6efe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Sat, 15 Jul 2023 21:55:23 +0100
Subject: [PATCH 7/7] join: promptly diagnose write errors
* src/join.c (prjoin): Check for write errors after each line.
* tests/misc/write-errors.sh: enable the test for join.
---
src/join.c | 3 +++
tests/misc/write-errors.sh | 2 +-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/join.c b/src/join.c
index 5b58ed4d5..afbf0307e 100644
--- a/src/join.c
+++ b/src/join.c
@@ -650,6 +650,9 @@ prjoin (struct line const *line1, struct line const *line2)
putchar (eolchar);
}
+
+ if (ferror (stdout))
+ write_error ();
}
/* Print the join of the files in FP1 and FP2. */
diff --git a/tests/misc/write-errors.sh b/tests/misc/write-errors.sh
index 65c2cec2b..b5cd24053 100755
--- a/tests/misc/write-errors.sh
+++ b/tests/misc/write-errors.sh
@@ -36,7 +36,7 @@ factor --version; yes 1 | factor
# TODO: fmt /dev/zero
# TODO: fold -b /dev/zero
head -z -n-1 /dev/zero
-# TODO: join -a 1 -z /dev/zero /dev/null
+join -a 1 -z /dev/zero /dev/null
# TODO: nl --version; yes | nl
# TODO: numfmt --version; yes 1 | numfmt
od -v /dev/zero
--
2.41.0
