Re: ls --dereference

[Paul Eggert]

Nobuyuki Tsuchimura <address@hidden> writes:

>   I'm not sure the new behaver is a bug or not,
> but it seems to be easy to reverse by an attached patch
> (if there is no side effect).
> In my opinion it is a bug, so I'll be glad if it is fixed.

I agree with you that it is a bug.  I think there is a better fix,
though, i.e., one that calls 'stat' less often.  Here is a patch.
This includes a test case to detect further instances of this bug,
should it recur.  Thanks again for reporting it.

2007-01-01  Paul Eggert  <address@hidden>

        * src/ls.c (gobble_file): Fix bug reported by
        Nobuyuki Tsuchimura in
        http://lists.gnu.org/archive/html/bug-coreutils/2006-12/msg00152.html
        where "ls -FRL" didn't follow a symbolic link in some cases on Linux.
        * tests/ls/follow-slink: Add a test for this case.

diff --git a/src/ls.c b/src/ls.c
index 63d363f..feba591 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -1,5 +1,5 @@
 /* `dir', `vdir' and `ls' directory listing programs for GNU.
-   Copyright (C) 85, 88, 90, 91, 1995-2006 Free Software Foundation, Inc.
+   Copyright (C) 85, 88, 90, 91, 1995-2007 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
@@ -2534,14 +2534,15 @@ gobble_file (char const *name, enum file
         direct.d_type), we have to stat it in order to indicate
         sticky and/or other-writable attributes.  */
       || (type == directory && print_with_color)
-      || (print_inode
-         && (inode == NOT_AN_INODE_NUMBER
-             /* When dereferencing symlinks, the inode must come from
-                stat, but readdir provides the inode of lstat.  Command
-                line dereferences are already taken care of by the above
-                assertion that the inode number is not yet known.  */
-             || (dereference == DEREF_ALWAYS
-                 && (type == symbolic_link || type == unknown))))
+      /* When dereferencing symlinks, the inode and type must come from
+        stat, but readdir provides the inode and type of lstat.  */
+      || ((print_inode || format_needs_type)
+         && (type == symbolic_link || type == unknown)
+         && (dereference == DEREF_ALWAYS
+             || (command_line_arg && dereference != DEREF_NEVER)))
+      /* Command line dereferences are already taken care of by the above
+        assertion that the inode number is not yet known.  */
+      || (print_inode && inode == NOT_AN_INODE_NUMBER)
       || (format_needs_type
          && (type == unknown || command_line_arg
              /* --indicator-style=classify (aka -F)
diff --git a/tests/ls/follow-slink b/tests/ls/follow-slink
index e79896b..00e9b1b 100755
--- a/tests/ls/follow-slink
+++ b/tests/ls/follow-slink
@@ -1,7 +1,7 @@
 #!/bin/sh
 # make sure ls -L always follows symlinks

-# Copyright (C) 2000, 2002, 2004, 2006 Free Software Foundation, Inc.
+# Copyright (C) 2000, 2002, 2004, 2006, 2007 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
@@ -33,9 +33,10 @@ mkdir $tmp || framework_failure=1
 cd $tmp || framework_failure=1

 # Isolate output files from directory being listed
-mkdir dir || framework_failure=1
+mkdir dir dir/sub dir1 || framework_failure=1
 cd dir || framework_failure=1
 ln -s link link || framework_failure=1
+ln -s ../../dir1 sub/link-to-dir || framework_failure=1

 # Make sure the symlink was created.
 # `ln -s link link' succeeds, but creates no file on
@@ -56,15 +57,30 @@ ls -L link 2> /dev/null && fail=1
 # list the link, provided no further information about the link needed
 # to be printed.  Since POSIX does not specify one way or the other, we
 # opt for compatibility (this was broken in 5.3.0 through 5.94).
-ls -L > ../out || fail=1
+LC_ALL=C ls -L > ../out-L || fail=1
+LC_ALL=C ls -FLR sub > ../out-FLR-sub || fail=1

 cd .. || fail=1

-cat <<\EOF > exp
+cat <<\EOF > exp-L
 link
+sub
 EOF

-cmp out exp || fail=1
-test $fail = 1 && diff out exp 2> /dev/null
+cat <<\EOF > exp-FLR-sub
+sub:
+link-to-dir/
+
+sub/link-to-dir:
+EOF
+
+cmp out-L exp-L || {
+  fail=1
+  diff out-L exp-L
+}
+cmp out-FLR-sub exp-FLR-sub || {
+  fail=1
+  diff out-FLR-sub exp-FLR-sub
+}

 (exit $fail); exit $fail
M ChangeLog
M src/ls.c
M tests/ls/follow-slink
Committed as e531e1e3abdacf7b55a99c88028a780ed7bd0339