ext/standard: Refactor unserialization callback function call #17484
Conversation
This also stores the callback function name as a zend_string
| /* Find unserialize callback */ | ||
| zend_function *callback_fn = zend_hash_find_ptr_lc(EG(function_table), PG(unserialize_callback_func)); |
There was a problem hiding this comment.
This breaks some valid callback names, such as:
\foo(leading\)Foo::bar(static method)
There was a problem hiding this comment.
Ah I didn't think of static methods, I'll add some tests to ensure this doesn't get missed. And I'll think about a solution if there is a good one.
There was a problem hiding this comment.
I didn't understand why you replaced call_user_function() with a manual function lookup + zend_call_known_function. Is there a particular reason?
There was a problem hiding this comment.
I am trying to see if it is possible to get rid of the function_name field of the FCI struct, and there are not many uses of call_user_function() (currently 29) on top of it having the now useless symbol_table argument (yes I know it's a macro so it doesn't matter).
| if (callback_fn == NULL) { | ||
| zend_string_release_ex(class_name, 0); | ||
| zend_throw_error(NULL, "Unserialization function %s is not defined", ZSTR_VAL(PG(unserialize_callback_func))); | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
Nit: This uses space indentation
This also stores the callback function name as a zend_string