PHPC-2111: Replace zend_parse_parameters calls with macros #1338
Conversation
| Z_PARAM_OPTIONAL | ||
| Z_PARAM_STRING_OR_NULL(uri_string, uri_string_len) | ||
| Z_PARAM_ARRAY_EX(options, 1, 1) | ||
| Z_PARAM_ARRAY_EX(driverOptions, 1, 1) |
There was a problem hiding this comment.
Noted that Z_PARAM_ARRAY_EX's second and third arguments correspond to check_null and separate, respectively (see: lxr).
| PHONGO_PARSE_PARAMETERS_START(1, 3) | ||
| Z_PARAM_ZVAL(mode) | ||
| Z_PARAM_OPTIONAL | ||
| Z_PARAM_ARRAY_EX(tagSets, 1, 1) |
There was a problem hiding this comment.
Noted this applies separate for the parsed zval. I believe the Manager and ReadPreference constructors were the only place where this was necessary.
| @@ -145,7 +144,7 @@ static PHP_METHOD(WriteConcern, __construct) | |||
|
|
|||
| switch (ZEND_NUM_ARGS()) { | |||
| case 3: | |||
| if (Z_TYPE_P(journal) != IS_NULL) { | |||
| if (journal && Z_TYPE_P(journal) != IS_NULL) { | |||
There was a problem hiding this comment.
Why was this change necessary? Did zend_parse_parameters previously always initialize zval *journal or what this a long-standing bug?
There was a problem hiding this comment.
Without the additional check I was presented with a compiler warning about journal potentially being uninitialised. Looking through the sources, we can see that zend_parse_arg_zval_deref will always assign the arg pointer to *dest if check_null is false (as is the case for journal). I'm not entirely sure what arg would be for optional arguments that weren't provided, or why this wasn't an issue before, especially considering that the call stack always ends up in the above zend_parse_arg_zval_deref function. I lack the internals knowledge to figure that one out, hence the simple NULL check.
alcaeus
force-pushed
the
feature/phpc-2111
branch
from
fd7a261 to
4958e36
Compare
PHPC-2111