On Tue, Feb 28, 2012 at 1:10 AM, Anthony Ferrara <[email protected]> wrote:
> I initially looked at the final fix when I discovered the issue.
> Follow me out on this. This is the current code as-implemented in
> r323563:
>
> 265 zval *obj;
> 266 MAKE_STD_ZVAL(obj);
> 267 if (Z_OBJ_HANDLER_P(*arg, cast_object)(*arg, obj, type
> TSRMLS_CC) == SUCCESS) {
> 268 zval_ptr_dtor(arg);
> 269 *arg = obj;
> 270 *pl = Z_STRLEN_PP(arg);
> 271 *p = Z_STRVAL_PP(arg);
> 272 return SUCCESS;
> 273 }
> 274 efree(obj);
>
> The issue that I originally identified (overwriting the argument
> pointer) is still happening. Is there any reason for overwriting the
> arg pointer? Wouldn't it be better to just do the Z_STRLEN_PP and
> Z_STRVAL_PP operations on obj instead, and zval_ptr_dtor it as well
Oops, you are right.. thanks for pointing this out.
:)
> (instead of efree, as that way if a reference is stored somewhere it
> won't result in a double free, or a segfault for accessing freed
> memory)?
>
> Thanks,
>
> Anthony
>
> On Mon, Feb 27, 2012 at 11:39 AM, Xinchen Hui <[email protected]> wrote:
>> Sent from my iPad
>>
>> 在 2012-2-28，0:10，Anthony Ferrara <[email protected]> 写道：
>>
>>> Out of curiosity, why are you changing it to copy the object for the
>>> result of the cast operation? cast_object should init the result
>>> zval, so why go through the step of copying the starting object to
>> plz look at the final fix： r323563
>>
>> thanks
>>> r323563
>>> Wouldn't it be easier just to do:
>>>
>>> if (Z_OBJ_HANDLER_PP(arg, cast_object)) {
>>> zval *result;
>>> ALLOC_ZVAL(result);
>>> if (Z_OBJ_HANDLER_P(*arg, cast_object)(*arg, result, type TSRMLS_CC)
>>> == SUCCESS) {
>>> zval_ptr_dtor(arg);
>>> *pl = Z_STRLEN_PP(result);
>>> *p = Z_STRVAL_PP(result);
>>> zval_ptr_dtor(result);
>>> return SUCCESS;
>>> }
>>> zval_ptr_dtor(result);
>>> }
>>>
>>> Keeping both completely separate, and not having the possibility of
>>> corrupting the arg object pointer? As it is right now (with the patch
>>> in the first mail), wouldn't the possibility still exist of nuking the
>>> arg object pointer which could be used elsewhere (and hence cause the
>>> memory leak and segfault when that variable is referenced again)?
>>>
>>> (Un tested as of yet, just throwing it out there as it seems kind of
>>> weird to overwrite the arg pointer for what seems like no reason)...
>>>
>>> Anthony
>>>
>>>
>>>
>>> On Mon, Feb 27, 2012 at 10:22 AM, Richard Lynch <[email protected]> wrote:
>>>> On Mon, February 27, 2012 2:31 am, Laruence wrote:
>>>>> On Mon, Feb 27, 2012 at 4:00 PM, Dmitry Stogov <[email protected]>
>>>>> wrote:
>>>>>> Hi Laruence,
>>>>>>
>>>>>> The attached patch looks wired. The patch on top of it (r323563)
>>>>>> makes it
>>>>>> better. However, in my opinion it fixes a common problem just in a
>>>>>> single
>>>>>> place. Each call to __toString() that makes "side effects" may
>>>>>> cause
>>>>>> the
>>>>>> similar problem. It would be great to make a "right" fix in
>>>>>> zend_std_cast_object_tostring() itself, but probably it would
>>>>>> require API
>>>>> Hi:
>>>>> before this fix, I thought about the same idea of that.
>>>>>
>>>>> but, you know, such change will need all exts who implmented
>>>>> their own cast_object handler change there codes too.
>>>>>
>>>>> for now, I exam the usage of std_cast_object_tostring, most of
>>>>> them do the similar things like this fix to avoid this issues(like
>>>>> ZEND_CAST handler).
>>>>>
>>>>> so I think, maybe it's okey for a temporary fix :)
>>>>
>>>> Perhaps a better solution would be to make a NEW function that uses
>>>> zval** and deprecate the old one with memory leaks.
>>>>
>>>> Old extensions remain functional, new extension consume less memory.
>>>>
>>>> (This presumes I actually understand the issue, which is questionable.)
>>>>
>>>> --
>>>> brain cancer update:
>>>> http://richardlynch.blogspot.com/search/label/brain%20tumor
>>>> Donate:
>>>> https://www.paypal.com/cgi-bin/webscr?cmd=_s-xclick&hosted_button_id=FS9NLTNEEKWBE
>>>>
>>>>
>>>>
>>>> --
>>>> PHP Internals - PHP Runtime Development Mailing List
>>>> To unsubscribe, visit: http://www.php.net/unsub.php
>>>>
--
惠新宸 laruence
Senior PHP Engineer
http://www.laruence.com