Hello Michael,
a few questions/comments so that i can understand better:
1) Zend/zend_API.c: <NEW>
+ZEND_API int zend_declare_class_constant_stringl(zend_class_entry *ce, char
*name, size_t name_length, char *value, size_t value_length TSRMLS_DC)
+{
+ zval *constant = new_zval(ce->type & ZEND_INTERNAL_CLASS);
+ if (ce->type & ZEND_INTERNAL_CLASS) {
+ ZVAL_STRINGL(constant, zend_strndup(value, value_length),
value_length, 0);
+ } else {
+ ZVAL_STRINGL(constant, value, value_length, 1);
+ }
+ return zend_declare_class_constant(ce, name, name_length, constant
TSRMLS_CC);
+}
+
If the internal part uses string duplication so shall use the user space
code.
2) Zend/zend_API.c: <NEW>
+ZEND_API int zend_update_static_property(zend_class_entry *scope, char *name,
size_t name_len, zval *value TSRMLS_DC)
+{
+ int retval;
+ zval **property = NULL;
+ zend_class_entry *old_scope = EG(scope);
+
+ EG(scope) = scope;
+
+ if (!(property = zend_std_get_static_property(scope, name, name_len, 0
TSRMLS_CC))) {
+ retval = FAILURE;
in the caose you utilize this value is a temp thing thus you need to free it
in all cases but those you directly use it. A failure is definitively a
thing where you do't use it so it must be fr here.
+ } 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.
+ } 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.
+
+ EG(scope) = old_scope;
+
+ return retval;
+}
Your static properties get duplicated and const updated from
3) Zend/zend.c: static void class_to_unicode(zend_class_entry **ce)
+ zend_u_hash_init_ex(&new_ce->default_static_properties,
(*ce)->default_static_properties.nNumOfElements, NULL,
(*ce)->default_static_properties.pDestructor, 1, 1, 0);
+ zend_hash_copy(&new_ce->default_static_properties,
&(*ce)->default_static_properties, (copy_ctor_func_t) zval_ptr_to_unicode,
&tmp_zval, sizeof(zval));
+ zend_u_hash_init_ex(&new_ce->static_properties,
(*ce)->static_properties.nNumOfElements, NULL,
(*ce)->default_static_properties.pDestructor, 1, 1, 0);
Why is the above not using
"(*ce)->default_static_properties.nNumOfElements", since that is the number
of entries the static property table will have exactly after the copy
operation on the next line so why not initialize the number to this?
+ zend_hash_copy(&new_ce->static_properties, &(*ce)->static_properties,
(copy_ctor_func_t) zval_ptr_to_unicode, &tmp_zval, sizeof(zval));
4) Zend/zend_API.c: ZEND_API void zend_update_class_constants(zend_class_entry
*class_type TSRMLS_DC)
zend_hash_apply_with_argument(&class_type->default_properties,
(apply_func_arg_t) zval_update_constant, (void *) 1 TSRMLS_CC);
- zend_hash_apply_with_argument(class_type->static_members,
(apply_func_arg_t) zval_update_constant, (void *) 1 TSRMLS_CC);
+
zend_hash_apply_with_argument(&class_type->default_static_properties,
(apply_func_arg_t) zval_update_constant, (void *) 1 TSRMLS_CC);
Before your patch we have the situation that consts can only be decalred in
user space. Thus it is ensured that their data is emalloc'd and only ever
needs to be const-updated once. Now that you have this as well as your
default static properties it (zend_update_class_constants()) must be called
once for every request time. Having said that i think we actuall need two
tables. cor consts as well. I think default_consts and
default_static_properies never need to be const updated because otherwise
you wouldn't react on define changes if a defined const is being referred
to. With this in mind we also face another problem, the problem that
const-update assumes that the memory was emalloc'd.So what we need to do is
ensuring that on the first use in a request the two tables default_const and
default_static_properties get copied to their working tables consts and
static_properties. After that copy we can update the consts. Now we also
ensured that for internal classes the values in the defualt_const table and
default_static_properties table can become malloc'd.
5) Zend/zend_compile.c: ZEND_API void
zend_initialize_class_data(zend_class_entry *ce, zend_bool nullify_handlers
TSRMLS_DC)
+ zend_u_hash_init_ex(&ce->default_static_properties, 0, NULL,
zval_ptr_dtor_func, persistent_hashes, UG(unicode), 0);
+ zend_u_hash_init_ex(&ce->static_properties, 0, NULL, ZVAL_PTR_DTOR,
persistent_hashes, UG(unicode), 0);
After reading 4) it should be clear that the values in
default_static_properties must be malloc'c zval's. With this in mind we of
course need to specify a corresponding free method insetad of the currently
used one that assumes the memory was emalloc'd.
6) Renamic static_members to static_properties is a very good idea if you
ask me but it requires that we have this patch ready and set before 5.1 is
out.
best regards
marcus
& nice work so far
Friday, August 12, 2005, 3:54:58 PM, you wrote:
> The patches are online at:
> http://dev.iworks.at/PATCHES/
> Thanks,
> --
> Michael - < mike(@)php.net >
Best regards,
Marcus
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
