apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From <...@covalent.net>
Subject Re: [PATCH] Fix apr_thread_exit(), change apr_thread_start_t, etc...
Date Wed, 18 Jul 2001 16:01:36 GMT

> > IMNSHO, this is wrong for two reasons.  #1, if the app has access to the
> > apr_thread_t, then they also have access to the pool.
> For the app to get access to the apr_thread_t, it must pass it through
> in it's own opaque data pointer. This is of course totally unobvious to
> the application developer.
> >                                                        We have a macro
> > called APR_POOL_IMPLEMENT_ACCESSOR that specifically creates accessor
> > functions for the pools.  Threads may not implement that function yet, but
> > it is far better to allow one method for accessing the pool.
> Threads may not implement that function yet because apr_thread_t is
> a private os-dependent data structure. Now an application developer
> must not only *always* pass the apr_thread_t through it's opaque data
> structure (in addition to whatever other data they need to pass), but
> once they are done doing that they're going to have to call some
> macro to extract the pool from the apr_thread_t? Ignoring the added
> complexity and obfuscation from an API user's standpoint just for a
> moment, and I still fail to see how this is portable.

you aren't looking at the code.  File's are implemented on a per platform
basis, so that unix and windows files share nothing when it comes to the
file type definition.  However, we have already implemented the pool
accessor functions using the macros I have pointed out.

As for the API, allow me to show what I am asking for

apr_thread_func thread_func(apr_thread_param_t *param)
   apr_thread_t *t = param->t;
   worker_data = param->data;
   apr_pool_t *p = apr_thread_get_pool(t);


> [Side note: In order to use APR_POOL_IMPLEMENT_ACCESSOR, will apr_thread_t
> have to be a non-os-dependent structure; one that contains the current
> os-defined private structure as a member? I see this as an alternative
> solution analog to the APR_POOL_IMPLEMENT_ACCESSOR routine, but I reject
> the idea that this would be a "good clean API".]

Please read the code.

> Every other function in APR that would need immediate access to a pool
> is passed explicity in the parameter list.

That is just not true.  Every APR function has access to a pool.  It gets
access to the pool in one of two ways.  1)  it is passed in through the
parameter list.  2)  It is passed in through some other variable on the
parameter list.  So, if a file function needs access to a pool, then if an
apr_file_t is passed in, then that function has access to the pool through
the file_t and generally another pool is not passed in.  This is not
always true.  If we are trying to change the scope of a file, then we may
pass in a pool to a file function, even if there is already a pool passed
in through another parameter.

> >                                                               #2, why are
> > we passing the apr_thread_t as a separate parameter to the thread?  That
> > is introducing a layer of indirection that isn't required here.  Just
> > create an apr_thread_param_t structure that is:
> >
> > struct apr_thread_param_t {
> >     apr_thread_t *t;
> >     void *data;
> > }
> >
> > Then, we can pass in this value as the void * to the thread function, and
> > be done with it.  This allows us to call the function passed into
> > apr_thread_create directly, and it allows us to add more parameters to
> > thread creation later if we have to.  I am not saying that we will
> > definately have to, but at least this allows us to do so easily.
> >
> > Now, we can re-define the APR_THREAD_FUNC api to be
> >
> > +typedef void *(APR_THREAD_FUNC *apr_thread_start_t)(apr_thread_param_t *);
> >
> So I'm willing to repost the patch (inline this time :) with the
> apr_thread_param_t modification suggested above, but I'm not willing
> to leave the apr_thread_t as a necessary but missing parameter, nor
> am I willing to leave the pool off the parameter list if we can't
> get to it easily and portably.

We can get to all of them easily and portably.


Ryan Bloom                        	rbb@apache.org
Covalent Technologies			rbb@covalent.net

View raw message