Re: Re: svn: /php/php-src/ branches/PHP_5_3/NEWS branches/PHP_5_3/Zend/zend_API.c trunk/NEWS trunk/Zend/zend_API.c

From: Date: Mon, 27 Feb 2012 16:39:54 +0000
Subject: Re: Re: svn: /php/php-src/ branches/PHP_5_3/NEWS branches/PHP_5_3/Zend/zend_API.c trunk/NEWS trunk/Zend/zend_API.c
References: 1 2 3 4 5 6  Groups: php.cvs php.internals 
Request: Send a blank email to [email protected] to get a copy of this message
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
>>


Thread (14 messages)

« previous php.cvs (#67748) next »