Last-minute testing found a minor problem. Here's the fix I'm considering: >From fc4d3f63b0c64992014b035e8b780eb230e0c855 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyer...@redhat.com> Date: Fri, 11 Dec 2009 12:15:13 +0100 Subject: [PATCH] tail: don't call fstat on an uninitialized FD
This bug showed up via valgrind as a "Conditional jump or move depends on uninitialized value(s)" error. * src/tail.c (ignore_fifo_and_pipe): New function. (main): Use it only when tailing forever. The code to compute n_viable and mark some F[i] as ignored would call isapipe on an uninitialized file descriptor. But n_viable and those .ignored marks are useful/used only when tailing forever. This bug was introduced via commit f0ff8c73 (7.6), "tail: make the new piped-stdin test as portable as the old one". * NEWS (Bug fixes): Mention it. --- NEWS | 3 +++ src/tail.c | 50 ++++++++++++++++++++++++++++++-------------------- 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/NEWS b/NEWS index 02f2ce0..1b56973 100644 --- a/NEWS +++ b/NEWS @@ -21,6 +21,9 @@ GNU coreutils NEWS -*- outline -*- if it uses helper processes for compression and its parent ignores CHLD signals. [bug introduced in coreutils-6.9] + tail without -f no longer access uninitialized memory + [bug introduced in coreutils-7.6] + timeout is now immune to the signal handling of its parent. Specifically timeout now doesn't exit with an error message if its parent ignores CHLD signals. [bug introduced in coreutils-7.6] diff --git a/src/tail.c b/src/tail.c index 2bd9e3d..71f8a32 100644 --- a/src/tail.c +++ b/src/tail.c @@ -1889,6 +1889,35 @@ parse_options (int argc, char **argv, } } +/* Mark as '.ignore'd each member of F that corresponds to a + pipe or fifo, and return the number of non-ignored members. */ +static size_t +ignore_fifo_and_pipe (struct File_spec *f, size_t n_files) +{ + /* When there is no FILE operand and stdin is a pipe or FIFO + POSIX requires that tail ignore the -f option. + Since we allow multiple FILE operands, we extend that to say: with -f, + ignore any "-" operand that corresponds to a pipe or FIFO. */ + size_t n_viable = 0; + + size_t i; + for (i = 0; i < n_files; i++) + { + bool is_a_fifo_or_pipe = + (STREQ (f[i].name, "-") + && !f[i].ignore + && 0 <= f[i].fd + && (S_ISFIFO (f[i].mode) + || (HAVE_FIFO_PIPES != 1 && isapipe (f[i].fd)))); + if (is_a_fifo_or_pipe) + f[i].ignore = true; + else + ++n_viable; + } + + return n_viable; +} + int main (int argc, char **argv) { @@ -1980,26 +2009,7 @@ main (int argc, char **argv) for (i = 0; i < n_files; i++) ok &= tail_file (&F[i], n_units); - /* When there is no FILE operand and stdin is a pipe or FIFO - POSIX requires that tail ignore the -f option. - Since we allow multiple FILE operands, we extend that to say: - ignore any "-" operand that corresponds to a pipe or FIFO. */ - size_t n_viable = 0; - for (i = 0; i < n_files; i++) - { - bool is_a_fifo_or_pipe = - (STREQ (F[i].name, "-") - && !F[i].ignore - && 0 <= F[i].fd - && (S_ISFIFO (F[i].mode) - || (HAVE_FIFO_PIPES != 1 && isapipe (F[i].fd)))); - if (is_a_fifo_or_pipe) - F[i].ignore = true; - else - ++n_viable; - } - - if (forever && n_viable) + if (forever && ignore_fifo_and_pipe (F, n_files)) { #if HAVE_INOTIFY /* If the user specifies stdin via a command line argument of "-", -- 1.6.6.rc1.319.g9b57d