Jim Meyering <j...@meyering.net> writes: > I considered that and discussed the > trade-off in the comment I committed. > There have been systems and configurations with > nonexistent and unusable /dev/stdin files.
sorry, I didn't read you comment. 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 <gscriv...@gnu.org> 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 -- 1.6.3.3 -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org