[Top][All Lists]
[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
- Re: inotify back end for tail -f on linux, (continued)
- Re: inotify back end for tail -f on linux, Giuseppe Scrivano, 2009/06/04
- Re: inotify back end for tail -f on linux, Jim Meyering, 2009/06/06
- Re: inotify back end for tail -f on linux, Giuseppe Scrivano, 2009/06/06
- Re: inotify back end for tail -f on linux, Jim Meyering, 2009/06/06
- Re: inotify back end for tail -f on linux, Jim Meyering, 2009/06/06
- Re: inotify back end for tail -f on linux, Giuseppe Scrivano, 2009/06/06
- Re: inotify back end for tail -f on linux, Jim Meyering, 2009/06/06
- Re: inotify back end for tail -f on linux, Giuseppe Scrivano, 2009/06/07
- Re: inotify back end for tail -f on linux, Jim Meyering, 2009/06/07
- Re: inotify back end for tail -f on linux, Jim Meyering, 2009/06/07
- Re: inotify back end for tail -f on linux,
Jim Meyering <=
- Re: inotify back end for tail -f on linux, Giuseppe Scrivano, 2009/06/11
- Re: inotify back end for tail -f on linux, Jim Meyering, 2009/06/11
- Re: inotify back end for tail -f on linux, Jim Meyering, 2009/06/13
- Re: inotify back end for tail -f on linux, Giuseppe Scrivano, 2009/06/14
- Re: inotify back end for tail -f on linux, Jim Meyering, 2009/06/15
- Re: inotify back end for tail -f on linux, Pádraig Brady, 2009/06/25
- Re: inotify back end for tail -f on linux, Jim Meyering, 2009/06/25
- Re: inotify back end for tail -f on linux, Pádraig Brady, 2009/06/25
- Re: inotify back end for tail -f on linux, Jim Meyering, 2009/06/25
- Re: inotify back end for tail -f on linux, Jim Meyering, 2009/06/06