fix printf negative infinity #15965
fix printf negative infinity #15965
Conversation
|
Oh I was unaware. Close this PR then? this is a low-effort fix~ |
ext/standard/formatted_print.c
Outdated
| // ideally negative should have been handled inside php_sprintf_appendstring , | ||
| // but the code is difficult to debug, and it's easy to break stuff inside there (I tried, I broke stuff), | ||
| // handling it here is much easier.. | ||
| php_sprintf_appendstring(buffer, pos, "-INF", 4, 0, padding, |
There was a problem hiding this comment.
Hard-coding the min_width argument (4) here, looks wrong to me (I am aware that this already had been done this way before); see https://3v4l.org/WtHcq. Why not simply pass width instead? Same for the NaN and the -INF cases.
seems the location is correct :) https://github.com/php/php-src/pull/15965/files#r1768840758
Co-authored-by: Christoph M. Becker <cmbecker69@gmx.de>
There was a problem hiding this comment.
Thank you! This looks good to me, but maybe adding some more test cases makes sense; escpecially giving a width in the format specifier to show the alignment, and maybe some more, e.g. using 0 padding.
And I would also change the max_width for NaN, but that can be done in a follow-up PR, too.
Anyhow, since we're late in the PHP 8.4.0 pre-release cycle, this requires approval from the @php/release-managers-84.
|
This fix has fallen through the cracks. I've now applied to PHP-8.4 and merged up. Thank you! |
sprintf("%.17g", -INF);should return-INFnotINFresolves #15964
I tried fixing it inside php_sprintf_appendstring as it seems the original author intended, but.. that was difficult, and I broke other stuff.
(I think php_sprintf_appendstring would benefit from using smart_str https://www.phpinternalsbook.com/php7/internal_types/strings/smart_str.html , but idk how much effort that would be)