Re: [PATCH RFC v3 2/5] lib: kstrtox: round to nearest, ties to even in kstrto[u]dec64()
From: Andy Shevchenko
Date: Wed Oct 21 2026 - 05:10:12 EST
On Tue, Oct 20, 2026 at 09:27:00AM -0300, Rodrigo Alencar via B4 Relay wrote:
> kstrtoudec64() and kstrtodec64() drop the digits beyond the requested
> scale, which truncates the result toward zero. The error is up to one
> unit in the last place and is always in the same direction.
>
> This is easy to hit with values produced by userspace floating point
> formatting. Printing 0.3 with "%.17g" gives "0.29999999999999999",
> which is parsed as 299999 with a scale of 6 instead of 300000. With E
> notation a negative exponent can also drop integer digits, so "19e-1"
> with a scale of 0 gives 1 instead of 2.
>
> Round the result to nearest instead, with ties to even. This is the
> IEEE 754 default rounding mode, as used by strtod() and printf(), and it
> does not bias results in either direction. The first discarded digit is
> known from where parsing stopped, so no extra pass over the string is
> needed: the remaining digits are only checked when the first discarded
> digit is a 5. Since round-half-even is symmetric, kstrtodec64() keeps
> rounding the magnitude and negates it.
>
> Inputs that fit within the scale are parsed exactly as before. An input
> that rounds up past the type limit, such as "18446744073.7095516155"
> with a scale of 9, now returns -ERANGE.
> Update the existing truncation test cases and add cases for ties and
> for rounding at the type limits.
...
> +static bool _round_half_even(const char *p, const char *end, u64 res)
> +{
> + if (p == end || *p < '5')
> + return false;
> + if (*p++ > '5')
This hidden ++ makes the whole if chain to be strict in order, gives less
readability and flexibility in my opinion.
> + return true;
So, this can be split to
// Does this need to be first?
if (p == end)
return false;
if (*p < '5')
return false;
if (*p > '5')
return true;
while (++p < end) {
...
}
> + /* tie: any nonzero digit after it breaks it, skipping the decimal point */
> + for (; p < end; p++)
> + if (*p != '0' && *p != '.')
This is a loop, so this also passes '.0.0.' case and some other similar to it.
Is this a problem?
> + return true;
> + return res & 1;
So, this res is used only once to get a parity. But this doesn't semantically
sound like boolean to me. What if we actually return 0 or 1 from this function
and use its output directly in the check_add_overflow()?
> +}
--
With Best Regards,
Andy Shevchenko