Skip to content

Use APPLY_STOP in pcre_clean_cache() #15839

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
Merged
merged 1 commit into from
Sep 12, 2024
Merged

Use APPLY_STOP in pcre_clean_cache() #15839

merged 1 commit into from
Sep 12, 2024

Conversation

iluuu1994
Copy link
Member

@iluuu1994 iluuu1994 commented Sep 11, 2024 •
edited
Loading

Once num_clean has reached 0, we never remove any more elements anyway.

Found while looking at #15205.

  • Edit: Ah, actually we shouldn't stop when only !pce->refcount is true. I'll fix that in a sec.

@iluuu1994 iluuu1994 requested a review from nielsdos September 11, 2024 12:14
@iluuu1994 iluuu1994 force-pushed the pcre-clean-use-ht-stop branch from 445eb3f to 6cb31fe Compare September 11, 2024 12:20
nielsdos
nielsdos approved these changes Sep 11, 2024
View reviewed changes
Copy link
Member

@nielsdos nielsdos left a comment

Choose a reason for hiding this comment

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

Seems right

@iluuu1994
Use APPLY_STOP in pcre_clean_cache()
c6f117c 
Once num_clean has reached 0, we never remove any more elements anyway.

Closes phpGH-15839
@iluuu1994 iluuu1994 force-pushed the pcre-clean-use-ht-stop branch from 6cb31fe to c6f117c Compare September 11, 2024 21:50
@iluuu1994
Copy link
Member Author

iluuu1994 commented Sep 11, 2024 •
edited
Loading

I realized the callback of zend_hash_apply_with_argument() accepts flags, so we can actually remove and stop at the same time. It also gets rid of the duplicate condition. Doesn't really make a difference though. 😄

nielsdos
nielsdos approved these changes Sep 12, 2024
View reviewed changes
Copy link
Member

@nielsdos nielsdos left a comment

Choose a reason for hiding this comment

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

TIL, indeed

@iluuu1994 iluuu1994 merged commit ffb4405 into php:master Sep 12, 2024
10 checks passed
Girgias pushed a commit to Girgias/php-src that referenced this pull request Sep 12, 2024
@iluuu1994 @Girgias
Use APPLY_STOP in pcre_clean_cache() (phpGH-15839)
c95baab 
Once num_clean has reached 0, we never remove any more elements anyway.

Closes phpGH-15839
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nielsdos nielsdos nielsdos approved these changes

Assignees
No one assigned
Labels
Extension: pcre
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@iluuu1994 @nielsdos