[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Bug#545422: coreutils: "tail -f -" fails
From: |
Giuseppe Scrivano |
Subject: |
Re: Bug#545422: coreutils: "tail -f -" fails |
Date: |
Tue, 22 Sep 2009 21:11:44 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.1.50 (gnu/linux) |
Hi Jim,
have you considered this patch for inclusion? I don't see a clearer way
to avoid polling without inotify fd support.
Regards,
Giuseppe
Giuseppe Scrivano <address@hidden> writes:
> This patch changes `tail' to handle stdin separately from inotify
> events, similar to what we are already doing when a --pid is specified.
>
> Regards,
> Giuseppe
>
>
> From f3010bebf9e25be9a83868b4ad9db2cc6cb6613f Mon Sep 17 00:00:00 2001
> From: Giuseppe Scrivano <address@hidden>
> Date: Mon, 7 Sep 2009 16:35:16 +0200
> Subject: [PATCH] tail: handle "-" properly
>
> * src/tail.c (tail_forever_inotify): Handle stdin (i.e., "-", but not
> /dev/stdin) separately from inotify.
> * tests/tail-2/wait: Ensure that when a stdin is watched, tail does not
> raise errors.
> ---
> src/tail.c | 176
> ++++++++++++++++++++++++++++++++++-------------------
> tests/tail-2/wait | 6 ++
> 2 files changed, 119 insertions(+), 63 deletions(-)
>
> diff --git a/src/tail.c b/src/tail.c
> index e3b9529..b817ecb 100644
> --- a/src/tail.c
> +++ b/src/tail.c
> @@ -134,7 +134,7 @@ struct File_spec
> int errnum;
>
> #if HAVE_INOTIFY
> - /* The watch descriptor used by inotify. */
> + /* The watch descriptor used by inotify, -1 on error, -2 if stdin. */
> int wd;
>
> /* The parent directory watch descriptor. It is used only
> @@ -1184,6 +1184,7 @@ tail_forever_inotify (int wd, struct File_spec *f,
> size_t n_files,
> char *evbuf;
> size_t evbuf_off = 0;
> size_t len = 0;
> + struct File_spec *stdin_spec = NULL;
>
> wd_table = hash_initialize (n_files, NULL, wd_hasher, wd_comparator, NULL);
> if (! wd_table)
> @@ -1196,6 +1197,34 @@ tail_forever_inotify (int wd, struct File_spec *f,
> size_t n_files,
> {
> if (!f[i].ignore)
> {
> + if (STREQ (f[i].name, "-"))
> + {
> + int old_flags = fcntl (f[i].fd, F_GETFL);
> + int new_flags = old_flags | O_NONBLOCK;
> +
> + stdin_spec = &f[i];
> + found_watchable = true;
> +
> + if (old_flags < 0
> + || (new_flags != old_flags
> + && fcntl (f[i].fd, F_SETFL, new_flags) == -1))
> + {
> + /* Don't update f[i].blocking if fcntl fails. */
> + if (S_ISREG (f[i].mode) && errno == EPERM)
> + {
> + /* This happens when using tail -f on a file with
> + the append-only attribute. */
> + }
> + else
> + error (EXIT_FAILURE, errno,
> + _("%s: cannot change stdin nonblocking mode"));
> + }
> + f[i].blocking = false;
> + f[i].wd = -2;
> + prev_wd = f[i].wd;
> + continue;
> + }
> +
> size_t fnlen = strlen (f[i].name);
> if (evlen < fnlen)
> evlen = fnlen;
> @@ -1235,6 +1264,8 @@ tail_forever_inotify (int wd, struct File_spec *f,
> size_t n_files,
> continue;
> }
>
> + prev_wd = f[i].wd;
> +
> if (hash_insert (wd_table, &(f[i])) == NULL)
> xalloc_die ();
>
> @@ -1245,8 +1276,6 @@ tail_forever_inotify (int wd, struct File_spec *f,
> size_t n_files,
> if (follow_mode == Follow_descriptor && !found_watchable)
> return;
>
> - prev_wd = f[n_files - 1].wd;
> -
> evlen += sizeof (struct inotify_event) + 1;
> evbuf = xmalloc (evlen);
>
> @@ -1259,12 +1288,12 @@ tail_forever_inotify (int wd, struct File_spec *f,
> size_t n_files,
> struct File_spec *fspec;
> uintmax_t bytes_read;
> struct stat stats;
> -
> + bool check_stdin = false;
> struct inotify_event *ev;
>
> - /* When watching a PID, ensure that a read from WD will not block
> - indefinetely. */
> - if (pid)
> + /* When watching a PID or stdin, ensure that a read from WD will not
> block
> + indefinitely. */
> + if (pid || stdin_spec)
> {
> fd_set rfd;
> struct timeval select_timeout;
> @@ -1284,78 +1313,92 @@ tail_forever_inotify (int wd, struct File_spec *f,
> size_t n_files,
>
> if (n_descriptors == 0)
> {
> - /* See if the process we are monitoring is still alive. */
> - if (kill (pid, 0) != 0 && errno != EPERM)
> - exit (EXIT_SUCCESS);
> + if (stdin_spec)
> + check_stdin = true;
> + if (pid)
> + {
> + /* See if the process we are monitoring is still alive. */
> + if (kill (pid, 0) != 0 && errno != EPERM)
> + exit (EXIT_SUCCESS);
>
> - continue;
> + if (!check_stdin)
> + continue;
> + }
> }
> }
>
> - if (len <= evbuf_off)
> + if (check_stdin)
> {
> - len = safe_read (wd, evbuf, evlen);
> - evbuf_off = 0;
> -
> - /* For kernels prior to 2.6.21, read returns 0 when the buffer
> - is too small. */
> - if ((len == 0 || (len == SAFE_READ_ERROR && errno == EINVAL))
> - && max_realloc--)
> + ev = NULL;
> + fspec = stdin_spec;
> + check_stdin = false;
> + }
> + else
> + {
> + if (len <= evbuf_off)
> {
> - len = 0;
> - evlen *= 2;
> - evbuf = xrealloc (evbuf, evlen);
> - continue;
> - }
> + len = safe_read (wd, evbuf, evlen);
> + evbuf_off = 0;
>
> - if (len == 0 || len == SAFE_READ_ERROR)
> - error (EXIT_FAILURE, errno, _("error reading inotify event"));
> - }
> + /* For kernels prior to 2.6.21, read returns 0 when the buffer
> + is too small. */
> + if ((len == 0 || (len == SAFE_READ_ERROR && errno == EINVAL))
> + && max_realloc--)
> + {
> + len = 0;
> + evlen *= 2;
> + evbuf = xrealloc (evbuf, evlen);
> + continue;
> + }
>
> - ev = (struct inotify_event *) (evbuf + evbuf_off);
> - evbuf_off += sizeof (*ev) + ev->len;
> + if (len == 0 || len == SAFE_READ_ERROR)
> + error (EXIT_FAILURE, errno, _("error reading inotify
> event"));
> + }
>
> - if (ev->len)
> - {
> - for (i = 0; i < n_files; i++)
> + ev = (struct inotify_event *) (evbuf + evbuf_off);
> + evbuf_off += sizeof (*ev) + ev->len;
> +
> + if (ev->len)
> {
> - /* With N=hundreds of frequently-changing files, this O(N^2)
> - process might be a problem. FIXME: use a hash table? */
> - if (f[i].parent_wd == ev->wd
> - && STREQ (ev->name, f[i].name + f[i].basename_start))
> - break;
> - }
> + for (i = 0; i < n_files; i++)
> + {
> + /* With N=hundreds of frequently-changing files, this
> O(N^2)
> + process might be a problem. FIXME: use a hash table?
> */
> + if (f[i].parent_wd == ev->wd
> + && STREQ (ev->name, f[i].name + f[i].basename_start))
> + break;
> + }
>
> - /* It is not a watched file. */
> - if (i == n_files)
> - continue;
> + /* It is not a watched file. */
> + if (i == n_files)
> + continue;
>
> - f[i].wd = inotify_add_watch (wd, f[i].name, inotify_wd_mask);
> + f[i].wd = inotify_add_watch (wd, f[i].name, inotify_wd_mask);
>
> - if (f[i].wd < 0)
> - {
> - error (0, errno, _("cannot watch %s"), quote (f[i].name));
> - continue;
> - }
> + if (f[i].wd < 0)
> + {
> + error (0, errno, _("cannot watch %s"), quote (f[i].name));
> + continue;
> + }
>
> - fspec = &(f[i]);
> - if (hash_insert (wd_table, fspec) == NULL)
> - xalloc_die ();
> + fspec = &(f[i]);
> + if (hash_insert (wd_table, fspec) == NULL)
> + xalloc_die ();
>
> - if (follow_mode == Follow_name)
> - recheck (&(f[i]), false);
> - }
> - else
> - {
> - struct File_spec key;
> - key.wd = ev->wd;
> - fspec = hash_lookup (wd_table, &key);
> + if (follow_mode == Follow_name)
> + recheck (&(f[i]), false);
> + }
> + else
> + {
> + struct File_spec key;
> + key.wd = ev->wd;
> + fspec = hash_lookup (wd_table, &key);
> + }
> }
> -
> if (! fspec)
> continue;
>
> - if (ev->mask & (IN_ATTRIB | IN_DELETE_SELF | IN_MOVE_SELF))
> + if (ev && ev->mask & (IN_ATTRIB | IN_DELETE_SELF | IN_MOVE_SELF))
> {
> if (ev->mask & (IN_DELETE_SELF | IN_MOVE_SELF))
> {
> @@ -1364,7 +1407,6 @@ tail_forever_inotify (int wd, struct File_spec *f,
> size_t n_files,
> }
> if (follow_mode == Follow_name)
> recheck (fspec, false);
> -
> continue;
> }
>
> @@ -1386,11 +1428,19 @@ tail_forever_inotify (int wd, struct File_spec *f,
> size_t n_files,
> fspec->size = stats.st_size;
> }
>
> - if (ev->wd != prev_wd)
> + if (fspec->mode == stats.st_mode
> + && (! S_ISREG (stats.st_mode) || fspec->size == stats.st_size)
> + && timespec_cmp (fspec->mtime, get_stat_mtime (&stats)) == 0)
> + continue;
> +
> + fspec->mtime = get_stat_mtime (&stats);
> + fspec->mode = stats.st_mode;
> +
> + if (fspec->wd != prev_wd)
> {
> if (print_headers)
> write_header (name);
> - prev_wd = ev->wd;
> + prev_wd = fspec->wd;
> }
>
> bytes_read = dump_remainder (name, fspec->fd, COPY_TO_EOF);
> diff --git a/tests/tail-2/wait b/tests/tail-2/wait
> index 62498d5..aa6e2dd 100755
> --- a/tests/tail-2/wait
> +++ b/tests/tail-2/wait
> @@ -42,6 +42,12 @@ for inotify in ---disable-inotify ''; do
> timeout 1 tail -s0.1 -f $inotify here 2>tail.err
> test $? = 124 || fail=1
>
> + timeout 1 tail -s0.1 -f $inotify < here 2>tail.err
> + test $? = 124 || fail=1
> +
> + echo hello | timeout 1 tail -s0.1 -f $inotify 2>tail.err
> + test $? = 124 || fail=1
> +
> # `tail -F' must wait in any case.
>
> timeout 1 tail -s0.1 -F $inotify here 2>>tail.err