apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@lyra.org>
Subject Re: New directory API...
Date Mon, 22 Jan 2001 10:23:15 GMT
On Mon, Jan 22, 2001 at 12:24:34AM -0600, William A. Rowe, Jr. wrote:
> > I dislike combining the readdir and the stat call.  We can provide what
> > comes for free, anything more should be the result of a separate stat
> > call.


> Alrighty then... I will live with that till we are proven wrong.  I presume 
> we mean to 'reuse' the apr_finfo_t struct in the subsequent call,

This won't be possible.

Consider if we try to reuse it. That would imply that if we *aren't* reusing
the structure, then we must zero it out first. Remember all of the problems
we had when apr_open() tried to reuse the apr_file_t structures? It was a
constant source of problems.

[ not to mention all the old code that *doesn't* zero this stuff out ]

I'd say: have the user pick up the finfo->filehand if available and call
apr_getfileinfo() for the bits they need (with a new structure).

> so we
> need to watch for any 'oops'.  Of course, the user is left testing what 
> they 'wanted' twice, once to see if apr_readdir picked it up, and again to 
> see if apr_stat did the second time around.  Very sub-optimal.

Yup. I'd guess that most apps will simply do a stat rather than trying to
really optimize the case. *If* optimization is important to them at that
point in the code path, then yes: they'll compute a new wanted flag.
Actually, it won't be too difficult:

wanted = MY_WANTED_FLAGS & ~finfo.valid;

> In our first bs session about the finfo idea, we kicked around the idea
> of -incrementally- calling apr_stat.

Shades of apr_open. I really don't think we want to try this.

> > > > +    finfo->fname = apr_pstrcat(thedir->cntxt, 
> > thedir->dirname, "/", thedir->entry->d_name, NULL);
> > > 
> > > This is a sub-optimal as the original.  I'd expect most apps would
> > > simply accept this;
> > > 
> > >     finfo->fcase = thedir->entry->dname;

Agreed. Let the app decide what to do with the name. In the DAV case of
readdir(), we want a name -- not a path. [see modules/dav/fs/repos.c]

> > That's how I originally had it.  The problem is that the testfile program
> > then fails miserably.  My goal was to have the readdir function return an
> > finfo with a name that we could then use for apr_finfo.  When I just used
> > the thedir->entry->dname field, I couldn't do that.
> Two choices.  apr_dir_fileinfo() which deals with it and merges them.
> Or preallocate a buffer in the opendir of the fname length (dir name) plus
> the length of the longest allowed filename, up to the limit of the total
> path length.  No sense returning a name >_MAX_PATH, I already have this
> exception in Win32 (and just skip over such bogus files.)

Third choice: let the caller deal with a concat if necessary. They can
optimize the snot out of it, if that is important to them. Other apps will
simply do the apr_pstrcat() and be done with it.

[ mod_dav optimizes, but I don't expect everybody to do that ]


Greg Stein, http://www.lyra.org/

View raw message