Return-Path: Delivered-To: apmail-apr-dev-archive@apr.apache.org Received: (qmail 23585 invoked by uid 500); 18 Jul 2001 15:17:48 -0000 Mailing-List: contact dev-help@apr.apache.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Delivered-To: mailing list dev@apr.apache.org Received: (qmail 23538 invoked from network); 18 Jul 2001 15:17:48 -0000 Date: Wed, 18 Jul 2001 08:18:25 -0700 (PDT) From: X-Sender: To: Aaron Bannert Cc: Subject: Re: [PATCH] Fix apr_thread_exit(), change apr_thread_start_t, etc... In-Reply-To: <20010718000442.Q23346@ebuilt.com> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Spam-Rating: h31.sny.collab.net 1.6.2 0/1000/N -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 rbb@apache.org Covalent Technologies rbb@covalent.net -----------------------------------------------------------------------------