FYI: two(!) new ls bugs

[Jim Meyering]

I found two bugs in ls.
Also, the df change mentioned below is actually more of a fix
than a feature.  For now, these changes are only on the trunk.

Here are the changes:

2006-07-21  Jim Meyering  <address@hidden>

        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
        a specified directory is on a file system of a given type or types.
        Otherwise, applications would have had to parse df's output.
        E.g., is "." either ext3 or reiserfs: df -t ext3 -t reiserfs .

        Fix a bug: ls --file-type worked like --indicator-style=slash,
        rather than like --indicator-style=file-type.
        * src/ls.c (FILE_TYPE_INDICATOR_OPTION): New enum member.
        (long_options): Map "file-type" to FILE_TYPE_INDICATOR_OPTION,
        not to 'p'.
        (decode_switches): Handle new case: FILE_TYPE_INDICATOR_OPTION.
        * NEWS: Mention the fix.
        * tests/ls-2/tests (file-type): New test, for the above fix.

Index: NEWS
===================================================================
RCS file: /fetish/cu/NEWS,v
retrieving revision 1.392
retrieving revision 1.394
diff -u -p -r1.392 -r1.394
--- NEWS        17 Jul 2006 03:02:45 -0000      1.392
+++ NEWS        21 Jul 2006 08:58:00 -0000      1.394
@@ -201,6 +201,12 @@ GNU coreutils NEWS
   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.
+
+  ls --file-type worked like --indicator-style=slash (-p),
+  rather than like --indicator-style=file-type.
+
   mv: moving a symlink into the place of an existing non-directory is
   now done atomically;  before, mv would first unlink the destination.
 
Index: src/df.c
===================================================================
RCS file: /fetish/cu/src/df.c,v
retrieving revision 1.172
retrieving revision 1.173
diff -u -p -r1.172 -r1.173
--- src/df.c    24 Jan 2006 23:30:46 -0000      1.172
+++ src/df.c    20 Jul 2006 15:42:26 -0000      1.173
@@ -68,6 +68,9 @@ static uintmax_t output_block_size;
 /* If true, use the POSIX output format.  */
 static bool posix_format;
 
+/* Count the number of valid arguments.  */
+unsigned int n_valid_args;
+
 /* If true, invoke the `sync' system call before getting any usage data.
    Using this option can make df very slow, especially with many or very
    busy disks.  Note that this may make a difference on some systems --
@@ -292,6 +295,8 @@ show_dev (char const *disk, char const *
   if (!selected_fstype (fstype) || excluded_fstype (fstype))
     return;
 
+  ++n_valid_args;
+
   /* If MOUNT_POINT is NULL, then the file system is not mounted, and this
      program reports on the file system that the special file is on.
      It would be better to report on the unmounted file system,
@@ -762,7 +767,6 @@ main (int argc, char **argv)
 {
   int c;
   struct stat *stats IF_LINT (= 0);
-  int n_valid_args = 0;
 
   initialize_main (&argc, &argv);
   program_name = argv[0];
@@ -894,10 +898,6 @@ main (int argc, char **argv)
              exit_status = EXIT_FAILURE;
              argv[i] = NULL;
            }
-         else
-           {
-             ++n_valid_args;
-           }
        }
     }
 
@@ -941,5 +941,8 @@ main (int argc, char **argv)
       show_all_entries ();
     }
 
+  if (n_valid_args == 0)
+    error (EXIT_FAILURE, 0, _("no file systems processed"));
+
   exit (exit_status);
 }
Index: src/ls.c
===================================================================
RCS file: /fetish/cu/src/ls.c,v
retrieving revision 1.419
retrieving revision 1.421
diff -u -p -r1.419 -r1.421
--- src/ls.c    19 Jul 2006 10:36:02 -0000      1.419
+++ src/ls.c    21 Jul 2006 08:49:25 -0000      1.421
@@ -737,6 +737,7 @@ enum
   GROUP_DIRECTORIES_FIRST_OPTION,
   HIDE_OPTION,
   INDICATOR_STYLE_OPTION,
+  FILE_TYPE_INDICATOR_OPTION,
 
   /* FIXME: --kilobytes is deprecated (but not -k); remove in late 2006 */
   KILOBYTES_LONG_OPTION,
@@ -770,7 +771,7 @@ static struct option const long_options[
   {"almost-all", no_argument, NULL, 'A'},
   {"ignore-backups", no_argument, NULL, 'B'},
   {"classify", no_argument, NULL, 'F'},
-  {"file-type", no_argument, NULL, 'p'},
+  {"file-type", no_argument, NULL, FILE_TYPE_INDICATOR_OPTION},
   {"si", no_argument, NULL, SI_OPTION},
   {"dereference-command-line", no_argument, NULL, 'H'},
   {"dereference-command-line-symlink-to-dir", no_argument, NULL,
@@ -1530,6 +1531,10 @@ decode_switches (int argc, char **argv)
          print_with_color = false;     /* disable --color */
          break;
 
+       case FILE_TYPE_INDICATOR_OPTION: /* --file-type */
+         indicator_style = file_type;
+         break;
+
        case 'g':
          format = long_format;
          print_owner = false;
@@ -2542,12 +2547,6 @@ gobble_file (char const *name, enum file
                  && (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.  */
Index: tests/ls/Makefile.am
===================================================================
RCS file: /fetish/cu/tests/ls/Makefile.am,v
retrieving revision 1.22
retrieving revision 1.23
diff -u -p -r1.22 -r1.23
--- tests/ls/Makefile.am        3 Jul 2006 12:55:33 -0000       1.22
+++ tests/ls/Makefile.am        21 Jul 2006 08:49:25 -0000      1.23
@@ -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
Index: tests/ls/stat-dtype
===================================================================
RCS file: tests/ls/stat-dtype
diff -N tests/ls/stat-dtype
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ tests/ls/stat-dtype 21 Jul 2006 08:49:25 -0000      1.1
@@ -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
Index: tests/ls-2/tests
===================================================================
RCS file: /fetish/cu/tests/ls-2/tests,v
retrieving revision 1.24
retrieving revision 1.25
diff -u -p -r1.24 -r1.25
--- tests/ls-2/tests    25 Sep 2005 20:46:30 -0000      1.24
+++ tests/ls-2/tests    20 Jul 2006 10:30:42 -0000      1.25
@@ -1,4 +1,5 @@
 #!/bin/sh
+# -*- perl -*-
 
 : ${PERL=perl}
 : ${srcdir=.}
@@ -31,6 +32,11 @@ my $slink_d = {PRE => sub {symlink '/', 
                            }};
 my $unlink_d = {POST => sub {unlink 'd' or die "d: $!\n"}};
 
+my $mkdir_d_slink = {PRE => sub {mkdir 'd',0755 or die "d: $!\n";
+                                symlink '/', 'd/s' or die "d/s: $!\n" }};
+my $rmdir_d_slink = {POST => sub {unlink 'd/s' or die "d/s: $!\n";
+                                 rmdir 'd' or die "d: $!\n" }};
+
 sub make_j_d ()
 {
   mkdir 'j', 0700 or die "creating j: $!\n";
@@ -126,6 +132,10 @@ my @Tests =
          unlink qw(setuid setgid);
         foreach my $dir (qw(owr owt sticky)) {rmdir $dir} }},
         ],
+
+     # For 5.97 and earlier, --file-type acted like --indicator-style=slash.
+     ['file-type', '--file-type d', {OUT => "address@hidden"},
+      $mkdir_d_slink, $rmdir_d_slink],
     );
 
 my $save_temps = $ENV{SAVE_TEMPS};