On 04/04/16 06:18, Assaf Gordon wrote:
Hello,
The attached patch add I/O error checking to seq, preventing infloop on certain
write-error conditions.
The typical case is
seq 1 inf > /dev/full
Nice one.
Note we'd only inf loop if seq was going to run forever anyway.
I've adjusted the NEWS accordingly.
but also a more rare case is of systems with SIGPIPE set to ignore by default
(same weird system in recent grep
thread:http://lists.gnu.org/archive/html/bug-grep/2016-03/msg00025.html ).
Trotting out my standard SIGPIPE response...
http://www.pixelbeat.org/programming/sigpipe_handling.html
On that weird system, commands like 'seq 999999 inf | head -n2' would run
forever because SIGPIPE is ignored,
Note if running stuff from python you may have SIGPIPE ignored.
See link above for details.
and seq did not check for errors. (This exact command is found in
./tests/misc/seq-precision.sh).
seq_fast() should exit() on EIO as it should only return
if nothing was output. Returning will only generate
duplicate outputs or free invalid pointers etc.
Now we could just exit() and rely on close_stdout to diagnose,
but here we should diagnose the actual error, rather than
relying on a generic "write error" because there are relatively
few output locations in this program.
+++ b/tests/misc/seq-io-errors.sh
+timed_seq_fail()
+{
+ timeout 5 seq "$@" > /dev/full 2>/dev/null
+ rc=$?
+ if test $rc -eq 1 ; then
+ :
+ elif test $rc -eq 124 ; then
+ fail=1
+ else
+ framework_failure_
+ fi
+}
The above could be simplified/standardized to:
returns_ 1 timeout 10 seq "$@" >/dev/full 2>/dev/null
Also it's worth checking for the SIGPIPE case.
updated patch is attached.
cheers,
Pádraig.
>From 65741749a324ff640e15a9b2959c8d6ee85131f9 Mon Sep 17 00:00:00 2001
From: Assaf Gordon <[email protected]>
Date: Mon, 4 Apr 2016 12:31:43 +0100
Subject: [PATCH] seq: detect and report I/O errors immediately
Ensure I/O errors are detected (and terminate seq), preventing seq
from infloop (or running for long time with a large
range) upon write errors or ignored SIGPIPE. Examples:
seq 1 inf > /dev/full (seq_fast)
seq 1.1 0.1 inf >/dev/full (print_numbers)
* src/seq.c (io_error): A new function to diagnose appropriate
stdio errors and exit the program with failure status.
(seq_fast, print_numbers): Explicitly check for write errors
and terminate the program with diagnostic.
* tests/misc/seq-io-errors.sh: Eest new error detection.
* tests/local.mk: Add new test.
* NEWS: Mention the fix.
---
NEWS | 3 +++
src/seq.c | 27 ++++++++++++++++++++-----
tests/local.mk | 1 +
tests/misc/seq-io-errors.sh | 48 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 74 insertions(+), 5 deletions(-)
create mode 100644 tests/misc/seq-io-errors.sh
diff --git a/NEWS b/NEWS
index dd3ee9c..ffb8641 100644
--- a/NEWS
+++ b/NEWS
@@ -15,6 +15,9 @@ GNU coreutils NEWS -*- outline -*-
stty --help no longer outputs extraneous gettext header lines
for translated languages. [bug introduced in coreutils-8.24]
+ seq now immediately exits upon write errors.
+ [This bug was present in "the beginning".]
+
** Changes in behavior
stat now outputs nanosecond information for time stamps even if
diff --git a/src/seq.c b/src/seq.c
index fbb94a0..3095715 100644
--- a/src/seq.c
+++ b/src/seq.c
@@ -267,6 +267,18 @@ long_double_format (char const *fmt, struct layout *layout)
}
}
+static void ATTRIBUTE_NORETURN
+io_error (void)
+{
+ int status = EXIT_FAILURE;
+ if (errno != EPIPE)
+ error (0, errno, _("standard output"));
+ else
+ status = EXIT_SUCCESS;
+ clearerr (stdout);
+ exit (status);
+}
+
/* Actually print the sequence of numbers in the specified range, with the
given or default stepping and format. */
@@ -284,7 +296,8 @@ print_numbers (char const *fmt, struct layout layout,
for (i = 1; ; i++)
{
long double x0 = x;
- printf (fmt, x);
+ if (printf (fmt, x) < 0)
+ io_error ();
if (out_of_range)
break;
x = first + i * step;
@@ -325,10 +338,12 @@ print_numbers (char const *fmt, struct layout layout,
break;
}
- fputs (separator, stdout);
+ if (fputs (separator, stdout) == EOF)
+ io_error ();
}
- fputs (terminator, stdout);
+ if (fputs (terminator, stdout) == EOF)
+ io_error ();
}
}
@@ -495,14 +510,16 @@ seq_fast (char const *a, char const *b)
output buffer so far, and reset to start of buffer. */
if (buf_end - (p_len + 1) < bufp)
{
- fwrite (buf, bufp - buf, 1, stdout);
+ if (fwrite (buf, bufp - buf, 1, stdout) != 1)
+ io_error ();
bufp = buf;
}
}
/* Write any remaining buffered output, and the terminator. */
*bufp++ = *terminator;
- fwrite (buf, bufp - buf, 1, stdout);
+ if (fwrite (buf, bufp - buf, 1, stdout) != 1)
+ io_error ();
IF_LINT (free (buf));
}
diff --git a/tests/local.mk b/tests/local.mk
index a83c3d0..8b2a19a 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -235,6 +235,7 @@ all_tests = \
tests/misc/ptx.pl \
tests/misc/test.pl \
tests/misc/seq.pl \
+ tests/misc/seq-io-errors.sh \
tests/misc/seq-long-double.sh \
tests/misc/seq-precision.sh \
tests/misc/head.pl \
diff --git a/tests/misc/seq-io-errors.sh b/tests/misc/seq-io-errors.sh
new file mode 100644
index 0000000..ac44ca2
--- /dev/null
+++ b/tests/misc/seq-io-errors.sh
@@ -0,0 +1,48 @@
+#!/bin/sh
+# Test for proper detection of I/O errors in seq
+
+# Copyright (C) 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=.}/tests/init.sh"; path_prepend_ ./src
+print_ver_ seq
+
+if ! test -w /dev/full || ! test -c /dev/full; then
+ skip_ '/dev/full is required'
+fi
+
+# Run 'seq' with a timeout, preventing infinite-loop run.
+# if seq fails (returns 1) - the test passed.
+# if seq times-out (timeout forces exitcode of 124) - the test failed.
+# any other code - framework failure.
+timed_seq_fail() { returns_ 1 timeout 10 seq "$@" >/dev/full 2>/dev/null; }
+
+
+# Test infinite sequence, using fast-path method (seq_fast).
+timed_seq_fail 1 inf
+
+# Test infinite sequence, using slow-path method (print_numbers).
+timed_seq_fail 1.1 .1 inf
+
+# Test non-infinite sequence, using slow-path method (print_numbers).
+# (despite being non-infinite, the entire sequence should take long time to
+# print. Thus, either an I/O error is detected immedaitely, or seq will
+# timeout).
+timed_seq_fail 1 0.0001 99999999
+
+(trap '' PIPE; timeout 10 sh -c 'seq inf 2>err | head -n1') || fail=1
+compare /dev/null err || fail=1
+
+Exit $fail
--
2.5.5
