-
Notifications
You must be signed in to change notification settings - Fork 7.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ext/pgsql: few internal changes. #17587
Conversation
e084700
to
06e40f6
Compare
ext/pgsql/pgsql.c
Outdated
if (zend_str_has_nul_byte(str)) { | ||
zend_argument_value_error(2, "must not contain any null bytes"); | ||
RETURN_THROWS(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't you use the ZPP PATH specifier here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM if CI is green.
Just saw a tiny nit, but that can be a follow-up PR.
@@ -6324,7 +6325,7 @@ PHP_FUNCTION(pg_close_stmt) | |||
|
|||
ZEND_PARSE_PARAMETERS_START(2, 2) | |||
Z_PARAM_OBJECT_OF_CLASS(pgsql_link, pgsql_link_ce) | |||
Z_PARAM_STR(stmt) | |||
Z_PARAM_PATH_STR(stmt) | |||
ZEND_PARSE_PARAMETERS_END(); | |||
|
|||
if (ZSTR_LEN(stmt) == 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit I just saw, could you update the below argument value error to use the helper? Can be a follow-up PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no pb I can do it now it s nothing.
Z_PARAM_STR*/Z_PARAM_PATH. lo_write checks any null byte.
e7a7a06
to
8ebc5e8
Compare
Z_PARAM_STR*/Z_PARAM_PATH. lo_write checks any null byte.