Re: 5dee3c11 break

From: Date: Tue, 06 May 2014 11:50:13 +0000
Subject: Re: 5dee3c11 break
References: 1 2 3 4  Groups: php.internals 
Request: Send a blank email to internals+get-73969@lists.php.net to get a copy of this message
Ah, I didn't get what did you mean by (check_empty == 2), but they probably should be changed into (check_empty != 1) 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 >

« previous php.internals (#73969) next »