Skip to content

ext/bcmath: Fixed pointer subtraction for scale - #17986

Closed
SakiTakamachi wants to merge 3 commits into
php:PHP-8.4from
SakiTakamachi:bcmath/fix_scale_calc
Closed

ext/bcmath: Fixed pointer subtraction for scale#17986
SakiTakamachi wants to merge 3 commits into
php:PHP-8.4from
SakiTakamachi:bcmath/fix_scale_calc

Conversation

@SakiTakamachi

Copy link
Copy Markdown
Member

No description provided.

@SakiTakamachi
SakiTakamachi marked this pull request as ready for review March 7, 2025 05:42
@ndossche

Copy link
Copy Markdown
Member

Right, do you have a test?

@SakiTakamachi

Copy link
Copy Markdown
MemberAuthor

@nielsdos
I've been trying all sorts of things, but I haven't found any code that's affected by this yet...

@SakiTakamachi

Copy link
Copy Markdown
MemberAuthor

Since this is the code for when auto_scale is false, only bccomp() is affected.

In the following code, scale is set to 20, but is only initialized to 10.

bccomp('0.0000000000000000000000001', '0', 10);

However, since only values ​​up to scale 10 are used for comparison, there will be no erroneous results.
It just allocates extra memory that is never used.

Comment threadext/bcmath/libbcmath/src/str2num.c Outdated
if (str_scale > 0) {
const char *fractional_new_end = bc_skip_zero_reverse(fractional_end, fractional_ptr);
str_scale -= fractional_new_end - fractional_end;
str_scale -= fractional_end - fractional_new_end; // fractional_end >= fractional_new_end

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please use c-style comments, i.e. /* */

@GirgiasGirgias left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems logical

SakiTakamachi added a commit that referenced this pull request Mar 14, 2025
* PHP-8.4:
Fixed pointer subtraction for scale (#17986)
@SakiTakamachi
SakiTakamachi deleted the bcmath/fix_scale_calc branch March 14, 2025 00:01
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@SakiTakamachi@ndossche@Girgias