>From 2a49ab8eae92859c02c0db7a14231b391287c5ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= Date: Thu, 15 Jun 2017 02:41:14 -0700 Subject: [PATCH 1/2] tail: with -f don't warn if doing a blocking read of a tty * src/tail.c: (main): Only issue the warning about -f being ineffective when we're not going into simple blocking mode. * tests/tail-2/follow-stdin.sh: Ensure the warning is output correctly. Fixes http://bugs.gnu.org/27368 --- NEWS | 4 ++++ src/tail.c | 18 ++++++++++++++---- tests/tail-2/follow-stdin.sh | 21 ++++++++++++++++++++- 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/NEWS b/NEWS index d2672e8..22805c8 100644 --- a/NEWS +++ b/NEWS @@ -56,6 +56,10 @@ GNU coreutils NEWS -*- outline -*- tail -f will now exit immediately if the output is piped and the reader of the pipe terminates. + tail -f will no longer erroneously warn about being ineffective + when following a single tty, as the simple blocking loop used + is effective in this case. + * Noteworthy changes in release 8.27 (2017-03-08) [stable] diff --git a/src/tail.c b/src/tail.c index 3918373..2f9b981 100644 --- a/src/tail.c +++ b/src/tail.c @@ -2365,12 +2365,22 @@ main (int argc, char **argv) if (found_hyphen && follow_mode == Follow_name) die (EXIT_FAILURE, 0, _("cannot follow %s by name"), quoteaf ("-")); - /* When following forever, warn if any file is '-'. + /* When following forever, and not using simple blocking, warn if + any file is '-' as the stats() used to check for input are ineffective. This is only a warning, since tail's output (before a failing seek, and that from any non-stdin files) might still be useful. */ - if (forever && found_hyphen && isatty (STDIN_FILENO)) - error (0, 0, _("warning: following standard input" - " indefinitely is ineffective")); + if (forever && found_hyphen) + { + struct stat in_stat; + bool blocking_stdin; + blocking_stdin = (pid == 0 && follow_mode == Follow_descriptor + && n_files == 1 && ! fstat (STDIN_FILENO, &in_stat) + && ! S_ISREG (in_stat.st_mode)); + + if (! blocking_stdin && isatty (STDIN_FILENO)) + error (0, 0, _("warning: following standard input" + " indefinitely is ineffective")); + } } /* Don't read anything if we'll never output anything. */ diff --git a/tests/tail-2/follow-stdin.sh b/tests/tail-2/follow-stdin.sh index 7e82835..c922ea1 100755 --- a/tests/tail-2/follow-stdin.sh +++ b/tests/tail-2/follow-stdin.sh @@ -1,5 +1,5 @@ #!/bin/sh -# tail -f - would fail with the initial inotify implementation +# Test tail -f - # Copyright (C) 2009-2017 Free Software Foundation, Inc. @@ -19,6 +19,9 @@ . "${srcdir=.}/tests/init.sh"; path_prepend_ ./src print_ver_ tail +################# +# tail -f - would fail with the initial inotify implementation + check_tail_output() { local delay="$1" @@ -51,6 +54,7 @@ for mode in '' '---disable-inotify'; do done +################# # Before coreutils-8.26 this would induce an UMR under UBSAN returns_ 1 timeout 10 tail -f - <&- 2>errt || fail=1 cat <<\EOF >exp || framework_failure_ @@ -63,4 +67,19 @@ sed 's/\(tail:.*\):.*/\1/' errt > err || framework_failure_ compare exp err || fail=1 +################# +# Before coreutils-8.28 this would erroneously issue a warning +if tty -s err || fail=1 + compare /dev/null err || fail=1 + done + + tail -f - /dev/null out & pid=$! + # Wait up to 12.7s for output to appear: + tail_re='ineffective' retry_delay_ check_tail_output .1 7 || + { cat out; fail=1; } + cleanup_ +fi + Exit $fail -- 2.9.3 >From 9066e57d421935747a4e99460478ec71b3191452 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= Date: Fri, 16 Jun 2017 23:50:47 -0700 Subject: [PATCH 2/2] tail: only use inotify with regular files * src/tail.c (any_non_regular): A new function to check passed files. (main): Use the above to skip inotify if any non regular files passed like /dev/tty or /dev/ttyUSB0 etc. * tests/tail-2/inotify-only-regular.sh: A new test. * tests/local.mk: Reference the new test. * NEWS: Mention the bug fix. Fixes http://bugs.gnu.org/21265 and http://bugs.gnu.org/27368 --- NEWS | 4 ++++ src/tail.c | 17 +++++++++++++++++ tests/local.mk | 1 + tests/tail-2/inotify-only-regular.sh | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 54 insertions(+) create mode 100755 tests/tail-2/inotify-only-regular.sh diff --git a/NEWS b/NEWS index 22805c8..86e6bc7 100644 --- a/NEWS +++ b/NEWS @@ -36,6 +36,10 @@ GNU coreutils NEWS -*- outline -*- or ignored until future events on the monitored files. [bug introduced with inotify support added in coreutils-7.5] + tail -f /dev/tty is now supported by not using inotify when any + non regular files are specified, as inotify is ineffective with these. + [bug introduced with inotify support added in coreutils-7.5] + uptime no longer outputs the AM/PM component of the current time, as that's inconsistent with the 24 hour time format used. [bug introduced in coreutils-7.0] diff --git a/src/tail.c b/src/tail.c index 2f9b981..b8be1d2 100644 --- a/src/tail.c +++ b/src/tail.c @@ -1339,6 +1339,22 @@ any_symlinks (const struct File_spec *f, size_t n_files) return false; } +/* Return true if any of the N_FILES files in F is not + a regular file. This is used to avoid adding inotify + watches on a device file for example, which inotify + will accept, but not give any events for. */ + +static bool +any_non_regular (const struct File_spec *f, size_t n_files) +{ + size_t i; + + for (i = 0; i < n_files; i++) + if (0 <= f[i].fd && ! S_ISREG (f[i].mode)) + return true; + return false; +} + /* Return true if any of the N_FILES files in F represents stdin and is tailable. */ @@ -2457,6 +2473,7 @@ main (int argc, char **argv) || any_remote_file (F, n_files) || ! any_non_remote_file (F, n_files) || any_symlinks (F, n_files) + || any_non_regular (F, n_files) || (!ok && follow_mode == Follow_descriptor))) disable_inotify = true; diff --git a/tests/local.mk b/tests/local.mk index fdf3edf..6112e88 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -177,6 +177,7 @@ all_tests = \ tests/tail-2/inotify-rotate.sh \ tests/tail-2/inotify-rotate-resources.sh \ tests/tail-2/inotify-dir-recreate.sh \ + tests/tail-2/inotify-only-regular.sh \ tests/chmod/no-x.sh \ tests/chgrp/basic.sh \ tests/rm/dangling-symlink.sh \ diff --git a/tests/tail-2/inotify-only-regular.sh b/tests/tail-2/inotify-only-regular.sh new file mode 100755 index 0000000..4d106fb --- /dev/null +++ b/tests/tail-2/inotify-only-regular.sh @@ -0,0 +1,32 @@ +#!/bin/sh +# ensure that tail -f only uses inotify for regular files + +# Copyright (C) 2017 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 . + +. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src +print_ver_ tail + +grep '^#define HAVE_INOTIFY 1' "$CONFIG_HEADER" >/dev/null \ + || skip_ 'inotify support required' + +require_strace_ 'inotify_add_watch' + +returns_ 124 timeout .1 strace -e inotify_add_watch -o strace.out \ + tail -f /dev/null || fail=1 + +grep 'inotify' strace.out && fail=1 + +Exit $fail -- 2.9.3