Mailing List Archive: Apache: Dev

Re: svn commit: r883816 - in /httpd/httpd/trunk: include/util_mutex.h server/util_mutex.c

Index | Next | Previous | View Threaded

wrowe at rowe-clan

Nov 24, 2009, 11:50 AM

Post #1 of 5 (730 views)
Permalink

Re: svn commit: r883816 - in /httpd/httpd/trunk: include/util_mutex.h server/util_mutex.c

trawick [at] apache wrote:
> --- httpd/httpd/trunk/include/util_mutex.h (original)
> +++ httpd/httpd/trunk/include/util_mutex.h Tue Nov 24 19:06:16 2009
> @@ -99,9 +99,9 @@
> const char **mutexfile);
>
> /* private function to process the Mutex directive */
> -AP_DECLARE(const char *) ap_set_mutex(cmd_parms *cmd, void *dummy,
> - const char *typelist,
> - const char *mechfile);

If this is a public function (and it is named as one) please revert ;-)

trawick at gmail

Nov 24, 2009, 12:11 PM

Post #2 of 5 (681 views)
Permalink

Re: svn commit: r883816 - in /httpd/httpd/trunk: include/util_mutex.h server/util_mutex.c [In reply to]

On Tue, Nov 24, 2009 at 2:50 PM, William A. Rowe Jr.
<wrowe [at] rowe-clan> wrote:
> trawick [at] apache wrote:
>> --- httpd/httpd/trunk/include/util_mutex.h (original)
>> +++ httpd/httpd/trunk/include/util_mutex.h Tue Nov 24 19:06:16 2009
>> @@ -99,9 +99,9 @@
>> const char **mutexfile);
>>
>> /* private function to process the Mutex directive */
>> -AP_DECLARE(const char *) ap_set_mutex(cmd_parms *cmd, void *dummy,
>> - const char *typelist,
>> - const char *mechfile);
>
> If this is a public function (and it is named as one) please revert ;-)

I used ap_set_name_virtual_host() as an example due to its unfortunate
locality to "Mutex" in the list of directives in server/core.c. It is
also an undecorated "const char *ap_set_name_virtual_host()".

Many if not all other functions which handle directives and which are
not implemented in the same file as the directive declaration also
have the ap_ prefix, so I don't follow you on the "public" aspect.

But most of these other functions are declared with
AP_DECLARE_NONSTD(). That seems to be appropriate for ap_set_mutex(),
ap_set_name_virtual_host(), and perhaps others.

Is that the fix?

nick at webthing

Nov 24, 2009, 12:17 PM

Post #3 of 5 (679 views)
Permalink

Re: svn commit: r883816 - in /httpd/httpd/trunk: include/util_mutex.h server/util_mutex.c [In reply to]

William A. Rowe Jr. wrote:
> trawick [at] apache wrote:
>> --- httpd/httpd/trunk/include/util_mutex.h (original)
>> +++ httpd/httpd/trunk/include/util_mutex.h Tue Nov 24 19:06:16 2009
>> @@ -99,9 +99,9 @@
>> const char **mutexfile);
>>
>> /* private function to process the Mutex directive */
>> -AP_DECLARE(const char *) ap_set_mutex(cmd_parms *cmd, void *dummy,
>> - const char *typelist,
>> - const char *mechfile);
>
> If this is a public function (and it is named as one) please revert ;-)

Um, it appears to have been introduced in r883540 only yesterday,
and this was a revert (to private).

Regarding r883540 itself, I've just taken a look at it.
It seems to enforce the same mutex type on all modules that
use a mutex. Can't see an obvious reason not to do that, but
it might merit discussion on-list. For example, maybe someone
has a good reason to use different mutex types for a proc
mutex and a global mutex?

--
Nick Kew

wrowe at rowe-clan

Nov 24, 2009, 12:18 PM

Post #4 of 5 (673 views)
Permalink

Re: svn commit: r883816 - in /httpd/httpd/trunk: include/util_mutex.h server/util_mutex.c [In reply to]

Jeff Trawick wrote:
>
> Many if not all other functions which handle directives and which are
> not implemented in the same file as the directive declaration also
> have the ap_ prefix, so I don't follow you on the "public" aspect.

s/public/meant to be exported/

> But most of these other functions are declared with
> AP_DECLARE_NONSTD(). That seems to be appropriate for ap_set_mutex(),
> ap_set_name_virtual_host(), and perhaps others.

AP_DECLARE_NONSTD() really has one goal, which is offering variable arg
lists. A secondary effect is to use a pure C calling convention, the sort
of thing you might need for some registered callbacks (e.g. atexit, etc).
Otherwise AP_DECLARE() would seem fine.

I think the registered cmd handler might need to be NONSTD.

wrowe at rowe-clan

Nov 24, 2009, 12:20 PM

Post #5 of 5 (676 views)
Permalink

Re: svn commit: r883816 - in /httpd/httpd/trunk: include/util_mutex.h server/util_mutex.c [In reply to]

Nick Kew wrote:
>
> Regarding r883540 itself, I've just taken a look at it.
> It seems to enforce the same mutex type on all modules that
> use a mutex. Can't see an obvious reason not to do that, but
> it might merit discussion on-list. For example, maybe someone
> has a good reason to use different mutex types for a proc
> mutex and a global mutex?

This permits your desired feature, please reread the thread between
Jeff and I, he explained it really well.

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads