bug-gnulib
[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 16:05:22 -0400

James Youngman <address@hidden> wrote:

> [ adding nug-gnulib to CC since we're discussinf its fts implementation ]
>
> On Sun, Mar 8, 2009 at 2:50 PM, George Spelvin <address@hidden> wrote:
>> James Youngman <address@hidden> wrote:
>>> The updated patches are what I actually pushed (into 4.5.x).
>>
>> Ah, thank you.  The posted patches wouldn't apply to savannah git HEAD
>> due to a conflict in the ChangeLog.  (Easy enough to work around, but
>> worrisome.)
>
> They're *in* HEAD.

Um, they weren't earlier today.  (I cloned at 2009-03-08 11:49:25 UTC.)

>
> >
> > The (tested) simplified version is, of course,
> > + ?boolean 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)));
> >
> > I poked at the leaf optimization, but it appears I'd have to change the
> > prototypes for the entire predicate system, which is extremely annoying.
>
> Yes.   I also believe it would gain little, since in the case of
> ftsfind at least, leaf is normally 0.

Oh.  Is that because ftsfind doesn't implement the optimization, or is there
some more fundamental reason?

>> One option is to understand the "state" structure and how its contents
>> relate to the predicate arguments.  I'm currently very confused by that.
>
> The state structure contains a bunch of what would otherwise be global
> variables.   The struct predicate instances represent tests or actions
> specified on the command line.  With a few exceptions (for example
> struct predicate_performance_info) the information in struct predicate
> is unchanged after we begin traversing the filesystem.

Thank you for the explanation, but due to my sloppy phrasing, it's not to
what I was wondering about.  I understand that the "struct predicate"
tree is constant; I was referring to the variety of 

boolean
pred_FOO (const char *pathname, struct stat *stat_buf, struct predicate 
*pred_ptr)
{
        ...
}

helper functions and those function arguments.  Consider, for example, the
pred_type function, which begins:

boolean
pred_type (const char *pathname, struct stat *stat_buf, struct predicate 
*pred_ptr)
{
  mode_t mode;
  mode_t type = pred_ptr->args.type;

  assert (state.have_type);

  if (0 == state.type)
    {
      /* This can sometimes happen with broken NFS servers.
       * See Savannah bug #16378.
       */
      return false;
    }

  (void) pathname;

  if (state.have_stat)
     mode = stat_buf->st_mode;
  else
     mode = state.type;

Notice how it seems to assume that state.have_stat describes the same
"current file" as the stat_buf argument.

So perhaps the leaf flag could be stuffed into the state structure.

(Ideally, the pathname and the stat_buf would be stuffed into the
state structure and a pointer to it would be passed down, so the
find engine would be reentrant.)

> FWIW, some broken NFS servers (or perhaps NFS servers serving
> corrupted filesystems) return invalid st_mode values; Savannah bug
> #16378 at https://savannah.gnu.org/bugs/?16378 gives an example.

Joy.




reply via email to

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