Re: [PATCH] simply and fix a race in 2 tail --follow tests
Pádraig Brady wrote: I had noticed these tests were a little verbose and had meant to simplify them. Coincidentally today I triggered a race in tail-2/pid, so the attached patch kills two birds with the one stone. Good timing. I hit this today, too. From ba37fb2e96334b3cc784a4387d74f726be9be98d Mon Sep 17 00:00:00 2001 From: =?utf-8?q?P=C3=A1draig=20Brady?= p...@draigbrady.com Date: Thu, 3 Sep 2009 00:39:17 +0100 Subject: [PATCH] tests: simplify and fix a race in 2 tail --follow tests * tests/tail-2/pid: Use the timeout command to determine process longevity, rather than querying /proc/$pid/status. That was racy in any case as I presume the test was copied slightly clearer: s/That/The latter/ FYI, tail -f with an unchanging file is different, now that it's based on inotify. Before, it really was always in the 'S' state. Now, it wakes up periodically. Hence this race. from tail-2/tail-n0f and wasn't updated to handle the case where the background process was in the R (running) state. * tests/tail-2/wait: Likewise. --- tests/tail-2/pid | 37 +++-- tests/tail-2/wait | 93 - 2 files changed, 19 insertions(+), 111 deletions(-) diff --git a/tests/tail-2/pid b/tests/tail-2/pid Looks fine, and works for me. Thanks!
Re: cp(1) extended attributes bug + [PATCH]: fix the leak in copy_reg()
Pádraig Brady wrote: Ondřej Vašík wrote: Ernest N. Mamikonyan wrote: [returning mailing list to CC, please keep it there] On Tue, 01 Sep 2009 02:16:28 -0400, Ondřej Vašík ova...@redhat.com wrote: Ernest N. Mamikonyan wrote: Cp(1) doesn't correctly copy extended attributes for read-only files: touch foo setfattr -n user.key -v value foo chmod a-w foo cp --preserve=xattr foo bar cp: setting attribute `user.key' for `user.key': Permission denied This error message is not produced by coreutils sources, but by libattr - all messages about extended attributes are generated there. I'm not sure why they are trying to set attributes for source file - maybe they are not and access rights for destination file are more relevant. Strace/ltrace of the failure could be helpful as well. The problem is quite simple! Cp(1) tries to change the xattrs of a file with mode 0400 (see attached trace). Is opening the destination file with initial an mode of 0600 a security (or some other) risk? I suppose that's what needs to be done. Sorry, I got confused ... it's obvious - not sure about the risk, so not trying to fix this now. What do you think, Jim/Pádraig? I haven't looked at it, but it seems like a bug from the description. Additionally it looks like there is a leak (about 36k per file for failing xattr preserve) in copy_reg() - as the return false; should be changed to return_val=false; - sending patch for this... but it's not fixing the reported issue. Eek! Well spotted. That also leaks the file descriptors. I've adjusted the logs slightly in the attached patch. cheers, Pádraig. From 7877731f6e7c59905355e71519bbee7d6eac5d32 Mon Sep 17 00:00:00 2001 From: =?utf-8?q?Ond=C5=99ej=20Va=C5=A1=C3=ADk?= ova...@redhat.com Date: Wed, 2 Sep 2009 10:34:27 +0100 Subject: [PATCH] cp: don't leak resources for each xattr preservation failure * src/copy.c (copy_reg): Don't exit from the function after an unsuccessful and required preservation of extended attributes. This resulted in leaking the copy buffer and file descriptors. Thanks Ernest, Ondřej, and Pádraig! Pádraig, you're welcome to push this fix. However, please adjust the log to s/exit/return/ or s/exit/return early/ Also, for each regression-fix like this, I find it worthwhile to include a pointer in the log to the commit that introduced it. E.g.,, http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commit;h=1762092901adf0404 http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commit;h=a977dbbe78f4dcb64 http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commit;h=493c48168490247c8 I've been including the coreutils version number, 8 bytes of the commit SHA1, date, and summary line, e.g., for the last one listed above, ... The bug was introduced in coreutils-7.0 via commit 0647f3eb, 2008-06-02, accommodate older SELinux which lacks matchpathcon_init_prefix.
Re: [bug #27373] sort -h performs incorrectly if in utf8 locale.
Pádraig Brady wrote: Follow-up Comment #1, bug #27373 (project coreutils): I can't reproduce this or see anything wrong with the code. All 720 of my locales work fine: $ for LANG in $(locale -a); do printf KnEnMnZn | ./sort -h | tr -d 'n'; echo; done | uniq -c 720 KMEZ Pádraig, The web interface ate all of your backslashes: for LANG in $(locale -a); do printf K\nE\nM\nZ\n \ | sort -h|tr -d '\n'; echo; done | uniq -c
please improve the documentation for install --compare (-C)
Hi, the -C (or --compare) option is currently not mentioned in the info documentation, and in man page it reads: compare each pair of source and destination files, and in some cases, do not modify the destination at all That's not very specific, making -C effectively unuseable. I found a nice description of what it does on this list, written by Kamil Dudka (http://lists.gnu.org/archive/html/bug-coreutils/2009-01/msg00122.html): With this option install checks an existing destination file and if it is not different (by content, owner, group and mode) from source, the file is not installed. Preserving destination's original mtime can significantly decrease time of building when a system library is reinstalled but the header files are not changed at all. IMHO this is adequate for the info documentation. For the shorter manpage, the description could be changed to read: compare each pair of source and destination files, and if identical (by content, ownership and mode), do not copy to preserve mtime Florian
Re: please improve the documentation for install --compare (-C)
Hi Florian,
On Thu September 3 2009 12:36:13 Florian Schlichting wrote:
Hi,
the -C (or --compare) option is currently not mentioned in the info
documentation, and in man page it reads:
compare each pair of source and destination files, and
in some cases, do not modify the destination at all
thanks for pointing this out! As for the missing info documentation, it's ugly
bug. A hunk of doc/coreutils.texi was misplaced somehow (lack of caffeine, who
knows...). The attached patch fixes it.
That's not very specific, making -C effectively unuseable. I found a
nice description of what it does on this list, written by Kamil Dudka
(http://lists.gnu.org/archive/html/bug-coreutils/2009-01/msg00122.html):
With this option install checks an existing destination file and if it
is not different (by content, owner, group and mode) from source, the
file is not installed. Preserving destination's original mtime can
significantly decrease time of building when a system library is
reinstalled but the header files are not changed at all.
IMHO this is adequate for the info documentation. For the shorter
manpage, the description could be changed to read:
compare each pair of source and destination files, and
if identical (by content, ownership and mode), do not
copy to preserve mtime
As for wording/reviewing the documentation text, we need to ask someone else.
My English is not good enough to write documentation...
Kamil
From a9bc3fc5e15672ac364dccd827265d3cc9d1aa14 Mon Sep 17 00:00:00 2001
From: Kamil Dudka kdu...@redhat.com
Date: Thu, 3 Sep 2009 14:08:27 +0200
Subject: [PATCH] install -C: fix bug in the texi documentation
* doc/coreutils.texi: Move the documentation for install --compare (-C)
option to the right place.
---
doc/coreutils.texi | 16
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 33877af..86394a1 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -2122,14 +2122,6 @@ The program accepts the following options. Also see @ref{Common options}.
@table @samp
-...@item -C
-...@itemx --compare
-...@opindex -C
-...@opindex --compare
-Compare each pair of source and destination files, and if the destination has
-identical content and any specified owner, group, permissions, and possibly
-SELinux context, then do not modify the destination at all.
-
@item -c
@itemx --crown-margin
@opindex -c
@@ -8072,6 +8064,14 @@ The program accepts the following options. Also see @ref{Common options}.
@optBackup
+...@item -C
+...@itemx --compare
+...@opindex -C
+...@opindex --compare
+Compare each pair of source and destination files, and if the destination has
+identical content and any specified owner, group, permissions, and possibly
+SELinux context, then do not modify the destination at all.
+
@item -c
@opindex -c
Ignored; for compatibility with old Unix versions of @command{install}.
--
1.6.2.5
[PATCH] rm: improve efficiency of rm -r (without -f) from O(N^2) to O(N)
I suppose this deserves mention in NEWS, too.
Not only does this fix a long-standing performance problem
with rm -r (but not with -f, which is already efficient),
but it makes rm -r behave correctly also for names longer than 8 KiB,
if your system has support for the faccessat system call.
While the O(N^2) - O(N) improvement is real, it is not as great
as you might expect because we capped the length (in remove.c)
of the file for which we would try to toe the standards line.
For longer names, we punted and used possibly inaccurate
permission bits (see remove.c for details).
Now, however, on modern systems, rm is correct, efficient,
and its core in remove.c is almost completely thread-safe.
I've just pushed this to next.
From d0c1381739cc1dbad8f9a9e8e2246d0a80f4acea Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Thu, 3 Sep 2009 15:15:09 +0200
Subject: [PATCH] rm: improve efficiency of rm -r (without -f) from O(N^2) to
O(N)
where N is the depth of the deepest hierarchy rm is processing.
* src/remove.c (write_protected_non_symlink): Use faccessat to
avoid O(N)-per-entry cost of calling euidaccess.
* m4/jm-macros.m4 (coreutils_MACROS): Check for faccessat.
---
m4/jm-macros.m4 |3 +++
src/remove.c|7 +++
2 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/m4/jm-macros.m4 b/m4/jm-macros.m4
index 416a0af..934d4ed 100644
--- a/m4/jm-macros.m4
+++ b/m4/jm-macros.m4
@@ -92,6 +92,9 @@ AC_DEFUN([coreutils_MACROS],
# for cp.c
AC_CHECK_FUNCS_ONCE([utimensat])
+ # for remove.c
+ AC_CHECK_FUNCS_ONCE([faccessat])
+
dnl This can't use AC_REQUIRE; I'm not quite sure why.
cu_PREREQ_STAT_PROG
diff --git a/src/remove.c b/src/remove.c
index 32f67a1..2db3859 100644
--- a/src/remove.c
+++ b/src/remove.c
@@ -172,6 +172,13 @@ write_protected_non_symlink (int fd_cwd,
mess up with long file names). */
{
+/* Use faccessat if possible, so as to avoid the expense
+ of processing an N-component name. */
+#if HAVE_FACCESSAT AT_EACCESS
+if (faccessat (fd_cwd, file, W_OK, AT_EACCESS) == 0)
+ return 0;
+#endif
+
/* This implements #5: */
size_t file_name_len = strlen (full_name);
--
1.6.4.2.395.ge3d52
Re: [bug #27373] sort -h performs incorrectly if in utf8 locale.
On Wed, 2009-09-02 at 21:43 +, Pádraig Brady wrote: Follow-up Comment #1, bug #27373 (project coreutils): I can't reproduce this or see anything wrong with the code. All 720 of my locales work fine: $ for LANG in $(locale -a); do printf KnEnMnZn | ./sort -h | tr -d 'n'; echo; done | uniq -c 720 KMEZ Can you give your libc version? Could you add a printf() to the find_unit_order() function in sort.c to see if it's called? Interestingly, it works here with the string you used, and fails in the following case: ~ $ for LANG in $(locale -a); do printf A b\nAA b\nAAA b\n | sort -h|tr -d '\n'; echo; done | uniq -c 1 A bAA bAAA b 21 AAA bAA bA b 1 A bAA bAAA b 2 AAA bAA bA b ~ $ ~ $ apt-cache policy libc6 libc6: Installed: 2.10.1-0ubuntu8 Candidate: 2.10.1-0ubuntu8 Version table: *** 2.10.1-0ubuntu8 0 500 http://archive.ubuntu.com karmic/main Packages 100 /var/lib/dpkg/status ~ $ sort --version sort (GNU coreutils) 7.5.42-1b2d2 Copyright (C) 2009 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html. This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Written by Mike Haertel and Paul Eggert. ~ $ I will add the printf you asked, and run it again. A similar issue has also been reported on Ubuntu (https://bugs.launchpad.net/bugs/422252), but for coreutils 6.10. Cheers, ..Carl.. signature.asc Description: This is a digitally signed message part
[PATCH] NEWS: mention recent improvements in rm
Pushed to next: [This entry is for 7.7 but currently resides in the block for 7.6. Obviously, it will be moved up right after release. ] From be4517a4c8cc8797251ad7c2703984bfbb11b983 Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Thu, 3 Sep 2009 16:11:58 +0200 Subject: [PATCH] NEWS: mention recent improvements in rm --- NEWS |9 + 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/NEWS b/NEWS index 50c40be..f8e4824 100644 --- a/NEWS +++ b/NEWS @@ -21,6 +21,15 @@ GNU coreutils NEWS-*- outline -*- because ls must stat every file in order to obtain a guaranteed-valid inode number. [bug introduced in coreutils-6.0] +** Improved robustness + + rm has been rewritten to use gnulib's fts + This makes it significantly faster (400-500%) in some pathological + cases, and slightly slower (20%) in at least one pathological case. + Unrelated to its use of fts, rm -r is now slightly more standards- + conformant when operating on write-protected relative file names + longer than 8KiB. + ** New features cp --reflink accepts a new auto parameter which falls back to -- 1.6.4.2.395.ge3d52
[PATCH] cp,mv: do preserve extended attributes even for read-only source files
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...
Greetings,
Ondřej Vašík
From cae691907fe50e2ab05198a7c647fe4140e3669e 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: mention that change
---
NEWS |4
src/copy.c | 24
tests/misc/xattr |7 +++
3 files changed, 31 insertions(+), 4 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..b645a9a 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -850,10 +850,26 @@ 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 (! copy_attr_by_fd (src_name, source_desc, dst_name, dest_desc, x)
+ x-require_preserve_xattr)
+ return_val = false;
+ }
+ else
+ {
+ if (!x-reduce_diagnostics || x-require_preserve_xattr)
+error (0, errno, _(can't write extended attributes to %s),
+ quote (dst_name));
+ if (x-require_preserve_xattr)
+return_val = false;
+ }
+}
+
if (x-preserve_mode || x-move_mode)
{
diff --git a/tests/misc/xattr b/tests/misc/xattr
index a27e1f6..828e871 100755
--- a/tests/misc/xattr
+++ b/tests/misc/xattr
@@ -77,6 +77,13 @@ cp -a a d 2err test -s err fail=1
getfattr -d d out_d || skip_test_ failed to get xattr of file
grep -F $xattr_pair out_d /dev/null || fail=1
+#test if --preserve=xattr works even for files without write rights
+chmod a-w a || framework_failure
+cp --preserve=xattr a e || fail=1
+getfattr -d e out_e || skip_test_ failed to get xattr of file
+grep -F $xattr_pair out_e /dev/null || fail=1
+chmod a+w a || framework_failure
+
rm b || framework_failure
# install should never preserve xattr
--
1.5.6.1.156.ge903b
signature.asc
Description: Toto je digitálně podepsaná část zprávy
Re: please improve the documentation for install --compare (-C)
From a9bc3fc5e15672ac364dccd827265d3cc9d1aa14 Mon Sep 17 00:00:00 2001 From: Kamil Dudka kdu...@redhat.com Date: Thu, 3 Sep 2009 14:08:27 +0200 Subject: [PATCH] install -C: fix bug in the texi documentation * doc/coreutils.texi: Move the documentation for install --compare (-C) option to the right place. Thanks! I've applied that, updated the log and will push shortly: Author: Kamil Dudka kdu...@redhat.com Date: Thu Sep 3 14:08:27 2009 +0200 doc: install -C: fix bug in the texi documentation * doc/coreutils.texi: Move the documentation for install --compare (-C) from the section on fmt to that for install. Reported by Florian Schlichting.
Re: please improve the documentation for install --compare (-C)
On Thu September 3 2009 16:25:07 Jim Meyering wrote: I've applied that, updated the log and will push shortly: Thanks. What's your opinion about change of the wording as Florian proposed? Kamil
Re: [bug #27373] sort -h performs incorrectly if in utf8 locale.
On Sep 3, 2009 10:08am, C de-Avillez hgg...@ubuntu.com wrote: Interestingly, it works here with the string you used, and fails in the following case: ~ $ for LANG in $(locale -a); do printf A b\nAA b\nAAA b\n | sort -h|tr -d '\n'; echo; done | uniq -c 1 A bAA bAAA b 21 AAA bAA bA b 1 A bAA bAAA b 2 AAA bAA bA b ~ $ I am receiving the same different sort for the C and POSIX locales with and without the -h option. These two discrepancies are due, I believe, to the collation functions for the C and POSIX locales specifying binary ordering and hence a space being sorted before an 'A'. In the other locales it seems that longer words are given preference. ~/src/core/fake$ locale -a | ./bin/sort | while read LANG ; do printf %10s $LANG ; echo -e 'A b\nAA b\nAAA b\n' | ./bin/sort -h | tr -d '\n' ; echo ; done ; C A bAA bAAA b POSIX A bAA bAAA b en_AU.utf8 AAA bAA bA b en_BW.utf8 AAA bAA bA b en_CA.utf8 AAA bAA bA b en_DK.utf8 AAA bAA bA b en_GB.utf8 AAA bAA bA b en_HK.utf8 AAA bAA bA b en_IE.utf8 AAA bAA bA b en_IN AAA bAA bA b en_NG AAA bAA bA b en_NZ.utf8 AAA bAA bA b en_PH.utf8 AAA bAA bA b en_SG.utf8 AAA bAA bA b en_US.utf8 AAA bAA bA b en_ZA.utf8 AAA bAA bA b en_ZW.utf8 AAA bAA bA b ~/src/core/fake$ locale -a | ./bin/sort | while read LANG ; do printf %10s $LANG ; echo -e 'A b\nAA b\nAAA b\n' | ./bin/sort | tr -d '\n' ; echo ; done ; C A bAA bAAA b POSIX A bAA bAAA b en_AU.utf8 AAA bAA bA b en_BW.utf8 AAA bAA bA b en_CA.utf8 AAA bAA bA b en_DK.utf8 AAA bAA bA b en_GB.utf8 AAA bAA bA b en_HK.utf8 AAA bAA bA b en_IE.utf8 AAA bAA bA b en_IN AAA bAA bA b en_NG AAA bAA bA b en_NZ.utf8 AAA bAA bA b en_PH.utf8 AAA bAA bA b en_SG.utf8 AAA bAA bA b en_US.utf8 AAA bAA bA b en_ZA.utf8 AAA bAA bA b en_ZW.utf8 AAA bAA bA b ~/src/core/fake$
Re: please improve the documentation for install --compare (-C)
Florian Schlichting wrote: the -C (or --compare) option is currently not mentioned in the info documentation, and in man page it reads: compare each pair of source and destination files, and in some cases, do not modify the destination at all That's not very specific, making -C effectively unuseable. I found a nice description of what it does on this list, written by Kamil Dudka (http://lists.gnu.org/archive/html/bug-coreutils/2009-01/msg00122.html): With this option install checks an existing destination file and if it is not different (by content, owner, group and mode) from source, the file is not installed. Preserving destination's original mtime can significantly decrease time of building when a system library is reinstalled but the header files are not changed at all. IMHO this is adequate for the info documentation. For the shorter manpage, the description could be changed to read: compare each pair of source and destination files, and if identical (by content, ownership and mode), do not copy to preserve mtime Thanks for the suggestion. How about this? From f7d2526c1c6eea4ae92fa1ce0b8a4f507adcf214 Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Thu, 3 Sep 2009 17:28:33 +0200 Subject: [PATCH] doc: improve install --help's description of --compare (-C) * src/install.c (usage): Improve description of -C option, based on a suggestion from Florian Schlichting. --- src/install.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/install.c b/src/install.c index fafa21a..80cc5ef 100644 --- a/src/install.c +++ b/src/install.c @@ -964,7 +964,8 @@ Mandatory arguments to long options are mandatory for short options too.\n\ -b like --backup but does not accept an argument\n\ -c (ignored)\n\ -C, --compare compare each pair of source and destination files, and\n\ -in some cases, do not modify the destination at all\n\ +if they have identical content, owner, group and mode,\n\ +do not modify the destination at all\n\ -d, --directory treat all arguments as directory names; create all\n\ components of the specified directories\n\ ), stdout); -- 1.6.4.2.395.ge3d52
[PATCH] df: don't fail due to an unreadable argument
Thanks to Olivier Fourdan and Ondřej Vašík.
Here's a patch for that bug:
From e0e8429c2433bd9820f42250236badc585bd9dd7 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Thu, 3 Sep 2009 19:36:34 +0200
Subject: [PATCH] df: don't fail due to an unreadable argument
* src/df.c (main): If open or fstat fails when we're trying to ensure
that all arg-partitions are automounted, fall back on using stat.
Inspired by the report and patch from Olivier Fourdan in
http://bugzilla.redhat.com/520630.
* NEWS (Bug fixes): Mention it.
* tests/df/unreadable: New test for the above.
* tests/Makefile.am (TESTS): Add df/unreadable.
The bug was introduced in coreutils-7.3 via commit dbd17157,
2009-04-28, df: use open(2), not stat, to trigger automounting.
---
NEWS|3 +++
THANKS |1 +
src/df.c|5 -
tests/Makefile.am |1 +
tests/df/unreadable | 32
5 files changed, 41 insertions(+), 1 deletions(-)
create mode 100755 tests/df/unreadable
diff --git a/NEWS b/NEWS
index 59270eb..cb01227 100644
--- a/NEWS
+++ b/NEWS
@@ -19,6 +19,9 @@ GNU coreutils NEWS-*-
outline -*-
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
diff --git a/THANKS b/THANKS
index ee18572..961785e 100644
--- a/THANKS
+++ b/THANKS
@@ -440,6 +440,7 @@ Olatunji Oluwabukunmi Ruwasetjruw...@stanford.edu
Olav Morkrido...@funcom.com
Ole Laursen o...@hardworking.dk
Oliver Kiddle okid...@yahoo.co.uk
+Olivier Fourdan ofour...@redhat.com
Ørn E. Hansen oehan...@daimi.aau.dk
Oskar Liljeblad o...@hem.passagen.se
Otavio Salvador ota...@ossystems.com.br
diff --git a/src/df.c b/src/df.c
index 787fcde..86fd0e3 100644
--- a/src/df.c
+++ b/src/df.c
@@ -994,8 +994,11 @@ main (int argc, char **argv)
stats = xnmalloc (argc - optind, sizeof *stats);
for (i = optind; i argc; ++i)
{
+ /* Prefer to open with O_NOCTTY and use fstat, but fall back
+ on using stat, in case the file is unreadable. */
int fd = open (argv[i], O_RDONLY | O_NOCTTY);
- if (fd 0 || fstat (fd, stats[i - optind]))
+ if ((fd 0 || fstat (fd, stats[i - optind]))
+ stat (argv[i], stats[i - optind]))
{
error (0, errno, %s, quote (argv[i]));
exit_status = EXIT_FAILURE;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 0151cb0..d9ff95b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -300,6 +300,7 @@ TESTS = \
cp/src-base-dot \
cp/symlink-slash \
cp/thru-dangling \
+ df/unreadable\
dd/direct\
dd/misc \
dd/not-rewound \
diff --git a/tests/df/unreadable b/tests/df/unreadable
new file mode 100755
index 000..8e60028
--- /dev/null
+++ b/tests/df/unreadable
@@ -0,0 +1,32 @@
+#!/bin/sh
+# ensure that df can handle an unreadable argument
+
+# 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 implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see http://www.gnu.org/licenses/.
+
+if test $VERBOSE = yes; then
+ set -x
+ df --version
+fi
+
+. $srcdir/test-lib.sh
+skip_if_root_
+
+fail=0
+touch unreadable || fail=1
+chmod a-r unreadable || fail=1
+df unreadable || fail=1
+
+Exit $fail
--
1.6.4.2.395.ge3d52
Re: please improve the documentation for install --compare (-C)
guys, I'm amazed by your lighning-fast reaction! On Thu, Sep 03, 2009 at 05:29:27PM +0200, Jim Meyering wrote: How about this? -C, --compare compare each pair of source and destination files, and\n\ -in some cases, do not modify the destination at all\n\ +if they have identical content, owner, group and mode,\n\ +do not modify the destination at all\n\ I like it thanks a lot! Florian
[PATCH] fix libcap configure flag handling
The current libcap flag handler treats any libcap configure option as --disable-libcap because it doesn't check $enableval at all. So make sure we do the sane thing: --disable-libcap - disable and don't run any tests --enable-libcap - run tests and fail if not found no choice - run tests and warn if not found Signed-off-by: Mike Frysinger vap...@gentoo.org --- m4/jm-macros.m4 | 24 1 files changed, 16 insertions(+), 8 deletions(-) diff --git a/m4/jm-macros.m4 b/m4/jm-macros.m4 index 416a0af..f4d43f1 100644 --- a/m4/jm-macros.m4 +++ b/m4/jm-macros.m4 @@ -105,17 +105,25 @@ AC_DEFUN([coreutils_MACROS], LIBS=$coreutils_saved_libs # Check whether libcap is usable -- for ls --color support + LIB_CAP= AC_ARG_ENABLE([libcap], -AC_HELP_STRING([--disable-libcap], [disable libcap support]), -AC_MSG_WARN([libcap support disabled by user]), -[AC_CHECK_LIB([cap], [cap_get_file], +AC_HELP_STRING([--disable-libcap], [disable libcap support])) + if test X$enable_libcap != Xno; then +AC_CHECK_LIB([cap], [cap_get_file], [AC_CHECK_HEADER([sys/capability.h], [LIB_CAP=-lcap - AC_DEFINE([HAVE_CAP], [1], [libcap usability])], -[AC_MSG_WARN([header sys/capability.h was not found, support for libcap will not be built])] - )], - [AC_MSG_WARN([libcap library was not found or not usable, support for libcap will not be built])]) -]) + AC_DEFINE([HAVE_CAP], [1], [libcap usability])] + )]) +if test X$LIB_CAP = X; then + if test X$enable_libcap = Xyes; then +AC_MSG_ERROR([libcap library was not found or not usable]) + else +AC_MSG_WARN([libcap library was not found or not usable, support for libcap will not be built]) + fi +fi + else +AC_MSG_WARN([libcap support disabled by user]) + fi AC_SUBST([LIB_CAP]) # See if linking `seq' requires -lm. -- 1.6.4
-T option help text
Philip Rowlands wrote: On Sun, 30 Aug 2009, James R. Van Zandt wrote: For the help text, here are some alternatives: if DEST is a directory, then delete it first This isn't what -T does. If DEST is an empty directory then it's overwritten with the rename(2) system call. Otherwise mv will fail e.g. if SOURCE isn't a directory or DEST is non-empty. Let's see... vanzandt:/tmp$ date source vanzandt:/tmp$ mkdir dest vanzandt:/tmp$ mv -T source dest mv: cannot overwrite directory `dest' with non-directory vanzandt:/tmp$ rm -rf dest vanzandt:/tmp$ touch test vanzandt:/tmp$ mv -T source dest vanzandt:/tmp$ I agree my proposed help text is wrong. However, I think the current description in the info file is wrong too. It says Do not treat the last operand specially when it is a directory or a symbolic link to a directory. Yet, this is an example where mv fails when DEST is a directory, and succeeds when it is a normal file. So a directory *is* treated differently. The extended discussion on the Target directory page says: For example, when the command `mv /tmp/source /tmp/dest' succeeds, there is no guarantee that `/tmp/source' was renamed to `/tmp/dest': it could have been renamed to `/tmp/dest/source' instead, if some other process created `/tmp/dest' as a directory. However, if `mv -T /tmp/source /tmp/dest' succeeds, there is no question that `/tmp/source' was renamed to `/tmp/dest'. I read the last sentence as saying /tmp/source would get renamed as /tmp/dest whether or not some other process had previously created /tmp/dest as a directory. The fact that it fails instead certainly should be stated. (To me it seems a much less useful behavior.) - Jim Van Zandt
ln -P/-L and linkat
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Now that POSIX 2008 requires ln to support user choice of whether to create hard links to symlinks, we can probably get rid of ENABLE_HARD_LINK_TO_SYMLINK_WARNING in the ln.c source code. I noticed this while considering how to implement linkat in gnulib. I guess platforms that follow link() by default will just have to fail with ENOSYS since they don't support -P (and keep their default of -L), while systems like GNU/Linux that have always behaved like -P are now justified. Do we want to make ln's behavior conditional on _POSIX2_VERSION (where conformance to 2001 tries to imply -L, even on Linux where it is more natural to imply -P), or is it not worth worrying about? - -- 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/ iEYEARECAAYFAkqgi+0ACgkQ84KuGfSFAYAV/gCeMbQNhg4z0G2qNOv1DuIZYCg9 EooAoMhShSReV6IsAfnn6s0QWR2V9Veu =aYbs -END PGP SIGNATURE-
Re: rm (remove.c): Rewrite to use fts: request for review
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Eric Blake on 8/28/2009 3:40 PM: Also, mingw does not provide dirfd and its struct DIR does not have room for an fd, so gnulib's dirfd replacement currently just returns ENOTSUP (as permitted by POSIX). fts handles this okay (by devolving gracefully into slower algorithms), but other clients don't. For example, on coreutils' master branch, remove.c blindly assumes that it can call openat(dirfd(dirp),..,O_RDONLY), which is not true (on coreutils' next branch, this code is deleted). At this point, since fdopendir is already GPL, I don't see a reason to create any new module; rather, I'll just work on improving the existing module to a) guarantee that dirfd(dirp) works, even on mingw after plain opendir, and b) where dirfd already works, guarantee that dirfd(dirp) returns the same fd as handed to fdopendir (either by writing back into the dirent-d[d]_fd member on platforms where it is accesssible, or by dup/close'ing the user's fd out of the way, then calling opendir(.) as many times as necessary until dirfd gives the right answer, and cleaning up the mess). - -- 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/ iEYEARECAAYFAkqgmu0ACgkQ84KuGfSFAYCaRgCg15hwz1YjpEbqvsL7e4QuNkGw 89YAoIYcyzZFf3sudb+Rj4d6UuXL+050 =0HqN -END PGP SIGNATURE-
Re: rm (remove.c): Rewrite to use fts: request for review
Eric Blake wrote: According to Jim Meyering on 9/1/2009 10:34 AM: Would you mind pushing it to savannah's coreutils.git next branch, to make the overall series easier to review, even though it won't be merged prior to 7.6? Sure. Just did that. Note that the first change set won't be pushed to master. Now that I've pushed faccessat to gnulib, we can benefit from O(N) even on older kernels that lack faccessat but have decent /proc/self/fd support. Plus a simplification and compilation warning fix. So, as long as we're going to be rebasing the next branch anyway, I thought I'd push this now to make it easier to review. ... diff --git a/ChangeLog b/ChangeLog index efda62b..fe02dd9 100644 diff --git a/MODULES.html.sh b/MODULES.html.sh ... diff --git a/lib/closein.c b/lib/closein.c ... Thanks. It looks like you pushed the right changes, and to next ;-)
Re: ln -P/-L and linkat
Eric Blake wrote: Now that POSIX 2008 requires ln to support user choice of whether to create hard links to symlinks, we can probably get rid of ENABLE_HARD_LINK_TO_SYMLINK_WARNING in the ln.c source code. Yes. Good point. I noticed this while considering how to implement linkat in gnulib. I guess platforms that follow link() by default will just have to fail with ENOSYS since they don't support -P (and keep their default of -L), while systems like GNU/Linux that have always behaved like -P are now justified. Agreed. Do we want to make ln's behavior conditional on _POSIX2_VERSION (where conformance to 2001 tries to imply -L, even on Linux where it is more natural to imply -P), or is it not worth worrying about? Let's not worry about this. That will be clearer, and besides, I doubt anyone would want to flip that switch.
