httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Roy T. Fielding" <roy.field...@gmail.com>
Subject Re: svn commit: r488780 - /httpd/httpd/branches/2.2.x/STATUS
Date Thu, 21 Dec 2006 02:51:19 GMT
On Dec 20, 2006, at 5:14 PM, William A. Rowe, Jr. wrote:
>>>>>      * mpm_winnt: Fix return values from wait_for_many_objects.
>>>>>          http://svn.apache.org/viewvc?view=rev&revision=428029
>>>>>        2.2.x version of patch:
>>>>>          Trunk version works
>>>>> +        http://people.apache.org/~wrowe/mpm_winnt_waits.patch
>>>>> +        is easier to read (-U8)

Well, I hate to be a stick in the mud, but that code sure seems
like a severely sucking hack.

Is WaitForMultipleObjects() a severely broken API that requires
knowledge of several constant values and their relative offsets?
Or are these private constants that lack documentation?

Why perform a spin loop that checks

   while((time(NULL) < tStopTime) && (dwRet == WAIT_FAILED));

the former condition being expensive while the latter is cheap?
It is actually testing WAIT_TIMEOUT (looping until we don't
get a timeout), but that is really hard to see now because of the
hackery on dwRet values.

And why are there three different exit points out of the loop?
And WTF doesn't it just increment dwIndex += MAXIMUM_WAIT_OBJECTS?
And what is the return value supposed to mean to callers?
And why is it different from the version on trunk?
Sleep(1000)?  Yowza.

So, I'm -0.9 on the patch, even though it isn't much worse than
the existing code.  I'd be -1 if I could be sure what it did.

....Roy

Mime
View raw message