You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "William A. Rowe, Jr." <wr...@rowe-clan.net> on 2003/03/31 08:53:12 UTC
[patch] two small problems in ssl_engine_mutex.c
One, win32 won't compile (nor any platform missing chown). In this
case we didn't need it and have a good macro to look at.
This raised another bug in the next line. We assumed because we
default to SYSV mutexes we should do that magic. I believe this is
wrong, and we should be looking for that sort of lock explicitly.
Please review.
Bill
Re: [patch] two small problems in ssl_engine_mutex.c
Posted by Brian Pane <br...@cnet.com>.
+1
Brian
On Sun, 2003-03-30 at 22:53, William A. Rowe, Jr. wrote:
> One, win32 won't compile (nor any platform missing chown). In this
> case we didn't need it and have a good macro to look at.
>
> This raised another bug in the next line. We assumed because we
> default to SYSV mutexes we should do that magic. I believe this is
> wrong, and we should be looking for that sort of lock explicitly.
>
> Please review.
>
> Bill
>
Re: [patch] two small problems in ssl_engine_mutex.c
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Monday, March 31, 2003 12:53 AM -0600 "William A. Rowe, Jr."
<wr...@rowe-clan.net> wrote:
> One, win32 won't compile (nor any platform missing chown). In this
> case we didn't need it and have a good macro to look at.
Yup. Bad. It'd never get executed on them, but how is the compiler to know?
> This raised another bug in the next line. We assumed because we
> default to SYSV mutexes we should do that magic. I believe this is
> wrong, and we should be looking for that sort of lock explicitly.
Yup. But, as a stylistic nit, I'd prefer following what prefork.c:977 has
instead. So:
+ #if APR_HAS_SYSVSEM_SERIALIZE
+ if (((mc->nMutexMech == APR_LOCK_DEFAULT) && APR_USE_SYSVSEM_SERIALIZE)
+ || (mc->nMutexMech == APR_LOCK_SYSVSEM)) {
would be:
#if APR_HAS_SYSVSEM_SERIALIZE
#if APR_USE_SYSVSEM_SERIALIZE
if (mc->nMutexMech == APR_LOCK_DEFAULT ||
mc->nMutexMech == APR_LOCK_SYSVSEM) {
#else
if (mc->nMutexMech == APR_LOCK_SYSVSEM) {
#endif
...
Take this for whatever it's worth. Looks good. +1. -- justin