-1. Please post patches in-line, because it makes it MUCH easier to reply
to them and make useful comments. Also, posting one patch at a time
would have made this patch easier to follow, and it would have
allowed me to commit the pieces that I like without having to go in
and modify the patch itself. Having said that, my comments follow.
Index: include/apr_thread_proc.h
===================================================================
RCS file: /home/cvspublic/apr/include/apr_thread_proc.h,v
retrieving revision 1.65
diff -u -r1.65 apr_thread_proc.h
--- include/apr_thread_proc.h 2001/06/06 18:11:06 1.65
+++ include/apr_thread_proc.h 2001/07/18 06:27:04
@@ -125,7 +125,7 @@
typedef struct apr_other_child_rec_t apr_other_child_rec_t;
#endif /* APR_HAS_OTHER_CHILD */
-typedef void *(APR_THREAD_FUNC *apr_thread_start_t)(void *);
+typedef void *(APR_THREAD_FUNC *apr_thread_start_t)(void *,
apr_thread_t *, apr_pool_t *);
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. 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. #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 *);
Done.
Ryan
_____________________________________________________________________________
Ryan Bloom [EMAIL PROTECTED]
Covalent Technologies [EMAIL PROTECTED]
-----------------------------------------------------------------------------
