Re: ls -i inefficiency

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Jim Meyering on 2/25/2006 4:54 AM:
> Thanks for the suggestion.
> If this isn't too invasive, I'm interested.
> I think `ls -i' (without some other option requiring stat info)
> is used far less often than, say `ls -F', so it's harder to justify
> adding much complexity to optimize for this relatively unusual case.

And here's the patch.  As a nice side effect, it also optimized 'ls -L' to
avoid stat() (after all, dereferencing makes no sense when all you need is
file names, and no information from the dereference).

2006-02-25  Eric Blake  <address@hidden>

        * src/pwd.c (NOT_AN_INODE_NUMBER, D_INO): Move to ...
        * src/system.h: ... here, for use in ...
        * src/ls.c (main): ... here.  Prefer dirent.d_ino to stat when
        possible.
        (gobble_file): Add inode argument.
        (print_dir): Pass inode if available.
        (usage): Remove inaccuracy.

- --
Life is short - so eat dessert first!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFEAGGb84KuGfSFAYARAsR9AJ9Oe4NFzALSh0DACSHCQ7zq82NNzACeOVsU
jgvBPfo2j2KZo8+otKgmJSk=
=jzq9
-----END PGP SIGNATURE-----

Index: src/system.h
===================================================================
RCS file: /sources/coreutils/coreutils/src/system.h,v
retrieving revision 1.142
diff -u -p -r1.142 system.h
--- src/system.h        7 Feb 2006 22:32:50 -0000       1.142
+++ src/system.h        25 Feb 2006 13:52:11 -0000
@@ -244,6 +244,18 @@ initialize_exit_failure (int status)
 # define CLOSEDIR(d) closedir (d)
 #endif
 
+enum
+{
+  NOT_AN_INODE_NUMBER = 0
+};
+
+#ifdef D_INO_IN_DIRENT
+# define D_INO(dp) ((dp)->d_ino)
+#else
+/* Some systems don't have inodes, so fake them to avoid lots of ifdefs.  */
+# define D_INO(dp) NOT_AN_INODE_NUMBER
+#endif
+
 /* Get or fake the disk device blocksize.
    Usually defined by sys/param.h (if at all).  */
 #if !defined DEV_BSIZE && defined BSIZE
Index: src/pwd.c
===================================================================
RCS file: /sources/coreutils/coreutils/src/pwd.c,v
retrieving revision 1.58
diff -u -p -r1.58 pwd.c
--- src/pwd.c   1 Feb 2006 14:43:24 -0000       1.58
+++ src/pwd.c   25 Feb 2006 13:52:11 -0000
@@ -40,18 +40,6 @@ struct file_name
   char *start;
 };
 
-enum
-{
-  NOT_AN_INODE_NUMBER = 0
-};
-
-#ifdef D_INO_IN_DIRENT
-# define D_INO(dp) ((dp)->d_ino)
-#else
-/* Some systems don't have inodes, so fake them to avoid lots of ifdefs.  */
-# define D_INO(dp) NOT_AN_INODE_NUMBER
-#endif
-
 /* The name this program was run with. */
 char *program_name;
 
Index: src/ls.c
===================================================================
RCS file: /sources/coreutils/coreutils/src/ls.c,v
retrieving revision 1.405
diff -u -p -r1.405 ls.c
--- src/ls.c    10 Jan 2006 07:31:21 -0000      1.405
+++ src/ls.c    25 Feb 2006 13:52:11 -0000
@@ -231,7 +231,8 @@ static char *make_link_name (char const 
 static int decode_switches (int argc, char **argv);
 static bool file_ignored (char const *name);
 static uintmax_t gobble_file (char const *name, enum filetype type,
-                             bool command_line_arg, char const *dirname);
+                             ino_t inode, bool command_line_arg,
+                             char const *dirname);
 static void print_color_indicator (const char *name, mode_t mode, int linkok);
 static void put_indicator (const struct bin_str *ind);
 static void add_ignore_pattern (const char *pattern);
@@ -1222,9 +1223,8 @@ main (int argc, char **argv)
 
   format_needs_stat = sort_type == sort_time || sort_type == sort_size
     || format == long_format
