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

Reply via email to