Skip to content

Conversation

@ndossche
Copy link
Member

@ndossche ndossche commented Feb 25, 2023

Two minor cleanups, descriptions:

  • This check is always false because of the undefined behaviour rule that
    says a NULL pointer must never be dereferenced: we already dereference name
    when checking the cache slot, before the NULL check. So the NULL may be
    optimised away by the compiler. It looks like the code isn't even
    supposed to work with name being NULL, so just remove the check.

  • Remove always-true check in zend_fetch_static_property_address_ex()

  • Simplify always-true conditions.

This check is always false because of the undefined behaviour rule that
says a NULL pointer must never be dereferenced: we already dereference name
when checking the cache slot, before the NULL check. So the NULL may be
optimised away by the compiler. It looks like the code isn't even
supposed to work with name being NULL, so just remove the check.
@ndossche
Copy link
Member Author

Failure in LINUX_X64_RELEASE_ZTS is oci_error() when oci_connect() fails [ext/oci8/tests/error1.phpt], so it's unrelated.

@devnexen devnexen requested a review from kocsismate February 26, 2023 08:30
lc_name = key;
} else {
if (name == NULL || !ZSTR_LEN(name)) {
if (!ZSTR_LEN(name)) {
Copy link
Member

Choose a reason for hiding this comment

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

Might make sense to add a ZEND_ASSERT(name); at the top of the function.

@Girgias Girgias merged commit 9108a32 into php:master Feb 26, 2023
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