Adjust base convert to allow negative numbers #3911
Conversation
Base convert currently does not handle negative numbers for example "-010" returns "8", "-8" would be expected This also emits a warning when using invalid chars Currently "01#0" is parsed exactly the same as "010" with no warning shown. The tests do fail currently due to conflicting behaviour, base_convert used to use zend_ulong, this PR would make that a zend_long. There are tests around the high ends of this which also fail. I imagine this will need an RFC but not sure how to start that process Targeting PHP8 would likely be best due to the BC changes
|
|
Thanks. I'm currently trying to get access to post to internals. Thank you |
| @@ -898,6 +904,15 @@ PHPAPI int _php_math_basetozval(zval *arg, int base, zval *ret) | |||
| } | |||
| } | |||
|
|
|||
| if (invalid_chars > 0) { | |||
There was a problem hiding this comment.
You'll probably need some extra handling to make sure that 0xDEADBEEF, 0b10101001 and 0o640 do not generate a warning.
|
Another thing to consider would be allowing leading and trailing whitespace. |
|
I will adjust to fix that. I think the first three work currently. Will adjust the whitespace one Still not had the email subscription to internals though though |
|
@derickr can you help this person out with internals subscription please ? @exussum12 send derick your email (privately) ... |
|
Sara helped out with it. Ive posted there now |
|
|
||
| if (c >= base) { | ||
| if (i == length -1) { | ||
| if (base == 16 && c == 33) { /* allow the second char to be x */ |
There was a problem hiding this comment.
Only if the first char is 0? I don't think we want to allow 9xAB for hex strings.
There was a problem hiding this comment.
#4328 This has been addressed here
|
Closing a new PR will be opened for 7.4 (#4328) and master |
|
Closed. This was 2 parts. The part remaining didn't pass the RFC |
Base convert currently does not handle negative numbers for example
"-010" returns "8", "-8" would be expected
This also emits a warning when using invalid chars
Currently
"01#0" is parsed exactly the same as "010" with no warning shown.
The tests do fail currently due to conflicting behavior, base_convert
used to use zend_ulong, this PR would make that a zend_long. There are
tests around the high ends of this which also fail. Not added new tests due to this as both sets couldn't pass and didn't want to change old tests without at least a discussion.
tests would be the two cases described above "-010" and "01#0" showing -8 and 8 (with a warning) respectively)
I imagine this will need an RFC but not sure how to start that process
Targeting PHP8 would likely be best due to the BC changes