bug-findutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [bug #24342] -inum predicate shoud use dirent.d_ino instead of stat.


From: George Spelvin
Subject: Re: [bug #24342] -inum predicate shoud use dirent.d_ino instead of stat.st_ino
Date: Sun, 08 Mar 2009 07:42:25 -0400

(Resend because I can't spell "bug-findutils".)

Thanks for the fix!  Sorry it fell off the bottom of my to-do pile.

If I might suggest, the code here does not express the logic very clearly:

diff --git a/find/util.c b/find/util.c
index 2570682..bbc8436 100644
--- a/find/util.c
+++ b/find/util.c
@@ -235,14 +235,51 @@ get_info (const char *pathname,
    * already have it, stat the file now.
    */
   if (pred_ptr->need_stat)
-    todo = true;
+    {
+      todo = true;             /* need full stat info */
+    }
   else if ((pred_ptr->need_type && (0 == state.have_type)))
-    todo = true;
-
+    {
+      todo = true;             /* need to stat to get the type */
+    }
+  if (!todo && pred_ptr->need_inum)
+    {
+      if (state.have_type)
+       {
+         /* For now we decide not to trust struct dirent.d_ino for
+          * directry entries that are subdirectories, in case this
+          * subdirectory is a mount point.  We also need to call a
+          * stat function if we don't have st_ino (i.e. it is zero).
+          */
+         if (S_ISDIR(p->st_mode) || (!p->st_ino))
+           todo = true;
+       }
+      else
+       {
+         todo = true;
+       }
+    }

First, I'd neaten it up to

  if (pred_ptr->need_stat)
    {
      todo = true;              /* need full stat info */
    }
  else if (pred_ptr->need_type && !state.have_type)
    {
      todo = true;              /* need to stat to get the type */
    }
  else if (pred_ptr->need_inum)
    {
      if (!p->st_ino)
        {
          todo = true;
        }
      eles if (!state.have_type || S_ISDIR(p->st_mode))
        {
          /* For now we decide not to trust struct dirent.d_ino for
           * directry entries that are subdirectories, in case this
           * subdirectory is a mount point.  We also need to call a
           * stat function if we don't have st_ino (i.e. it is zero).
           */
          todo = true;
        }
    }

And then it could be further compacted, if you like, to,

  /* For now we decide not to trust struct dirent.d_ino for directry
   * entries that are subdirectories, in case this subdirectory is a
   * mount point.  We also need to call a stat function if we don't have
   * st_ino (i.e. it is zero).
   */
  todo = pred_ptr->need_stat || (pred_ptr->need_type && !state.have_type) ||
         (pred_ptr->need_inum && (!p->st-ino || !state.have_type || 
S_ISDIR(p->st_mode)));

And then you could pass "leaf" down to apply_predicate -> get_info and optimize 
it further to:

  todo = pred_ptr->need_stat || (pred_ptr->need_type && !state.have_type) ||
         (pred_ptr->need_inum && (!p->st-ino || (state.have_type ? 
S_ISDIR(p->st_mode) : !leaf)));

If you prefer the nested-if form, the leaf optimization looks like:

  if (pred_ptr->need_stat)
    {
      todo = true;              /* need full stat info */
    }
  else if (pred_ptr->need_type && !state.have_type)
    {
      todo = true;              /* need to stat to get the type */
    }
  else if (pred_ptr->need_inum)
    {
      if (!p->st_ino)
        {
          todo = true;
        }
      else if (!state.have_type)
        {
          todo = !leaf;
        }
      else if (S_ISDIR(p->st_mode))
        {
          /* For now we decide not to trust struct dirent.d_ino for
           * directry entries that are subdirectories, in case this
           * subdirectory is a mount point.  We also need to call a
           * stat function if we don't have st_ino (i.e. it is zero).
           */
          todo = true;
        }
    }

(Any and all copyrights to the above code abandoned to the public domain.
Go nuts.)




reply via email to

[Prev in Thread] Current Thread [Next in Thread]