On Wed, 4 Feb 2026 16:57:13 +0300 Dmitry Antipov <[email protected]> wrote:
> In '_parse_integer_limit()', adjust native integer arithmetic > with near-to-overflow branch where 'check_mul_overflow()' and > 'check_add_overflow()' are used to check whether an intermediate > result goes out of range, and denote such a case with ULLONG_MAX, > thus making the function more similar to standard C library's > 'strtoull()'. Adjust comment to kernel-doc style as well. > > Reviewed-by: Andy Shevchenko <[email protected]> > Signed-off-by: Dmitry Antipov <[email protected]> > --- > v5: minor brace style adjustment > v4: restore plain integer arithmetic and use check_xxx_overflow() > on near-to-overflow branch only > v3: adjust commit message and comments as suggested by Andy > v2: initial version to join the series > --- > lib/kstrtox.c | 39 ++++++++++++++++++++++++++------------- > 1 file changed, 26 insertions(+), 13 deletions(-) > > diff --git a/lib/kstrtox.c b/lib/kstrtox.c > index bdde40cd69d7..8691f85cf2ce 100644 > --- a/lib/kstrtox.c > +++ b/lib/kstrtox.c > @@ -39,20 +39,26 @@ const char *_parse_integer_fixup_radix(const char *s, > unsigned int *base) > return s; > } > > -/* > - * Convert non-negative integer string representation in explicitly given > radix > - * to an integer. A maximum of max_chars characters will be converted. > +/** > + * _parse_integer_limit - Convert integer string representation to an integer > + * @s: Integer string representation > + * @base: Radix > + * @p: Where to store result > + * @max_chars: Maximum amount of characters to convert > + * > + * Convert non-negative integer string representation in explicitly given > + * radix to an integer. If overflow occurs, value at @p is set to ULLONG_MAX. > * > - * Return number of characters consumed maybe or-ed with overflow bit. > - * If overflow occurs, result integer (incorrect) is still returned. > + * This function is the workhorse of other string conversion functions and it > + * is discouraged to use it explicitly. Consider kstrto*() family instead. > * > - * Don't you dare use this function. > + * Return: Number of characters consumed, maybe ORed with overflow bit > */ > noinline > unsigned int _parse_integer_limit(const char *s, unsigned int base, unsigned > long long *p, > size_t max_chars) > { > - unsigned long long res; > + unsigned long long tmp, res; > unsigned int rv; > > res = 0; > @@ -72,14 +78,21 @@ unsigned int _parse_integer_limit(const char *s, unsigned > int base, unsigned lon > if (val >= base) > break; > /* > - * Check for overflow only if we are within range of > - * it in the max base we support (16) > + * Accumulate result if no overflow detected. > + * Otherwise just consume valid characters. > */ > - if (unlikely(res & (~0ull << 60))) { > - if (res > div_u64(ULLONG_MAX - val, base)) > - rv |= KSTRTOX_OVERFLOW; > + if (likely(res != ULLONG_MAX)) { > + if (unlikely(res & (~0ull << 60))) { Aren't those two checks in the wrong order? The likely/unlikely really don't make that much difference you want the main test first. In any case what is the first check for? I think it just stops 0xffffffffffffffff0 being treated as an error. If you are trying to skip the rest of the digits after an overflow you need to check 'rv'. Although I wonder whether strtoul() (etc) should stop 'eating' input when the value would overflow and return a pointer to the digit that caused the error. Code looking at the terminating character wont be expecting a digit and will treat it as a syntax error - which is what you are trying to do. That is a much easier API to use, and a 'drop-in' for existing code. David > + /* We're close to possible overflow. */ > + if (check_mul_overflow(res, base, &tmp) || > + check_add_overflow(tmp, val, &res)) { > + res = ULLONG_MAX; > + rv |= KSTRTOX_OVERFLOW; > + } > + } else { > + res = res * base + val; > + } > } > - res = res * base + val; > rv++; > s++; > }
