bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: inotify back end for tail -f on linux


From: Jim Meyering
Subject: Re: inotify back end for tail -f on linux
Date: Thu, 11 Jun 2009 10:54:02 +0200

Giuseppe Scrivano wrote:
> The new version includes all these changes.
>
...
> +/* Tail N_FILES files forever, or until killed.
> +   Check modifications using the inotify events system.  */
> +static void
> +tail_forever_inotify (int wd, struct File_spec *f, int n_files)
> +{
> +  unsigned int i;
> +  unsigned int max_realloc = 3;
> +  Hash_table *wd_table;
> +
> +  bool found_watchable = false;
> +  size_t last;

Looking through this again, I wonder what "last" means.
So I inspect the uses...

> +  size_t evlen = 0;
> +  char *evbuf;
> +  size_t evbuf_off = 0;
> +  ssize_t len = 0;
> +
> +  wd_table = hash_initialize (n_files, NULL, wd_hasher, wd_comparator, NULL);
> +  if (! wd_table)
> +    xalloc_die ();
> +
> +  for (i = 0; i < n_files; i++)
> +    {
> +      if (!f[i].ignore)
> +        {
...
> +          f[i].wd = inotify_add_watch (wd, f[i].name,
> +                                       (IN_MODIFY | IN_ATTRIB
> +                                        | IN_DELETE_SELF | IN_MOVE_SELF));
> +
> +          if (last == old_wd)
> +            last = f[i].wd;

This comparison looks like it must use "last" uninitialized.

> +
...
> +        }
> +    }
> +
> +  if (follow_mode == Follow_descriptor && !found_watchable)
> +    return;
> +
> +  last = f[n_files - 1].wd;

...
[more below]

Perhaps "prev_wd" would be more accurate?
"last" can mean final or preceding, so is rarely a good variable name,
due to that ambiguity.  It'd be good to write a comment describing
it, too.

Speaking of which, please add a comment saying what
each of the two main loops in that function does.

Another regression:

    touch k; chmod 0 k; tail -F k

fails to open "k" (as it must), but even
when I run "chmod u+r k" in another window,
the inotify-based version of tail fails to open it.

When I repeat the experiment with non-inotify-based tail, it does
what I want:

    $ touch k; chmod 0 k; tail -F k
    tail: cannot open `k' for reading: Permission denied
    tail: `k' has become accessible




reply via email to

[Prev in Thread] Current Thread [Next in Thread]