-
Notifications
You must be signed in to change notification settings - Fork 7.7k
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 calculation of aligned buffer size #16161
Conversation
As is, for requested size which are already aligned, we over-allocate. This issue has been reported by @kkmuffme.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks correct to me. Thanks!
Will this also fix it for the php.ini EDIT: will this ship with PHP 8.4? or will it also be backported to earlier PHP versions? |
Renaming things is hard. ;)
This looks like a good old off-by-one error. See <php/doc-en#3815 (comment)>.
Yes, that uses the same code, so has the same issue, and will be fixed as well.
Unlikely, since at least in a strict sense, this patch constitutes an ABI break. |
Unlikely that it's backported? |
Just to add on to this then to clarify this (and get some feedback if there might be an issue): This might be something that should perhaps be clarified in the docs, as it's an easy mistake to make when switching between .ini and PHP |
Both are unlikely; introducing an ABI is already too late for PHP 8.4, so this fix should probably only get into PHP 8.5 (or 9.0, whatever comes next).
Oh, good catch! Since there is special handling for Lines 1862 to 1863 in 95fa39d
we should document that. |
As is, for requested size which are already aligned, we over-allocate.
This issue has been reported by @kkmuffme.