Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use known zend_string pointer to check for equality instead of C strings #11370

Merged
merged 2 commits into from Jun 5, 2023
Merged

Use known zend_string pointer to check for equality instead of C strings #11370

merged 2 commits into from Jun 5, 2023

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Jun 4, 2023

Not sure about using the one for wakeup, as currently it is unused and maybe better to get rid of it altogether?

@Girgias Girgias requested a review from dstogov as a code owner June 4, 2023 13:50
@iluuu1994
Copy link
Member

Not sure about using the one for wakeup, as currently it is unused and maybe better to get rid of it altogether?

@Girgias I can see ZEND_STR_WAKEUP used from ext/standard/var_unserializer.re. Known strings are inexpensive and shared across processes, so I don't see any harm in this.

dstogov
dstogov approved these changes Jun 5, 2023
View reviewed changes
Copy link
Member

@dstogov dstogov 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 this makes sense.

@Girgias Girgias merged commit 9ce6980 into php:master Jun 5, 2023
12 of 13 checks passed
@Girgias Girgias deleted the magic-invoke-str branch June 5, 2023 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iluuu1994 iluuu1994 iluuu1994 approved these changes

@dstogov dstogov dstogov approved these changes

Assignees
No one assigned
Labels
Category: Engine
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Girgias @iluuu1994 @dstogov