GNU bug report logs - #27368
Minor concern: Confusing tail warning

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 27368 in the body.
You can then email your comments to 27368 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Charlie Hagedorn <charlie.hagedorn <at> gmail.com>
To: bug-coreutils <at> gnu.org
Subject: Minor concern: Confusing tail warning
Date: Wed, 14 Jun 2017 16:03:07 -0700

[Message part 1 (text/plain, inline)]

Hi!

Thank you for maintaining such useful and reliable tools.

Today I came across an unexpected warning in tail. The warning is intended
to handle this case:

[:~]$ tail -f
tail: warning: following standard input indefinitely is ineffective

which is both important and fun.

Today, however, I was surprised to see it appear in this context:

[:~]$ tail -f < /dev/ttyUSB0  > data.dat
tail: warning: following standard input indefinitely is ineffective

The warning was confusing and perhaps inappropriate, as this call actually
*does* something, and is very effective at doing what I want; streaming the
port's output into data.dat.

(Importantly, for reasons I don't yet understand, tail -f /dev/ttyUSB0 >
data.dat does not reliably tail the port; it redirects only one line of
output instead of a continuous stream).

System is Debian Jessie -- coreutils v8.23-4.

Thanks!

Charlie

[Message part 2 (text/html, inline)]

Message #8 received at 27368 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Charlie Hagedorn <charlie.hagedorn <at> gmail.com>, 27368 <at> debbugs.gnu.org
Subject: Re: bug#27368: Minor concern: Confusing tail warning
Date: Thu, 15 Jun 2017 02:40:46 -0700

On 14/06/17 16:03, Charlie Hagedorn wrote:
> Hi!
> 
> Thank you for maintaining such useful and reliable tools.
> 
> Today I came across an unexpected warning in tail. The warning is intended
> to handle this case:
> 
> [:~]$ tail -f
> tail: warning: following standard input indefinitely is ineffective
> 
> which is both important and fun.
> 
> Today, however, I was surprised to see it appear in this context:
> 
> [:~]$ tail -f < /dev/ttyUSB0  > data.dat
> tail: warning: following standard input indefinitely is ineffective
> 
> The warning was confusing and perhaps inappropriate, as this call actually
> *does* something, and is very effective at doing what I want; streaming the
> port's output into data.dat.

Right. The above command will go into non inotify blocking mode
and will thus work as you expect.  Note tail will not output
anything until the first time read() returns nothing.
The following patch should suppress the warning if we would be using this mode.

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"));
+      }
   }

> (Importantly, for reasons I don't yet understand, tail -f /dev/ttyUSB0 >
> data.dat does not reliably tail the port; it redirects only one line of
> output instead of a continuous stream).

That looks like another case where we should be disabling inotify.
I.E. you can see this waits forever for inotify events if you hit ctrl-d a few times:

  strace tail -f /dev/tty

Ah yes that was discussed at http://bugs.gnu.org/21265
I suppose we should improve things here with a couple of patches.

1. Disable inotify if any non regular files (except fifo/pipe)
(the kernel should really disallow this, but best handle I think)

2. Generalise the warning in the above patch to be:
if (n_files > 1 && any_device_or_tty)
  printf ("warning: following a stdin/device in combination with other inputs is ineffective")

cheers,
Pádraig

Message #13 received at 27368-done <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Charlie Hagedorn <charlie.hagedorn <at> gmail.com>, 27368-done <at> debbugs.gnu.org
Subject: Re: bug#27368: Minor concern: Confusing tail warning
Date: Sat, 17 Jun 2017 01:32:34 -0700

[Message part 1 (text/plain, inline)]

On 15/06/17 02:40, Pádraig Brady wrote:
> On 14/06/17 16:03, Charlie Hagedorn wrote:
>> Hi!
>>
>> Thank you for maintaining such useful and reliable tools.
>>
>> Today I came across an unexpected warning in tail. The warning is intended
>> to handle this case:
>>
>> [:~]$ tail -f
>> tail: warning: following standard input indefinitely is ineffective
>>
>> which is both important and fun.
>>
>> Today, however, I was surprised to see it appear in this context:
>>
>> [:~]$ tail -f < /dev/ttyUSB0  > data.dat
>> tail: warning: following standard input indefinitely is ineffective
>>
>> The warning was confusing and perhaps inappropriate, as this call actually
>> *does* something, and is very effective at doing what I want; streaming the
>> port's output into data.dat.
> 
> Right. The above command will go into non inotify blocking mode
> and will thus work as you expect.  Note tail will not output
> anything until the first time read() returns nothing.
> The following patch should suppress the warning if we would be using this mode.
> 
> 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"));
> +      }
>    }
> 
>> (Importantly, for reasons I don't yet understand, tail -f /dev/ttyUSB0 >
>> data.dat does not reliably tail the port; it redirects only one line of
>> output instead of a continuous stream).
> 
> That looks like another case where we should be disabling inotify.
> I.E. you can see this waits forever for inotify events if you hit ctrl-d a few times:
> 
>   strace tail -f /dev/tty
> 
> Ah yes that was discussed at http://bugs.gnu.org/21265
> I suppose we should improve things here with a couple of patches.
> 
> 1. Disable inotify if any non regular files (except fifo/pipe)
> (the kernel should really disallow this, but best handle I think)
> 
> 2. Generalise the warning in the above patch to be:
> if (n_files > 1 && any_device_or_tty)
>   printf ("warning: following a stdin/device in combination with other inputs is ineffective")

