Re: bug with ls: unnecessary stat calls on symlink targets

[Jim Meyering]

Jeremy Maitin-Shepard <address@hidden> wrote:

> Jim Meyering <address@hidden> writes:
>
>> Jeremy Maitin-Shepard <address@hidden> wrote:
>>> Currently, if there is a color for LINK, and EXEC, but not for ORPHAN or
>>> MISSING, ls still stats the target of every symlink, but never uses the
>>> information.  It does this as a result of the following commit:
>>>
>>>
>> http://cvs.savannah.gnu.org/viewvc/coreutils/src/ls.c?root=coreutils&r1=1.369&r2=1.370
>> ...
>
>> Thanks for reporting this.
>> I'll investigate in the next few days.
>
> Have you come to any conclusions regarding this bug?

The problem is that the ln=target attribute is not well documented,
and it makes ls color symlinks according to the type of the
referenced (target) file.  Removing the test that you suggest
would break that usage.

To get closer to the behavior you want, you can turn off coloring
of ORPHAN and MISSING (things that require dereferencing symlinks)
and put ls in no-dereference mode with -F:

  touch x
  chmod a+x x
  ln -s x link-to-x
  eval "$(dircolors --p |perl -pe 's/^(ORPHAN|MISSING).*/$1 0/'|dircolors -)"; 
strace -e stat ls -F --color x link-to-x

That didn't quite work, because of EXEC, hence first hunk of the
patch below.  Also, it didn't color the symlink, which led to the
second hunk.

The lack of output from strace -e stat shows that the above
avoids those stat calls.

Of course, you may find the side effect of -F (appending the
type indicator byte to the name) undesirable.  I looked for ls's
--no-dereference option, but it doesn't exist.  -F -d and -l all enable
that internal bit of functionality.

I'll check in something like the following once I've written a test.

diff --git a/src/ls.c b/src/ls.c
index 840f310..78e90c8 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -1173,7 +1173,7 @@ main (int argc, char **argv)
     {
       /* Avoid following symbolic links when possible.  */
       if (is_colored (C_ORPHAN)
-         || is_colored (C_EXEC)
+         || (is_colored (C_EXEC) && color_symlink_as_referent)
          || (is_colored (C_MISSING) && format == long_format))
        check_symlink_color = true;

@@ -2722,6 +2722,12 @@ gobble_file (char const *name, enum filetype type, ino_t 
inode,
          free (linkname);
        }

+      /* When not distinguishing types of symlinks, pretend we know that
+        it is stat'able, so that it will be colored as a regular symlink,
+        and not as an orphan.  */
+      if (S_ISLNK (f->stat.st_mode) && !check_symlink_color)
+       f->linkok = true;
+
       if (S_ISLNK (f->stat.st_mode))
        f->filetype = symbolic_link;
       else if (S_ISDIR (f->stat.st_mode))