Re: [Boost-build] Patch for bjam's process termination waiting on Windows.

From: Jurko Gospodnetić (jurko.gospodnetic_at_[hidden])
Date: 2008-04-07 05:57:30

   Hi Volodya.

> First a question -- what is the 259 value? You mentioned MSDN article,
> do you have a link?

   See http://msdn2.microsoft.com/en-us/library/ms683189(VS.85).aspx
(or any other MSDN documentation page for the GetExitCodeProcess() 
Windows API function).

   Here's a quote:

>   Important  The GetExitCodeProcess function returns a valid error code
> defined by the application only after the thread terminates. Therefore,
> an application should not use STILL_ACTIVE (259) as an error code. If a
> thread returns STILL_ACTIVE (259) as an error code, applications that
> test for this value could interpret it to mean that the thread is still
> running and continue to test for the completion of the thread after the
> thread has terminated, which could put the application into an infinite
> loop.

>> -/*  waits for a single child process command to complete, or the
>> -    timeout, whichever is first. returns the index of the completed
>> -    command, or -1. */
>> +/*  waits for a single child process command to complete, or the timeout,
>> +    whichever comes first. returns the index of the completed command, or -1. */
> 
> If we're cleaning the comments, how about this:
> 
> /*  Waits for a single child process command to complete, or the timeout,
>     whichever comes first. Returns the index of the completed command in the
>     cmdtab array, or -1. */

   Done.

>> +    /* wait for a child to complete, or for our timeout window to expire */
>> +    wait_api_result = WaitForMultipleObjects( num_active, active_handles,
>> +        FALSE, timeoutMillis );
>> +    if
>> +    (
>> +        ( WAIT_OBJECT_0 <= wait_api_result ) &&
>> +        ( wait_api_result < WAIT_OBJECT_0 + num_active )
>> +    )
> 
> Funny indentation you have here :-) Can opening paren go to the same
> line as "if" and closing one don't go on a separate line?

   Ok, I have changed it to

     if ( ( WAIT_OBJECT_0 <= wait_api_result ) &&
         ( wait_api_result < WAIT_OBJECT_0 + num_active ) )

   I actually prefer the original style in C-style code where functions 
are longer than 5-10 lines, but... :-)

> From my limited knowledge of windows API, this seems to be OK. Can you please
> commit the patch?

   Ok, all the changes you noted applied (including updating the commit 
comment with the appropriate MSDN link) and the patch committed.

   Changeset link: http://svn.boost.org/trac/boost/changeset/44087

   Thanks for the review!

   Best regards,
     Jurko Gospodnetić