Skip to content

Use zend_call_known_function() to call #[\SensitiveParameter]’s constructor #14254

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

Merged
merged 1 commit into from
May 16, 2024

Conversation

TimWolla
Copy link
Member

zend_call_method_with_1_params() causes needless overhead, due to the capability of looking up the function by name. It’s also very rarely used (only in ext/spl).

zend_call_known_function() is the standard methodology to call a known function and going through Z_OBJCE_P() avoids the repeated hardcoding of the CE name.

…onstructor

`zend_call_method_with_1_params()` causes needless overhead, due to the
capability of looking up the function by name. It’s also very rarely used (only
in ext/spl).

`zend_call_known_function()` is the standard methodology to call a known
function and going through `Z_OBJCE_P()` avoids the repeated hardcoding of the
CE name.
Copy link
Member

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think no lookup by name happens, because you're passing fn_proxy with an already set value. Even so, zend_call_known_function avoids one useless level of indirection.

@TimWolla
Copy link
Member Author

I think no lookup by name happens, because you're passing fn_proxy with an already set value.

Correct, but that's unnecessary if() branches for the theoretical support of it (including the ugly double-pointer parameter) 😄

@TimWolla TimWolla merged commit 4988816 into php:master May 16, 2024
10 checks passed
@TimWolla TimWolla deleted the sensitive-parameter-construct branch May 16, 2024 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants