apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@lyra.org>
Subject Re: apr_fileinfo_t is ___BOGUS___
Date Thu, 11 Jan 2001 00:35:29 GMT
[ moving this to dev@apr.apache.org, where it belongs... ]

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?

>   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.

>   truename requires a FindFile call.

truename? eh?

> 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?

> 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.

> So if you disagree, scream.  If you don't like the call/return
> issues, consider that 'transparent' types ought to someday grow
> inline performance optimizations.  That's a seperate topic for
> another day.

I'm kind of screaming right now. I'd like to hear more on why this changed
is *needed*.

> 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)


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).

*) 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.

*) possibly the same fall thru on apr_lstat()

*) 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.

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

Cheers,
-g

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

Mime
View raw message