apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael Clark <mich...@metaparadigm.com>
Subject Re: apr_os_dir_put fixes
Date Thu, 13 Dec 2007 06:40:26 GMT
William A. Rowe, Jr. wrote:
> Iain Wade wrote:
>> 1/ Create a new apr_os_dir_put_ex function, which adds a dirname field
>> and register_cleanup flag.
> IFF you agree with my comments on 2) below, is there an advantage to an
> extensible flags arg, with the same flag bit for this function?

an additional flag argument might make more sense - and then use 

>> 2/ Create a new apr_os_file_put_ex function, which adds a 
>> register_cleanup flag.
> Why a bool?  We already pass a flags arg, why not a toggle bit?

I guess it was because of the existing precedence for the 
register_cleanup arg on apr_os_pipe_put_ex

So for this one we would use the existing apr_os_file_put but add an 
extra APR_FILE_CLEANUP flag?

>> 3/ Create a new apr_dir_name_get function which returns the directory 
>> name
>> 4/ Re-factor apr_(file|dir)_open to use apr_os_(file|dir)_put for 
>> consistency.
> Interesting, I'll need to review this a bit.  Perhaps that should be 
> broken
> out as a seperate patch, and 1 and 3 considered together, 2 considered 
> on it's
> own (in conjunction with 1).

I will do some testing on these patches.

 From what I can see, API-wise, this solves most of the issues we had 
with apr in implementing a wrapped apr_dir_open that creates an 
apr_dir_t from a file descriptor and doing this only with public apr 
interfaces (i.e. not looking inside and/or fiddling with the opaque 
apr_dir_t structure using platform headers).

There is one outstanding issue from an apr API point of view. We have a 
wrapper for apr_dir_read that needs the pool but there is no pool 
argument so the only context is the apr_dir_t itself but we can't access 
the pool from the opaque structure from outside of apr.

Do you think it is reasonable to have an apr_dir_pool_get?

View raw message