[RFC] Help debugging overloaded objects

php.internals

Marcus Börger

19 years ago
Hello internals, the attached patch introduces a new handler to the engine that is supposed to help debugging overloaded objects. It allows to return a temporary hash rather then the object properties. This way an extension can show non properties in var_dump() and print_r(). It will be used in extensions like SimpleXML. To show how it will look like the changes for said extension are alsopresent. Last but not least the handler can be NULL in which case the old behavior is maintained. If noone objects I will commit this by the end of the week. Any comments? Best regards, Marcus

Sara Golemon

19 years ago
> the attached patch introduces a new handler to the engine that is > supposed to help debugging overloaded objects. It allows to return a > temporary hash rather then the object properties. This way an extension > can show non properties in var_dump() and print_r(). It will be used > in extensions like SimpleXML. To show how it will look like the changes > for said extension are alsopresent. Last but not least the handler can > be NULL in which case the old behavior is maintained. If noone objects > I will commit this by the end of the week. > > Any comments? >
Looks useful, I like it. +1 -Sara

Derick Rethans

19 years ago
On Mon, 15 Jan 2007, Marcus Boerger wrote:
> Any comments?
Shouldn't you init is_temp here? As far as I can see it is not initialized in every code path, and there is an if() using it in the 3rd patch element here:
> @@ -581,6 +581,7 @@ ZEND_API void zend_print_zval_r_ex(zend_ > HashTable *properties = NULL; > zstr class_name = NULL_ZSTR; > zend_uint clen; > + int is_temp; > > > if (Z_OBJ_HANDLER_P(expr, get_class_name)) { > Z_OBJ_HANDLER_P(expr, get_class_name)(expr, &class_name, &clen, 0 TSRMLS_CC); > @@ -593,8 +594,11 @@ ZEND_API void zend_print_zval_r_ex(zend_ > if (class_name.v) { > efree(class_name.v); > } > - if (Z_OBJ_HANDLER_P(expr, get_properties)) { > + if (Z_OBJ_HANDLER_P(expr, get_debug_info)) { > + properties = Z_OBJ_HANDLER_P(expr, get_debug_info)(expr, &is_temp TSRMLS_CC); > + } else if (Z_OBJ_HANDLER_P(expr, get_properties)) { > properties = Z_OBJPROP_P(expr); > + is_temp = 0; > } > if (properties) { > if (++properties->nApplyCount>1) { > @@ -604,6 +608,10 @@ ZEND_API void zend_print_zval_r_ex(zend_ > } > print_hash(properties, indent, 1 TSRMLS_CC); > properties->nApplyCount--; > + if (is_temp) { > + zend_hash_destroy(properties); > + efree(properties); > + } > } > break; > }
regards, Derick
-- Derick Rethans http://derickrethans.nl | http://ez.no | http://xdebug.org

Marcus Börger

19 years ago
Hello Derick, I changed the code now to be easier understandable. In the patch the following happens. In the two if cases properties get initialized and in the third case where none of the ifs apply properties will remain NULL. Now in the later code is_temp isonlybeingused if properties is not NULL. Anyway i made the code more readable. best regards marcus Tuesday, January 16, 2007, 12:17:04 AM, you wrote:
> On Mon, 15 Jan 2007, Marcus Boerger wrote:
>> Any comments?
> Shouldn't you init is_temp here? As far as I can see it is not initialized in > every code path, and there is an if() using it in the 3rd patch element here:
>> @@ -581,6 +581,7 @@ ZEND_API void zend_print_zval_r_ex(zend_ >> HashTable *properties = NULL; >> zstr class_name = NULL_ZSTR; >> zend_uint clen; >> + int is_temp; >> >> >> if (Z_OBJ_HANDLER_P(expr, get_class_name)) { >> Z_OBJ_HANDLER_P(expr, get_class_name)(expr, &class_name, &clen, 0 TSRMLS_CC); >> @@ -593,8 +594,11 @@ ZEND_API void zend_print_zval_r_ex(zend_ >> if (class_name.v) { >> efree(class_name.v); >> } >> - if (Z_OBJ_HANDLER_P(expr, get_properties)) { >> + if (Z_OBJ_HANDLER_P(expr, get_debug_info)) { >> + properties = Z_OBJ_HANDLER_P(expr, get_debug_info)(expr, &is_temp TSRMLS_CC); >> + } else if (Z_OBJ_HANDLER_P(expr, get_properties)) { >> properties = Z_OBJPROP_P(expr); >> + is_temp = 0; >> } >> if (properties) { >> if (++properties->nApplyCount>1) { >> @@ -604,6 +608,10 @@ ZEND_API void zend_print_zval_r_ex(zend_ >> } >> print_hash(properties, indent, 1 TSRMLS_CC); >> properties->nApplyCount--; >> + if (is_temp) { >> + zend_hash_destroy(properties); >> + efree(properties); >> + } >> } >> break; >> }
> regards, > Derick > -- > Derick Rethans > http://derickrethans.nl | http://ez.no | http://xdebug.org
Best regards, Marcus

Michael Wallner

19 years ago
Marcus Boerger wrote:
> Hello internals, > > the attached patch introduces a new handler to the engine that is > supposed to help debugging overloaded objects. It allows to return a > temporary hash rather then the object properties. This way an extension > can show non properties in var_dump() and print_r(). It will be used > in extensions like SimpleXML. To show how it will look like the changes > for said extension are alsopresent. Last but not least the handler can > be NULL in which case the old behavior is maintained. If noone objects > I will commit this by the end of the week. > > Any comments?
A strong +1 from me. Though, I don't like the name of the handler... Regards,
-- Michael

Marcus Börger

19 years ago
Hello Michael, ok current name and other options in one list: 1) get_debug_info() 2) get_debug_hash() 3) get_dump_vars() 4) get_dump_hash() does this list contain anythign you like? best regards marcus Tuesday, January 16, 2007, 8:32:41 AM, you wrote:
> Marcus Boerger wrote: >> Hello internals, >> >> the attached patch introduces a new handler to the engine that is >> supposed to help debugging overloaded objects. It allows to return a >> temporary hash rather then the object properties. This way an extension >> can show non properties in var_dump() and print_r(). It will be used >> in extensions like SimpleXML. To show how it will look like the changes >> for said extension are alsopresent. Last but not least the handler can >> be NULL in which case the old behavior is maintained. If noone objects >> I will commit this by the end of the week. >> >> Any comments?
> A strong +1 from me. Though, I don't like the name of the handler...
> Regards, > -- > Michael
Best regards, Marcus

Evert Pot

19 years ago
Ignore this message if it doesn't make sense.. wouldn't this essentially be the same as __toArray() ? Evert Marcus Boerger wrote:

Marcus Börger

19 years ago
Hello Evert, assuming you mean that as an array cast you were wrong. If you convert a SimpleXML instance to an array you get either the current element, the current attribute or the text or all elements or all attributes. For ArrayObject/ArrayIterator you would get either the properties or a copy of the inner array. But maybeyou are thinking of a new magic method. Ifso wellthat would be the use. But we don't have it and it is questionable whether we want that. Anybody on that part? best regards marcus Tuesday, January 16, 2007, 9:27:35 PM, you wrote:
> Ignore this message if it doesn't make sense..
> wouldn't this essentially be the same as __toArray() ?
> Evert
> Marcus Boerger wrote: >> Hello Michael, >> >> ok current name and other options in one list: >> >> 1) get_debug_info() >> 2) get_debug_hash() >> 3) get_dump_vars() >> 4) get_dump_hash() >> >> does this list contain anythign you like? >> >> best regards >> marcus >> >> Tuesday, January 16, 2007, 8:32:41 AM, you wrote: >> >> >>> Marcus Boerger wrote: >>> >>>> Hello internals, >>>> >>>> the attached patch introduces a new handler to the engine that is >>>> supposed to help debugging overloaded objects. It allows to return a >>>> temporary hash rather then the object properties. This way an extension >>>> can show non properties in var_dump() and print_r(). It will be used >>>> in extensions like SimpleXML. To show how it will look like the changes >>>> for said extension are alsopresent. Last but not least the handler can >>>> be NULL in which case the old behavior is maintained. If noone objects >>>> I will commit this by the end of the week. >>>> >>>> Any comments? >>>> >> >> >>> A strong +1 from me. Though, I don't like the name of the handler... >>> >> >> >>> Regards, >>> -- >>> Michael >>> >> >> >> >> >> Best regards, >> Marcus >> >>
Best regards, Marcus

Michael Wallner

19 years ago
Marcus Boerger wrote:
> Hello Michael, > > ok current name and other options in one list: > > 1) get_debug_info() > 2) get_debug_hash() > 3) get_dump_vars() > 4) get_dump_hash() > > does this list contain anythign you like?
No, sorry :) How about get_tmp_properties? Cheers,
-- Michael