Boost logo

Boost-Build :

From: Vladimir Prus (ghost_at_[hidden])
Date: 2008-04-07 03:44:48


On Sunday 20 January 2008 23:38:36 Jurko Gospodnetić wrote:

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

> Index: execnt.c
> ===================================================================
> --- execnt.c    (revision 42883)
> +++ execnt.c    (working copy)
> @@ -479,12 +479,12 @@
>      /* wait for a command to complete, while snarfing up any output */
>      do
>      {
> +        /* check for a complete command, briefly */
> +        i = try_wait(500);
>          /* read in the output of all running commands */
>          read_output();
>          /* close out pending debug style dialogs */
>          close_alerts();
> -        /* check for a complete command, briefly */
> -        if ( i < 0 ) i = try_wait(500);
>          /* check if a command ran out of time */
>          if ( i < 0 ) i = try_kill_one();
>      }
> @@ -499,7 +499,7 @@
>          /* the time data for the command */
>          record_times(cmdtab[i].pi.hProcess, &time);
>  
> -        /* Clear the temp file */
> +        /* clear the temp file */
>          if ( cmdtab[i].tempfile_bat )
>          {
>              unlink( cmdtab[ i ].tempfile_bat );
> @@ -507,6 +507,9 @@
>              cmdtab[i].tempfile_bat = NULL;
>          }
>  
> +        /* find out the process exit code */
> +        GetExitCodeProcess( cmdtab[i].pi.hProcess, &cmdtab[i].exitcode );
> +
>          /* the dispossition of the command */
>          if( intr )
>              rstat = EXEC_CMD_INTR;
> @@ -884,43 +887,41 @@
>      }
>  }
>  
> -/*  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. */

>  static int try_wait(int timeoutMillis)
>  {
> -    int i, num_active, waiting;
> +    int i;
> +    int num_active;
> +    int wait_api_result;
>      HANDLE active_handles[MAXJOBS];
>      int active_procs[MAXJOBS];
>  
> -    for ( waiting = 1; waiting;  )
> +    /* prepare a list of all active processes to wait for */
> +    for ( num_active = 0, i = 0; i < globs.jobs; ++i )
>      {
> -        /* find the first completed child process */
> -        for ( num_active = 0, i = 0; i < globs.jobs; ++i )
> +        if ( cmdtab[i].pi.hProcess )
>          {
> -            /* if we have an already dead process, return it. */
> -            cmdtab[i].exitcode = 0;
> -            if ( GetExitCodeProcess( cmdtab[i].pi.hProcess, &cmdtab[i].exitcode ) )
> -            {
> -                if ( STILL_ACTIVE != cmdtab[i].exitcode )
> -                {
> -                    return i;
> -                }
> -            }
> -            /* it's running, add it to the list to watch for */
>              active_handles[num_active] = cmdtab[i].pi.hProcess;
>              active_procs[num_active] = i;
> -            num_active += 1;
> +            ++num_active;
>          }
> -        
> -        /* wait for a child to complete, or for our timeout window to expire */
> -        if ( waiting )
> -        {
> -            WaitForMultipleObjects( num_active, active_handles, FALSE, timeoutMillis );
> -            waiting = 0;
> -        }
>      }
> -    
> +
> +    /* 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?

> +    {
> +        /* terminated process detected - return its index */
> +        return active_procs[ wait_api_result - WAIT_OBJECT_0 ];
> +    }
> +
> +    /* timeout */
>      return -1;
>  }
>  
> @@ -941,9 +942,7 @@
>                  close_alert(cmdtab[i].pi.hProcess);
>                  /* we have a "runaway" job, kill it */
>                  kill_process_tree(0,cmdtab[i].pi.hProcess);
> -                /* and return it as complete, with the failure code */
> -                GetExitCodeProcess( cmdtab[i].pi.hProcess, &cmdtab[i].exitcode );
> -                /* mark it as a timeout */
> +                /* and return it marked as a timeout */
>                  cmdtab[i].exit_reason = EXIT_TIMEOUT;
>                  return i;
>              }

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

Thanks,
Volodya


Boost-Build list run by bdawes at acm.org, david.abrahams at rcn.com, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk