tail: fix output of redundant headers when resuming

* src/tail.c (check_fspec): Only enable printing of the file header
if we've actually read some data and this is a new file.  Also
move printing of the file header to...
(dump_remainder): ...here, to allow printing only when data read.
* tests/tail-2/overlay-headers.sh: A new test for suspension
and resumption of tail.
* tests/local.mk: Reference the new test.
* NEWS: Mention the fix.
Fixes http://bugs.gnu.org/23539

diff --git a/NEWS b/NEWS
index 24082f5be32662cec6229d89232552a37f490b19..9528f89e07cef8867d7a7d69e741755c4ddedee3 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -12,6 +12,10 @@ GNU coreutils NEWS                                    -*- outline -*-
   158909489063877810457 and 222087527029934481871.
   [bug introduced in coreutils-8.20]
 
+  tail no longer prints redundant file headers with interleaved inotify events,
+  which could be triggered especially when tail was suspended and resumed.
+  [bug introduced with inotify support added in coreutils-7.5]
+
   wc --bytes --files0-from now correctly reports byte counts.
   Previously it may have returned values that were too large,
   depending on the size of the first file processed.

diff --git a/src/tail.c b/src/tail.c
index 0c9c3e8827747d5163b6ea73372ba11b0ed3aed4..b24757f3d7e715bc94dd5fcf5ac4cc933babf450 100644 (file)
--- a/src/tail.c
+++ b/src/tail.c
@@ -399,7 +399,8 @@ xwrite_stdout (char const *buffer, size_t n_bytes)
    Return the number of bytes read from the file.  */
 
 static uintmax_t
-dump_remainder (const char *pretty_filename, int fd, uintmax_t n_bytes)
+dump_remainder (bool want_header, const char *pretty_filename, int fd,
+                uintmax_t n_bytes)
 {
   uintmax_t n_written;
   uintmax_t n_remaining = n_bytes;
@@ -419,6 +420,11 @@ dump_remainder (const char *pretty_filename, int fd, uintmax_t n_bytes)
         }
       if (bytes_read == 0)
         break;
+      if (want_header)
+        {
+          write_header (pretty_filename);
+          want_header = false;
+        }
       xwrite_stdout (buffer, bytes_read);
       n_written += bytes_read;
       if (n_bytes != COPY_TO_EOF)
