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.

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

Fix required memory size passed to expunge() #534

Merged
merged 1 commit into from
Mar 2, 2025

Conversation

madmajestro
Copy link
Contributor

The size passed to expunge() must match the size passed to find_block().

apc_sma.c Outdated
@@ -449,7 +449,7 @@ PHP_APCU_API void *apc_sma_malloc_ex(apc_sma_t *sma, size_t n, size_t *allocated

/* Expunge cache in hope of freeing up memory, but only once */
if (!nuked) {
sma->expunge(*sma->data, n+fragment);
sma->expunge(*sma->data, ALIGNWORD(n + ALIGNWORD(sizeof(struct block_t))));
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we should be passing the real size to the expunge hook. Instead apc_sma_get_avail_size should be computing the real size for its check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I absolutely agree. Fixed.

@madmajestro madmajestro force-pushed the fix-size-passed-to-expunge branch from fefa7a6 to 10ffa35 Compare March 2, 2025 16:45
The function apc_sma_get_avail_size() now takes care of computing the
real size to achieve better separation of layers. In addition, the
computation of the real size has been fixed.
@madmajestro madmajestro force-pushed the fix-size-passed-to-expunge branch from 10ffa35 to 037ea17 Compare March 2, 2025 16:50
@nikic nikic merged commit 5f37033 into krakjoe:master Mar 2, 2025
31 checks passed
@madmajestro madmajestro deleted the fix-size-passed-to-expunge branch March 9, 2025 13:39
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.

2 participants