bug#15173: [PATCH] cp: with --link always use linkat() if available
Thanks, looks good.
bug#15173: [PATCH] cp: with --link always use linkat() if available
On 02/10/2014 02:29 AM, Paul Eggert wrote:
> Pádraig Brady wrote:
>> +#if ! defined HAVE_LINKAT
>> && !(LINK_FOLLOWS_SYMLINKS && S_ISLNK (src_mode)
>> -&& x->dereference == DEREF_NEVER))
>> +&& x->dereference == DEREF_NEVER)
>> +#endif
>
> Could you reword that sort of thing so as not to use the #if inside an
> expression? Something like this instead, perhaps, earlier in the code:
>
> #if !defined HAVE_LINKAT && LINK_FOLLOWS_SYMLINKS
> # define LINKAT_FOLLOWS_SYMLINKS 1
> #else
> # define LINKAT_FOLLOWS_SYMLINKS 0
> #endif
That's slightly confusing as linkat() emulation never
actually symlinks the referent. I went for the more direct:
#if defined HAVE_LINKAT || ! LINK_FOLLOWS_SYMLINKS
# define CAN_HARDLINK_SYMLINKS 1
#else
# define CAN_HARDLINK_SYMLINKS 0
#endif
Full patch is attached.
thanks,
Pádraig.
>From 9196a8c3375a3c8f7478b2ca8373ceeef31eaeb2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?=
Date: Thu, 12 Dec 2013 18:31:48 +
Subject: [PATCH] cp: with --link always use linkat() if available
* src/copy.c (copy_reg): If linkat() is available it doesn't
matter about the gnulib emulation provided, and thus the
LINK_FOLLOWS_SYMLINKS should not have significance here.
This was noticed on FreeBSD and the consequence is that
cp --link will create hardlinks to symlinks there, rather
than emulating with symlinks to symlinks.
* tests/cp/link-deref.sh: Adjust to the checks to cater
for all cases where hardlinks to symlinks are supported.
---
src/copy.c | 20 ++--
tests/cp/link-deref.sh |7 +++
2 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/src/copy.c b/src/copy.c
index 3e4cbff..127eed2 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -98,6 +98,14 @@ rpl_mkfifo (char const *file, mode_t mode)
#define SAME_GROUP(A, B) ((A).st_gid == (B).st_gid)
#define SAME_OWNER_AND_GROUP(A, B) (SAME_OWNER (A, B) && SAME_GROUP (A, B))
+/* LINK_FOLLOWS_SYMLINKS is tri-state; if it is -1, we don't know
+ how link() behaves, so assume we can't hardlink symlinks in that case. */
+#if defined HAVE_LINKAT || ! LINK_FOLLOWS_SYMLINKS
+# define CAN_HARDLINK_SYMLINKS 1
+#else
+# define CAN_HARDLINK_SYMLINKS 0
+#endif
+
struct dir_list
{
struct dir_list *parent;
@@ -2484,16 +2492,15 @@ copy_internal (char const *src_name, char const *dst_name,
should not follow the link. We can approximate the desired
behavior by skipping this hard-link creating block and instead
copying the symlink, via the 'S_ISLNK'- copying code below.
- LINK_FOLLOWS_SYMLINKS is tri-state; if it is -1, we don't know
- how link() behaves, so we use the fallback case for safety.
Note gnulib's linkat module, guarantees that the symlink is not
dereferenced. However its emulation currently doesn't maintain
timestamps or ownership so we only call it when we know the
emulation will not be needed. */
else if (x->hard_link
- && !(LINK_FOLLOWS_SYMLINKS && S_ISLNK (src_mode)
-&& x->dereference == DEREF_NEVER))
+ && !(! CAN_HARDLINK_SYMLINKS && S_ISLNK (src_mode)
+&& x->dereference == DEREF_NEVER)
+ )
{
if (! create_hard_link (src_name, dst_name, false, false, dereference))
goto un_backup;
@@ -2632,8 +2639,9 @@ copy_internal (char const *src_name, char const *dst_name,
/* If we've just created a hard-link due to cp's --link option,
we're done. */
if (x->hard_link && ! S_ISDIR (src_mode)
- && !(LINK_FOLLOWS_SYMLINKS && S_ISLNK (src_mode)
- && x->dereference == DEREF_NEVER))
+ && !(! CAN_HARDLINK_SYMLINKS && S_ISLNK (src_mode)
+ && x->dereference == DEREF_NEVER)
+ )
return delayed_ok;
if (copied_as_regular)
diff --git a/tests/cp/link-deref.sh b/tests/cp/link-deref.sh
index e56d592..5bbd8d4 100755
--- a/tests/cp/link-deref.sh
+++ b/tests/cp/link-deref.sh
@@ -20,10 +20,9 @@
print_ver_ cp
if grep '^#define HAVE_LINKAT 1' "$CONFIG_HEADER" > /dev/null \
- && grep '^#define LINK_FOLLOWS_SYMLINKS 0' "$CONFIG_HEADER" > /dev/null; then
- # With this config (which is the case on GNU/Linux) cp will attempt to
- # linkat() to hardlink a symlink. So now see if the current file system
- # supports this operation.
+ || grep '^#define LINK_FOLLOWS_SYMLINKS 0' "$CONFIG_HEADER" > /dev/null; then
+ # With this config cp will attempt to linkat() to hardlink a symlink.
+ # So now see if the current file system supports this operation.
ln -s testtarget test_sl || framework_failure_
ln -P test_sl test_hl_sl || framework_failure_
ino_sl="$(stat -c '%i' test_sl)" || framework_failure_
--
1.7.7.6
bug#15173: [PATCH] cp: with --link always use linkat() if available
Pádraig Brady wrote: +#if ! defined HAVE_LINKAT && !(LINK_FOLLOWS_SYMLINKS && S_ISLNK (src_mode) -&& x->dereference == DEREF_NEVER)) +&& x->dereference == DEREF_NEVER) +#endif Could you reword that sort of thing so as not to use the #if inside an expression? Something like this instead, perhaps, earlier in the code: #if !defined HAVE_LINKAT && LINK_FOLLOWS_SYMLINKS # define LINKAT_FOLLOWS_SYMLINKS 1 #else # define LINKAT_FOLLOWS_SYMLINKS 0 #endif That way, the rest of the code can simply replace LINK_FOLLOWS_SYMLINKS with LINKAT_FOLLOWS_SYMLINKS, which will make it easier to read.
bug#15173: [PATCH] cp: with --link always use linkat() if available
On 12/12/2013 06:44 PM, Pádraig Brady wrote:
> On 12/09/2013 02:24 AM, Pádraig Brady wrote:
>> Sorry if you get multiple copies of this.
>>
>> The test for this is failing on solaris 10 (NFS)
>> It does seem that hardlinks to symlinks are supported:
>>
>> $ touch tfile
>> $ ln -s tfile tlink
>> $ src/ln -L tlink tlink-ln-L
>> $ src/ln -P tlink tlink-ln-P
>> $ src/ln tlink tlink-ln
>> $ ls -li tfile tlink*
>> 8550 -rw-r--r-- 3 padraig csw0 Dec 9 01:18 tfile
>> 8551 lrwxrwxrwx 2 padraig csw5 Dec 9 01:19 tlink ->
>> tfile
>> 8550 -rw-r--r-- 3 padraig csw0 Dec 9 01:18 tlink-ln
>> 8550 -rw-r--r-- 3 padraig csw0 Dec 9 01:18 tlink-ln-L
>> 8551 lrwxrwxrwx 2 padraig csw5 Dec 9 01:19 tlink-ln-P
>> -> tfile
>>
>> But we have linkat() emulation in place I think:
>>
>> $ grep LINK lib/config.h
> ...
>> /* #undef HAVE_LINKAT */
>> #define LINK_FOLLOWS_SYMLINKS -1
>
>> FAIL: tests/cp/link-deref
>> =
>
>> --- exp Mon Dec 9 01:00:08 2013
>> +++ out Mon Dec 9 01:00:08 2013
>> @@ -1,1 +1,1 @@
>> -cp --link -P dirlink dst|result=0|inode=8436|type=symbolic link|error=
>> +cp --link -P dirlink dst|result=0|inode=8467|type=symbolic link|error=
>> + rm -f diff.out
>> + false
>> + ls -lid dirlink dir dst
>> 8434 drwxr-xr-x 2 padraig csw 2 Dec 9 01:00 dir
>> 8436 lrwxrwxrwx 1 padraig csw 3 Dec 9 01:00 dirlink -> dir
>> 8467 lrwxrwxrwx 1 padraig csw 3 Dec 9 01:00 dst -> dir
>> + fail=1
>
> So the attached should address this on FreeBSD ast least
> where we HAVE_LINKAT so don't need to fallback to
> the symlink -> symlink emulation in copy.c
I still think this is correct, and while I didn't think it
worth the small risk right before the 8.22 release,
I hope to push this rebased patch soon.
thanks,
Pádraig.
>From 1893578fb0cdbb1875a42bc86cad452f6c62e5dc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?=
Date: Thu, 12 Dec 2013 18:31:48 +
Subject: [PATCH] cp: with --link always use linkat() if available
* src/copy.c (copy_reg): If linkat() is available it doesn't
matter about the gnulib emulation provided, and thus the
LINK_FOLLOWS_SYMLINKS should not have significance here.
This was noticed on FreeBSD and the consequence is that
cp --link will create hardlinks to symlinks there, rather
than emulating with symlinks to symlinks.
* tests/cp/link-deref.sh: Adjust to remove the no longer
significant LINK_FOLLOWS_SYMLINKS check.
---
src/copy.c | 10 --
tests/cp/link-deref.sh |3 +--
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/copy.c b/src/copy.c
index 3e4cbff..d560dfd 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -2492,8 +2492,11 @@ copy_internal (char const *src_name, char const *dst_name,
timestamps or ownership so we only call it when we know the
emulation will not be needed. */
else if (x->hard_link
+#if ! defined HAVE_LINKAT
&& !(LINK_FOLLOWS_SYMLINKS && S_ISLNK (src_mode)
-&& x->dereference == DEREF_NEVER))
+&& x->dereference == DEREF_NEVER)
+#endif
+ )
{
if (! create_hard_link (src_name, dst_name, false, false, dereference))
goto un_backup;
@@ -2632,8 +2635,11 @@ copy_internal (char const *src_name, char const *dst_name,
/* If we've just created a hard-link due to cp's --link option,
we're done. */
if (x->hard_link && ! S_ISDIR (src_mode)
+#if ! defined HAVE_LINKAT
&& !(LINK_FOLLOWS_SYMLINKS && S_ISLNK (src_mode)
- && x->dereference == DEREF_NEVER))
+ && x->dereference == DEREF_NEVER)
+#endif
+ )
return delayed_ok;
if (copied_as_regular)
diff --git a/tests/cp/link-deref.sh b/tests/cp/link-deref.sh
index e56d592..b249fce 100755
--- a/tests/cp/link-deref.sh
+++ b/tests/cp/link-deref.sh
@@ -19,8 +19,7 @@
. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
print_ver_ cp
-if grep '^#define HAVE_LINKAT 1' "$CONFIG_HEADER" > /dev/null \
- && grep '^#define LINK_FOLLOWS_SYMLINKS 0' "$CONFIG_HEADER" > /dev/null; then
+if grep '^#define HAVE_LINKAT 1' "$CONFIG_HEADER" > /dev/null; then
# With this config (which is the case on GNU/Linux) cp will attempt to
# linkat() to hardlink a symlink. So now see if the current file system
# supports this operation.
--
1.7.7.6
bug#16578: Wish: Support for non-native endianness in od
On 02/10/2014 01:59 AM, Paul Eggert wrote: > Pádraig Brady wrote: >> $ time od.new -tx8 --endian=bug od.in >> 4.97 elapsed > > If you really used "--endian=bug" and there was no diagnostic, then there > must have been a bug. :-) Ha! I retyped incorrectly rather than copy/pasted. I can confirm the params are checked correctly: $ od -tx8 --endian=bug od.in od: invalid argument ‘bug’ for ‘--endian’ Valid arguments are: - ‘little’ - ‘big’ Try 'src/od --help' for more information.
bug#16578: Wish: Support for non-native endianness in od
Pádraig Brady wrote: $ time od.new -tx8 --endian=bug od.in 4.97 elapsed If you really used "--endian=bug" and there was no diagnostic, then there must have been a bug. :-)
bug#16700: [PATCH] - Add audio file type .m4a to LS_COLORS
On 02/09/2014 11:51 AM, John wrote: > Trivial patch to display .m4a files in the same cyan color as all other audio > files via LS_COLORS. This [m4a] is a very common format generated by iTunes > so I would think many users would like to see this implemented. Please cc me > on any replies; I am not subscribed to the ML. Pushed. thanks, Pádraig.
bug#16700: [PATCH] - Add audio file type .m4a to LS_COLORS
Trivial patch to display .m4a files in the same cyan color as all other audio files via LS_COLORS. This [m4a] is a very common format generated by iTunes so I would think many users would like to see this implemented. Please cc me on any replies; I am not subscribed to the ML.--- a/src/dircolors.hin 2013-12-04 09:48:30.0 -0500 +++ b/src/dircolors.hin 2014-02-09 06:37:21.456576522 -0500 @@ -210,6 +210,7 @@ EXEC 01;32 .aac 00;36 .au 00;36 .flac 00;36 +.m4a 00;36 .mid 00;36 .midi 00;36 .mka 00;36
bug#16578: Wish: Support for non-native endianness in od
On 02/09/2014 08:42 AM, Niels Möller wrote:
> Pádraig Brady writes:
>
>> Attached in the patch I intend to push in your name.
>
> Nice.
>
>> I also added docs to usage() and the texinfo file, and added a test.
>
> I don't quite understand how the test works, but as far as I see, it
> doesn't test floats? So that's inconsistent with the commit message.
Oops, I removed an 'f' while developing. Added that back now
which also gets sizes up to 16 tested.
>> BTW I checked if there was any speed difference with the new code.
>> I wasn't expecting this to be a bottleneck, and true enough
>> there is only a marginal change. The new code is consistently
>> a little _faster_ though on my i3-2310M which is a bit surprising.
>
> Odd. But performance of x86 is usually pretty hard to predict by just
> looking at the source or assembly code. I was hoping that in the
> non-swapped case, the false conditional
>
>if (input_swap && sizeof(T) > 1)
>
> should be very friendly to the branch predictor, and hence almost free.
>
> Jim Meyering writes:
>
>> One nit: please change the type of "j" here (identical in attached)
>> to be unsigned, to match that of the upper bound.
>
> Makes sense. In my own projects, I tend to use unsigned int for loop
> counts whereever I don't need to iterate over any negative values. But
> my impression is that most others prefer to use signed int for
> everything which doesn't rely on mod 2^n arithmetic, so that's why I
> made j signed here.
done
>> That would be our first use of "rev". Is it ubiquitous enough to depend on?
Ugh good point.
> It appears *not* to be available on my closest solaris box. While on my
> gnu/linux system, it's provided by util-linux. For the test, I guess rev
> could be implemented something like
>
> while read line
> printf "%s" line | tr -d '\n' | sed 's/./.\n/' | tac | tr -d '\n'
> echo
> done
I went with:
rev() {
while read line; do
printf '%s' "$line" | sed 's/./&\n/g' | tac | paste -s -d ''
done
}
> Maybe rev should be provided by coreutils, similarly to tac? I'd prefer
> not to think about the unicode issues for rev, though...
I think so too. It's not Linux specific and we've previously
mentioned rev in alternative for adding various functionality to coreutils.
Thanks to both of you for the review!
I've now pushed:
http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commit;h=b370924c
Pádraig.
bug#16578: Wish: Support for non-native endianness in od
Pádraig Brady writes: > Attached in the patch I intend to push in your name. Nice. > I also added docs to usage() and the texinfo file, and added a test. I don't quite understand how the test works, but as far as I see, it doesn't test floats? So that's inconsistent with the commit message. > BTW I checked if there was any speed difference with the new code. > I wasn't expecting this to be a bottleneck, and true enough > there is only a marginal change. The new code is consistently > a little _faster_ though on my i3-2310M which is a bit surprising. Odd. But performance of x86 is usually pretty hard to predict by just looking at the source or assembly code. I was hoping that in the non-swapped case, the false conditional if (input_swap && sizeof(T) > 1) should be very friendly to the branch predictor, and hence almost free. Jim Meyering writes: > One nit: please change the type of "j" here (identical in attached) > to be unsigned, to match that of the upper bound. Makes sense. In my own projects, I tend to use unsigned int for loop counts whereever I don't need to iterate over any negative values. But my impression is that most others prefer to use signed int for everything which doesn't rely on mod 2^n arithmetic, so that's why I made j signed here. > That would be our first use of "rev". Is it ubiquitous enough to depend on? It appears *not* to be available on my closest solaris box. While on my gnu/linux system, it's provided by util-linux. For the test, I guess rev could be implemented something like while read line printf "%s" line | tr -d '\n' | sed 's/./.\n/' | tac | tr -d '\n' echo done Maybe rev should be provided by coreutils, similarly to tac? I'd prefer not to think about the unicode issues for rev, though... Regards, /Niels -- Niels Möller. PGP-encrypted email is preferred. Keyid C0B98E26. Internet email is subject to wholesale government surveillance.
