Re: [PATCH] ls: allow stat-free use of --color

[Jim Meyering]

Jim Meyering wrote:
> Pádraig Brady wrote:
>> On 11/05/11 16:52, Jim Meyering wrote:
>>> diff --git a/src/ls.c b/src/ls.c
>>> index 4262923..54876ce 100644
>>> --- a/src/ls.c
>>> +++ b/src/ls.c
>>> @@ -2740,7 +2740,10 @@ gobble_file (char const *name, enum filetype type, 
>>> ino_t inode,
>>>        /* When coloring a directory (we may know the type from
>>>           direct.d_type), we have to stat it in order to indicate
>>>           sticky and/or other-writable attributes.  */
>>> -      || (type == directory && print_with_color)
>>> +      || (type == directory && print_with_color
>>> +          && (is_colored (C_OTHER_WRITABLE)
>>> +              || is_colored (C_STICKY)
>>> +              || is_colored (C_STICKY_OTHER_WRITABLE)))
>>>        /* 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)
>>
>> Looks good.
>
> Thanks for the review.

The initial patch was by far the quickest and easiest part of this change.
Thanks to Pádraig for most of the d_type-check script used in the test.

>From 94768db6077253c1387e0a3de2b4cdbc7f63118b Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 11 May 2011 17:13:53 +0200
Subject: [PATCH] ls: allow stat-free use of --color
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Even on a system with d_type support, the default use of --color
makes ls stat every file in order to be able to honor settings like
EXEC, STICKY, ORPHAN, SETUID, etc., because those settings require
information that is not provided by dirent.d_type.  However, if
for a potentially large performance gain, you are willing to disable
those settings, you can now make ls --color give type-related coloring
and perform no stat calls at all (other than the unavoidable call-per-
command-line argument).  Before this change, even with all of those
attributes disabled, ls --color would still stat every directory.
Now, we're down to the minimum of one stat call per command-line arg.
* src/ls.c (gobble_file): With --color, don't stat a
non-command-line-specified directory when no directory-coloring
attribute is enabled.
* tests/init.cfg (require_dirent_d_type_): New function.
* tests/d_type-check: New script, mostly from Pádraig Brady.
* tests/Makefile.am (EXTRA_DIST): Add it.
* tests/ls/stat-free-color: New test.
* tests/Makefile.am (TESTS): Add it.
* doc/coreutils.texi (General output formatting): Describe how
to use dircolors to make ls --color refrain from calling stat
on a d_type-enabled file system.
Prompted by a query from Josef Bacik.
---
 doc/coreutils.texi       |   17 ++++++++++++++
 src/ls.c                 |    5 +++-
 tests/Makefile.am        |    2 +
 tests/d_type-check       |   37 +++++++++++++++++++++++++++++++
 tests/init.cfg           |   10 ++++++++
 tests/ls/stat-free-color |   54 ++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 124 insertions(+), 1 deletions(-)
 create mode 100644 tests/d_type-check
 create mode 100755 tests/ls/stat-free-color

diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 457ecab..b77d8df 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -7042,6 +7042,23 @@ General output formatting
 @command{less} usually produces unreadable results.  However, using
 @code{more -f} does seem to work.

+@vindex LS_COLORS
+@vindex SHELL @r{environment variable, and color}
+Note that using the @option{--color} option may incur a noticeable
+performance penalty when run in a directory with very many entries,
+because the default settings require that @command{ls} @code{stat} every
+single file it lists.
+However, if you would like most of the file-type coloring
+but can live without the other coloring options (e.g.,
+executable, orphan, sticky, other-writable, capability), use
+@command{dircolors} to set the @env{LS_COLORS} environment variable like this,
+@example
+eval $(dircolors -p | perl -pe \
+  's/^((CAP|S[ET]|O[TR]|M|E)\w+).*/$1 00/' | dircolors -)
+@end example
+and on a @code{dirent.d_type}-capable file system, @command{ls}
+will perform only one @code{stat} call per command line argument.
+
 @item -F
 @itemx --classify
 @itemx --indicator-style=classify
diff --git a/src/ls.c b/src/ls.c
index 4262923..54876ce 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -2740,7 +2740,10 @@ gobble_file (char const *name, enum filetype type, ino_t 
inode,
       /* When coloring a directory (we may know the type from
          direct.d_type), we have to stat it in order to indicate
          sticky and/or other-writable attributes.  */
-      || (type == directory && print_with_color)
+      || (type == directory && print_with_color
+          && (is_colored (C_OTHER_WRITABLE)
+              || is_colored (C_STICKY)
+              || is_colored (C_STICKY_OTHER_WRITABLE)))
       /* 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)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 6a8b79d..9bff3f0 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -10,6 +10,7 @@ EXTRA_DIST =          \
   CuSkip.pm            \
   CuTmpdir.pm          \
   check.mk             \
+  d_type-check         \
   envvar-check         \
   filefrag-extent-compare \
   fiemap-capable       \
@@ -426,6 +427,7 @@ TESTS =                                             \
   ls/rt-1                                      \
   ls/stat-dtype                                        \
   ls/stat-failed                               \
+  ls/stat-free-color                           \
   ls/stat-free-symlinks                                \
   ls/stat-vs-dirent                            \
   ls/symlink-slash                             \
diff --git a/tests/d_type-check b/tests/d_type-check
new file mode 100644
index 0000000..e409049
--- /dev/null
+++ b/tests/d_type-check
@@ -0,0 +1,37 @@
+#!/usr/bin/python
+# Exit 0 if "." has useful d_type information, else 1.
+# Intended to exit 0 only on Linux/GNU systems.
+import sys
+
+fail = 1
+try:
+  import ctypes
+
+  (DT_UNKNOWN, DT_DIR,) = (0, 4,)
+
+  class dirent(ctypes.Structure):
+    _fields_ = [
+      ("d_ino", ctypes.c_long),
+      ("d_off", ctypes.c_long),
+      ("d_reclen", ctypes.c_ushort),
+      ("d_type", ctypes.c_ubyte),
+      ("d_name", ctypes.c_char*256)]
+
+  direntp = ctypes.POINTER(dirent)
+
+  # FIXME: find a way to hard-coding libc's so-name.
+  libc = ctypes.cdll.LoadLibrary("libc.so.6")
+  libc.readdir.restype = direntp
+
+  dirp = libc.opendir(".")
+  if dirp:
+    ep = libc.readdir(dirp)
+    if ep:
+      name = ep.contents.d_name
+      if (name == "." or name == "..") and ep.contents.d_type == DT_DIR:
+        fail = 0
+
+except:
+  pass
+
+sys.exit(fail)
diff --git a/tests/init.cfg b/tests/init.cfg
index 92f841f..b7ecb30 100644
--- a/tests/init.cfg
+++ b/tests/init.cfg
@@ -322,6 +322,16 @@ fiemap_capable_()
   python $abs_srcdir/fiemap-capable "$@"
 }

+# Skip the current test if "." lacks d_type support.
+require_dirent_d_type_()
+{
+  python < /dev/null \
+    || skip_ python missing: assuming no d_type support
+
+  python $abs_srcdir/d_type-check \
+    || skip_ requires d_type support
+}
+
 # Does the current (working-dir) file system support sparse files?
 require_sparse_support_()
 {
diff --git a/tests/ls/stat-free-color b/tests/ls/stat-free-color
new file mode 100755
index 0000000..d11c6f4
--- /dev/null
+++ b/tests/ls/stat-free-color
@@ -0,0 +1,54 @@
+#!/bin/sh
+# Show that --color need not use stat, as long as we have d_type support.
+
+# Copyright (C) 2011 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
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../src
+print_ver_ ls
+require_strace_ stat
+require_dirent_d_type_
+
+ln -s nowhere dangle || framework_failure_
+
+# Disable enough features via LS_COLORS so that ls --color
+# can do its job without calling stat (other than the obligatory
+# one-call-per-command-line argument).
+cat <<EOF > color-without-stat || framework_failure_
+RESET 0
+DIR 01;34
+LINK 01;36
+FIFO 40;33
+SOCK 01;35
+DOOR 01;35
+BLK 40;33;01
+CHR 40;33;01
+ORPHAN 00
+SETUID 00
+SETGID 00
+CAPABILITY 00
+STICKY_OTHER_WRITABLE 00
+OTHER_WRITABLE 00
+STICKY 00
+EXEC 00
+MULTIHARDLINK 00
+EOF
+eval $(dircolors -b color-without-stat)
+
+strace -o log -e stat,lstat ls --color=always . || fail=1
+n_lines=$(wc -l < log)
+test $n_lines = 1 || fail=1
+
+Exit $fail
--
1.7.5.1.398.g86d1d