Re: Merge PR 621

On Wed, Mar 26, 2014 at 12:14 PM, Tjerk Meesters <tjerk.meesters@gmail.com>wrote: > Hi Levi, > > On Tue, Mar 25, 2014 at 9:51 PM, Levi Morrison <morrison.levi@gmail.com>wrote: > >> In a similar vein have we checked offsetSet? It seems >> offsetSet is >> not called with +=; I am not sure if this is intended; see >> https://bugs.php.net/bug.php?id=66641 >> > > This is due to how binary assignment is applied to properties vs arrays: > > http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_vm_execute.h#31979 > > The workaround here is to disable the get_property_ptr_ptr > inside > spl_array.c, like this: > > https://gist.github.com/datibbaw/703597322ad411287461 > The test suite proved me wrong about this, as bug65328.phpt fails and causes some memory leaks when applied; some tweaks may be required or an altogether different approach :) > > > Not sure if everyone will agree to that change, but it's one way to fix > the bug other than changing the API itself :) > > >> >> >> On Tue, Mar 25, 2014 at 4:38 AM, Tjerk Meesters <tjerk.meesters@gmail.com >> > wrote: >> >>> Hi, >>> >>> >>> On Tue, Mar 25, 2014 at 11:54 AM, Stas Malyshev <smalyshev@sugarcrm.com >>> >wrote: >>> >>> > Hi! >>> > >>> > > It's my understanding that you should (have to) only call >>> > > "get" if >>> you >>> > > know that the offset exists. >>> > >>> > Yes, you are right. I'd still add some example that changes the keys to >>> > the tests to ensure all cases are covered properly. But looks like it >>> > works OK in all cases I could think of. >>> > >>> >>> I've updated the test case with such an example. >>> >>> PHP-5.6 and master have been updated. >>> >>> >>> >>> > -- >>> > Stanislav Malyshev, Software Architect >>> > SugarCRM: http://www.sugarcrm.com/ >>> > (408)454-6900 ext. 227 >>> > >>> >>> >>> >>> -- >>> -- >>> Tjerk >>> >> >> > > > -- > -- > Tjerk > -- -- Tjerk