Fix another bug: ls --indicator-style=file-type would call

stat for a symlink, even though it wasn't always needed.
In some cases, that unnecessary stat would cause ls to fail.
* src/ls.c (gobble_file): Don't treat symlinks specially (in
requiring a stat syscall).  Remove the offending exclusion.
* NEWS: Mention the fix.
* tests/ls/stat-dtype: New file/test, for the above fix.
Also exercises the new df feature, below.

diff --git a/ChangeLog b/ChangeLog
index 2f4a20e..874e987 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,4 +1,13 @@
-2006-07-20  Jim Meyering  <jim@meyering.net>
+2006-07-21  Jim Meyering  <jim@meyering.net>
+
+       Fix another bug: ls --indicator-style=file-type would call
+       stat for a symlink, even though it wasn't always needed.
+       In some cases, that unnecessary stat would cause ls to fail.
+       * src/ls.c (gobble_file): Don't treat symlinks specially (in
+       requiring a stat syscall).  Remove the offending exclusion.
+       * NEWS: Mention the fix.
+       * tests/ls/stat-dtype: New file/test, for the above fix.
+       Also exercises the new df feature, below.
 
        * src/df.c (main): Fail and don't print the headers if no
        file system is processed.  This makes it easy to test whether

diff --git a/NEWS b/NEWS
index 0ec1148..b1991b1 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -201,6 +201,9 @@ GNU coreutils NEWS                                    -*- outline -*-
   a very long symlink chain as a dangling symlink.  Before, such a
   misinterpretation would cause these tools not to diagnose an ELOOP error.
 
+  ls --indicator-style=file-type would sometimes stat a symlink
+  unnecessarily.
+
   mv: moving a symlink into the place of an existing non-directory is
   now done atomically;  before, mv would first unlink the destination.
 

diff --git a/src/ls.c b/src/ls.c
index ef0b902..4a42132 100644 (file)
--- a/src/ls.c
+++ b/src/ls.c
@@ -2547,12 +2547,6 @@ gobble_file (char const *name, enum filetype type, ino_t inode,
                  && (type == symbolic_link || type == unknown))))
       || (format_needs_type
          && (type == unknown
-
-             /* FIXME: remove this disjunct.
-                I don't think we care about symlinks here, but for now
-                this won't make a big performance difference.  */
-             || type == symbolic_link
-
              /* --indicator-style=classify (aka -F)
                 requires that we stat each regular file
                 to see if it's executable.  */

diff --git a/tests/ls/Makefile.am b/tests/ls/Makefile.am
index 87f2040..7c6ae5a 100644 (file)
--- a/tests/ls/Makefile.am
+++ b/tests/ls/Makefile.am
@@ -3,6 +3,7 @@
 AUTOMAKE_OPTIONS = 1.2 gnits
 
 TESTS = \
+  stat-dtype \
   inode dangle file-type recursive dired infloop \
   rt-1 time-1 symlink-slash follow-slink no-arg m-option \
   stat-vs-dirent

diff --git a/tests/ls/stat-dtype b/tests/ls/stat-dtype
new file mode 100755 (executable)
index 0000000..30a84b6
--- /dev/null
+++ b/tests/ls/stat-dtype
@@ -0,0 +1,51 @@
+#!/bin/sh
+# Ensure that ls --file-type does not call stat unnecessarily.
+
+if test "$VERBOSE" = yes; then
+  set -x
+  ls --version
+fi
+
+. $srcdir/../envvar-check
+
+# Skip this test unless "." is on a file system with useful d_type info.
+# FIXME: use a more dynamic test for this, since whether
+# d_type is useful depends on much more than the file system type.
+# For example, with linux-2.6.15, at least tmpfs, ext3 and reiserfs work,
+# but xfs doesn't.  Here's hoping that this kludge is enough for now.
+df -t tmpfs -t ext3 -t reiserfs . 2> /dev/null ||
+  {
+    echo "$0: '.' is not on a suitable file system for this test" 1>&2
+    echo "$0: skipping this test" 1>&2
+    (exit 77); exit 77
+  }
+
+pwd=`pwd`
+t0=`echo "$0"|sed 's,.*/,,'`.tmp; tmp=$t0/$$
+trap 'status=$?; cd $pwd; chmod -R u+rwx $t0; rm -rf $t0 && exit $status' 0
+trap '(exit $?); exit $?' 1 2 13 15
+
+framework_failure=0
+mkdir -p $tmp || framework_failure=1
+cd $tmp || framework_failure=1
+
+mkdir d || framework_failure=1
+ln -s / d/s || framework_failure=1
+chmod 600 d || framework_failure=1
+
+if test $framework_failure = 1; then
+  echo "$0: failure in testing framework" 1>&2
+  (exit 1); exit 1
+fi
+
+fail=0
+
+ls --file-type d > out || fail=1
+cat <<\EOF > exp || fail=1
+s@
+EOF
+
+cmp out exp || fail=1
+test $fail = 1 && diff out exp 2> /dev/null
+
+(exit $fail); exit $fail