Add string or object ZPP macros #5788
Conversation
Zend/zend_API.h
Outdated
| @@ -1972,6 +2016,30 @@ static zend_always_inline int zend_parse_arg_class_name_or_obj( | |||
| return 1; | |||
| } | |||
|
|
|||
| static zend_always_inline int zend_parse_arg_str_or_object( | |||
| zval *arg, zend_string **destination_string, zval **destination_object, zend_class_entry *base_ce, int allow_null | |||
There was a problem hiding this comment.
I'm not sure what type the object destination should be. zval **? zend_object **?
There was a problem hiding this comment.
I personally would use a zend_object as I find the current behaviour of the object ZPP a bit weird that it returns a zval, but that's only me.
There was a problem hiding this comment.
I've changed it to zend_object :)
|
If we don't have any function to which this would apply right now, I would suggest adding some to zend_test and testing those. Might actually be better to do that than using random functions for zpp tests. |
|
Thanks for the suggestion! In my recent commit, I've added such tests, and the macros seem to work fine now. |
ext/zend_test/test.c
Outdated
| ZEND_PARSE_PARAMETERS_END(); | ||
|
|
||
| if (str) { | ||
| RETURN_STR(str); |
There was a problem hiding this comment.
| RETURN_STR(str); | |
| RETURN_STR_COPY(str); |
Zend/zend_API.c
Outdated
| return; | ||
| } | ||
|
|
||
| zend_argument_type_error(num, "must be of type string|%s, %s given", name, zend_zval_type_name(arg)); |
There was a problem hiding this comment.
I believe the canonical type ordering is %s|string.
There was a problem hiding this comment.
What about the string|object case? Or that's okay, because object is an internal type?
There was a problem hiding this comment.
$ sapi/cli/php -r '(function(string|object $x){})([]);'
Fatal error: Uncaught TypeError: {closure}(): Argument #1 ($x) must be of type object|string, array given
It's object|string as well.
There was a problem hiding this comment.
I've just adapted the argument types to follow this order, that's why the new commit.
Zend/zend_API.h
Outdated
| if (EXPECTED(Z_TYPE_P(arg) == IS_STRING)) { | ||
| *destination_string = Z_STR_P(arg); | ||
| *destination_object = NULL; | ||
| } else if (EXPECTED(Z_TYPE_P(arg) == IS_OBJECT)) { |
There was a problem hiding this comment.
I'd move the object case first. I believe in places where it's used it's the "more expected" one.
This is a preparation for OpenSSL's resource to object migration where quite a few parameters (e.g. most X.509-related ones) have to be parsed as
string|object-of-type(seephp_openssl_x509_from_zval()).