-    || dereference == DEREF_ALWAYS
-    || print_block_size || print_inode;
-  format_needs_type = (!format_needs_stat
+    || print_block_size;
+  format_needs_type = (! format_needs_stat
                       && (recursive || print_with_color
                           || indicator_style != none));
 
@@ -1245,13 +1245,13 @@ main (int argc, char **argv)
   if (n_files <= 0)
     {
       if (immediate_dirs)
-       gobble_file (".", directory, true, "");
+       gobble_file (".", directory, NOT_AN_INODE_NUMBER, true, "");
       else
        queue_directory (".", NULL, true);
     }
   else
     do
-      gobble_file (argv[i++], unknown, true, "");
+      gobble_file (argv[i++], unknown, NOT_AN_INODE_NUMBER, true, "");
     while (i < argc);
 
   if (files_index)
@@ -2351,7 +2351,8 @@ print_dir (char const *name, char const 
                  || next->d_type == DT_SOCK)
                type = next->d_type;
 #endif
-             total_blocks += gobble_file (next->d_name, type, false, name);
+             total_blocks += gobble_file (next->d_name, type, D_INO (next),
+                                          false, name);
            }
        }
       else if (errno != 0)
@@ -2496,12 +2497,16 @@ clear_files (void)
    Return the number of blocks that the file occupies.  */
 
 static uintmax_t
-gobble_file (char const *name, enum filetype type, bool command_line_arg,
-            char const *dirname)
+gobble_file (char const *name, enum filetype type, ino_t inode,
+            bool command_line_arg, char const *dirname)
 {
   uintmax_t blocks;
   struct fileinfo *f;
 
+  /* An inode value prior to gobble_file necessarily came from readdir,
+     which is not used for command line arguments.  */
+  assert (! command_line_arg || inode == NOT_AN_INODE_NUMBER);
+
   if (files_index == nfiles)
     {
       files = xnrealloc (files, nfiles, 2 * sizeof *files);
@@ -2515,6 +2520,14 @@ gobble_file (char const *name, enum file
 
   if (command_line_arg
       || format_needs_stat
+      || (print_inode
+         && (inode == NOT_AN_INODE_NUMBER
+             /* When dereferencing symlinks, the inode must come from
+                stat, but readdir provides the inode of lstat.  Command
+                line dereferences are already taken care of by the above
+                assertion that the inode number is not yet known.  */
+             || (dereference == DEREF_ALWAYS
+                 && (type == symbolic_link || type == unknown))))
       || (format_needs_type
          && (type == unknown
 
@@ -2617,8 +2630,8 @@ gobble_file (char const *name, enum file
              f->linkok = true;
 
              /* Symbolic links to directories that are mentioned on the
-                command line are automatically traced if not being
-                listed as files.  */
+                command line are automatically traced if not being
+                listed as files.  */
              if (!command_line_arg || format == long_format
                  || !S_ISDIR (linkstats.st_mode))
                {
@@ -2643,13 +2656,6 @@ gobble_file (char const *name, enum file
       else
        f->filetype = normal;
 
-      {
-       char buf[INT_BUFSIZE_BOUND (uintmax_t)];
-       int len = strlen (umaxtostr (f->stat.st_ino, buf));
-       if (inode_number_width < len)
-         inode_number_width = len;
-      }
-
       blocks = ST_NBLOCKS (f->stat);
       {
        char buf[LONGEST_HUMAN_READABLE + 1];
@@ -2715,12 +2721,21 @@ gobble_file (char const *name, enum file
   else
     {
       f->filetype = type;
+      f->stat.st_ino = inode;
 #if HAVE_STRUCT_DIRENT_D_TYPE && defined DTTOIF
       f->stat.st_mode = DTTOIF (type);
 #endif
       blocks = 0;
     }
 
+  if (print_inode)
+      {
+       char buf[INT_BUFSIZE_BOUND (uintmax_t)];
+       int len = strlen (umaxtostr (f->stat.st_ino, buf));
+       if (inode_number_width < len)
+         inode_number_width = len;
+      }
+
   f->name = xstrdup (name);
   files_index++;
 
@@ -4162,7 +4177,7 @@ Mandatory arguments to long options are 
       --indicator-style=WORD append indicator with style WORD to entry 
names:\n\
                                none (default), slash (-p),\n\
                                file-type (--file-type), classify (-F)\n\
-  -i, --inode                with -l, print the index number of each file\n\
+  -i, --inode                print the index number of each file\n\
   -I, --ignore=PATTERN       do not list implied entries matching shell 
PATTERN\n\
   -k                         like --block-size=1K\n\
 "), stdout);