Re: tail -f: --pid *and* inotify

[Jim Meyering]

Giuseppe Scrivano wrote:

> Jim Meyering <address@hidden> writes:
>
>> A couple of points:
>>
>> Please move these declarations down into the scope where they are used.
>>
>>
>> It would be better not to perform the kill test after every
>> single select call when actively tailing files.
>> Considering how --pid is documented (in the texinfo manual),
>> it should be ok to call kill only when select times out (returns 0).
>> Of course, that means a constantly-running tail -f will
>> never check for process death, but that is consistent
>> with the documentation.
>
> Thanks for the advises.  I cleaned it following them.
>
> I was checking for the pid every time exactly to avoid the problem that
> it is never checked and can run indefinitely even if the process is
> already not running.
> Maybe we can add a real clock controlling how much time passed since
> last check or even simpler, a counter like "don't exit without check
> more than N consecutive times."  What do you think?

Thanks.
I've pushed it with this change to avoid a warning about the
PID parameter shadowing the global.  It's best always to enable
all warnings, if you can:

  ./configure --enable-gcc-warnings

then such "warnings" are promoted to errors and stop the build.

diff --git a/src/tail.c b/src/tail.c
index 1474b06..a73ffa2 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -1164,7 +1164,7 @@ wd_comparator (const void *e1, const void *e2)
    Check modifications using the inotify events system.  */

 static void
-tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, int pid,
+tail_forever_inotify (int wd, struct File_spec *f, size_t n_files,
                       double sleep_interval)
 {
   size_t i;
@@ -1256,7 +1256,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t 
n_files, int pid,

       struct inotify_event *ev;

-      /* If a process is watched be sure that read from wd will not block
+      /* When watching a PID, ensure that a read from WD will not block
          indefinetely.  */
       if (pid)
         {
@@ -1278,7 +1278,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t 
n_files, int pid,

           if (n_descriptors == 0)
             {
-              /* Check if the process we are monitoring is still alive.  */
+              /* See if the process we are monitoring is still alive.  */
               if (kill (pid, 0) != 0 && errno != EPERM)
                 break;

@@ -1978,7 +1978,7 @@ main (int argc, char **argv)
         error (0, errno, _("inotify cannot be used, reverting to polling"));
       else
         {
-          tail_forever_inotify (wd, F, n_files, pid, sleep_interval);
+          tail_forever_inotify (wd, F, n_files, sleep_interval);

           /* The only way the above returns is upon failure.  */
           exit (EXIT_FAILURE);