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 Sat, 22 Dec 2007 12:53:37 GMT
Lucian Adrian Grijincu wrote:
> I'd like to raise an API consistency issue here.
> (I've included Iain's original patches so that you can look at the code easily)
> the file and pipe APIs allocate space for apr_file_t regardless of the
> value of the passed in parameter.
> the dir API allocates space for apr_dir_t only if the passed in
> parameter is not a pointer to NULL. If it's a pointer to a non-NULL
> value, we consider that at that location is a full apr_dir_t object
> and use it as such.
> Apart from the different behavior there's an ugliness issues here too:
> say I have two pools : p0 and p1.
> I allocate a valid apr_dir_t from p0  with apr_dir_open(&dir, ..., p0).
> then I do a apr_os_dir_put_ex(&dir, ..., p1).
> now my object uses two pools to allocate it's data and is dependent on
> both, which I find ugly.
> Ugly are the next alternatives too ...
> A) decide the pool to use based on the value of the apr_dir_t.
>   if apr_dir_t * is NULL allocate it from the pool pased in to
> apr_os_dir_put[_ex]
>   if it's not NULL ignore the passed in pool and allocate it from the
> original pool (which can be accessed through dir->pool
> B) provide a flag through the user can specify which behavior he wants
> in case the passed in parameter is not null
>     this can currently be done by making dir = NULL before passing it
> to apr_os_dir_put[_ex] but with a flag it's more explicit.
> C) always allocate from the new pool and ignore the value of the
> apr_dir_t object (this is the behavior for apr_file_t objects)
I would go for C.

The particular use case that raised the need for the patches was to 
create a file or directory from an apr_os_file_t or apr_os_dir_t (which 
means we always need it to allocate from the passed in pool).

We have the existing apr_os_file_put and apr_os_dir_put for where we 
want to replace the descriptor in an existing apr_file_t or apr_dir_t 
(well at least the put sort of implies that)

Perhaps we could instead name them "apr_file_open_os" and 
"apr_dir_open_os" (then we don't need to have an _ex variant) and make 
them always allocate as do apr_file_open and apr_dir_open?


View raw message