From 0944f56181e0487c3a45185aa9730b0fde0a49a7 Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sun, 23 Aug 2020 00:35:36 +0200 Subject: [PATCH] AK: Fix human_readable_size corner cases In particular: consistent rounding and extreme values. Before, rounding was something like 'away from 0.999...', which led to surprising corner cases in which the value was rounded up. Now, rounding is always 'down'. This even works for 0xffffffff, and also for 0xffffffffffffffffULL on 64-bit. --- AK/NumberFormat.h | 12 ++++++------ AK/Tests/TestNumberFormat.cpp | 20 ++++++++++---------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/AK/NumberFormat.h b/AK/NumberFormat.h index 7a2d1a681a7..0f172ed3bb0 100644 --- a/AK/NumberFormat.h +++ b/AK/NumberFormat.h @@ -32,10 +32,10 @@ namespace AK { // FIXME: Remove this hackery once printf() supports floats. -static String number_string_with_one_decimal(float number, const char* suffix) +static String number_string_with_one_decimal(u64 number, u32 unit, const char* suffix) { - float decimals = number - (int)number; - return String::format("%d.%d %s", (int)number, (int)(decimals * 10), suffix); + int decimal = (number % unit) * 10 / unit; + return String::format("%llu.%d %s", number / unit, decimal, suffix); } static inline String human_readable_size(size_t size) @@ -43,10 +43,10 @@ static inline String human_readable_size(size_t size) if (size < 1 * KiB) return String::format("%zu B", size); if (size < 1 * MiB) - return number_string_with_one_decimal((float)size / (float)KiB, "KiB"); + return number_string_with_one_decimal(size, KiB, "KiB"); if (size < 1 * GiB) - return number_string_with_one_decimal((float)size / (float)MiB, "MiB"); - return number_string_with_one_decimal((float)size / (float)GiB, "GiB"); + return number_string_with_one_decimal(size, MiB, "MiB"); + return number_string_with_one_decimal(size, GiB, "GiB"); } } diff --git a/AK/Tests/TestNumberFormat.cpp b/AK/Tests/TestNumberFormat.cpp index fb9619ead5f..f78e87bbebe 100644 --- a/AK/Tests/TestNumberFormat.cpp +++ b/AK/Tests/TestNumberFormat.cpp @@ -98,7 +98,7 @@ TEST_CASE(fraction_MiB) TEST_CASE(border_MiB_GiB) { EXPECT_EQ(human_readable_size(1000 * MiB), "1000.0 MiB"); - EXPECT_EQ(human_readable_size(1024 * MiB - 1), "1024.0 MiB"); // TODO + EXPECT_EQ(human_readable_size(1024 * MiB - 1), "1023.9 MiB"); EXPECT_EQ(human_readable_size(1024 * MiB), "1.0 GiB"); EXPECT_EQ(human_readable_size(1024 * MiB + 1), "1.0 GiB"); } @@ -114,9 +114,9 @@ TEST_CASE(fraction_GiB) EXPECT_EQ(human_readable_size(1154272461), "1.0 GiB"); EXPECT_EQ(human_readable_size(1181115968), "1.0 GiB"); - EXPECT_EQ(human_readable_size(1181115969), "1.1 GiB"); // TODO - EXPECT_EQ(human_readable_size(1181116000), "1.1 GiB"); // TODO - EXPECT_EQ(human_readable_size(1181116006), "1.1 GiB"); // TODO + EXPECT_EQ(human_readable_size(1181115969), "1.0 GiB"); + EXPECT_EQ(human_readable_size(1181116000), "1.0 GiB"); + EXPECT_EQ(human_readable_size(1181116006), "1.0 GiB"); // 1024 * 1024 * 1024 * 1.1 = 1181116006.4 EXPECT_EQ(human_readable_size(1181116007), "1.1 GiB"); EXPECT_EQ(human_readable_size(1202590842), "1.1 GiB"); @@ -124,9 +124,9 @@ TEST_CASE(fraction_GiB) TEST_CASE(extremes_4byte) { - EXPECT_EQ(human_readable_size(0x7fffffff), "2.0 GiB"); // TODO + EXPECT_EQ(human_readable_size(0x7fffffff), "1.9 GiB"); EXPECT_EQ(human_readable_size(0x80000000), "2.0 GiB"); - EXPECT_EQ(human_readable_size(0xffffffff), "4.0 GiB"); // TODO + EXPECT_EQ(human_readable_size(0xffffffff), "3.9 GiB"); } template @@ -149,10 +149,10 @@ void actual_extremes_8byte<8>() EXPECT_EQ(human_readable_size(0x800000000ULL), "32.0 GiB"); EXPECT_EQ(human_readable_size(0x10000000000ULL), "1024.0 GiB"); - // Ehh, too bad. - EXPECT_EQ(human_readable_size(0x7fffffffffffffffULL), "-2147483648.-2147483648 GiB"); - EXPECT_EQ(human_readable_size(0x8000000000000000ULL), "-2147483648.-2147483648 GiB"); - EXPECT_EQ(human_readable_size(0xffffffffffffffffULL), "-2147483648.-2147483648 GiB"); + // Oh yeah! These are *correct*! + EXPECT_EQ(human_readable_size(0x7fffffffffffffffULL), "8589934591.9 GiB"); + EXPECT_EQ(human_readable_size(0x8000000000000000ULL), "8589934592.0 GiB"); + EXPECT_EQ(human_readable_size(0xffffffffffffffffULL), "17179869183.9 GiB"); } TEST_CASE(extremes_8byte)