Re: More exact scan_scaled(3)
Damien Miller <[email protected]>
| Newsgroups | gmane.os.openbsd.tech |
|---|---|
| Message-ID | <[email protected]> |
I'd like to commit this. Ok?
On Mon, 1 Jun 2026, Damien Miller wrote:
> Hi,
>
> This is adapted from a Github PR for OpenSSH, which imports scan_scaled(3)
> https://github.com/openssh/openssh-portable/pull/671/
>
> It changes the conversion to interleve divisions and multiplies, yielding
> much more accurate resultsfor large exponents.
>
> This also adapts the unit test to perform inexact comparisons for larger
> numbers, with a tolerance starting at 0.1% and rising slowly from there.
> The expanded tests show how far off the conversions get for the existing
> code when given large inputs (these use the 'E' == exa suffix):
>
> ** FAILURE: test 18 check 3, expect 1037629354146162279, result 0 diff 1037629354146162279 > tolerance 1000000 **
> ** FAILURE: test 20 check 3, expect 2190550858753009255, result 1152921504606846976 diff 1037629354146162279 > tolerance 10000000 **
> ** FAILURE: test 21 check 3, expect 4265809567045333807, result 3458764513820540928 diff 807045053224792879 > tolerance 10000000 **
> ** FAILURE: test 22 check 3, expect 9108079886394091105, result 8070450532247928832 diff 1037629354146162273 > tolerance 10000000 **
Index: lib/libutil/fmt_scaled.c
===================================================================
RCS file: /cvs/src/lib/libutil/fmt_scaled.c,v
diff -u -p -r1.23 fmt_scaled.c
--- lib/libutil/fmt_scaled.c 27 Dec 2022 17:10:08 -0000 1.23
+++ lib/libutil/fmt_scaled.c 1 Jun 2026 08:04:25 -0000
@@ -73,7 +73,7 @@ scan_scaled(char *scaled, long long *res
{
char *p = scaled;
int sign = 0;
- unsigned int i, ndigits = 0, fract_digits = 0;
+ unsigned int i, ndigits = 0, fract_digits = 0, muls = 0, divs = 0;
long long scale_fact = 1, whole = 0, fpart = 0;
/* Skip leading whitespace */
@@ -182,17 +182,33 @@ scan_scaled(char *scaled, long long *res
/* scale whole part */
whole *= scale_fact;
- /* truncate fpart so it doesn't overflow.
- * then scale fractional part.
- */
- while (fpart >= LLONG_MAX / scale_fact) {
- fpart /= 10;
- fract_digits--;
- }
- fpart *= scale_fact;
- if (fract_digits > 0) {
- for (i = 0; i < fract_digits -1; i++)
+ /*
+ * Scale fractional part: compute
+ * fpart * scale_fact / 10^(fract_digits-1)
+ * without intermediate overflow. scale_fact is 1024^i,
+ * i.e. a power of 1024 = 2^10. Interleave
+ * multiply-by-1024 with divide-by-10 so the running
+ * value stays within long long range while preserving
+ * precision.
+ */
+ muls = i;
+ divs = fract_digits > 0 ? fract_digits - 1 : 0;
+ while (muls > 0 && divs > 0) {
+ if (fpart <= LLONG_MAX / 1024) {
+ fpart *= 1024;
+ muls--;
+ } else {
fpart /= 10;
+ divs--;
+ }
+ }
+ while (muls > 0) {
+ fpart *= 1024;
+ muls--;
+ }
+ while (divs > 0) {
+ fpart /= 10;
+ divs--;
}
if (sign == -1)
whole -= fpart;
Index: regress/lib/libutil/fmt_scaled/fmt_test.c
===================================================================
RCS file: /cvs/src/regress/lib/libutil/fmt_scaled/fmt_test.c,v
diff -u -p -r1.19 fmt_test.c
--- regress/lib/libutil/fmt_scaled/fmt_test.c 4 Dec 2022 23:50:46 -0000 1.19
+++ regress/lib/libutil/fmt_scaled/fmt_test.c 1 Jun 2026 08:04:25 -0000
@@ -150,6 +150,13 @@ fmt_test(void)
#define IMPROBABLE (-42)
+static const long long K = 1024LL;
+static const long long M = K * K;
+static const long long G = M * K;
+static const long long T = G * K;
+static const long long P = T * K;
+static const long long E = P * K;
+
struct { /* the test cases */
char *input;
long long result;
@@ -157,22 +164,29 @@ struct { /* the test cases */
} sdata[] = {
{ "0", 0, 0 },
{ "123", 123, 0 },
- { "1k", 1024, 0 }, /* lower case */
+ { "1k", K, 0 }, /* lower case */
{ "100.944", 100, 0 }, /* should --> 100 (truncates fraction) */
{ "10099", 10099LL, 0 },
- { "1M", 1048576LL, 0 },
- { "1.1M", 1153433LL, 0 }, /* fractions */
- { "1.111111111111111111M", 1165084LL, 0 }, /* fractions */
- { "1.55M", 1625292LL, 0 }, /* fractions */
- { "1.9M", 1992294LL, 0 }, /* fractions */
- { "-2K", -2048LL, 0 }, /* negatives */
- { "-2.2K", -2252LL, 0 }, /* neg with fract */
- { "4.5k", 4608, 0 },
- { "3.333755555555t", 3665502936412, 0 },
- { "-3.333755555555t", -3665502936412, 0 },
+ { "1M", M, 0 },
+ { "1.1M", M + (M / 10), 0 }, /* fractions */
+ { "1.111111111111111111M", 1165084LL, 0 }, /* fractions */
+ { "1.55M", M + (M / 2) + (M / 20), 0 }, /* fractions */
+ { "1.9M", M + (9 * (M / 10)), 0 }, /* fractions */
+ { "-2K", -2 * K, 0 }, /* negatives */
+ { "-2.2K", -((2 * K) + (2 * (K / 10))), 0 },/* neg with fract */
+ { "4.5k", (4 * K) + (5 * (K / 10)), 0 },
+ { "3.333755555555t", 3665502997495, 0 }, /* exact 3665502997495.561 */
+ { "-3.333755555555t", -3665502997495, 0 },
{ "4.5555555555555555K", 4664, 0 },
{ "4.5555555555555555555K", 4664, 0 }, /* handle enough digits? */
{ "4.555555555555555555555555555555K", 4664, 0 }, /* ignores extra digits? */
+ { "0.9E", E - (E / 10), 0 },
+ { "0.1E", E/10, 0 },
+ { "1.9E", E + (E - (E / 10)), 0 },
+ { "3.7E", (3 * E) + (7 * (E / 10)), 0 },
+ { "7.9E", (7 * E) + (9 * (E / 10)), 0 },
+ { "1.234567T", T + (T * 234567LL) / 1000000LL, 0 },
+ { "0.0001P", P / 10000, 0 },
{ "1G", 1073741824LL, 0 },
{ "G", 0, 0 }, /* should == 0G? */
{ "1234567890", 1234567890LL, 0 }, /* should work */
@@ -324,10 +338,23 @@ assert_errno(int testnum, int check, int
static int
assert_llong(int testnum, int check, long long expect, long long result)
{
+ long long tmp, abs_expect, abs_diff, diff, tolerance = 0;
+
if (expect == result)
return 0;
- printf("** FAILURE: test %d check %d, expect %lld, result %lld **\n",
- testnum, check, expect, result);
+ abs_expect = expect < 0 ? -expect : expect;
+ if (abs_expect > 1024) {
+ tolerance = 1;
+ for (tmp = abs_expect; tmp != 0; tmp /= 1024)
+ tolerance *= 10;
+ }
+ diff = expect - result;
+ abs_diff = diff < 0 ? -diff : diff;
+ if (abs_diff < tolerance)
+ return 0;
+ printf("** FAILURE: test %d check %d, expect %lld, result %lld "
+ "diff %lld > tolerance %lld **\n", testnum, check, expect,
+ result, diff, tolerance);
return 1;
}