From 9e0a095be648284bdc8a47d2b01120fdb01c001f Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Fri, 21 Jul 2006 08:49:24 +0000 Subject: [PATCH] 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. --- ChangeLog | 11 ++++++++++- NEWS | 3 +++ src/ls.c | 6 ------ tests/ls/Makefile.am | 1 + tests/ls/stat-dtype | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 65 insertions(+), 7 deletions(-) create mode 100755 tests/ls/stat-dtype diff --git a/ChangeLog b/ChangeLog index 2f4a20e..874e987 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,13 @@ -2006-07-20 Jim Meyering +2006-07-21 Jim Meyering + + 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 --- 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 --- 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 --- 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 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 -- 2.7.4