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