On 12/13/07, Michael Clark <[EMAIL PROTECTED]> wrote: > 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?
Ok, I've got it using flags now with APR_DIR_CLEANUP. I agree it's nicer. > >> 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 It was. I now have an apr_os_pipe_flags_put function which may or may not be worthwhile. > So for this one we would use the existing apr_os_file_put but add an > extra APR_FILE_CLEANUP flag? The apr_os_file_put function already took a flags set, but it overrode APR_FILE_CLEANUP. I've got a new _ex version which corrects the flag behaviour. > Do you think it is reasonable to have an apr_dir_pool_get? Sure. It's only a few lines to write. New summary: dir.patch: 1/ add apr_dir_put_ex, adding dirname and flags 2/ refactor apr_dir_open to use apr_dir_put_ex 3/ add apr_dir_name_get 4/ add apr_dir_pool_get file.patch: 1/ add apr_os_file_put_ex, changing behavior to honour APR_FILE_NOCLEANUP set/unset. 2/ refactor apr_file_open to use apr_dir_put_ex pipe.patch (not stricly necessary): 1/ add apr_os_pipe_flags_put, replacing register_cleanup bool with flags 2/ refactor apr_file_pipe_create to use apr_os_pipe_flags_put --Iain
dir.patch
Description: Binary data
file.patch
Description: Binary data
pipe.patch
Description: Binary data
