Giuseppe Scrivano wrote: > Jim Meyering <j...@meyering.net> writes: > >> No test is too small or too "obvious" to add ;-) >> >> Adding this simple test would be great. >> Thanks!
Hi Giuseppe, > I found another problem. The inotify version can't be used when --pid > is specified. There are other inotify events that I want to investigate > better if they can be used with --pid and in this case we can add them > later when the inotify back end is stable enough for me to break it > again :) What do you think? Good catch. > This is the updated version, I integrated your test-case in the > tests/tail-2/wait file and I added a new file (tests/tail-2/pid) with > additional tests for the --pid option. Thanks! >>From f0824b8b79c1fb22e9a48cbf831a87433fc4d3e8 Mon Sep 17 00:00:00 2001 > From: Giuseppe Scrivano <gscriv...@gnu.org> > Date: Tue, 2 Jun 2009 08:28:23 +0200 > Subject: [PATCH] tail: Use inotify if it is available. > ... > + tail uses inotify if it is present. We can be slightly more precise: tail uses inotify when possible > * Noteworthy changes in release 7.4 (2009-05-07) [stable] > > diff --git a/configure.ac b/configure.ac > index 4eb640e..a63ac0c 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -410,6 +410,8 @@ AM_GNU_GETTEXT_VERSION([0.15]) > # For a test of uniq: it uses the $LOCALE_FR envvar. > gt_LOCALE_FR > > +AC_CHECK_FUNCS([inotify_init], [AC_DEFINE([HAVE_INOTIFY], [1], [Check for > libinotify])]) > + We like to avoid adding lines that occupy more than 80 columns. How about something like this instead? The comment is the one that ends up in lib/config.h. This is not checking for a library. AC_CHECK_FUNCS([inotify_init], [AC_DEFINE([HAVE_INOTIFY], [1], [Define to 1 if you have usable inotify support.])]) Oh, and it'd be better to put that test in m4/jm-macros.m4, not in configure.ac. > AC_CONFIG_FILES( > Makefile > doc/Makefile ... > + if (len <= evbuff_off) > + { > + len = read (wd, evbuff, evlen); Please use safe_read here. Then you can drop the EINTR test/block below. > + evbuff_off = 0; > + > + if (len <= 0 && errno == EINVAL && max_realloc--) > + { > + len = 0; > + evlen *= 2; > + evbuff = xrealloc (evbuff, evlen); > + continue; > + } > + > + if (len <= 0 && errno == EINTR) > + { > + len = 0; > + continue; > + } > + > + if (len <= 0) > + error (EXIT_FAILURE, errno, _("error reading inotify event")); > + } ... > if (forever) > - tail_forever (F, n_files, sleep_interval); > + { > +#ifdef HAVE_INOTIFY > + int wd = inotify_init (); > + if (wd > 0 && pid == 0) > + tail_forever_inotify (wd, F, n_files); > +#endif > + tail_forever (F, n_files, sleep_interval); > + } > > if (have_read_stdin && close (STDIN_FILENO) < 0) > error (EXIT_FAILURE, errno, "-"); Thanks. I had to make some small changes: - chmod a+x tests/tail-2/{pid,wait} That works around this "make check" failure: --- t1 2009-06-06 09:50:14.000000000 +0200 +++ t2 2009-06-06 09:50:17.000000000 +0200 @@ -359,11 +359,9 @@ tail-2/assert-2 tail-2/big-4gb tail-2/infloop-1 -tail-2/pid tail-2/proc-ksyms tail-2/start-middle tail-2/tail-n0f -tail-2/wait touch/dangling-symlink touch/dir-1 touch/empty-file make[2]: *** [vc_exe_in_TESTS] Error 1 make[2]: *** Waiting for unfinished jobs.... -------------------------------------- Notice how when tail_forever_inotify returns, execution continues into tail_forever. It shouldn't. -------------------------------------- Notice how this waits, as it should: $ touch k; chmod 0 k; ./tail --pid $$ -F k ./tail: cannot open `k' for reading: Permission denied yet this exits immediately: [also, it'd be nice not to give two diagnostics about the same problem] $ touch k; chmod 0 k; ./tail -F k ./tail: cannot open `k' for reading: Permission denied ./tail: cannot watch `k': Permission denied [Exit 1] -------------------------------------- I haven't fixed that last problem. Please fold the following patch into yours for the next iteration. >From 2a200db8a5f3b36d23b179bc4343c647957d6d13 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyer...@redhat.com> Date: Sat, 6 Jun 2009 10:12:02 +0200 Subject: [PATCH] tail: minor changes * src/tail.c (main): Don't call inotify_init when waiting for a PID. [HAVE_INOTIFY]: Exit nonzero whenever tail_forever_inotify returns. Change "wd > 0" to "0 <= wd", since 0 is a valid file descriptor. --- src/tail.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/tail.c b/src/tail.c index 6d5f870..4d9270f 100644 --- a/src/tail.c +++ b/src/tail.c @@ -1929,9 +1929,15 @@ main (int argc, char **argv) if (forever) { #ifdef HAVE_INOTIFY - int wd = inotify_init (); - if (wd > 0 && pid == 0) - tail_forever_inotify (wd, F, n_files); + if (pid == 0) + { + int wd = inotify_init (); + if (0 <= wd) + tail_forever_inotify (wd, F, n_files); + + /* The only way the above returns is upon failure. */ + exit (EXIT_FAILURE); + } #endif tail_forever (F, n_files, sleep_interval); } -- 1.6.3.2.277.gd10543 _______________________________________________ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils