Skip to content

Fix incorrect type for return value in zend_update_static_property_ex() #10691

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

Closed
wants to merge 1 commit into from

Conversation

nielsdos
Copy link
Member

@nielsdos nielsdos commented Feb 24, 2023

zend_update_static_property_ex() returns a zend_result, but the return value is stored here in a bool. A bool is unsigned on my system, so in case zend_update_static_property_ex() returns FAILURE (== -1) this gets converted to 1 instead. This is not a valid zend_result value. This means that (transitive) callers could mistakingly think the function succeeded while it did in fact not succeed. Fix it by changing the type to zend_result.

Found using an experimental static analyser I'm developing.

zend_update_static_property_ex() returns a zend_result, but the return
value is stored here in a bool. A bool is unsigned on my system, so in
case zend_update_static_property_ex() returns FAILURE (== -1) this gets
converted to 1 instead. This is not a valid zend_result value. This
means that (transitive) callers could mistakingly think the function
succeeded while it did in fact not succeed. Fix it by changing the type
to zend_result.
@nielsdos nielsdos changed the title Fix incorrect type for return value of zend_update_static_property_ex() Fix incorrect type for return value in zend_update_static_property_ex() Feb 24, 2023
Copy link
Member

@devnexen devnexen left a comment

Choose a reason for hiding this comment

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

Looks sensible to me.

@devnexen devnexen closed this in 8959ff3 Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants