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
_______________________________________________
Bug-coreutils mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/bug-coreutils