Two proposed patches for this are attached.

cheers,
Pádraig.

[tail-pipe.diff (text/x-patch, attachment)]

Message #16 received at 27368 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: 27368 <at> debbugs.gnu.org, Pádraig Brady <P <at> draigbrady.com>, 
 charlie.hagedorn <at> gmail.com
Cc: 27368-done <at> debbugs.gnu.org
Subject: Re: bug#27368: Minor concern: Confusing tail warning
Date: Sat, 17 Jun 2017 07:35:45 -0700

On Sat, Jun 17, 2017 at 1:32 AM, Pádraig Brady <P <at> draigbrady.com> wrote:
...
> Two proposed patches for this are attached.

Nice fixes. Thank you!

In the NEWS addition:

   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.

Please don't use "will" in these blurbs.
I.e., use the present tense instead. Referencing the future makes
sense only now, prior to the release.

   tail -f now exits immediately when the output is piped
   and the reader of the pipe terminates.

   tail -f no longer erroneously warns about being ineffective
   when following a single tty, as the simple blocking loop used
   is effective in this case.

   tail -f /dev/tty is now supported by avoiding inotify when any
   non regular file is specified, as inotify is ineffective with these.
   [bug introduced with inotify support added in coreutils-7.5]

----------------------
In this new function, please move the declaration of "i" into the for-loop:

+static bool
+any_non_regular (const struct File_spec *f, size_t n_files)
+{
+  size_t i;
+
+  for (i = 0; i < n_files; i++)

Message #22 received at 27368 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: 27368 <at> debbugs.gnu.org
Subject: Re: bug#27368: Minor concern: Confusing tail warning
Date: Sat, 17 Jun 2017 14:30:29 -0700

On 17/06/17 07:35, Jim Meyering wrote:
> On Sat, Jun 17, 2017 at 1:32 AM, Pádraig Brady <P <at> draigbrady.com> wrote:
> ...
>> Two proposed patches for this are attached.
> 
> Nice fixes. Thank you!
> 
> In the NEWS addition:
> 
>    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.

> Please don't use "will" in [NEWS].
> I.e., use the present tense instead. Referencing the future makes
> sense only now, prior to the release.
> 
> ----------------------
> In this new function, please move the declaration of "i" into the for-loop:
> 
> +static bool
> +any_non_regular (const struct File_spec *f, size_t n_files)
> +{
> +  size_t i;
> +
> +  for (i = 0; i < n_files; i++)
> 

Cool.
Thanks for the review

Message #25 received at 27368 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: 27368 <at> debbugs.gnu.org
Subject: Re: bug#27368: Minor concern: Confusing tail warning
Date: Sat, 17 Jun 2017 15:57:02 -0700

On 17/06/17 14:30, Pádraig Brady wrote:
> On 17/06/17 07:35, Jim Meyering wrote:
>> In this new function, please move the declaration of "i" into the for-loop:
>>
>> +static bool
>> +any_non_regular (const struct File_spec *f, size_t n_files)
>> +{
>> +  size_t i;
>> +
>> +  for (i = 0; i < n_files; i++)

I did that in about 100 instances at:
http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=1379bdc

cheers,
Pádraig

Message #28 received at 27368 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Pádraig Brady <P <at> draigbrady.com>
Cc: 27368 <at> debbugs.gnu.org
Subject: Re: bug#27368: Minor concern: Confusing tail warning
Date: Sat, 17 Jun 2017 16:47:24 -0700

On Sat, Jun 17, 2017 at 3:57 PM, Pádraig Brady <P <at> draigbrady.com> wrote:
> On 17/06/17 14:30, Pádraig Brady wrote:
>> On 17/06/17 07:35, Jim Meyering wrote:
>>> In this new function, please move the declaration of "i" into the for-loop:
>>>
>>> +static bool
>>> +any_non_regular (const struct File_spec *f, size_t n_files)
>>> +{
>>> +  size_t i;
>>> +
>>> +  for (i = 0; i < n_files; i++)
>
> I did that in about 100 instances at:
> http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=1379bdc

Nice! Thank you.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.