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 it?
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 <c...@l-i-e.com> wrote:
> On Mon, February 27, 2012 2:31 am, Laruence wrote:
>> On Mon, Feb 27, 2012 at 4:00 PM, Dmitry Stogov <dmi...@zend.com>
>> 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
>
--
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
