Giuseppe Scrivano wrote: > Jim Meyering <[email protected]> writes: >> Hi Giuseppe, >> >> I've realized that there is a good way to remove the ugly exclusion >> that currently disables inotify-based tail -f when --pid is specified. >> Instead of the existing while-1-loop around code that reads the inotify >> FD, we can use a loop that polls that single FD with a 1-2-second timeout. >> Then, it can periodically run the usual PID-alive check. >> Are you interested in doing that? >> >> Between this, the ls -1U bug, and the btrfs O(1) copy, >> I'm now expecting to defer the release by at least a week. > > I modified tail as you suggested.
Thanks! A couple of points: >>From 862c9d934dc2ee188e9fe29985e463e2ec4d16ca Mon Sep 17 00:00:00 2001 > From: Giuseppe Scrivano <[email protected]> > 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 | 49 ++++++++++++++++++++++++++++++++++++++----------- > 1 files changed, 38 insertions(+), 11 deletions(-) > > diff --git a/src/tail.c b/src/tail.c > index fd44e22..74bda55 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; > @@ -1174,6 +1177,8 @@ tail_forever_inotify (int wd, struct File_spec *f, > size_t n_files) > char *evbuf; > size_t evbuf_off = 0; > size_t len = 0; > + fd_set rfd; > + struct timeval select_timeout; Please move these declarations down into the scope where they are used. > wd_table = hash_initialize (n_files, NULL, wd_hasher, wd_comparator, NULL); > if (! wd_table) > @@ -1253,6 +1258,31 @@ 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) > + { > + 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")); > + > + /* Check if the process we are monitoring is still alive. */ > + if (kill (pid, 0) != 0 && errno != EPERM) > + break; > + > + if (n_descriptors == 0) > + continue; 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. _______________________________________________ Bug-coreutils mailing list [email protected] http://lists.gnu.org/mailman/listinfo/bug-coreutils
