Converted the zend_parse_parameters in xml.c to the fast API. #5561
Conversation
There was a problem hiding this comment.
Indentation should be tabs for C files and not spaces.
Moreover, I'm not sure it makes massive sense to change this.
I could understand it more if you need special functionality (like union types or extended FUNC) from the ZPP API which is not available in the old one.
| } else { | ||
| ZEND_PARSE_PARAMETERS_START(0, 2) | ||
| Z_PARAM_OPTIONAL | ||
| Z_PARAM_STRING_EX(encoding_param, encoding_param_len, 1, 0) |
There was a problem hiding this comment.
Should be the Z_PARAM_STRING_OR_NULL version, moreover is there a reason for it to not use zend_strings instead of keeping the char* with a separate length.
|
@jensdenies Thank you for contributing to PHP! Unfortunately, fast ZPP is not always preferable to the "old" ZPP mechanism so mass changing all the parameter parsing in an extension most probably won't work well. :( For the background, please see #3041 (comment). |
|
@kocsismate Thanks for clarifying that! I'll close the PR then. |
Hi,
This is my very first pull request for the PHP repository, so i apologize in advance if i did something wrong. The aim of this pull request is to convert some old
zend_parse_parametersinto their fast API variant.The
CONTRIBUTING.mdstates that "Failure conditions of zend_parse_parameters, ZEND_PARSE_PARAMETERS() and similar functions should not be tested.", so i deliberately didn't add tests.Thanks.