Hi Marcus Boerger, you wrote:
> Hello Michael,
>
> Wednesday, August 17, 2005, 9:14:50 AM, you wrote:
>>>+ } else if (*property == value) {
>>>+ retval = SUCCESS;
>>>Here you compare the address of a zval, was that intended?
>>>In case you meant it, then i think it cannot be correct because that should
>>>never happen, or is that the case you actually want to prevent?
>>>In case you meant to prohibit from setting the same value again you need
>>>actual zval comparison which won't work easily for strings.
>
>>This is from zend_reflection_api ReflectionProperty::setValue().
>>I assume it is meant to prevent useless operations,
>>because when else should the zvals point to the same address?
>
> Yep in reflection api it is used for the exact same reason. I was only
> wondering whether that can happen here. Since your code is called outside
> request time it can only happen if some code tries to update the same prop
> with the same value twice.
It theoretically avoids
zpp = zend_std_get_static_property(...);
zend_update_static_property(..., *zpp);
Also zend_std_write_property uses the same bit, so
I think better be on the safe side...
>>>+ } else {
>>>+ if (PZVAL_IS_REF(*property)) {
>>>+ zval_dtor(*property);
>>>+ (*property)->type = value->type;
>>>+ (*property)->value = value->value;
>>>+
>>>+ if (value->refcount) {
>>>+ zval_copy_ctor(*property);
>>>+ }
>>>+ } else {
>>>+ **property = *value;
>>>+ zval_copy_ctor(*property);
>>>+ }
>>>+ retval = SUCCESS;
>>>+ }
>>>+
>>>+ if (!value->refcount) {
>>>+ zval_dtor(value);
>>>+ FREE_ZVAL(value);
>>>+ }
>>>This only works because your zvals are initialized with refcount=0 which
>>>cannot be right. In the end it should always read here
>>>zval_ptr_dtor(&value); And tmp_zval() should initialize with refcount=1.
>
>
>>I actually thought that there should be an "else { zval_ptr_dtor(&value) }"?
>>Every place I looked at where "temporary zvals" are used, initialize them
>>with refcount=0.
>
>
> Temp zvals are used in scripts and can never be stored at least that was the
> goal. In your case always and unfortunatley also in the engine usage for
> edge cases they are going to be stored. So probably it is easier here to go
> with normal props initilized with refcount=1. You have to try and see.
zend_update_property() and zend_std_write_property() use the same form
of temporary zvals, though the destruction part of the passed value zval
should probably be adopted to that of zend_update_property().
>>They're not supposed to change their values after
>>declaration, are they?
>
>
> php -r '$d=42; define("a",$d); class t { const x=a;} var_dump(t::x);'
>
> That's why.
So, what about disallowing IS_CONSTANT zvals for internal zvals as
already done for arrays or other complex types and never actually
zval_update_constant() those zvals and skip internal classes in
zend_update_class_constants()?
Regards,
--
Michael - < mike(@)php.net >
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
