[Top][All Lists]
[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.)
Re: [bug #24342] -inum predicate shoud use dirent.d_ino instead of stat.st_ino,
George Spelvin <=