Re: Patch for bug 25063 - _pclose should wait for the command processor to terminate and return it's exit status

[Alexandre Julliard]

Borut Razem  writes:
> +static void msvcrt_add_handle(MSVCRT_FILE *fp, HANDLE *hProcess)
> +{
> - LOCK_POPEN;
> - if (fp2handle == NULL)
> - {
> - /* array not allocated yet: allocate it */
> - if ((fp2handle = MSVCRT_malloc(sizeof (struct fp2handle_s) * 16)) != NULL)
> - {
> -
>  /* write handles to the first slot */
> -
>  fp2handle[0].fp = fp;
> -
>  fp2handle[0].hProcess = hProcess;
> -
> -
>  /* zero the remaining space */
> -
>  memset(&fp2handle[1], 0, sizeof (struct fp2handle_s) * (16 - 1));
> -
> -
>  /* update the number of slots */
> -
>  fp2handle_entries = 16;
> - }
> - }
> - else
> - {
> - int i;
> -
> - /* search for free or slot with the same fp */
> - for (i = 0; i < fp2handle_entries; ++i)
> - {
> -
>  if (fp2handle[i].fp == NULL || fp2handle[i].fp == fp)
> -
>  {
> -
>    /* found: write handles to the slot */
> -
>    fp2handle[i].fp = fp;
> -
>    fp2handle[i].hProcess = hProcess;
> -
>    break;
> -
>  }
> - }
> - if (i >= fp2handle_entries)
> - {
> -
>  /* no space left: double the array size */
> -
>  struct fp2handle_s *newFp2handle;
> -
> -
>  if ((newFp2handle = MSVCRT_realloc(fp2handle, sizeof (struct fp2handle_s) * (fp2handle_entries * 2))) != NULL)
> -
>  {
> -
>    fp2handle = newFp2handle;
> -
> -
>    /* write handles to the first exetended slot */
> -
>    fp2handle[fp2handle_entries].fp = fp;
> -
>    fp2handle[fp2handle_entries].hProcess = hProcess;
> -
> -
>    /* zero the remaining extended space */
> -
>    memset(&fp2handle[fp2handle_entries + 1], 0, sizeof (struct fp2handle_s) * (fp2handle_entries - 1));
> -
> -
>    /* update the number of slots */
> -
>    fp2handle_entries *= 2;
> -
>  }
> - }
> - }
> - UNLOCK_POPEN;
>
> +}

That's an awful lot of code for such a simple thing. You probably want
to use an array indexed by file descriptor or something like that. Also
please don't add a comment before every line stating what the line
does, that should be clear from the code itself.