apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lucian Adrian Grijincu" <lucian.griji...@gmail.com>
Subject Re: apr_os_dir_put fixes
Date Sat, 22 Dec 2007 11:53:58 GMT
On Dec 13, 2007 9:12 AM, Iain Wade <iwade@optusnet.com.au> wrote:
[snip]
> 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
>

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)



One more thing about apr_os_file_put[_ex]:
            rv = apr_thread_mutex_create(&((*file)->thlock),
                                         APR_THREAD_MUTEX_DEFAULT, pool);
We allocate this mutex for every call. Couldn't we reuse a previously
allocated one (in case the passed in apr_file_t* was not NULL and
pointing to a valid object)?


-- 
Lucian

Mime
View raw message