Re: Merge PR 621

From: Date: Wed, 26 Mar 2014 04:56:06 +0000
Subject: Re: Merge PR 621
References: 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18  Groups: php.internals 
Request: Send a blank email to [email protected] to get a copy of this message
On Wed, Mar 26, 2014 at 12:14 PM, Tjerk Meesters
<[email protected]>wrote:

> Hi Levi,
>
> On Tue, Mar 25, 2014 at 9:51 PM, Levi Morrison <[email protected]>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 <[email protected]
>> > wrote:
>>
>>> Hi,
>>>
>>>
>>> On Tue, Mar 25, 2014 at 11:54 AM, Stas Malyshev <[email protected]
>>> >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


Thread (23 messages)

« previous php.internals (#73441) next »