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

number_format: No precesion loss on formatting int's #11584

Closed
wants to merge 1 commit into from

Conversation

marc-mabe
Copy link
Contributor

@marc-mabe marc-mabe commented Jul 3, 2023

This is based on #11487 supporting negative decimal places on number_format.

  • Passing an int to number_format will perform formatting the int (zend_long) value without casting it to float (double)
  • With negative decimal places it will perform rounding to power of 10 using zend_ulong to be able to round high numbers without integer overflow (e.g. 9223372036854775806 -> 10,000,000,000,000,000,000)
  • Use of zend_long for decimals (dec argument) as there is a possible integer overflow with big numbers. This issue still exists on formatting floats but fixing it would be a BC break. (opened a separate PR number_format: prevent integer overflow on big decimal number #11649)

PS: I'm not a C developer and this is the first time for me handling pointers and malloc ... please review carefully that I haven't done a mistake ending up in crashes :)

@bukka
Copy link
Member

bukka commented Jul 9, 2023

@marc-mabe I merged #11487 so please rebase this. Also please squash it to a single commit when you are in it.

@marc-mabe
Copy link
Contributor Author

@bukka done

@marc-mabe
Copy link
Contributor Author

Internals: https://externals.io/message/120769

Copy link
Member

@bukka bukka left a comment

Choose a reason for hiding this comment

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

I went through the logic couple of times it looks reasonable to me.

ext/standard/math.c Show resolved Hide resolved
@bukka bukka closed this in 591f3f6 Jul 13, 2023
@marc-mabe marc-mabe deleted the num_format_int branch July 13, 2023 21:43
Sign up for free to 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.

None yet

2 participants