apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Aaron Bannert <aa...@clove.org>
Subject Re: [PATCH] new lock APIs, rwlocks, condition variables
Date Tue, 04 Sep 2001 05:06:50 GMT
On Mon, Sep 03, 2001 at 09:53:26PM -0700, Ryan Bloom wrote:
> I have a couple of comments.  :-)
> 
> Overall, with just a quick review, +1 for the concept, I have a few issues with
> the implementation.  Most of my problem is the size of the patch, the good
> thing is that this patch is mostly empty declarations so that we keep
> compiling.
> 
> The problem I have, is that this is actually doing two things.  1)  It re-writes the
> locking API.  2)  It introduces a new condition variable API.  Can we get the patch
> split in two?  To make this a bit easier, I would also recommend that the condition
> variable API belongs in it's own file in the locks subdirectory.

I was thinking that we might want to have a rwlock.c and cond.c for
each implementation, but opted to leave it in until reviewed. It should
be easy enough to split it out.


> Adding that API to the thread locks API that is already there is making the code
> harder to read.  If you split it out, then you can re-post the entire patch in 
> two pieces.  One, the current API re-org, which can be generated by just doing
> a CVS diff.  The other the condition variable API, which can be done by just 
> posting the files with the API to the list.

Are you proposing I do something like apr_cond.h and apr_rwlock.h, leaving
apr_thread_mutex_t and apr_proc_mutex_t in apr_lock.h? That may make this
whole process much easier to digest.


> If you can do that, I can review this patch tomorrow or Tuesday, and hopefully
> apply it the same day.  Also, very nice that this doesn't remove any of the
> current API.  That makes this patch very easy to apply, because I don't have
> to worry about breaking things on half our platforms.  :-)


-aaron

Mime
View raw message