Re: Bug#545422: coreutils: tail -f - fails
Bill Brelsford wrote:
Package: coreutils
Version: 7.5-3
Severity: normal
tail -f no longer works with stdin. E.g. commands such as
somecommand | tail -f -
somecommand | tail -f
tail -f /var/log/kern
fail with the message:
tail: cannot watch `-': No such file or directory
Worked under 7.4-2 and previous versions.
Thanks for the report.
I'm fixing it like this, upstream.
Test coming momentarily.
From 30269c9ca38c06b31a7c764c192562e3b0268725 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Mon, 7 Sep 2009 08:37:08 +0200
Subject: [PATCH] tail -f: work on - once again
* src/tail.c (main) [HAVE_INOTIFY]: When stdin (i.e., -, but not
/dev/stdin) is specified on the command line, don't use inotify.
Reported by Bill Brelsford in http://bugs.debian.org/545422.
* NEWS (Bug fixes): Mention it.
This bug was introduced in coreutils-7.5 via commit ae494d4b,
2009-06-02, tail: use inotify if it is available.
---
NEWS |9 +
src/tail.c | 14 +-
2 files changed, 22 insertions(+), 1 deletions(-)
diff --git a/NEWS b/NEWS
index b02d2da..5c7fb82 100644
--- a/NEWS
+++ b/NEWS
@@ -31,6 +31,15 @@ GNU coreutils NEWS-*-
outline -*-
Before, this would print nothing and wait: stdbuf -o 4K tail -f /etc/passwd
Note that this bug affects tail -f only when its standard output is buffered,
which is relatively unusual.
+ [bug introduced in coreutils-7.5]
+
+ tail -f once again works with standard input. inotify-enabled tail -f
+ would fail when operating on a nameless stdin. I.e., tail -f /etc/passwd
+ would say tail: cannot watch `-': No such file or directory, yet the
+ relatively baroque tail -f /dev/stdin /etc/passwd would work. Now, the
+ offending usage causes tail to revert to its conventional sleep-based
+ (i.e., not inotify-based) implementation.
+ [bug introduced in coreutils-7.5]
** New features
diff --git a/src/tail.c b/src/tail.c
index e3b9529..c53df9e 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -1982,7 +1982,19 @@ main (int argc, char **argv)
if (forever)
{
#if HAVE_INOTIFY
- if (!disable_inotify)
+ /* If the user specifies stdin via a command line argument of -,
+ or implicitly by providing no arguments, we won't use inotify.
+ Technically, on systems with a working /dev/stdin, we *could*,
+ but would it be worth it? Verifying that it's a real device
+ and hooked up to stdin is not trivial, while reverting to
+ non-inotify-based tail_forever is easy and portable. */
+ bool stdin_cmdline_arg = false;
+
+ for (i = 0; i n_files; i++)
+if (STREQ (file[i], -))
+ stdin_cmdline_arg = true;
+
+ if (!disable_inotify !stdin_cmdline_arg)
{
int wd = inotify_init ();
if (wd 0)
--
1.6.4.2.419.gab238
Re: [PATCH] cp, mv: do preserve extended attributes even for read-only source files
Hi,
Jim Meyering wrote:
Pádraig Brady wrote:
Ondřej Vašík wrote:
As reported in
http://lists.gnu.org/archive/html/bug-coreutils/2009-08/msg00342.html by
Ernest N. Mamikonyan, cp/mv fails to preserve extended attributes for
read-only source files.
Following patch fixes the issue for me, although maybe it's not perfect
solution. But I don't know about better one at the moment.
Test included...
Thanks for that, and especially the test.
It looks to me that the cause of this is actually a bug
in fsetxattr() (called by attr_copy_fd) as it's being
passed a writable fd, but still giving permission denied?
Hi Ondřej,
Thanks for working on that.
Note that your patch relaxes permissions
on the destination and does not restore them.
If you continue to work on this, please use the adjusted patch below.
It makes the test script detect that failure, and also removes most
of the redirections to /dev/null. Now that nearly all test-related
output is directed to a log file, there's no point in redirecting small
outputs like that, and seeing them in the log can even make it easier
to diagnose problems.
Since this problem affects only users of file systems
mounted with user_xattr, I may defer the fix until coreutils-7.7.
Ah, I knew I forgot to do something :). Thanks for spotting this.
Restoring to dest_mode ~omitted_permissions done in attached patch,
dropped redirections from the test as well. Additionally - I modified
the copy.c patch a bit - failure of mode change now doesn't mean that I
don't try to preserve extended attributes (as it still could pass).
Pádraig is right that it looks like some kind of bug in libattr and
fsetxattr() function, as the descriptor should be writable, anyway this
should workaround it - at least until they'll fix/change it or other way
of solution will be found.
Ok with passing to 7.7, although with such small impact and relatively
low danger, it could maybe included to 7.6 (if more snapshots will be
before real release).
Greetings,
Ondřej
From 5fca1fbaf2e7594496c854f4c3eef60bd3013697 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Ond=C5=99ej=20Va=C5=A1=C3=ADk?= ova...@redhat.com
Date: Thu, 3 Sep 2009 16:10:21 +0200
Subject: [PATCH] cp,mv: do preserve extended attributes even for read-only source files
* src/copy.c (copy_reg): Set mode on file descriptor to 0600 for copying
extended attributes to prevent failures when source file doesn't have
write access rights. Reported by Ernest N. Mamikonyan.
* tests/misc/xattr: Test that change.
* NEWS (Bug fixes): Mention it.
---
NEWS |4
src/copy.c | 21 +
tests/misc/xattr | 42 --
3 files changed, 49 insertions(+), 18 deletions(-)
diff --git a/NEWS b/NEWS
index 59270eb..8c511c6 100644
--- a/NEWS
+++ b/NEWS
@@ -15,6 +15,10 @@ GNU coreutils NEWS-*- outline -*-
cp --preserve=xattr no longer leaks resources on each preservation failure.
[bug introduced in coreutils-7.1]
+ cp --preserve=xattr and --archive now preserves extended attributes even
+ when the source file doesn't have write access rights
+ [bug introduced in coreutils-7.1]
+
dd now returns non-zero status if it encountered a write error while
printing a summary to stderr.
[bug introduced in coreutils-6.11]
diff --git a/src/copy.c b/src/copy.c
index e604ec5..9506fb4 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -850,10 +850,23 @@ copy_reg (char const *src_name, char const *dst_name,
set_author (dst_name, dest_desc, src_sb);
- if (x-preserve_xattr ! copy_attr_by_fd (src_name, source_desc,
- dst_name, dest_desc, x)
- x-require_preserve_xattr)
-return_val = false;
+ /* to allow copying extended attributes on read-only files, change
+ destination file descriptor access rights to 0600 for a while. */
+ if (x-preserve_xattr)
+ {
+ if (! fchmod_or_lchmod (dest_desc, dst_name, 0600))
+ {
+ if (!x-reduce_diagnostics || x-require_preserve_xattr)
+error (0, errno,
+ _(failed to set writable mode for %s, copying xattrs may fail),
+ quote (dst_name));
+ }
+ if (! copy_attr_by_fd (src_name, source_desc, dst_name, dest_desc, x)
+ x-require_preserve_xattr)
+ return_val = false;
+ fchmod_or_lchmod (dest_desc, dst_name, dst_mode ~omitted_permissions);
+ }
+
if (x-preserve_mode || x-move_mode)
{
diff --git a/tests/misc/xattr b/tests/misc/xattr
index a27e1f6..404085f 100755
--- a/tests/misc/xattr
+++ b/tests/misc/xattr
@@ -29,7 +29,7 @@ fi
# Skip this test if cp was built without xattr support:
touch src dest || framework_failure
-cp --preserve=xattr -n src dest 2/dev/null \
+cp --preserve=xattr -n src dest \
|| skip_test_ coreutils built without xattr support
# this code was taken from test mv/backup-is-src
@@ -46,13 +46,13 @@
seq, why so slow?
The conversion of everything to long doubles internally makes seq a lot slower than it needs to be in integer cases, I assume from the use of floating-point multiplication for every line of output: seq.c:257 x = first + i * step; $ time seq 100 /dev/null real0m1.616s user0m1.610s sys 0m0.004s $ time ./myseq 100 /dev/null real0m0.280s user0m0.275s sys 0m0.005s Would it be possible to detect arguments which require no floating-point, and handle them with integer addition? Cheers, Phil
Re: seq, why so slow?
Philip Rowlands wrote: The conversion of everything to long doubles internally makes seq a lot slower than it needs to be in integer cases, I assume from the use of floating-point multiplication for every line of output: seq.c:257 x = first + i * step; $ time seq 100 /dev/null real0m1.616s user0m1.610s sys 0m0.004s $ time ./myseq 100 /dev/null real0m0.280s user0m0.275s sys 0m0.005s Would it be possible to detect arguments which require no floating-point, and handle them with integer addition? I've noticed that too, but it wasn't on my priority list. Here are some alternatives and timings: $ time seq 100 /dev/null real0m1.236s $ time yes '' | nl -ba -w1 -s' ' | head -n100 /dev/null real0m0.623s $ time shuf -i 1-100 --random-source=/dev/zero /dev/null real0m0.568s # This uses ascii add from getlimits.c and is arbitrary precision $ time seq2 1 1 100 /dev/null real0m0.379s # simple for(...) printf(i); $ time seq.simple /dev/null real0m0.268s # cat -n uses ascii add optimised for +1 case (next_line_num) $ time yes '' | cat -n | head -n100 /dev/null real0m0.142s cheers, Pádraig.
Re: [PATCH] cp, mv: do preserve extended attributes even for read-only source files
Ondřej Vašík wrote: Ah, I knew I forgot to do something :). Thanks for spotting this. Restoring to dest_mode ~omitted_permissions done in attached patch, dropped redirections from the test as well. Additionally - I modified the copy.c patch a bit - failure of mode change now doesn't mean that I don't try to preserve extended attributes (as it still could pass). Pádraig is right that it looks like some kind of bug in libattr and fsetxattr() function, as the descriptor should be writable, anyway this should workaround it - at least until they'll fix/change it or other way of solution will be found. What's the best place to report that? It would be good to add a comment in the code that this is a workaround rather than expected behaviour (after confirming the bug of course). Ok with passing to 7.7, although with such small impact and relatively low danger, it could maybe included to 7.6 (if more snapshots will be before real release). To minimize side affects perhaps we should only do the chmod(600) if (geteuid () != 0 !access (src_name, W_OK)) ? cheers, Pádraig.
new snapshot available: coreutils-7.5.65-61cc6
There have been disproportionately many bug fixes since coreutils-7.5. It's an interesting mix of fixes for recent regressions and for a few older bugs. coreutils snapshot: http://meyering.net/cu/coreutils-ss.tar.gz http://meyering.net/cu/coreutils-ss.tar.xz http://meyering.net/cu/coreutils-ss.tar.gz.sig http://meyering.net/cu/coreutils-ss.tar.xz.sig aka http://meyering.net/cu/coreutils-7.5.65-61cc6.tar.gz http://meyering.net/cu/coreutils-7.5.65-61cc6.tar.xz There have been many changes in gnulib, too, and this snapshot includes the very latest. I expect to release coreutils-7.6 on Thursday or Friday. Here's the NEWS I expect to see for coreutils-7.6: ** Bug fixes cp, mv now ignore failure to preserve a symlink time stamp, when it is due to their running on a kernel older than what was implied by headers and libraries tested at configure time. [bug introduced in coreutils-7.5] cp --reflink --preserve now preserves attributes when cloning a file. [bug introduced in coreutils-7.5] cp --preserve=xattr no longer leaks resources on each preservation failure. [bug introduced in coreutils-7.1] dd now exits with non-zero status when it encounters a write error while printing a summary to stderr. [bug introduced in coreutils-6.11] df no longer requires that each command-line argument be readable [bug introduced in coreutils-7.3] ls -i now prints consistent inode numbers also for mount points. This makes ls -i DIR less efficient on systems with dysfunctional readdir, because ls must stat every file in order to obtain a guaranteed-valid inode number. [bug introduced in coreutils-6.0] tail -f (inotify-enabled) now flushes any initial output before blocking. Before, this would print nothing and wait: stdbuf -o 4K tail -f /etc/passwd Note that this bug affects tail -f only when its standard output is buffered, which is relatively unusual. [bug introduced in coreutils-7.5] tail -f once again works with standard input. inotify-enabled tail -f would fail when operating on a nameless stdin. I.e., tail -f /etc/passwd would say tail: cannot watch `-': No such file or directory, yet the relatively baroque tail -f /dev/stdin /etc/passwd would work. Now, the offending usage causes tail to revert to its conventional sleep-based (i.e., not inotify-based) implementation. [bug introduced in coreutils-7.5] ** New features cp --reflink accepts a new auto parameter which falls back to a standard copy if creating a copy-on-write clone is not possible. -- Changes in coreutils since 7.5 Eric Blake (5): dd: detect closed stderr build: avoid unused variable warnings on cygwin mv, cp: tweak LINK_FOLLOWS_SYMLINKS logic ln: add comments related to POSIX 2008 build: update from gnulib Jim Meyering (43): post-release administrivia build: update from gnulib build: update from *public* gnulib tests: skip (don't fail) a cp test, upon mount-related failure cp: ignore obscure failure to preserve symlink time stamps, global: convert indentation-TABs to spaces maint: teach make syntax-check the space-only indentation rule doc: HACKING: mention the new space-only indentation policy maint: remove Local Variables: indent-tabs-mode: nil from all sources maint: ensure we don't embed Emacs indent-tabs-mode setting lines tests: tail-2/assert: avoid risk of race condition tests: mkdir/selinux: avoid spurious failure on some SELinux systems build: stop earlier if touching ChangeLog fails build: prefix a few rules with $(AM_V_GEN) maint: ignore only man/*.1, not all *.1 files tests: cp/reflink-auto guard against a pathological $TMPDIR tests: move a coreutils-specific test from maint.mk to Makefile.am build: update from gnulib tests: other-fs-tmpdir: don't misbehave for quote-unfriendly $TMPDIR build: update bootstrap from gnulib doc: cp: update note on preserving symlink time stamps build: quiet make check in src/ maint: stdbuf: move a declaration; no-semantic-change maint: revert my stdbuf change: the result didn't even compile ls -i: print consistent inode numbers also for mount points maint: mbsalign.c: remove unnecessary assignment maint: tail: remove unnecessary initialization maint: dd: remove unnecessary initialization maint: shred: remove unnecessary initialization maint: chown, chgrp, chmod, chcon: remove unnecessary initialization maint: du: remove unnecessary initialization chcon, chmod, chgrp, chown, du: do not ignore fts_close failure build: update from gnulib df: don't fail due to an unreadable argument build: update gnulib submodule to latest tests: ls/stat-vs-dirent: avoid spurious test failure maint: remove unused file:
Re: [PATCH] cp, mv: do preserve extended attributes even for read-only source files
Ondřej Vašík wrote: Ah, I knew I forgot to do something :). Thanks for spotting this. Restoring to dest_mode ~omitted_permissions done in attached patch, dropped redirections from the test as well. Additionally - I modified the copy.c patch a bit - failure of mode change now doesn't mean that I don't try to preserve extended attributes (as it still could pass). Pádraig is right that it looks like some kind of bug in libattr and fsetxattr() function, as the descriptor should be writable, anyway this should workaround it - at least until they'll fix/change it or other way of solution will be found. Ok with passing to 7.7, although with such small impact and relatively low danger, it could maybe included to 7.6 (if more snapshots will be before real release). Thanks for the update. However, I'd rather avoid that permission-relaxing code completely. Not only does it appear to constitute a security problem when run by root, but it may also fail, when copying, as non-priveleged, to a file that is writable but owned by someone else.
Re: [PATCH] cp, mv: do preserve extended attributes even for read-only source files
Pádraig Brady wrote: Ondřej Vašík wrote: Ah, I knew I forgot to do something :). Thanks for spotting this. Restoring to dest_mode ~omitted_permissions done in attached patch, dropped redirections from the test as well. Additionally - I modified the copy.c patch a bit - failure of mode change now doesn't mean that I don't try to preserve extended attributes (as it still could pass). Pádraig is right that it looks like some kind of bug in libattr and fsetxattr() function, as the descriptor should be writable, anyway this should workaround it - at least until they'll fix/change it or other way of solution will be found. What's the best place to report that? It would be good to add a comment in the code that this is a workaround rather than expected behaviour (after confirming the bug of course). libattr upstream has mailing list xfs at oss.sgi.com , so maybe the best place is there. Ok with passing to 7.7, although with such small impact and relatively low danger, it could maybe included to 7.6 (if more snapshots will be before real release). To minimize side affects perhaps we should only do the chmod(600) if (geteuid () != 0 !access (src_name, W_OK)) ? Good idea, it would reduce possibility of security leak, playing with access rights is always a bit dangerous (although here we play with rights on destination descriptor, which is imho much more safe). Additionally - Jim is correct that for different owner 0600 rights are not sufficient for different owner of the file - and 0666 is too much devil-like ;) . Any idea? Greetings, Ondřej signature.asc Description: Toto je digitálně podepsaná část zprávy
Re: Bug#545422: coreutils: tail -f - fails
Hi Jim,
what do you think about the following solution? It avoids to revert to
the old polling mechanism using /dev/stdin instead of - to
inotify_add_watch.
Cheers,
Giuseppe
diff --git a/src/tail.c b/src/tail.c
index e3b9529..016b712 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -1152,6 +1152,12 @@ tail_forever (struct File_spec *f, size_t n_files,
double sleep_interval)
#if HAVE_INOTIFY
+static char const *
+map_inotify_fname (char const *name)
+{
+ return STREQ (name, -) ? /dev/stdin : name;
+}
+
static size_t
wd_hasher (const void *entry, size_t tabsize)
{
@@ -1226,7 +1232,8 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t
n_files,
}
}
- f[i].wd = inotify_add_watch (wd, f[i].name, inotify_wd_mask);
+ f[i].wd = inotify_add_watch (wd, map_inotify_fname (f[i].name),
+ inotify_wd_mask);
if (f[i].wd 0)
{
@@ -1330,7 +1337,8 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t
n_files,
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, map_inotify_fname (f[i].name),
+ inotify_wd_mask);
if (f[i].wd 0)
{
Jim Meyering j...@meyering.net writes:
Bill Brelsford wrote:
Package: coreutils
Version: 7.5-3
Severity: normal
tail -f no longer works with stdin. E.g. commands such as
somecommand | tail -f -
somecommand | tail -f
tail -f /var/log/kern
fail with the message:
tail: cannot watch `-': No such file or directory
Worked under 7.4-2 and previous versions.
Thanks for the report.
I'm fixing it like this, upstream.
Test coming momentarily.
From 30269c9ca38c06b31a7c764c192562e3b0268725 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Mon, 7 Sep 2009 08:37:08 +0200
Subject: [PATCH] tail -f: work on - once again
* src/tail.c (main) [HAVE_INOTIFY]: When stdin (i.e., -, but not
/dev/stdin) is specified on the command line, don't use inotify.
Reported by Bill Brelsford in http://bugs.debian.org/545422.
* NEWS (Bug fixes): Mention it.
This bug was introduced in coreutils-7.5 via commit ae494d4b,
2009-06-02, tail: use inotify if it is available.
---
NEWS |9 +
src/tail.c | 14 +-
2 files changed, 22 insertions(+), 1 deletions(-)
diff --git a/NEWS b/NEWS
index b02d2da..5c7fb82 100644
--- a/NEWS
+++ b/NEWS
@@ -31,6 +31,15 @@ GNU coreutils NEWS-*-
outline -*-
Before, this would print nothing and wait: stdbuf -o 4K tail -f /etc/passwd
Note that this bug affects tail -f only when its standard output is
buffered,
which is relatively unusual.
+ [bug introduced in coreutils-7.5]
+
+ tail -f once again works with standard input. inotify-enabled tail -f
+ would fail when operating on a nameless stdin. I.e., tail -f /etc/passwd
+ would say tail: cannot watch `-': No such file or directory, yet the
+ relatively baroque tail -f /dev/stdin /etc/passwd would work. Now, the
+ offending usage causes tail to revert to its conventional sleep-based
+ (i.e., not inotify-based) implementation.
+ [bug introduced in coreutils-7.5]
** New features
diff --git a/src/tail.c b/src/tail.c
index e3b9529..c53df9e 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -1982,7 +1982,19 @@ main (int argc, char **argv)
if (forever)
{
#if HAVE_INOTIFY
- if (!disable_inotify)
+ /* If the user specifies stdin via a command line argument of -,
+ or implicitly by providing no arguments, we won't use inotify.
+ Technically, on systems with a working /dev/stdin, we *could*,
+ but would it be worth it? Verifying that it's a real device
+ and hooked up to stdin is not trivial, while reverting to
+ non-inotify-based tail_forever is easy and portable. */
+ bool stdin_cmdline_arg = false;
+
+ for (i = 0; i n_files; i++)
+if (STREQ (file[i], -))
+ stdin_cmdline_arg = true;
+
+ if (!disable_inotify !stdin_cmdline_arg)
{
int wd = inotify_init ();
if (wd 0)
--
1.6.4.2.419.gab238
Re: Bug#545422: coreutils: tail -f - fails
Giuseppe Scrivano wrote:
what do you think about the following solution? It avoids to revert to
the old polling mechanism using /dev/stdin instead of - to
inotify_add_watch.
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.
commit cdfb703c5da31a798557722df516e0d01dac828a
Author: Jim Meyering meyer...@redhat.com
Date: Mon Sep 7 08:37:08 2009 +0200
tail -f: handle -/stdin once again
* src/tail.c (main) [HAVE_INOTIFY]: When stdin (i.e., -, or no args,
but not /dev/stdin) is specified on the command line, don't use inotify.
Reported by Bill Brelsford in http://bugs.debian.org/545422.
* tests/tail-2/follow-stdin: New file. Test for this.
* tests/Makefile.am (TESTS): Add the test.
* NEWS (Bug fixes): Mention it.
This bug was introduced in coreutils-7.5 via commit ae494d4b,
2009-06-02, tail: use inotify if it is available.
diff --git a/src/tail.c b/src/tail.c
index e3b9529..c53df9e 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -1982,7 +1982,19 @@ main (int argc, char **argv)
if (forever)
{
#if HAVE_INOTIFY
- if (!disable_inotify)
+ /* If the user specifies stdin via a command line argument of -,
+ or implicitly by providing no arguments, we won't use inotify.
+ Technically, on systems with a working /dev/stdin, we *could*,
+ but would it be worth it? Verifying that it's a real device
+ and hooked up to stdin is not trivial, while reverting to
+ non-inotify-based tail_forever is easy and portable. */
+ bool stdin_cmdline_arg = false;
+
+ for (i = 0; i n_files; i++)
+if (STREQ (file[i], -))
+ stdin_cmdline_arg = true;
+
+ if (!disable_inotify !stdin_cmdline_arg)
{
int wd = inotify_init ();
if (wd 0)
Re: new snapshot available: coreutils-7.5.65-61cc6
Eric Blake wrote: According to Jim Meyering on 9/7/2009 3:09 AM: ls -i now prints consistent inode numbers also for mount points. This makes ls -i DIR less efficient on systems with dysfunctional readdir, because ls must stat every file in order to obtain a guaranteed-valid inode number. [bug introduced in coreutils-6.0] On systems with working d_type, is it sufficient to limit the stat() to situations where we know that the entry might be a directory (DT_DIR or DT_UNKNOWN)? In other words, are we guaranteed that mount points can only occur atop directories, and that we can avoid the stat() for regular files? Recently I saw a note in which someone mentioned bind mounts on non-directories, so at least in general, that may not be sufficient. However, I don't have the reference handy and can't dig right now.
Re: [PATCH] cp, mv: do preserve extended attributes even for read-only source files
Ondřej Vašík wrote: Pádraig Brady wrote: To minimize side affects perhaps we should only do the chmod(600) if (geteuid () != 0 !access (src_name, W_OK)) ? Good idea, it would reduce possibility of security leak, playing with access rights is always a bit dangerous (although here we play with rights on destination descriptor, which is imho much more safe). Additionally - Jim is correct that for different owner 0600 rights are not sufficient for different owner of the file - and 0666 is too much devil-like ;) . Any idea? preserve_xattr before preserve_ownership ? cheers, Pádraig.
Re: new snapshot available: coreutils-7.5.65-61cc6
Eric Blake e...@byu.net writes: In other words, are we guaranteed that mount points can only occur atop directories, and that we can avoid the stat() for regular files? You can also (bind-)mount regular files. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different.
Re: new snapshot available: coreutils-7.5.65-61cc6
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Andreas Schwab on 9/7/2009 5:17 AM: Eric Blake e...@byu.net writes: In other words, are we guaranteed that mount points can only occur atop directories, and that we can avoid the stat() for regular files? You can also (bind-)mount regular files. In other words, d_type is unreliable, just like d_ino. Yuck. - -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkqk7SYACgkQ84KuGfSFAYDKPgCdEzhdHCZzmZNzd3TbtUVctszD nuIAnjYCyP9PCMcSlsA4GgSOwJcwY8u4 =67pI -END PGP SIGNATURE-
Re: new snapshot available: coreutils-7.5.65-61cc6
Jim Meyering wrote: Eric Blake wrote: According to Jim Meyering on 9/7/2009 3:09 AM: ls -i now prints consistent inode numbers also for mount points. This makes ls -i DIR less efficient on systems with dysfunctional readdir, because ls must stat every file in order to obtain a guaranteed-valid inode number. [bug introduced in coreutils-6.0] On systems with working d_type, is it sufficient to limit the stat() to situations where we know that the entry might be a directory (DT_DIR or DT_UNKNOWN)? In other words, are we guaranteed that mount points can only occur atop directories, and that we can avoid the stat() for regular files? Recently I saw a note in which someone mentioned bind mounts on non-directories, so at least in general, that may not be sufficient. However, I don't have the reference handy and can't dig right now. It may have been me: http://lists.gnu.org/archive/html/bug-coreutils/2009-03/msg00321.html I use bind mounted files on my stateless system here. cheers, Pádraig.
Re: [PATCH] cp, mv: do preserve extended attributes even for read-only source files
Pádraig Brady wrote:
Ondřej Vašík wrote:
Pádraig Brady wrote:
To minimize side affects perhaps we should only do the chmod(600)
if (geteuid () != 0 !access (src_name, W_OK)) ?
Good idea, it would reduce possibility of security leak, playing with
access rights is always a bit dangerous (although here we play with
rights on destination descriptor, which is imho much more safe).
Additionally - Jim is correct that for different owner 0600 rights are
not sufficient for different owner of the file - and 0666 is too much
devil-like ;) . Any idea?
preserve_xattr before preserve_ownership ?
Good idea, moved there and used that (geteuid () != 0 access
(src_name, W_OK)) construction - additionally I tried to reduce those
chmod calls (call for returning permissions only when the write_access
granting call was used) - so it should be safer now.
Anyway, added comment that real problem is in libattr and this is just
workaround and added FIXME. Better now?
Greetings,
Ondřej
From 3aae1734c715cb8246a4961f5bea5e6fa58c1d61 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Ond=C5=99ej=20Va=C5=A1=C3=ADk?= ova...@redhat.com
Date: Thu, 3 Sep 2009 16:10:21 +0200
Subject: [PATCH] cp,mv: do preserve extended attributes even for read-only source files
* src/copy.c (copy_reg): Set mode on file descriptor to 0600 for copying
extended attributes to prevent failures when source file doesn't have
write access rights. Reported by Ernest N. Mamikonyan.
* tests/misc/xattr: Test that change.
* NEWS (Bug fixes): Mention it.
---
NEWS |4
src/copy.c | 34 +-
tests/misc/xattr | 42 --
3 files changed, 61 insertions(+), 19 deletions(-)
diff --git a/NEWS b/NEWS
index 59270eb..8c511c6 100644
--- a/NEWS
+++ b/NEWS
@@ -15,6 +15,10 @@ GNU coreutils NEWS-*- outline -*-
cp --preserve=xattr no longer leaks resources on each preservation failure.
[bug introduced in coreutils-7.1]
+ cp --preserve=xattr and --archive now preserves extended attributes even
+ when the source file doesn't have write access rights
+ [bug introduced in coreutils-7.1]
+
dd now returns non-zero status if it encountered a write error while
printing a summary to stderr.
[bug introduced in coreutils-6.11]
diff --git a/src/copy.c b/src/copy.c
index e604ec5..c8f0a45 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -834,6 +834,35 @@ copy_reg (char const *src_name, char const *dst_name,
}
}
+ /* To allow copying extended attributes on read-only files, change
+ destination file descriptor access rights to 0600 for a while
+ if required.
+ Note: it's just workaround - fsetxattr from libattr needs write
+ access rights even with file descriptor opened with O_WRONLY
+ FIXME: remove that ugly access rights change. */
+ if (x-preserve_xattr)
+ {
+ bool access_unchanged = true;
+
+ if (geteuid () != 0 !access (src_name, W_OK)
+ (access_unchanged = fchmod_or_lchmod (dest_desc, dst_name, 0600)))
+ {
+ if (!x-reduce_diagnostics || x-require_preserve_xattr)
+error (0, errno,
+ _(failed to set writable mode for %s, copying xattrs may fail),
+ quote (dst_name));
+ }
+
+ if (! copy_attr_by_fd (src_name, source_desc, dst_name, dest_desc, x)
+ x-require_preserve_xattr)
+ return_val = false;
+
+ if (!access_unchanged)
+ fchmod_or_lchmod (dest_desc, dst_name,
+ dst_mode ~omitted_permissions);
+ }
+
+
if (x-preserve_ownership ! SAME_OWNER_AND_GROUP (*src_sb, sb))
{
switch (set_owner (x, dst_name, dest_desc, src_sb, *new_dst, sb))
@@ -850,11 +879,6 @@ copy_reg (char const *src_name, char const *dst_name,
set_author (dst_name, dest_desc, src_sb);
- if (x-preserve_xattr ! copy_attr_by_fd (src_name, source_desc,
- dst_name, dest_desc, x)
- x-require_preserve_xattr)
-return_val = false;
-
if (x-preserve_mode || x-move_mode)
{
if (copy_acl (src_name, source_desc, dst_name, dest_desc, src_mode) != 0
diff --git a/tests/misc/xattr b/tests/misc/xattr
index a27e1f6..404085f 100755
--- a/tests/misc/xattr
+++ b/tests/misc/xattr
@@ -29,7 +29,7 @@ fi
# Skip this test if cp was built without xattr support:
touch src dest || framework_failure
-cp --preserve=xattr -n src dest 2/dev/null \
+cp --preserve=xattr -n src dest \
|| skip_test_ coreutils built without xattr support
# this code was taken from test mv/backup-is-src
@@ -46,13 +46,13 @@ xattr_pair=$xattr_name=\$xattr_value\
# create new file and check its xattrs
touch a || framework_failure
getfattr -d a out_a || skip_test_ failed to get xattr of file
-grep -F $xattr_pair out_a /dev/null framework_failure
+grep -F $xattr_pair out_a framework_failure
# try to set user xattr on file
setfattr -n $xattr_name
Re: the unicode arrow
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Michal Svoboda on 9/6/2009 5:33 AM: When doing cp -va I can see neat quotes (depending on locale), as in „blah“, but the arrow is still composed of a dash and a greater-than symbol, as in -. Is there any plan to make the arrow also neat, using the unicore arrow symbol? This was discussed last month. The verdict is no. http://lists.gnu.org/archive/html/bug-coreutils/2009-08/msg00048.html - -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkqlCT8ACgkQ84KuGfSFAYAjJgCfVC6W/+UglmNz+uVIUx5AB70Y jygAnimlpjMMJc0gr7ZKhFdJmHX7uqoQ =NCtp -END PGP SIGNATURE-
Re: the unicode arrow
Hi, On Mon, Sep 07, 2009 at 07:23:12AM -0600, Eric Blake wrote: According to Michal Svoboda on 9/6/2009 5:33 AM: When doing cp -va I can see neat quotes (depending on locale), as in „blah“, but the arrow is still composed of a dash and a greater-than symbol, as in -. Is there any plan to make the arrow also neat, using the unicore arrow symbol? This was discussed last month. The verdict is no. http://lists.gnu.org/archive/html/bug-coreutils/2009-08/msg00048.html Actually that discussion was about ls -l, which has a POSIX specified output format. The cp -v case is different in that it is not POSIX specified and already uses special characters (those neat quotes). I'd say that cp -v could very well use an arrow symbol (but I don't intend to write a patch, since this is not important to me ;-). Erik -- But hey, don't listen to me - I like C++, and approve of Java. -- Andrew Morton
Re: [PATCH] cp, mv: do preserve extended attributes even for read-only source files
Ondřej Vašík wrote: Pádraig Brady wrote: Ondřej Vašík wrote: Pádraig Brady wrote: To minimize side affects perhaps we should only do the chmod(600) if (geteuid () != 0 !access (src_name, W_OK)) ? Good idea, it would reduce possibility of security leak, playing with access rights is always a bit dangerous (although here we play with rights on destination descriptor, which is imho much more safe). Additionally - Jim is correct that for different owner 0600 rights are not sufficient for different owner of the file - and 0666 is too much devil-like ;) . Any idea? preserve_xattr before preserve_ownership ? Good idea, moved there and used that (geteuid () != 0 access (src_name, W_OK)) construction - additionally I tried to reduce those chmod calls (call for returning permissions only when the write_access granting call was used) - so it should be safer now. Anyway, added comment that real problem is in libattr and this is just workaround and added FIXME. Better now? That looks much better, thanks. Since we're only doing u+rw, and we've already stat'd it's probably better to just (sb.mode S_IWUSR) rather than access(...). Also a couple of the if statements are indented too far. This should now be safer but as Jim says it only effects file systems mounted user_xattr. Perhaps we should wait until coreutils-7.7 and also feedback from libattr devs so as we can put an accurate comment in the code. cheers, Pádraig.
Re: the unicode arrow
Erik Auerswald wrote: Hi, On Mon, Sep 07, 2009 at 07:23:12AM -0600, Eric Blake wrote: According to Michal Svoboda on 9/6/2009 5:33 AM: When doing cp -va I can see neat quotes (depending on locale), as in „blah“, but the arrow is still composed of a dash and a greater-than symbol, as in -. Is there any plan to make the arrow also neat, using the unicore arrow symbol? This was discussed last month. The verdict is no. http://lists.gnu.org/archive/html/bug-coreutils/2009-08/msg00048.html Actually that discussion was about ls -l, which has a POSIX specified output format. The cp -v case is different in that it is not POSIX specified and already uses special characters (those neat quotes). What cp -v displays for quotes can be seen using `ls -l --quoting-style=locale` The German local on my system has the quotes the OP referred to: LANG=de_DE.utf8 ls -l --quoting-style=locale I'd say that cp -v could very well use an arrow symbol (but I don't intend to write a patch, since this is not important to me ;-). It's a trivial patch, though it's of marginal benefit. Also some scripts may be depending on the output from `cp -v`, so I'm not on for changing it. cheers, Pádraig.
Re: the unicode arrow
On Mon, 7 Sep 2009, Eric Blake wrote: This was discussed last month. The verdict is no. http://lists.gnu.org/archive/html/bug-coreutils/2009-08/msg00048.html This list archive has done strange things with character encodings which make the discussion difficult to follow. Something along the way has converted all the non-ascii characters into 'Ã', e.g. Pádraig into PÃdraig. Anyone know who maintains lists.gnu.org (or is this a mailman/mhonarc issue)? Cheers, Phil
Re: the unicode arrow
Philip Rowlands wrote: On Mon, 7 Sep 2009, Eric Blake wrote: This was discussed last month. The verdict is no. http://lists.gnu.org/archive/html/bug-coreutils/2009-08/msg00048.html This list archive has done strange things with character encodings which make the discussion difficult to follow. Something along the way has converted all the non-ascii characters into 'Ã', e.g. Pádraig into PÃdraig. gmane's interface displays them properly: http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/17612/focus=17617 There's also the marc.info mirror, but it seems to mis-handle multi-byte output: http://marc.info/?t=12495917586r=1w=2
Re: the unicode arrow
Philip Rowlands wrote:
On Mon, 7 Sep 2009, Eric Blake wrote:
This was discussed last month. The verdict is no.
http://lists.gnu.org/archive/html/bug-coreutils/2009-08/msg00048.html
This list archive has done strange things with character encodings which
make the discussion difficult to follow. Something along the way has
converted all the non-ascii characters into 'Ã', e.g. Pádraig into PÃdraig.
Interestingly it's just that particular mail that seems to be mangled.
Note the l alias mentioned there can't take parameters, so I'll
paste an l() function here to see if it has the same issue:
# quick dir listing with latest files/dirs at the bottom,
# prettify symlink arrows.
# using eval to precompute the tput sequences.
eval
l() {
ls -lrt --color=always \\...@\ |
sed 's/ - / $(tput bold)▪▶$(tput sgr0) /'
}
If the above is mangled in the archives, one can get l() from:
http://www.pixelbeat.org/settings/.bashrc
Anyone know who maintains lists.gnu.org (or is this a mailman/mhonarc
issue)?
cheers,
Pádraig.
Re: the unicode arrow
Pádraig Brady wrote 1276 bytes: Also some scripts may be depending on the output from `cp -v`, so I'm not on for changing it. The output is already changed (the neat quotes). And one can use LC_ALL=POSIX to get rid of both.
Re: the unicode arrow
Pádraig Brady wrote: Philip Rowlands wrote: On Mon, 7 Sep 2009, Eric Blake wrote: This was discussed last month. The verdict is no. http://lists.gnu.org/archive/html/bug-coreutils/2009-08/msg00048.html This list archive has done strange things with character encodings which make the discussion difficult to follow. Something along the way has converted all the non-ascii characters into 'Ã', e.g. Pádraig into PÃdraig. Interestingly it's just that particular mail that seems to be mangled. Actually it seems to be randomly mangling the mails. Seems like some state is not initialized for multibyte routines somewhere. cheers, Pádraig.
Re: Bug#545422: coreutils: tail -f - fails
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 ||
Re: new snapshot available: coreutils-7.5.65-61cc6
Hi Jim, On Mon, Sep 07, 2009 at 11:09:21AM +0200, Jim Meyering wrote: There have been disproportionately many bug fixes since coreutils-7.5. It's an interesting mix of fixes for recent regressions and for a few older bugs. coreutils snapshot: http://meyering.net/cu/coreutils-ss.tar.gz http://meyering.net/cu/coreutils-ss.tar.xz http://meyering.net/cu/coreutils-ss.tar.gz.sig http://meyering.net/cu/coreutils-ss.tar.xz.sig aka http://meyering.net/cu/coreutils-7.5.65-61cc6.tar.gz http://meyering.net/cu/coreutils-7.5.65-61cc6.tar.xz I've run the non-root checks without failures on my debian/sid x86 (32 bit) system. All 356 tests passed (32 tests were not run) All 135 tests passed (16 tests were not run) Erik
Re: new snapshot available: coreutils-7.5.65-61cc6
Erik Auerswald wrote: ... http://meyering.net/cu/coreutils-7.5.65-61cc6.tar.gz http://meyering.net/cu/coreutils-7.5.65-61cc6.tar.xz Hi Erik, I've run the non-root checks without failures on my debian/sid x86 (32 bit) system. Good to know. That's one I hadn't tested. Thanks!
Re: Bug#545422: coreutils: tail -f - fails
Giuseppe Scrivano wrote: 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. Thanks! This sounds good in principle, but it's too invasive for 7.6. I'll look at it later.
Re: the unicode arrow
Pádraig Brady p...@draigbrady.com writes:
# quick dir listing with latest files/dirs at the bottom,
# prettify symlink arrows.
# using eval to precompute the tput sequences.
eval
l() {
ls -lrt --color=always \\...@\ |
sed 's/ - / $(tput bold)▪▶$(tput sgr0) /'
FWIW, I find this arrow pretty ugly. Why not →?
Andreas.
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
And now for something completely different.
ln can't backup directories
Hi all, I'm failing in creating a symlink where the destination is a directory and do a backup of this directory. This is what I tried: $ mkdir a $ ln -s --no-target-directory --backup=numbered b a ln: `a': cannot overwrite directory Looking at the code I can seen the reason, the backup rename() is never executed, because the check whether the target is a directory comes first. Any hints whether this is a bug of ln or how I can create the link while preserving the destination as an backup are more than welcomed. Thanks. Bert
Re: the unicode arrow
Andreas Schwab wrote:
Pádraig Brady p...@draigbrady.com writes:
# quick dir listing with latest files/dirs at the bottom,
# prettify symlink arrows.
# using eval to precompute the tput sequences.
eval
l() {
ls -lrt --color=always \\...@\ |
sed 's/ - / $(tput bold)▪▶$(tput sgr0) /'
FWIW, I find this arrow pretty ugly. Why not →?
Because I can't see it with my font (Bitstream Vera Mono 10).
Feel free to change it in your copy of l() :)
cheers,
Pádraig.
Re: new snapshot available: coreutils-7.5.65-61cc6
Jim Meyering wrote: There have been disproportionately many bug fixes since coreutils-7.5. It's an interesting mix of fixes for recent regressions and for a few older bugs. Passed Skipped Failed \- Fedora core 5 x86 | 346 42 0 Fedora 11 x86 | 341 47 0 Solaris 10 x86| 325 61 2 Solaris 9 x86 build failed with: fstatat.c, line 39: undefined symbol: AT_SYMLINK_NOFOLLOW Solaris 10 failures: tail-2/flush-initial (patch attached) ls/color-clear-to-eol (due to sed ignoring last line without \n) cheers, Pádraig. From 14427d9174b880d469d8c5083fba6d24429b2a2b Mon Sep 17 00:00:00 2001 From: =?utf-8?q?P=C3=A1draig=20Brady?= p...@draigbrady.com Date: Mon, 7 Sep 2009 17:03:08 +0100 Subject: [PATCH] tests: tail-2/flush-initial should not rely on stdbuf * tests/tail-2/flush-initial: stdbuf is not built on all systems. In addition it's redundant since stdout will already be buffered since we're redirecting to file. So just call tail without using stdbuf. --- tests/tail-2/flush-initial |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/tests/tail-2/flush-initial b/tests/tail-2/flush-initial index 515b29d..e0d79fe 100755 --- a/tests/tail-2/flush-initial +++ b/tests/tail-2/flush-initial @@ -25,7 +25,9 @@ fi fail=0 echo line in || fail=1 -stdbuf --output=1K tail -f in out +# Output should be buffered since we're writing to file +# so we're depending on the flush to write out +tail -f in out tail_pid=$! # Wait for 1 second for the file to be flushed. -- 1.6.2.5
Re: merge rename and rename-dest-slash
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Eric Blake on 9/7/2009 10:30 AM: Any objections to deleting the rename-dest-slash module? It performs a subset of the rename module, Correction. As currently written, rename.m4 checks whether: rm -rf d1 d2 mkdir d1 rename(d1/,d2) fails, but POSIX 2008 permits this case, so the test is wrong. But POSIX _does_ require rm -rf d1 d2 touch d1 rename(d1/,d2) to fail (in other words, the existing code needs to account for whether a directory is being renamed, rather than blindly rejecting trailing slash). Meanwhile, rename-dest-slash.m4 checks whether: rm -rf d1 d2 mkdir d1 rename(d1,d2/) fails, which POSIX 2008 does indeed require to fail. But note: rm -rf d1 d2 mkdir d1 d2 rename(d1,d2/) is required to pass. All of this is independent of the cygwin 1.5 bug, where POSIX requires any rename attempt targeting an explicit . or .. to fail with EINVAL. I guess it's time for me to add a unit test. - -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkqlPvIACgkQ84KuGfSFAYCxZACgsXNKwV8AjPrPTZPN0dfHEeIX gfMAnAx25gaUe73an4YHzI+bi5vaOcVq =BknH -END PGP SIGNATURE-
[PATCH] tests: tail-2/infloop-1: avoid rare test failure on a busy system
I've seen this test fail a few times, recently, but it's not easy to trigger. With this change, I saw no failure in 60 iterations. From 494fed027114d63719439b399a7602f8d0384bcf Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Mon, 7 Sep 2009 11:39:19 +0200 Subject: [PATCH] tests: tail-2/infloop-1: avoid rare test failure on a busy system * tests/tail-2/infloop-1: Sleep 3 seconds, not 1, but in increments of 0.1 second. Before, this test would fail ~1 time in 20 via make -j9 check on a quad-core system. Correct comment. --- tests/tail-2/infloop-1 | 12 +--- 1 files changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/tail-2/infloop-1 b/tests/tail-2/infloop-1 index 6a456de..72d51d9 100755 --- a/tests/tail-2/infloop-1 +++ b/tests/tail-2/infloop-1 @@ -27,16 +27,22 @@ yes t yes_pid=$! while :; do test -s t break + sleep .1 done tail -n 1 t tail_pid=$! kill $yes_pid # This test is racy, and can fail under unusual circumstances. -# On a busy system, yes will fail to write -# (and hence fail to be killed by SIGPIPE) in that 1-second interval. +# On a very busy system, tail will fail to notice that $yes_pid is gone. # Then the following kill will succeed and cause this test to fail. -sleep 1 + +# Wait for up to 3 seconds for tail to detect the death of $yes_pid. +for i in $(seq 30); do +kill -0 $tail_pid || break +echo sleep 0.1s +sleep .1 +done fail=0 kill $tail_pid fail=1 || : -- 1.6.4.2.419.gab238
Re: new snapshot available: coreutils-7.5.65-61cc6
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Pádraig Brady on 9/7/2009 10:22 AM: Solaris 9 x86 build failed with: fstatat.c, line 39: undefined symbol: AT_SYMLINK_NOFOLLOW Hmm. That should be taken care of by the gnulib fcntl.h replacement. Are we missing a #include? Can you post the full failure? Solaris 10 failures: tail-2/flush-initial (patch attached) Hmm. We should reword the NEWS entry (right now, it states that the failure was rare because stdbuf was not always used; but your analysis of this failure proves that it can indeed be more common). ls/color-clear-to-eol (due to sed ignoring last line without \n) In autoconf, we frequently work around that sort of problem by using echo to append a blank line to the captured output and expected output. Do you need me to submit a patch? - -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkqlQAEACgkQ84KuGfSFAYBYxgCeMfSXwOoKsh/XCWmSDjpL+dw4 PlkAnjpnG7/3N7Q0NmomLU4cjEDKWNwb =/JrE -END PGP SIGNATURE-
Re: new snapshot available: coreutils-7.5.65-61cc6
Pádraig Brady wrote: Jim Meyering wrote: There have been disproportionately many bug fixes since coreutils-7.5. It's an interesting mix of fixes for recent regressions and for a few older bugs. Passed Skipped Failed \- Fedora core 5 x86 | 346 42 0 Fedora 11 x86 | 341 47 0 Solaris 10 x86| 325 61 2 Solaris 9 x86 build failed with: fstatat.c, line 39: undefined symbol: AT_SYMLINK_NOFOLLOW Solaris 10 failures: tail-2/flush-initial (patch attached) ls/color-clear-to-eol (due to sed ignoring last line without \n) Thanks for the testing! From 14427d9174b880d469d8c5083fba6d24429b2a2b Mon Sep 17 00:00:00 2001 From: =?utf-8?q?P=C3=A1draig=20Brady?= p...@draigbrady.com Date: Mon, 7 Sep 2009 17:03:08 +0100 Subject: [PATCH] tests: tail-2/flush-initial should not rely on stdbuf * tests/tail-2/flush-initial: stdbuf is not built on all systems. In addition it's redundant since stdout will already be buffered since we're redirecting to file. So just call tail without using stdbuf. Good change. Please push that.
Re: windows.h: No such file or directory
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
According to Jim Meyering on 8/22/2009 1:58 AM:
John Taylor wrote:
I'm trying to build coreutils-7.4 with a cross-compilator (gcc-4.4.1)
that runs on i686-unknown-linux-gnu and produces code for
x86_64-unknown-linux-gnu. When building coreutils, I get the following
error message:
../../coreutils-7.4/lib/rename.c:33:21: error: windows.h: No such file
or directory
...
That is an unfortunate consequence of the fact that the rename
module must perform a so-called run test to determine whether
it must work around certain rename bugs.
When cross-compiling, it assumes the worst.
However, in your case, the m4 macro could obviously do better,
knowing the target is linux/gnu-based.
A patch would be welcome.
I'm pushing this, which improves the rename module (after first making the
stdio module easier to modify) to cross-compile to non-mingw scenarios.
If we then follow through with my proposal to remove the rename-dest-slash
module in favor of rename, then the compilation error mentioned above
should also be solved. Then there's still the matter of writing a unit
test and improving behavior for other known or discovered bugs.
- --
Don't work too hard, make some time for fun as well!
Eric Blake e...@byu.net
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAkqlQ2UACgkQ84KuGfSFAYDNrQCghIVews7NvrqUxPGppwuGmmLJ
ZyEAniFCZalzsO+dGsbx135iqlkJuOdL
=ZDOK
-END PGP SIGNATURE-
From bd24844d02c2add6a689c2963d35b9f9bb216147 Mon Sep 17 00:00:00 2001
From: Eric Blake e...@byu.net
Date: Mon, 7 Sep 2009 05:51:20 -0600
Subject: [PATCH 1/3] getcwd: minor cleanups
* lib/getcwd.c (AT_FDCWD): Delete; rely on fcntl.h instead.
(is_ENAMETOOLONG): Delete; ENAMETOOLONG is portable.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog|4
lib/getcwd.c | 16 +---
2 files changed, 5 insertions(+), 15 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index f7ac99c..771a9a9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
2009-09-07 Eric Blake e...@byu.net
+ getcwd: minor cleanups
+ * lib/getcwd.c (AT_FDCWD): Delete; rely on fcntl.h instead.
+ (is_ENAMETOOLONG): Delete; ENAMETOOLONG is portable.
+
openat: provide more convenience names
* modules/faccessat (configure.ac): Add C witness.
* lib/unistd.in.h (readlinkat): Fix typo.
diff --git a/lib/getcwd.c b/lib/getcwd.c
index 2da1aee..6658ed5 100644
--- a/lib/getcwd.c
+++ b/lib/getcwd.c
@@ -59,20 +59,6 @@
#include limits.h
-/* Work around a bug in Solaris 9 and 10: AT_FDCWD is positive. Its
- value exceeds INT_MAX, so its use as an int doesn't conform to the
- C standard, and GCC and Sun C complain in some cases. */
-#if 0 AT_FDCWD AT_FDCWD == 0xffd19553
-# undef AT_FDCWD
-# define AT_FDCWD (-3041965)
-#endif
-
-#ifdef ENAMETOOLONG
-# define is_ENAMETOOLONG(x) ((x) == ENAMETOOLONG)
-#else
-# define is_ENAMETOOLONG(x) 0
-#endif
-
#ifndef MAX
# define MAX(a, b) ((a) (b) ? (b) : (a))
#endif
@@ -164,7 +150,7 @@ __getcwd (char *buf, size_t size)
# undef getcwd
dir = getcwd (buf, size);
- if (dir || (errno != ERANGE !is_ENAMETOOLONG (errno) errno != ENOENT))
+ if (dir || (errno != ERANGE errno != ENAMETOOLONG errno != ENOENT))
return dir;
#endif
--
1.6.3.3.334.g916e1
From 1132a93bd6177115d1047846c62dc96fb7facb56 Mon Sep 17 00:00:00 2001
From: Eric Blake e...@byu.net
Date: Mon, 7 Sep 2009 10:16:42 -0600
Subject: [PATCH 2/3] stdio: sort witness names
* modules/stdio (Makefile.am): Sort replacements.
* m4/stdio_h.m4 (gl_STDIO_H_DEFAULTS): Likewise.
* lib/stdio.in.h: Likewise.
Signed-off-by: Eric Blake e...@byu.net
---
ChangeLog |5 +
lib/stdio.in.h | 631
m4/stdio_h.m4 | 104 +-
modules/stdio | 98 +-
4 files changed, 421 insertions(+), 417 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 771a9a9..e3af685 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
2009-09-07 Eric Blake e...@byu.net
+ stdio: sort witness names
+ * modules/stdio (Makefile.am): Sort replacements.
+ * m4/stdio_h.m4 (gl_STDIO_H_DEFAULTS): Likewise.
+ * lib/stdio.in.h: Likewise.
+
getcwd: minor cleanups
* lib/getcwd.c (AT_FDCWD): Delete; rely on fcntl.h instead.
(is_ENAMETOOLONG): Delete; ENAMETOOLONG is portable.
diff --git a/lib/stdio.in.h b/lib/stdio.in.h
index 9dfb679..495baca 100644
--- a/lib/stdio.in.h
+++ b/lib/stdio.in.h
@@ -68,154 +68,6 @@
extern C {
#endif
-
-#if @GNULIB_FPRINTF_POSIX@
-# if @REPLACE_FPRINTF@
-# define fprintf rpl_fprintf
-extern int fprintf (FILE *fp, const char *format, ...)
- __attribute__ ((__format__ (__printf__, 2, 3)));
-# endif
-#elif @GNULIB_FPRINTF@ @REPLACE_STDIO_WRITE_FUNCS@
Re: new snapshot available: coreutils-7.5.65-61cc6
Eric Blake wrote:
According to Pádraig Brady on 9/7/2009 10:22 AM:
Solaris 9 x86 build failed with:
fstatat.c, line 39: undefined symbol: AT_SYMLINK_NOFOLLOW
Hmm. That should be taken care of by the gnulib fcntl.h replacement.
Are we missing a #include? Can you post the full failure?
Solaris 10 failures:
tail-2/flush-initial (patch attached)
Hmm. We should reword the NEWS entry (right now, it states that the
failure was rare because stdbuf was not always used; but your analysis of
this failure proves that it can indeed be more common).
ls/color-clear-to-eol (due to sed ignoring last line without \n)
In autoconf, we frequently work around that sort of problem by using echo
to append a blank line to the captured output and expected output. Do you
need me to submit a patch?
Here's one, albeit not yet tested on Solaris:
From 9547a78bda1d4f01a782c869be248b5e0faf31f9 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Mon, 7 Sep 2009 19:36:21 +0200
Subject: [PATCH] tests: ls/color-clear-to-eol: append NL to accommodate old sed
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* tests/ls/color-clear-to-eol: Some vendor sed programs fail
to operate on lines that are not NL-terminated.
This affects at least Solaris 10's /bin/sed.
Reported by Pádraig Brady.
---
tests/ls/color-clear-to-eol |6 +-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/tests/ls/color-clear-to-eol b/tests/ls/color-clear-to-eol
index 92cf8a8..fd65ced 100755
--- a/tests/ls/color-clear-to-eol
+++ b/tests/ls/color-clear-to-eol
@@ -30,11 +30,15 @@ e='\33'
color_code='0;31;42'
c_pre=$e[0m$e[${color_code}m
c_post=$e[0m$e[K\n$e[m
-printf $c_pre$long_name$c_post exp || framework_failure
+printf $c_pre$long_name$c_post\n exp || framework_failure
fail=0
env TERM=xterm COLUMNS=80 LS_COLORS=*.foo=$color_code TIME_STYLE=+T \
ls -og --color=always $long_name out || fail=1
+
+# Append a newline, to accommodate less-capable versions of sed.
+echo out || fail=1
+
sed 's/.*T //' out k mv k out
compare out exp || fail=1
--
1.6.4.2.419.gab238
Re: new snapshot available: coreutils-7.5.65-61cc6
Eric Blake wrote: Solaris 10 failures: tail-2/flush-initial (patch attached) Hmm. We should reword the NEWS entry (right now, it states that the failure was rare because stdbuf was not always used; but your analysis of this failure proves that it can indeed be more common). Do you know of many uses of tail -f that write to a file? Most of the ones I've seen would output to a tty or similar device.
Re: new snapshot available: coreutils-7.5.65-61cc6
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Jim Meyering on 9/7/2009 11:41 AM: Eric Blake wrote: Solaris 10 failures: tail-2/flush-initial (patch attached) Hmm. We should reword the NEWS entry (right now, it states that the failure was rare because stdbuf was not always used; but your analysis of this failure proves that it can indeed be more common). Do you know of many uses of tail -f that write to a file? Most of the ones I've seen would output to a tty or similar device. Good point. I've reread the text, and although it mentions stdbuf, it does not imply that stdbuf is the only way to get this behavior. So I'm okay with the existing wording: tail -f (inotify-enabled) now flushes any initial output before blocking. Before, this would print nothing and wait: stdbuf -o 4K tail -f /etc/passwd Note that this bug affects tail -f only when its standard output is buffered, which is relatively unusual. - -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkqlRrQACgkQ84KuGfSFAYCt3wCfZ7cVFNW2mmp+CQV9BBrDE405 IsgAn2K0T/8Q5GxseWXjDd1Zh/ojp6e9 =GreE -END PGP SIGNATURE-
misc/cat-buf test failure
Hi Pádraig, On my 6th run of make check, the cat-buf test failed. It's an inherently racy test. If the dd process is starved until 2 is printed, it will end up printing both lines to out. Here's the code: # Use a fifo rather than a pipe in the tests below # so that the producer (cat) will wait until the # consumer (dd) opens the fifo therefore increasing # the chance that dd will read the data from each # write separately. mkfifo fifo || framework_failure echo '1' exp dd count=1 if=fifo out (echo '1'; sleep .2; echo '2') | cat -v fifo wait #for dd to complete compare out exp || fail=1 Exit $fail We could allow either one line or both, but then how can the test fail? Alternatives: print a warning (a la skip_test_) and pass the test, or just skip it when we lose the race. No big deal.
[PATCH] tests: misc/cat-buf: clean up syntax
While I was looking at this test... From a4a864da365fe70eb3a69fd4347f8f747a258efd Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Mon, 7 Sep 2009 20:23:03 +0200 Subject: [PATCH] tests: misc/cat-buf: clean up syntax * tests/misc/cat-buf: Don't suppress dd's stderr. Remove useless quotes. --- tests/misc/cat-buf |8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/misc/cat-buf b/tests/misc/cat-buf index fb9ae88..d37f9f4 100755 --- a/tests/misc/cat-buf +++ b/tests/misc/cat-buf @@ -30,11 +30,11 @@ fi # write separately. mkfifo fifo || framework_failure -echo '1' exp +echo 1 exp -dd count=1 if=fifo out 2 err -(echo '1'; sleep .2; echo '2') | cat -v fifo -wait #for dd to complete +dd count=1 if=fifo out +(echo 1; sleep .2; echo 2) | cat -v fifo +wait # for dd to complete compare out exp || fail=1 -- 1.6.4.2.419.gab238
[PATCH] tail: don't give up on inotify mode for an already-ignored -
My recent change in this area wasn't quite right. For example, before, : k; ./tail -f - k . would revert to sleep-based implementation. With this patch, since - ends up being ignored, the inotify-based implementation is used. From a8d26b3ce1630b6e9213b79d213ad7c699ee9861 Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Mon, 7 Sep 2009 20:56:38 +0200 Subject: [PATCH] tail: don't give up on inotify mode for an already-ignored - * src/tail.c (main): Adjust today's change to honor the F[i].ignore flag that may have been set in tail_file. --- src/tail.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/tail.c b/src/tail.c index c53df9e..9288007 100644 --- a/src/tail.c +++ b/src/tail.c @@ -1991,7 +1991,7 @@ main (int argc, char **argv) bool stdin_cmdline_arg = false; for (i = 0; i n_files; i++) -if (STREQ (file[i], -)) +if (!F[i].ignore STREQ (F[i].name, -)) stdin_cmdline_arg = true; if (!disable_inotify !stdin_cmdline_arg) -- 1.6.4.2.419.gab238
Re: tail -f problem
Ulrich Drepper wrote:
Jim Meyering wrote:
Thanks for the report!
Ulrich and Ren reported that the command,
echo foobar | tail -c3 -f
would block indefinitely, while POSIX says that it must not.
Here's how I expect to fix this.
Maybe I'm splitting hairs, but rather than
call this a bug in NEWS, (it's more like GIGO) I've put
it in a new POSIX conformance category.
This seems small and safe enough that
I'm leaning toward including it in coreutils-7.6,
but I'll wait for a second opinion and/or review.
From d4d7daeb391faafda795dff87212bbfe9611d26c Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Mon, 7 Sep 2009 22:10:10 +0200
Subject: [PATCH 1/2] tail: ignore -f for piped-stdin, as POSIX requires
* src/tail.c (main): Tailing a pipe forever is not useful,
and POSIX specifies that tail ignore the -f when there is no
file argument and stdin is a FIFO or pipe. So we do that.
In addition, GNU tail excludes - arguments from the list of files
to tail forever, when the associated file descriptor is connected
to a FIFO or pipe. Before this change, :|tail -f would hang.
Reported by Ren Yang and Ulrich Drepper.
* tests/tail-2/pipe-f: Test for this.
* tests/tail-2/pipe-f2: Ensure tail doesn't exit early for a fifo.
* tests/Makefile.am (TESTS): Add these tests.
* NEWS (POSIX conformance): Mention it.
---
NEWS |6 ++
THANKS |1 +
src/tail.c | 16 +++-
tests/Makefile.am|2 ++
tests/tail-2/pipe-f | 32
tests/tail-2/pipe-f2 | 37 +
6 files changed, 93 insertions(+), 1 deletions(-)
create mode 100755 tests/tail-2/pipe-f
create mode 100755 tests/tail-2/pipe-f2
diff --git a/NEWS b/NEWS
index a5a6094..9efe5fc 100644
--- a/NEWS
+++ b/NEWS
@@ -46,6 +46,12 @@ GNU coreutils NEWS-*-
outline -*-
cp --reflink accepts a new auto parameter which falls back to
a standard copy if creating a copy-on-write clone is not possible.
+** POSIX conformance
+
+ tail -f now ignores - when stdin is a pipe or FIFO, per POSIX.
+ Now, : | tail -f terminates immediately. Before, it'd block indefinitely.
+ [the old behavior dates back to the original implementation]
+
* Noteworthy changes in release 7.5 (2009-08-20) [stable]
diff --git a/THANKS b/THANKS
index 2410866..c6655eb 100644
--- a/THANKS
+++ b/THANKS
@@ -490,6 +490,7 @@ Ralph Loaderloa...@maths.ox.ac.uk
Raul Miller m...@magenta.com
Raúl Núñez de Arenas Coronado r...@pleyades.net
Reuben Thomas r...@sc3d.org
+Ren Yangry...@redhat.com
Richard A Downing richard.down...@bcs.org.uk
Richard Braakmand...@xs4all.nl
Richard Dawer...@phekda.freeserve.co.uk
diff --git a/src/tail.c b/src/tail.c
index 9288007..d75d9b3 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -1979,7 +1979,21 @@ main (int argc, char **argv)
for (i = 0; i n_files; i++)
ok = tail_file (F[i], n_units);
- if (forever)
+ /* When there is no FILE operand and stdin is a pipe or FIFO
+ POSIX requires that tail ignore the -f option.
+ Since we allow multiple FILE operands, we extend that to say:
+ ignore any - operand that corresponds to a pipe or FIFO. */
+ size_t n_viable = 0;
+ for (i = 0; i n_files; i++)
+{
+ if (STREQ (F[i].name, -) !F[i].ignore
+ 0 = F[i].fd S_ISFIFO (F[i].mode))
+F[i].ignore = true;
+ else
+++n_viable;
+}
+
+ if (forever n_viable)
{
#if HAVE_INOTIFY
/* If the user specifies stdin via a command line argument of -,
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 6b3c2b1..42a12cf 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -429,6 +429,8 @@ TESTS = \
tail-2/big-4gb \
tail-2/flush-initial \
tail-2/follow-stdin \
+ tail-2/pipe-f\
+ tail-2/pipe-f2 \
tail-2/proc-ksyms\
tail-2/start-middle \
touch/dangling-symlink \
diff --git a/tests/tail-2/pipe-f b/tests/tail-2/pipe-f
new file mode 100755
index 000..b9f6ae3
--- /dev/null
+++ b/tests/tail-2/pipe-f
@@ -0,0 +1,32 @@
+#!/bin/sh
+# ensure that :|tail -f doesn't hang, per POSIX
+
+# Copyright (C) 2009 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the
Re: tail -f problem
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Jim Meyering on 9/7/2009 2:48 PM: This seems small and safe enough that I'm leaning toward including it in coreutils-7.6, but I'll wait for a second opinion and/or review. +** POSIX conformance + + tail -f now ignores - when stdin is a pipe or FIFO, per POSIX. + Now, : | tail -f terminates immediately. Before, it'd block indefinitely. + [the old behavior dates back to the original implementation] Let's spell out the abbreviation (it'd is colloquial; using it would sounds better). Beyond that, I didn't see anything wrong in the review (either coding-wise, or compliance-wise). - -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkqld0AACgkQ84KuGfSFAYCQ7QCgnWmZ9y2rP0+15eBjQhQ1ry0I 8TIAoK0JXqS0NPbIywOcI14iO9HUmmUW =D/Xo -END PGP SIGNATURE-
Re: tail -f problem
Eric Blake wrote: According to Jim Meyering on 9/7/2009 2:48 PM: This seems small and safe enough that I'm leaning toward including it in coreutils-7.6, but I'll wait for a second opinion and/or review. +** POSIX conformance + + tail -f now ignores - when stdin is a pipe or FIFO, per POSIX. + Now, : | tail -f terminates immediately. Before, it'd block indefinitely. + [the old behavior dates back to the original implementation] Let's spell out the abbreviation (it'd is colloquial; using it would sounds better). Beyond that, I didn't see anything wrong in the review (either coding-wise, or compliance-wise). Thanks for the review. I prefer it would, too and wrote that, at first, but that made it a little too long ;-) Adjusted: ** POSIX conformance tail -f now ignores - when stdin is a pipe or FIFO, per POSIX. Now, :|tail -f terminates immediately. Before, it would block indefinitely. [the old behavior dates back to the original implementation]
Re: merge rename and rename-dest-slash
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Eric Blake on 9/7/2009 11:12 AM: According to Eric Blake on 9/7/2009 10:30 AM: Any objections to deleting the rename-dest-slash module? It performs a subset of the rename module, Correction. As currently written, rename.m4 checks whether: rm -rf d1 d2 mkdir d1 rename(d1/,d2) Meanwhile, rename-dest-slash.m4 checks whether: rm -rf d1 d2 mkdir d1 rename(d1,d2/) fails And POSIX 2008 is still awkward on rules for trailing slashes. I think the intent is that a directory may be specified with a trailing slash, but that a regular file must not (so, for example, the fact that Solaris 10 allows: touch a rename(a,b/) to succeed if b exists as a regular file, but fails with ENOTDIR if b does not exist, is a bug in Solaris). But I'd rather wait for the Austin group ruling on my bug report: https://www.opengroup.org/sophocles/show_mail.tpl?CALLER=index.tplsource=Llistname=austin-group-lid=12739 as well as delay the merging of these two modules until after coreutils 7.6 is released. I guess it's time for me to add a unit test. Still true, although it's hard to know what to test until we are certain what POSIX requires. - -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkqle4sACgkQ84KuGfSFAYDimACffcikrffznSm+jbGVyNj3/aGA NAsAoKPuryJSyFQcmkXUqRktC8HEkFGt =Zg6u -END PGP SIGNATURE-
Re: misc/cat-buf test failure
Jim Meyering wrote: Hi Pádraig, On my 6th run of make check, the cat-buf test failed. It's an inherently racy test. If the dd process is starved until 2 is printed, it will end up printing both lines to out. Hmm. 0.2s was too short between writes so. I've bumped it up to 0.5s since 0.2 failed so infrequently. I've also changed to just skip on possible failure. thanks, Pádraig. From 047dae65f432eedec43639afcb9075ea42358c00 Mon Sep 17 00:00:00 2001 From: =?utf-8?q?P=C3=A1draig=20Brady?= p...@draigbrady.com Date: Mon, 7 Sep 2009 23:50:19 +0100 Subject: [PATCH] tests: address a race condition in misc/cat-buf * tests/misc/cat-buf: Increase the delay between writes to decrease the chance that dd will read both at once. Since the test is inherently racy, print a warning via skip_test_ rather than failing outright. Reported by Jim Meyering. --- tests/misc/cat-buf |9 +++-- 1 files changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/misc/cat-buf b/tests/misc/cat-buf index d37f9f4..9ca2d7b 100755 --- a/tests/misc/cat-buf +++ b/tests/misc/cat-buf @@ -30,12 +30,17 @@ fi # write separately. mkfifo fifo || framework_failure +fail=0 + echo 1 exp dd count=1 if=fifo out -(echo 1; sleep .2; echo 2) | cat -v fifo +(echo 1; sleep .5; echo 2) | cat -v fifo wait # for dd to complete -compare out exp || fail=1 +# Though unlikely, this test may fail because dd was starved +# between opening the fifo and reading from it, rather than +# the failure we're testing where cat doesn't output immediately. +compare out exp || skip_test_ possible test failure. Please verify. Exit $fail -- 1.6.2.5
Re: tail -f problem
Jim Meyering wrote: Ulrich Drepper wrote: Jim Meyering wrote: Thanks for the report! Ulrich and Ren reported that the command, echo foobar | tail -c3 -f would block indefinitely, while POSIX says that it must not. This seems to be handled already (see line 1926): echo foobar | POSIXLY_CORRECT= tail -c3 -f Also there is the stdin_cmdline_arg bit below this new chunk. Is that now rendered redundant? It seems that these 3 chunks should be merged. cheers, Pádraig.
Re: new snapshot available: coreutils-7.5.65-61cc6
Eric Blake wrote: According to Pádraig Brady on 9/7/2009 10:22 AM: Solaris 9 x86 build failed with: fstatat.c, line 39: undefined symbol: AT_SYMLINK_NOFOLLOW Hmm. That should be taken care of by the gnulib fcntl.h replacement. Are we missing a #include? Can you post the full failure? gnulib 52c658e seems to have removed #include openat.h from fstatat.c but not replaced it with #include fcntl.h cheers, Pádraig.
Re: new snapshot available: coreutils-7.5.65-61cc6
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Pádraig Brady on 9/7/2009 6:32 PM: fstatat.c, line 39: undefined symbol: AT_SYMLINK_NOFOLLOW Hmm. That should be taken care of by the gnulib fcntl.h replacement. Are we missing a #include? Can you post the full failure? gnulib 52c658e seems to have removed #include openat.h from fstatat.c but not replaced it with #include fcntl.h Yep; and only Solaris had the problem, because that is the only platform with broken fstatat (the other platforms either lack it or it works entirely). Fixed as follows; coreutils needs to update the gnulib submodule. - -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkqlwIgACgkQ84KuGfSFAYCYWQCbBC7k9nNOQ7kWrZb97lGz/Tpw ADkAniQxA4kKKzEv7vognH3MDcKFRpGV =nQmf -END PGP SIGNATURE- From 2c90f1af0fd6fcf444f5c16de9ff465651a854fc Mon Sep 17 00:00:00 2001 From: Eric Blake e...@byu.net Date: Mon, 7 Sep 2009 20:16:00 -0600 Subject: [PATCH] fstatat: fix compilation on Solaris MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * lib/fstatat.c (includes): Add fcntl.h. Reported by Pádraig Brady. Signed-off-by: Eric Blake e...@byu.net --- ChangeLog |6 ++ lib/fstatat.c |1 + 2 files changed, 7 insertions(+), 0 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0b9e3a0..7df9226 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2009-09-08 Eric Blake e...@byu.net + + fstatat: fix compilation on Solaris + * lib/fstatat.c (includes): Add fcntl.h. + Reported by Pádraig Brady. + 2009-09-07 Eric Blake e...@byu.net rename: modernize replacement diff --git a/lib/fstatat.c b/lib/fstatat.c index 2bf547e..9b0c1af 100644 --- a/lib/fstatat.c +++ b/lib/fstatat.c @@ -22,6 +22,7 @@ #include sys/stat.h #include errno.h +#include fcntl.h #include string.h #undef fstatat -- 1.6.3.3.334.g916e1
Re: misc/cat-buf test failure
Pádraig Brady wrote: Hmm. 0.2s was too short between writes so. I've bumped it up to 0.5s since 0.2 failed so infrequently. I've also changed to just skip on possible failure. That should do it. Thanks.
