lib/string_helpers.c | 12 ++++++++++++ lib/tests/string_helpers_kunit.c | 25 +++++++++++++++++++++++++ 2 files changed, 37 insertions(+)
string_get_size() rounds the fractional part before formatting the
result. If that carry pushes the integer part to the unit divisor, the
helper still prints the old unit and produces strings like "1000 kB"
and "1024 KiB".
That misformatted carry case comes from the arithmetic rounding added by
commit 564b026fbd0d ("string_helpers: fix precision loss for some
inputs"). The helper already rounds those values up numerically; it
just fails to renormalize them into the next unit before formatting.
Renormalize the carried result into the next named unit so those cases
print as "1.00 MB" and "1.00 MiB" instead. Only do that when another
named unit exists, so the existing "UNK" fallback is preserved at the
top end. Extend the KUnit coverage around the first decimal and binary
boundaries, including adjacent values and non-byte block sizes that hit
the same carry path.
Fixes: 564b026fbd0d ("string_helpers: fix precision loss for some inputs")
Signed-off-by: Shuvam Pandey <shuvampandey1@gmail.com>
---
Changes in v2:
- correct the Fixes tag to 564b026fbd0d
- renormalize only when another named unit exists
- preserve the top-end UNK fallback
- add decimal and binary boundary/neighbor KUnit coverage
- add non-byte block-size cases that hit the same carry path
lib/string_helpers.c | 12 ++++++++++++
lib/tests/string_helpers_kunit.c | 25 +++++++++++++++++++++++++
2 files changed, 37 insertions(+)
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 169eaf5834949..bcabb24fe0dbf 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -121,6 +121,18 @@ int string_get_size(u64 size, u64 blk_size, const enum string_size_units units,
size += 1;
}
+ /*
+ * Renormalize into the next named unit, but preserve the top-end
+ * UNK fallback. After promotion the value is exactly 1 of the next
+ * unit, so keep two fractional digits for the usual 1.00 formatting.
+ */
+ if (size >= divisor[units_base] && i + 1 < ARRAY_SIZE(units_2)) {
+ size = 1;
+ remainder = 0;
+ i++;
+ j = 2;
+ }
+
if (j) {
snprintf(tmp, sizeof(tmp), ".%03u", remainder);
tmp[j+1] = '\0';
diff --git a/lib/tests/string_helpers_kunit.c b/lib/tests/string_helpers_kunit.c
index c853046183d24..67822cb48fd15 100644
--- a/lib/tests/string_helpers_kunit.c
+++ b/lib/tests/string_helpers_kunit.c
@@ -558,6 +558,31 @@ static void test_get_size(struct kunit *test)
/* weird block sizes */
test_string_get_size_one(3000, 1900, "5.70 MB", "5.44 MiB");
+ /* rounding carry into the next unit at the first decimal boundary */
+ test_string_get_size_one(999499, 1, "999 kB", "976 KiB");
+ test_string_get_size_one(999500, 1, "1.00 MB", "976 KiB");
+ test_string_get_size_one(999999, 1, "1.00 MB", "977 KiB");
+ test_string_get_size_one(1000000, 1, "1.00 MB", "977 KiB");
+ test_string_get_size_one(1000001, 1, "1.00 MB", "977 KiB");
+
+ /* rounding carry into the next unit at the first binary boundary */
+ test_string_get_size_one(1048063, 1, "1.05 MB", "1023 KiB");
+ test_string_get_size_one(1048064, 1, "1.05 MB", "1.00 MiB");
+ test_string_get_size_one(1048575, 1, "1.05 MB", "1.00 MiB");
+ test_string_get_size_one(1048576, 1, "1.05 MB", "1.00 MiB");
+ test_string_get_size_one(1048577, 1, "1.05 MB", "1.00 MiB");
+
+ /* values already in the next binary unit stay unchanged */
+ test_string_get_size_one(2097151, 1, "2.10 MB", "2.00 MiB");
+ test_string_get_size_one(2097152, 1, "2.10 MB", "2.00 MiB");
+ test_string_get_size_one(2097153, 1, "2.10 MB", "2.00 MiB");
+
+ /* non-byte block sizes hit the same carry path */
+ test_string_get_size_one(4997, 200, "999 kB", "976 KiB");
+ test_string_get_size_one(4998, 200, "1.00 MB", "976 KiB");
+ test_string_get_size_one(9981, 105, "1.05 MB", "1023 KiB");
+ test_string_get_size_one(9982, 105, "1.05 MB", "1.00 MiB");
+
/* huge values */
test_string_get_size_one(U64_MAX, 4096, "75.6 ZB", "64.0 ZiB");
test_string_get_size_one(4096, U64_MAX, "75.6 ZB", "64.0 ZiB");
--
2.50.1 (Apple Git-155)Please, keep the author of the original change in the loop.
James, what do you think?
On Sun, Apr 19, 2026 at 6:31 PM Shuvam Pandey <shuvampandey1@gmail.com> wrote:
>
> string_get_size() rounds the fractional part before formatting the
> result. If that carry pushes the integer part to the unit divisor, the
> helper still prints the old unit and produces strings like "1000 kB"
> and "1024 KiB".
>
> That misformatted carry case comes from the arithmetic rounding added by
> commit 564b026fbd0d ("string_helpers: fix precision loss for some
> inputs"). The helper already rounds those values up numerically; it
> just fails to renormalize them into the next unit before formatting.
>
> Renormalize the carried result into the next named unit so those cases
> print as "1.00 MB" and "1.00 MiB" instead. Only do that when another
> named unit exists, so the existing "UNK" fallback is preserved at the
> top end. Extend the KUnit coverage around the first decimal and binary
> boundaries, including adjacent values and non-byte block sizes that hit
> the same carry path.
>
> Fixes: 564b026fbd0d ("string_helpers: fix precision loss for some inputs")
> Signed-off-by: Shuvam Pandey <shuvampandey1@gmail.com>
> ---
> Changes in v2:
> - correct the Fixes tag to 564b026fbd0d
> - renormalize only when another named unit exists
> - preserve the top-end UNK fallback
> - add decimal and binary boundary/neighbor KUnit coverage
> - add non-byte block-size cases that hit the same carry path
>
> lib/string_helpers.c | 12 ++++++++++++
> lib/tests/string_helpers_kunit.c | 25 +++++++++++++++++++++++++
> 2 files changed, 37 insertions(+)
>
> diff --git a/lib/string_helpers.c b/lib/string_helpers.c
> index 169eaf5834949..bcabb24fe0dbf 100644
> --- a/lib/string_helpers.c
> +++ b/lib/string_helpers.c
> @@ -121,6 +121,18 @@ int string_get_size(u64 size, u64 blk_size, const enum string_size_units units,
> size += 1;
> }
>
> + /*
> + * Renormalize into the next named unit, but preserve the top-end
> + * UNK fallback. After promotion the value is exactly 1 of the next
> + * unit, so keep two fractional digits for the usual 1.00 formatting.
> + */
> + if (size >= divisor[units_base] && i + 1 < ARRAY_SIZE(units_2)) {
> + size = 1;
> + remainder = 0;
> + i++;
> + j = 2;
> + }
> +
> if (j) {
> snprintf(tmp, sizeof(tmp), ".%03u", remainder);
> tmp[j+1] = '\0';
> diff --git a/lib/tests/string_helpers_kunit.c b/lib/tests/string_helpers_kunit.c
> index c853046183d24..67822cb48fd15 100644
> --- a/lib/tests/string_helpers_kunit.c
> +++ b/lib/tests/string_helpers_kunit.c
> @@ -558,6 +558,31 @@ static void test_get_size(struct kunit *test)
> /* weird block sizes */
> test_string_get_size_one(3000, 1900, "5.70 MB", "5.44 MiB");
>
> + /* rounding carry into the next unit at the first decimal boundary */
> + test_string_get_size_one(999499, 1, "999 kB", "976 KiB");
> + test_string_get_size_one(999500, 1, "1.00 MB", "976 KiB");
> + test_string_get_size_one(999999, 1, "1.00 MB", "977 KiB");
> + test_string_get_size_one(1000000, 1, "1.00 MB", "977 KiB");
> + test_string_get_size_one(1000001, 1, "1.00 MB", "977 KiB");
> +
> + /* rounding carry into the next unit at the first binary boundary */
> + test_string_get_size_one(1048063, 1, "1.05 MB", "1023 KiB");
> + test_string_get_size_one(1048064, 1, "1.05 MB", "1.00 MiB");
> + test_string_get_size_one(1048575, 1, "1.05 MB", "1.00 MiB");
> + test_string_get_size_one(1048576, 1, "1.05 MB", "1.00 MiB");
> + test_string_get_size_one(1048577, 1, "1.05 MB", "1.00 MiB");
> +
> + /* values already in the next binary unit stay unchanged */
> + test_string_get_size_one(2097151, 1, "2.10 MB", "2.00 MiB");
> + test_string_get_size_one(2097152, 1, "2.10 MB", "2.00 MiB");
> + test_string_get_size_one(2097153, 1, "2.10 MB", "2.00 MiB");
> +
> + /* non-byte block sizes hit the same carry path */
> + test_string_get_size_one(4997, 200, "999 kB", "976 KiB");
> + test_string_get_size_one(4998, 200, "1.00 MB", "976 KiB");
> + test_string_get_size_one(9981, 105, "1.05 MB", "1023 KiB");
> + test_string_get_size_one(9982, 105, "1.05 MB", "1.00 MiB");
> +
> /* huge values */
> test_string_get_size_one(U64_MAX, 4096, "75.6 ZB", "64.0 ZiB");
> test_string_get_size_one(4096, U64_MAX, "75.6 ZB", "64.0 ZiB");
> --
> 2.50.1 (Apple Git-155)
>
--
With Best Regards,
Andy Shevchenko
On Mon, 2026-04-20 at 09:56 +0300, Andy Shevchenko wrote:
> Please, keep the author of the original change in the loop.
> James, what do you think?
Well firstly I would note that the original behaviour isn't a bug. It
works that way because of the binary case. So if you feed in 1023 in
for STRING_UNITS_2 it will return 1023 B. If we're going to allow that
slop for binary, it didn't seem reasonable to be precious about the
over by 1 of decimal.
However, if the desire is for this only ever to print out three
significant figures (is that the desire? because I don't really get it
from the changelog) then this:
> > + if (size >= divisor[units_base] && i + 1 <
> > ARRAY_SIZE(units_2)) {
> > + size = 1;
> > + remainder = 0;
> > + i++;
> > + j = 2;
Isn't right. Size is base_10 here, so it should always be compared
against 1000 and, for the STRING_UNITS_2 case, the lower two digits
need to be carried over to the remainder (with roundup). I'd also drop
the comparison against ARRAY_SIZE because you'd still violate the 3sf
rule even if the unit is UNK.
Regards,
James
Hi James, > Well firstly I would note that the original behaviour isn't a bug. > ... > However, if the desire is for this only ever to print out three > significant figures ... > Isn't right. Thanks, that clarifies it. I was looking at the 1000 kB / 1024 KiB cases as a narrow formatting issue after the rounding carry, but that is too narrow a reading. As you point out, the current helper is not based on a strict always 3 significant figures rule, and my patch only adjusts one carry case rather than addressing the broader behaviour. So I agree this should not go forward as a bug fix in its current form. I'll drop this patch here and revisit it only if I can come back with a better understanding of the intended semantics. Thanks, Shuvam
On Tue, Apr 21, 2026 at 6:38 AM Shuvam Pandey <shuvampandey1@gmail.com> wrote: > > Well firstly I would note that the original behaviour isn't a bug. James, thank you for the prompt reply! > So I agree this should not go forward as a bug fix in its current form. > I'll drop this patch here and revisit it only if I can come back with a > better understanding of the intended semantics. Shuvam, perhaps we need to add a few more test cases first to understand current behaviour better? -- With Best Regards, Andy Shevchenko
On Tue, 2026-04-21 at 07:53 +0300, Andy Shevchenko wrote: > On Tue, Apr 21, 2026 at 6:38 AM Shuvam Pandey > <shuvampandey1@gmail.com> wrote: > > > > Well firstly I would note that the original behaviour isn't a > > > bug. > > James, thank you for the prompt reply! > > > So I agree this should not go forward as a bug fix in its current > > form. I'll drop this patch here and revisit it only if I can come > > back with a better understanding of the intended semantics. > > Shuvam, perhaps we need to add a few more test cases first to > understand current behaviour better? Sure, but the base 2 one is the problem. It will go up to 1023 in whatever units but for binary that's under 1 of the next unit up. So 1023 B == 1.00 KiB 1018 B == 0.99 KiB 1000 B == 0.98 KiB And so on. However, technically the latter two (when converted to the upper unit) are only 2sf not 3 because the leading zero doesn't count. If you run the above to 3sf it comes out 1023 B == 0.999 KiB 1018 B == 0.994 KiB 1000 B == 0.977 KiB If the desire is really to be 3sf everywhere, you need the latter and the routine can be adjusted to do that: the while around the do_div would have to be checking strictly >= 1000 and you now have to set j=0 if size comes out 0. Rounding also has to be done inside that loop now (because too much precision is lost in the j = 3 case). But, like I said, this isn't a bug fix it's a behaviour change. Regards, James
On Tue, Apr 21, 2026 at 10:39 AM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > Shuvam, perhaps we need to add a few more test cases first to > understand current behaviour better? Yes, that makes sense. I'll put together a separate test-only patch covering the boundary cases first, so the current behaviour is explicit before proposing any behavioural change. Thanks, Shuvam
© 2016 - 2026 Red Hat, Inc.