@@ -528,7 +534,7 @@ file_lines (const char *pretty_filename, int fd, uintmax_t n_lines,
                  output the part that is after it.  */
               if (n != bytes_read - 1)
                 xwrite_stdout (nl + 1, bytes_read - (n + 1));
-              *read_pos += dump_remainder (pretty_filename, fd,
+              *read_pos += dump_remainder (false, pretty_filename, fd,
                                            end_pos - (pos + bytes_read));
               return true;
             }
@@ -540,7 +546,7 @@ file_lines (const char *pretty_filename, int fd, uintmax_t n_lines,
           /* Not enough lines in the file; print everything from
              start_pos to the end.  */
           xlseek (fd, start_pos, SEEK_SET, pretty_filename);
-          *read_pos = start_pos + dump_remainder (pretty_filename, fd,
+          *read_pos = start_pos + dump_remainder (false, pretty_filename, fd,
                                                   end_pos);
           return true;
         }
@@ -1223,7 +1229,7 @@ tail_forever (struct File_spec *f, size_t n_files, double sleep_interval)
           else
             bytes_to_read = COPY_TO_EOF;
 
-          bytes_read = dump_remainder (name, fd, bytes_to_read);
+          bytes_read = dump_remainder (false, name, fd, bytes_to_read);
 
           any_input |= (bytes_read != 0);
           f[i].size += bytes_read;
@@ -1336,7 +1342,8 @@ wd_comparator (const void *e1, const void *e2)
   return spec1->wd == spec2->wd;
 }
 
-/* Output (new) data for FSPEC->fd.  */
+/* Output (new) data for FSPEC->fd.
+   PREV_FSPEC records the last File_spec for which we output.  */
 static void
 check_fspec (struct File_spec *fspec, struct File_spec **prev_fspec)
 {
@@ -1371,18 +1378,18 @@ check_fspec (struct File_spec *fspec, struct File_spec **prev_fspec)
            && timespec_cmp (fspec->mtime, get_stat_mtime (&stats)) == 0)
     return;
 
-  if (fspec != *prev_fspec)
-    {
-      if (print_headers)
-        write_header (name);
-      *prev_fspec = fspec;
-    }
+  bool want_header = print_headers && (fspec != *prev_fspec);
 
-  uintmax_t bytes_read = dump_remainder (name, fspec->fd, COPY_TO_EOF);
+  uintmax_t bytes_read = dump_remainder (want_header, name, fspec->fd,
+                                         COPY_TO_EOF);
   fspec->size += bytes_read;
 
-  if (fflush (stdout) != 0)
-    die (EXIT_FAILURE, errno, _("write error"));
+  if (bytes_read)
+    {
+      *prev_fspec = fspec;
+      if (fflush (stdout) != 0)
+        die (EXIT_FAILURE, errno, _("write error"));
+    }
 }
 
 /* Attempt to tail N_FILES files forever, or until killed.
@@ -1792,7 +1799,7 @@ tail_bytes (const char *pretty_filename, int fd, uintmax_t n_bytes,
       *read_pos = current_pos;
     }
 
-  *read_pos += dump_remainder (pretty_filename, fd, n_bytes);
+  *read_pos += dump_remainder (false, pretty_filename, fd, n_bytes);
   return true;
 }
 
@@ -1816,7 +1823,7 @@ tail_lines (const char *pretty_filename, int fd, uintmax_t n_lines,
       int t = start_lines (pretty_filename, fd, n_lines, read_pos);
       if (t)
         return t < 0;
-      *read_pos += dump_remainder (pretty_filename, fd, COPY_TO_EOF);
+      *read_pos += dump_remainder (false, pretty_filename, fd, COPY_TO_EOF);
     }
   else
     {

diff --git a/tests/local.mk b/tests/local.mk
index 30d6d7c87f5bffd84871889e0c948dfd5fa65576..27170240e935751000d81cbe45cd3bb9373bdf32 100644 (file)
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -248,6 +248,7 @@ all_tests =                                 \
   tests/misc/date-next-dow.pl                  \
   tests/misc/ptx-overrun.sh                    \
   tests/misc/xstrtol.pl                                \
+  tests/tail-2/overlay-headers.sh              \
   tests/tail-2/pid.sh                          \
   tests/misc/od.pl                             \
   tests/misc/od-endian.sh                      \

diff --git a/tests/tail-2/overlay-headers.sh b/tests/tail-2/overlay-headers.sh
new file mode 100755 (executable)
index 0000000..cc4f789
--- /dev/null
+++ b/tests/tail-2/overlay-headers.sh
@@ -0,0 +1,81 @@
+#!/bin/sh
+# inotify-based tail would output redundant headers for
+# overlapping inotify events while it was suspended
+
+# 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 <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
+print_ver_ tail sleep
+
+# Function to count number of lines from tail
+# while ignoring transient errors due to resource limits
+countlines_ ()
+{
+  grep -Ev 'inotify (resources exhausted|cannot be used)' out | wc -l
+}
+
+# Function to check the expected line count in 'out'.
+# Called via retry_delay_().  Sleep some time - see retry_delay_() - if the
+# line count is still smaller than expected.
+wait4lines_ ()
+{
+  local delay=$1
+  local elc=$2   # Expected line count.
+  [ "$(countlines_)" -ge "$elc" ] || { sleep $delay; return 1; }
+}
+
+# Speedup the non inotify case
+fastpoll='---dis -s.1 --max-unchanged-stats=1'
+
+# Terminate any background tail process
+cleanup_() {
+  kill $pid 2>/dev/null && wait $pid;
+  kill $sleep 2>/dev/null && wait $sleep
+}
+
+echo start > file1 || framework_failure_
+echo start > file2 || framework_failure_
+
+# Use this as a way to gracefully terminate tail
+env sleep 20 & sleep=$!
+
+tail $fastpoll --pid=$sleep -f file1 file2 > out & pid=$!
+
+kill -0 $pid || fail=1
+
+# Wait for 5 initial lines
+retry_delay_ wait4lines_ .1 6 5 || fail=1
+
+# Suspend tail so single read() caters for multiple inotify events
+kill -STOP $pid || fail=1
+
+# Interleave writes to files to generate overlapping inotify events
+echo line >> file1 || framework_failure_
+echo line >> file2 || framework_failure_
+echo line >> file1 || framework_failure_
+echo line >> file2 || framework_failure_
+
+# Resume tail processing
+kill -CONT $pid || fail=1
+
+# Wait for 8 more lines
+retry_delay_ wait4lines_ .1 6 13 || fail=1
+
+kill $sleep && wait || framework_failure_
+
+test "$(countlines_)" = 13 || fail=1
+
+Exit $fail