apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@lyra.org>
Subject [wrowe@rowe-clan.net: RE: apr_fileinfo_t is ___BOGUS___]
Date Thu, 11 Jan 2001 05:43:16 GMT
misfired... forwarding to dev@ ...

----- Forwarded message from "William A. Rowe, Jr." <wrowe@rowe-clan.net> -----

From: "William A. Rowe, Jr." <wrowe@rowe-clan.net>
Subject: RE: apr_fileinfo_t is ___BOGUS___
To: "'Greg Stein'" <gstein@lyra.org>
Date: Wed, 10 Jan 2001 19:03:18 -0600

> From: Greg Stein [mailto:gstein@lyra.org]
> Sent: Wednesday, January 10, 2001 6:35 PM
> [ moving this to dev@apr.apache.org, where it belongs... ]

ack... typo the third time I tried retransmitting (my usual account
isn't moderated in.)

> On Wed, Jan 10, 2001 at 02:21:37PM -0600, William A. Rowe, Jr. wrote:
> > Ok guys...
> > 
> >   apr_fileinfo_t must be restored to an incomplete type.
> > 
> > On win32, we have the following scenario;
> > 
> >   inode, dev, and ctype require a CreateFile call.
> ctype?

file, pipe, char, dev etc.  Simple typo.

> >   type (pipe, etc) requires CreateFile/GetFileType calls.
> >   query owner/access kills performance by two orders of magnitude.
> that doesn't happen today. user/group fields are zeroed.

Ack.  I certainly expect to see an accessor for these where they are
-required-, e.g. permission properties of dav?

> >   truename requires a FindFile call.
> truename? eh?

eh, ya, eh.  We may not need it everywhere, but we need it some places.
Both to distinguish case and mappings (shortnames).  Just because it
it a noop returning the original string passed under unix doesn't imply
apps don't need it.  And explain that to the folks trying to do security
on case insensitive unixes such as OS X, or a case-insensitive volume
mount point.  Noone but the kernel knows what case folding was accepted.
On unix, that's a stat - readdir - match for inode.  But we obviously
need some 'inside information' that there is case folding going on.
That part, on unix, I have no idea.

> > Now, there is no SINGLE ATOMIC CALL that does all this work for
> > us.  It -should- be our problem (the win32 hackers) to get this
> > right, but we can only do that with accessor functions.
> You're talking about performance, not correctness... right?

It's (nearly) correct now, but as Stoddard observed, performancewise
we have work to do.

> > The changes need to also augment the lstat flavor, so that the
> > one can be queried from the other (a significant performance
> > boost on win32.)
> I don't understand this "queried from the other" bit.

If we have resolved the truename of the link file, do we really need
to call findfirstfile again as the target and get a truename again?  No.

> > BTW - we also need an optimized form of apr_file_open that accepts
> > an apr_fileinfo_t ... that handle we grabbed to stat the file can
> > be Duplicated into a read or read/write handle for file access!  The
> > file handle in the win32 flavor will be cached with a registered
> > cleanup against the pool, so until we are destroyed, we can keep
> > grabbing the file for different flavors of access.
> That is a bit of a stretch. How about saying people can open the file, then
> use apr_getfileinfo() on it. (as opposed to supporting the other direction)

Both.  Either way, we have a handle embedded in both structures.  If it's
there, we can DuplicateHandle on win32.

> Some other comments on filestat.c, from some other 
> discussions with Bill
> Tutt, and looking at it now:
> *) is_exe() should be static and/or #if'd out.


> *) the code needs refactoring big time (reduce code dup errors).

Ack.  Already once-throughed and reduced utf folding to four blocks, and
even that can be compressed.  Other opportunities there.

> *) in apr_stat(): if the CreateFileW() fails because the user doesn't have
>    permission to open the file, then the code should fall thru to the
>    GetFileAttributesEx() path.

Cool thought!  

> *) possibly the same fall thru on apr_lstat()

Perhaps... that's trickier.

> *) refactoring would prevent the fake construction of an incomplete
>    apr_file_t (which could pose maintenance problems if apr_getfileinfo()
>    assumed that another field was valid, but the fake ones didn't set it
>    up). the refactoring would just pass a filehandle into an
>    attribute-fetching funtcion, and apr_getfileinfo() would also call this
>    function.

Actually, based on the volume and other factors, several fields are invalid
on a runtime basis (CreateFileW fell through?  No inode, it's APR_ENOTIMPL.
Same always on Win9x.)

> *) note that finfo->size computation is done differently in two places.
>    again: a suggestion to refactor to remove duplication.

Concur, thanks for all the pointers.

The fact remains that non-unicies may require more than one call to get
complete info, and even unicies have file info that isn't part of a stat
call (e.g. true name of file on a case-insensitive mount point.)

If we are going to optimize, this cruft must become transparent to the user.

----- End forwarded message -----

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

View raw message