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