Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:55711
Return-Path: <dk@uw.no>
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Delivered-To: mailing list internals@lists.php.net
Received: (qmail 86785 invoked from network); 6 Oct 2011 16:26:19 -0000
Received: from unknown (HELO lists.php.net) (127.0.0.1)
  by localhost with SMTP; 6 Oct 2011 16:26:19 -0000
Authentication-Results: pb1.pair.com header.from=dk@uw.no; sender-id=unknown
Authentication-Results: pb1.pair.com smtp.mail=dk@uw.no; spf=permerror; sender-id=unknown
Received-SPF: error (pb1.pair.com: domain uw.no from 193.71.32.2 cause and error)
X-PHP-List-Original-Sender: dk@uw.no
X-Host-Fingerprint: 193.71.32.2 mx-1.vendo.no Linux 2.4/2.6
Received: from [193.71.32.2] ([193.71.32.2:54984] helo=mx-1.vendo.no)
	by pb1.pair.com (ecelerity 2.1.1.9-wez r(12769M)) with ESMTP
	id 70/E0-16264-AA6DD8E4 for <internals@lists.php.net>; Thu, 06 Oct 2011 12:26:18 -0400
Received: (qmail 452 invoked from network); 6 Oct 2011 16:26:15 -0000
Received: from daniel.tbg.sysedata.no (HELO ?10.0.1.127?) (195.159.98.89)
  by mx-1.vendo.no with SMTP; 6 Oct 2011 16:26:15 -0000
Message-ID: <4E8DD6A7.60601@uw.no>
Date: Thu, 06 Oct 2011 18:26:15 +0200
User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.21) Gecko/20110831 Thunderbird/3.1.13
MIME-Version: 1.0
To: Etienne Kneuss <colder@php.net>
CC: internals@lists.php.net
References: <4E7C764C.3000808@uw.no>	<4E8DC9B4.7040809@uw.no> <CACQesk4hh9FxO=L3bERu5-=+vWCqGP28X5Gr6UXczQRN9C3RXA@mail.gmail.com>
In-Reply-To: <CACQesk4hh9FxO=L3bERu5-=+vWCqGP28X5Gr6UXczQRN9C3RXA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [PATCH] Fix for bug #55754
From: dk@uw.no ("Daniel K.")

On 10/06/2011 06:03 PM, Etienne Kneuss wrote:
> On Thu, Oct 6, 2011 at 17:31, Daniel K. <dk@uw.no> wrote:
>> The patch still applies to trunk, and I think it should be applied to
>> the 5.3 and 5.4 branches as well.
> 
> The new patch seems to be indeed better. My only concern is whether it
> covers all those cases now. Did you try passing all possible kinds of
> value expressions ? Or is it guaranteed to cover all cases by design?
> If not, we should probably add more of those cases in the tests.

It now covers all possible cases by design. The patched function
(zend_do_pass_param) is only called from the language parser, and in
Zend/zend_language_parser.y zend_do_pass_param() is called with the
second parameter set to ZEND_SEND_VAL for all expr_without_variable type
parameters.

From php5.4-201110061230/Zend/zend_language_parser.y: line 538-541

non_empty_function_call_parameter_list:
        expr_without_variable   { Z_LVAL($$.u.constant) = 1;
zend_do_pass_param(&$1, ZEND_SEND_VAL, Z_LVAL($$.u.constant) TSRMLS_CC); }
    |   variable                { Z_LVAL($$.u.constant) = 1;
zend_do_pass_param(&$1, ZEND_SEND_VAR, Z_LVAL($$.u.constant) TSRMLS_CC); }
    |   '&' w_variable          { Z_LVAL($$.u.constant) = 1;
zend_do_pass_param(&$2, ZEND_SEND_REF, Z_LVAL($$.u.constant) TSRMLS_CC); }
[...]

So your concern should be addressed.


> Other than that, this looks fine.

Super.


Daniel K.