Re: [PATCH] Fix for bug #55754
| From: | Daniel K. | Date: | Thu, 06 Oct 2011 16:26:15 +0000 |
| Subject: | Re: [PATCH] Fix for bug #55754 | ||
| References: | 1 2 3 | Groups: | php.internals |
| Request: | Send a blank email to internals+get-55711@lists.php.net to get a copy of this message | ||
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.