Hi,
On Tue, May 6, 2014 at 8:43 PM, Dmitry Stogov <dmitry@zend.com> wrote:
> zend_std_has_dimension() doesn't know what (check_empty == 2) means.
>
> check_empty == 0 - ISSET => we don't need to call offsetGet() and relay on
> offsetExists() return value.
> check_empty == 1 - ISEMPTY => we should call offsetGet() after
> offsetExists().
>
> NULL values should be handled by offsetExists().
>
Okay, I suppose it's fair to make its behaviour the same as for zend_std :)
I'll make the necessary changes by tonight for 5.6 and master.
>
> Thanks. Dmitry.
>
>
> On Tue, May 6, 2014 at 4:36 PM, Tjerk Anne Meesters <datibbaw@php.net>wrote:
>
>>
>>
>> On Tue, May 6, 2014 at 7:50 PM, Dmitry Stogov <dmitry@zend.com> wrote:
>>
>>> Ah, I didn't get what did you mean by (check_empty == 2), but they
>>> probably should be changed into (check_empty != 1)
>>>
>>
>> When I said mostly, perhaps I should have mentioned that the isset()
>> behaviour was deliberately updated as well; in other words, the following
>> works as expected:
>>
>> $x['foo'] = null;
>> isset($x['foo']); // false
>>
>> After taking a closer look at zend_std_has_dimension() it seems that both
>> isset() and empty() behave as if empty() was always used; of course, if
>> this behaviour was translated into spl_array.c, the infinite recursion
>> would still be an issue. So, at this point I'm unsure what the proper
>> action point should be.
>>
>>
>>>
>>>
>>> It fixes the problem.
>>> Could you please verify and commit into all relevant branches.
>>>
>>> Thanks. Dmitry.
>>>
>>> --- a/ext/spl/spl_array.c
>>> +++ b/ext/spl/spl_array.c
>>> @@ -603,7 +603,7 @@ static int spl_array_has_dimension_ex(int
>>> check_inherited, zval *object, zval *o
>>>
>>> if (rv && zend_is_true(rv TSRMLS_CC)) {
>>> zval_ptr_dtor(&rv);
>>> - if (check_empty == 2) {
>>> + if (check_empty != 1) {
>>> return 1;
>>> } else if (intern->fptr_offset_get) {
>>> value = spl_array_read_dimension_ex(1,
>>> object, offset, BP_VAR_R TSRMLS_CC);
>>> @@ -622,7 +622,7 @@ static int spl_array_has_dimension_ex(int
>>> check_inherited, zval *object, zval *o
>>> switch(Z_TYPE_P(offset)) {
>>> case IS_STRING:
>>> if (zend_symtable_find(ht,
>>> Z_STRVAL_P(offset), Z_STRLEN_P(offset)+1, (void **) &tmp) != FAILURE) {
>>> - if (check_empty == 2) {
>>> + if (check_empty != 1) {
>>> return 1;
>>> }
>>> } else {
>>> @@ -639,7 +639,7 @@ static int spl_array_has_dimension_ex(int
>>> check_inherited, zval *object, zval *o
>>> index = Z_LVAL_P(offset);
>>> }
>>> if (zend_hash_index_find(ht, index,
>>> (void **)&tmp) != FAILURE) {
>>> - if (check_empty == 2) {
>>> + if (check_empty != 1) {
>>> return 1;
>>> }
>>> } else {
>>>
>>>
>>>
>>> On Tue, May 6, 2014 at 3:29 PM, Tjerk Anne Meesters <datibbaw@php.net>wrote:
>>>
>>>> Hi,
>>>>
>>>>
>>>> On Tue, May 6, 2014 at 7:25 PM, Dmitry Stogov <dmitry@zend.com> wrote:
>>>>
>>>>> I didn't review you patch careful, but why do you need to call
>>>>> offsetGet() when offsetExists() must be enough?
>>>>> is it for empty()?
>>>>>
>>>>
>>>> Mostly for empty(), yes. It fixes the expected empty() behaviour for
>>>> $a['foo'] == 0 when using ArrayObject. This is further
>>>> explained in this
>>>> bug report: https://bugs.php.net/bug.php?id=66834
>>>>
>>>>
>>>>> Then you probably should implement it a way similar to
>>>>> zend_std_has_dimension() in Zend/zend_object_handlers.c.
>>>>> call offsetExists() and then offsetGet() if necessary.
>>>>>
>>>>
>>>> That's already the case :)
>>>>
>>>> http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_object_handlers.c#719
>>>>
>>>>
>>>>>
>>>>> Thanks. Dmitry.
>>>>>
>>>>>
>>>>> On Tue, May 6, 2014 at 2:31 PM, Tjerk Anne Meesters
>>>>> <datibbaw@php.net>wrote:
>>>>>
>>>>>> Hi Dmitry,
>>>>>>
>>>>>> On Tue, May 6, 2014 at 4:11 PM, Dmitry Stogov <dmitry@zend.com>wrote:
>>>>>>
>>>>>>> Hi Tjerk,
>>>>>>>
>>>>>>> your commit broke the code that worked fine before (still works in
>>>>>>> 5.5 but broken in 5.6 and above).
>>>>>>> It leads into infinity recursion until stack overflow.
>>>>>>>
>>>>>>> It must be fixed or reverted.
>>>>>>>
>>>>>>
>>>>>> This has been mentioned by Jakub before and a fix to ZF2 has already
>>>>>> been merged:
>>>>>>
>>>>>> https://github.com/zendframework/zf2/pull/6096
>>>>>>
>>>>>> The previous code and my patch basically cannot coexist; it used to
>>>>>> work in 5.6 before, but only by the "virtue" of an unfortunate
>>>>>> implementation.
>>>>>>
>>>>>> I believe this is not the only 5.6 issue that ZF2 is dealing with,
>>>>>> but if you feel that this breaks too many things for a 5.x release I
>>>>>> suppose we can revert it in PHP-5.6 and keep it for PHP-6?
>>>>>>
>>>>>> Let me know.
>>>>>>
>>>>>>
>>>>>>> Thanks. Dmitry.
>>>>>>>
>>>>>>> <?php
>>>>>>> class Parameters extends ArrayObject {
>>>>>>> public function __construct(array $values = null) {
>>>>>>> if (null === $values) {
>>>>>>> $values = array();
>>>>>>> }
>>>>>>> parent::__construct($values, ArrayObject::ARRAY_AS_PROPS);
>>>>>>> }
>>>>>>> public function offsetGet($name) {
>>>>>>> if (isset($this[$name])) {
>>>>>>> return parent::offsetGet($name);
>>>>>>> }
>>>>>>> return null;
>>>>>>> }
>>>>>>> }
>>>>>>> $x = new Parameters();
>>>>>>> var_dump($x['foo']);
>>>>>>> $x['foo'] = 'bar';
>>>>>>> var_dump($x['foo']);
>>>>>>> ?>
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> --
>>>>>> Tjerk
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> --
>>>> Tjerk
>>>>
>>>
>>>
>>
>>
>> --
>> --
>> Tjerk
>>
>
>
--
--
Tjerk