apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Aaron Bannert <aa...@clove.org>
Subject Re: Pools rewrite [2]
Date Wed, 05 Dec 2001 16:31:33 GMT
On Wed, Dec 05, 2001 at 12:35:00PM +0100, Sander Striker wrote:
> > <comment>
> > Ack.  No.  Declare a new scope like so:
> > #define UNLOCK(mutex) \
> >   do { \
> >     if (mutex) \
> >         apr_thread_mutex_unlock(mutex); \
> >   } while(0)
> > 
> > Calling it like UNLOCK(mutex) (i.e. no semicolon) is just badness
> > and will shoot us down the road later.
> > </comment>
> 
> Ok, will change that.

If the semicolon is the only problem, just remove the semicolon from
the macro:

#define UNLOCK(mutex) \
    if (mutex) \
        apr_thread_mutex_unlock(mutex)


IMHO, I don't see the use of a macro for this kind of thing in the first
place, because it's a simple enough construct. Also, we might want to check
for errors on that lock call (up to you).

[snip]

> > 
> > /*
> >  * Initialization
> >  */
> > 
> > APR_DECLARE(apr_status_t) apr_pool_alloc_init(apr_pool_t *global)
> > {
> >     apr_status_t rv;
> > 
> >     if (global_allocator_initialized)
> >         return APR_SUCCESS; /* Is this correct? */
> > 
> > <comment>
> > Sure.  apr_thread_once is probably a much better way to handle this
> > though I think.
> > </comment>
> 
> My point was if it was correct to return APR_SUCCESS.  People are
> initializing twice if they reach that line of code.
> 
> Isn't apr_thread_once restricted to platforms where we actually have
> threads?  Hmmm, I now start to wonder about my apr_thread_mutex_xxx
> calls.  Aaron?

apr_thread_once seems to be only usable on platforms where APR_HAS_THREADS
is defined. On platforms where there are no threads, apr_thread_once
can just be an int flag to make sure it doesn't get called twice.

As for the locking functions, the old way was that the locking routines
were always available on platforms with thread support or not, but at
runtime we'd get an APR_ENOTIMPL on platforms that didn't support it. If
we run across a platform that doesn't support apr_thread_mutex_t right now,
we'll at least get a compile-time error. Since that is undesirable, all
uses of apr_thread_mutex_t should be protected in APR_HAS_THREADS #ifdefs.

[snip]

-aaron 

Mime
View raw message