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 <andriy.shevchenko@intel.com>
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v7: drop redundant check against ULLONG_MAX and restore
original comment
v6: more compact for-loop and minor style adjustments again
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 | 33 +++++++++++++++++++++------------
1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index bdde40cd69d7..ffcf0219b1f1 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -39,14 +39,20 @@ 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,
@@ -56,8 +62,7 @@ unsigned int _parse_integer_limit(const char *s, unsigned int base, unsigned lon
unsigned int rv;
res = 0;
- rv = 0;
- while (max_chars--) {
+ for (rv = 0; max_chars--; rv++, s++) {
unsigned int c = *s;
unsigned int lc = _tolower(c);
unsigned int val;
@@ -76,12 +81,16 @@ unsigned int _parse_integer_limit(const char *s, unsigned int base, unsigned lon
* it in the max base we support (16)
*/
if (unlikely(res & (~0ull << 60))) {
- if (res > div_u64(ULLONG_MAX - val, base))
+ unsigned long long tmp;
+
+ 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++;
}
*p = res;
return rv;
--
2.53.0On Thu, 12 Feb 2026 15:56:24 +0300
Dmitry Antipov <dmantipov@yandex.ru> 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 <andriy.shevchenko@intel.com>
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
> v7: drop redundant check against ULLONG_MAX and restore
> original comment
> v6: more compact for-loop and minor style adjustments again
> 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 | 33 +++++++++++++++++++++------------
> 1 file changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/lib/kstrtox.c b/lib/kstrtox.c
> index bdde40cd69d7..ffcf0219b1f1 100644
> --- a/lib/kstrtox.c
> +++ b/lib/kstrtox.c
> @@ -39,14 +39,20 @@ 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,
> @@ -56,8 +62,7 @@ unsigned int _parse_integer_limit(const char *s, unsigned int base, unsigned lon
> unsigned int rv;
>
> res = 0;
> - rv = 0;
> - while (max_chars--) {
> + for (rv = 0; max_chars--; rv++, s++) {
> unsigned int c = *s;
I think it would be better to use a separate variable for OVERFLOW.
Then the above would be the much more readable:
overflow = 0;
for (rv = 0; rv < max_chars; rv++) {
unsigned int c = s[rv];
with a final:
return rv | overflow;
('rv' is probably not a good name any more...)
I'd guess the code comes out a bit smaller, rather depends on how the
compiler pessimises it.
> unsigned int lc = _tolower(c);
> unsigned int val;
> @@ -76,12 +81,16 @@ unsigned int _parse_integer_limit(const char *s, unsigned int base, unsigned lon
> * it in the max base we support (16)
> */
> if (unlikely(res & (~0ull << 60))) {
> - if (res > div_u64(ULLONG_MAX - val, base))
> + unsigned long long tmp;
> +
> + if (check_mul_overflow(res, base, &tmp) ||
> + check_add_overflow(tmp, val, &res)) {
Do you need 'tmp' at all?
I think you can just use 'res'.
David
> + res = ULLONG_MAX;
> rv |= KSTRTOX_OVERFLOW;
> + }
> + } else {
> + res = res * base + val;
> }
> - res = res * base + val;
> - rv++;
> - s++;
> }
> *p = res;
> return rv;
On Thu, Feb 12, 2026 at 03:56:24PM +0300, Dmitry Antipov 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.
...
> + for (rv = 0; max_chars--; rv++, s++) {
Hmm... is max_chars being used inside for-loop body? I would rather use regular
pattern here:
for (rv = 0; rv < max_chars; rv++, s++) {
(just check that any integer / sign promotion doesn't change the logic).
--
With Best Regards,
Andy Shevchenko
On Thu, 12 Feb 2026 15:41:59 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> On Thu, Feb 12, 2026 at 03:56:24PM +0300, Dmitry Antipov 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.
>
> ...
>
> > + for (rv = 0; max_chars--; rv++, s++) {
>
> Hmm... is max_chars being used inside for-loop body? I would rather use regular
> pattern here:
>
> for (rv = 0; rv < max_chars; rv++, s++) {
>
> (just check that any integer / sign promotion doesn't change the logic).
>
The 'problem' is that the loop sets 'OVERFLOW' in rv.
But that is easily solved.
The smallest code is probably from the slightly horrid:
rv = max_chars;
lim = s + max_chars;
while (s < lim) {
ch = *s++;
..
}
return rv - (lim - s);
although that relies on max_chars being sane.
David
On Thu, Feb 12, 2026 at 02:32:39PM +0000, David Laight wrote:
> On Thu, 12 Feb 2026 15:41:59 +0200
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > On Thu, Feb 12, 2026 at 03:56:24PM +0300, Dmitry Antipov wrote:
...
> > > + for (rv = 0; max_chars--; rv++, s++) {
> >
> > Hmm... is max_chars being used inside for-loop body? I would rather use regular
> > pattern here:
> >
> > for (rv = 0; rv < max_chars; rv++, s++) {
> >
> > (just check that any integer / sign promotion doesn't change the logic).
>
> The 'problem' is that the loop sets 'OVERFLOW' in rv.
Ah, true...
--
With Best Regards,
Andy Shevchenko
© 2016 - 2026 Red Hat, Inc.