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

[Giuseppe Scrivano]

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?

Regards,
Giuseppe



>From 65f2737fa6e2519fbccbad7d285ca8923a893057 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano <address@hidden>
Date: Sun, 26 Jul 2009 13:22:57 +0200
Subject: [PATCH] tail: use the inotify backend when a PID is specified.

* src/tail.c (tail_forever_inotify): When a PID is specified avoid read
to sleep indefinitely.  Check if the specified PID if it is still alive,
otherwise exit from tail.
(main): Use the new tail_forever_inotify interface.
---
 src/tail.c |   52 +++++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/src/tail.c b/src/tail.c
index fd44e22..1474b06 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -50,6 +50,8 @@
 #if HAVE_INOTIFY
 # include "hash.h"
 # include <sys/inotify.h>
+/* `select' is used by tail_forever_inotify.  */
+# include <sys/select.h>
 #endif
 
 /* The official name of this program (e.g., no `g' prefix).  */
@@ -1162,7 +1164,8 @@ 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)
+tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, int pid,
+                      double sleep_interval)
 {
   size_t i;
   unsigned int max_realloc = 3;
@@ -1253,6 +1256,36 @@ tail_forever_inotify (int wd, struct File_spec *f, 
size_t n_files)
 
       struct inotify_event *ev;
 
+      /* If a process is watched be sure that read from wd will not block
+         indefinetely.  */
+      if (pid)
+        {
+          fd_set rfd;
+          struct timeval select_timeout;
+          int n_descriptors;
+
+          FD_ZERO (&rfd);
+          FD_SET (wd, &rfd);
+
+          select_timeout.tv_sec = (time_t) sleep_interval;
+          select_timeout.tv_usec = 1000000000 * (sleep_interval
+                                                 - select_timeout.tv_sec);
+
+          n_descriptors = select (wd + 1, &rfd, NULL, NULL, &select_timeout);
+
+          if (n_descriptors == -1)
+            error (EXIT_FAILURE, errno, _("error monitoring inotify event"));
+
+          if (n_descriptors == 0)
+            {
+              /* Check if the process we are monitoring is still alive.  */
+              if (kill (pid, 0) != 0 && errno != EPERM)
+                break;
+
+              continue;
+            }
+        }
+
       if (len <= evbuf_off)
         {
           len = safe_read (wd, evbuf, evlen);
@@ -1940,18 +1973,15 @@ main (int argc, char **argv)
   if (forever)
     {
 #if HAVE_INOTIFY
-      if (pid == 0)
+      int wd = inotify_init ();
+      if (wd < 0)
+        error (0, errno, _("inotify cannot be used, reverting to polling"));
+      else
         {
-          int wd = inotify_init ();
-          if (wd < 0)
-            error (0, errno, _("inotify cannot be used, reverting to 
polling"));
-          else
-            {
-              tail_forever_inotify (wd, F, n_files);
+          tail_forever_inotify (wd, F, n_files, pid, sleep_interval);
 
-              /* The only way the above returns is upon failure.  */
-              exit (EXIT_FAILURE);
-            }
+          /* The only way the above returns is upon failure.  */
+          exit (EXIT_FAILURE);
         }
 #endif
       tail_forever (F, n_files, sleep_interval);
-- 
1.6.3.3