Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:13853
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws;
        s=beta; d=gmail.com;
        h=received:message-id:date:from:reply-to:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:references;
        b=PmUMSYI7ze20/h2NV2zwOVlL2jwmFXWOWpVjpgC3O+xvsF+R6iwcSklid7p4UB/LzehmPJ4hRTtgBOyYu2LJGgIKd1CQw+0nOt+PDtXMbtzpsmjfqJCiIlFgBqEdJxvoOEwiT95WudNpFIu+9YZbhQtOs7WbztLnJhsIbuzcm7E=
Message-ID: <4e89b42604111416277a868900@mail.gmail.com>
Date: Sun, 14 Nov 2004 19:27:47 -0500
Reply-To: Wez Furlong <kingwez@gmail.com>
To: "ilya77@gmail.com" <ilya77@gmail.com>
Cc: internals@lists.php.net
In-Reply-To: <ec6233bc0411140311723494c0@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
References: <ec6233bc0411140311723494c0@mail.gmail.com>
Subject: Re: [PHP-DEV] [PATCH] proc_open() invokes unnecessary instances of CMD.EXE on Win32 for *each* started process, even if it's not necessary
From: kingwez@gmail.com (Wez Furlong)

I can see the need for this kind of thing, but I'm not so sure about
the implementation.
It's also important to consider backward compatibility and security
before adjusting this function.

Needs some brains on it; mine is currently occupied, but I'll try and
think on it over the next week.

--Wez.

On Sun, 14 Nov 2004 13:11:38 +0200, ilya77@gmail.com <ilya77@gmail.com> wrote:
> Here's a patch, which logic is pretty simple:
> 
> 1st try to invoke without CMD.EXE (this will work for executable
> files), and if that fails - invoke with CMD.EXE.
> Shouldn't be too much of a burden for the CPU, but saves memory.
> It also allows to kill processes later, which wouldn't happen if
> CMD.EXE would wrap the child process - only CMD.EXE would be killed.
> 
> Also, attached the patch in text file form since gmail might screw
> some characters.
> 
> -----------------------------------------------------------------
> 
> Index: php-src/ext/standard/proc_open.c
> ===================================================================
> RCS file: /repository/php-src/ext/standard/proc_open.c,v
> retrieving revision 1.30
> diff -u -p -w -r1.30 proc_open.c
> --- php-src/ext/standard/proc_open.c    10 Nov 2004 19:47:15 -0000      1.30
> +++ php-src/ext/standard/proc_open.c    14 Nov 2004 10:45:56 -0000
> @@ -729,23 +729,30 @@ PHP_FUNCTION(proc_open)
>         }
> 
> +       if (suppress_errors) {
> +               old_error_mode = SetErrorMode(SEM_FAILCRITICALERRORS|SEM_NOGPFAULTERRORBOX);
> +       }
> +
>         memset(&pi, 0, sizeof(pi));
> 
> +       // Try invoking without CMD first
> +       newprocok = CreateProcess(NULL, command, &security, &security, TRUE,
> NORMAL_PRIORITY_CLASS, env.envp, cwd, &si, &pi);
> +
> +       if (newprocok == FALSE)
> +       {
> +               // If failed - invoke with CMD
>         command_with_cmd = emalloc(command_len + sizeof(COMSPEC_9X) + 1 +
> sizeof(" /c "));
>         sprintf(command_with_cmd, "%s /c %s", GetVersion() < 0x80000000 ?
> COMSPEC_NT : COMSPEC_9X, command);
> 
> -       if (suppress_errors) {
> -               old_error_mode = SetErrorMode(SEM_FAILCRITICALERRORS|SEM_NOGPFAULTERRORBOX);
> -       }
> -
>         newprocok = CreateProcess(NULL, command_with_cmd, &security,
> &security, TRUE, NORMAL_PRIORITY_CLASS, env.envp, cwd, &si, &pi);
> 
> +               efree(command_with_cmd);
> +       }
> +
>         if (suppress_errors) {
>                 SetErrorMode(old_error_mode);
>         }
> 
> -       efree(command_with_cmd);
> -
>         if (FALSE == newprocok) {
>                 php_error_docref(NULL TSRMLS_CC, E_WARNING, "CreateProcess failed");
>                 goto exit_fail;
> 
> 
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
> 
> 
>