Hi Andy,
Sorry, I should have kept the discussion on-list.
This is about the string_get_size() patch for outputs like "1000 kB" and
"1024 KiB", i.e. cases where the current code has already rounded to the
unit divisor but still prints the old unit.
> I'm trying to understand the current behaviour better and the logic of your
> change. But looking at the above I think your patch doesn't improve the case.
> And might even be another bug in the current code. I mean, do we have to round
> the numbers? 1048575 and lower are not equal 1.00 MiB strictly speaking.
> It's rather 0.999 MiB.
I took another look at the code and the old versions. I think the issue
here is narrower than whether values should cross the boundary at all.
For the base-10 values you asked about earlier, and for the binary
boundary case, the current code already rounds across the boundary.
With blk_size = 1, it prints:
999999 -> 1000 kB
1000000 -> 1.00 MB
1000001 -> 1.00 MB
and
1048575 -> 1024 KiB
1048576 -> 1.00 MiB
1048577 -> 1.00 MiB
So the patch does not introduce new cross-boundary rounding. It only changes
how those already-rounded results are displayed: it renormalizes 1000 kB to
1.00 MB and 1024 KiB to 1.00 MiB.
Around the first MB/MiB boundary with blk_size = 1, the display changes for:
STRING_UNITS_10: 999500 .. 999999
STRING_UNITS_2: 1048064 .. 1048575
with the adjacent values staying unchanged, e.g.:
999499 -> 999 kB
1000000 -> 1.00 MB
1048063 -> 1023 KiB
1048576 -> 1.00 MiB
That behavior comes from the arithmetic-rounding logic added by
564b026fbd0d ("string_helpers: fix precision loss for some inputs").
Before that change, 999999 printed 999 kB and 1048575 printed 1023 KiB,
so my original Fixes tag was wrong.
The v2 will carry:
Fixes: 564b026fbd0d ("string_helpers: fix precision loss for some inputs")
If the intended policy is instead that values below 1.00 MB / MiB must
never round across the unit boundary, then I agree this is broader than a
renormalization bug and the rounding policy itself would need changing.
But with the current documented 3 significant figures behavior, leaving
the result as 1000 kB / 1024 KiB is inconsistent with that formatting,
because the existing rounding has already carried the value to the next
unit boundary.
Unless there is a reason to take a different approach, I'll send a v2 with
the corrected Fixes tag, an updated changelog framing this as post-rounding
renormalization, and threshold tests around the affected ranges.
Thanks,
Shuvam
