Re: [PATCH v12 02/11] lib: kstrtox: add kstrtoudec64() and kstrtodec64()

[Rodrigo Alencar]

On 26/05/12 12:39PM, Jonathan Cameron wrote:
> On Sun, 10 May 2026 13:42:20 +0100
> Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@xxxxxxxxxx> wrote:
> 
> > From: Rodrigo Alencar <rodrigo.alencar@xxxxxxxxxx>
> > 
> > Add helpers that parses decimal numbers into 64-bit number, i.e., decimal
> > point numbers with pre-defined scale are parsed into a 64-bit value (fixed
> > precision). After the decimal point, digits beyond the specified scale
> > are ignored.
> > 
> > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@xxxxxxxxxx>
> 
> Whilst Rodrigo has already replied to say there will be another version
> I'd like to request final feedback from those who were involved in the parser
> discussions.  
> 
> They got very involved and I'm far from an expert in the right way to do
> this stuff.  
> 
> I don't think David Laight was +CC so I've added that.
> David, Andy - I think you two were most involved in that discussion:
> Any objections to the end result? 

I am evaluating on taking sashiko's feedback here too, so it is a good
time to check this again indeed.

> Thanks,
> 
> Jonathan
> 
> 
> > ---
> >  include/linux/kstrtox.h |   3 ++
> >  lib/kstrtox.c           | 107 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 110 insertions(+)
> > 
> > diff --git a/include/linux/kstrtox.h b/include/linux/kstrtox.h
> > index 6ea897222af1..bec2fc17bde0 100644
> > --- a/include/linux/kstrtox.h
> > +++ b/include/linux/kstrtox.h
> > @@ -97,6 +97,9 @@ int __must_check kstrtou8(const char *s, unsigned int base, u8 *res);
> >  int __must_check kstrtos8(const char *s, unsigned int base, s8 *res);
> >  int __must_check kstrtobool(const char *s, bool *res);
> >  
> > +int __must_check kstrtoudec64(const char *s, unsigned int scale, u64 *res);
> > +int __must_check kstrtodec64(const char *s, unsigned int scale, s64 *res);
> > +
> >  int __must_check kstrtoull_from_user(const char __user *s, size_t count, unsigned int base, unsigned long long *res);
> >  int __must_check kstrtoll_from_user(const char __user *s, size_t count, unsigned int base, long long *res);
> >  int __must_check kstrtoul_from_user(const char __user *s, size_t count, unsigned int base, unsigned long *res);
> > diff --git a/lib/kstrtox.c b/lib/kstrtox.c
> > index 97be2a39f537..da7b5f83a3c5 100644
> > --- a/lib/kstrtox.c
> > +++ b/lib/kstrtox.c
> > @@ -17,6 +17,7 @@
> >  #include <linux/export.h>
> >  #include <linux/kstrtox.h>
> >  #include <linux/math64.h>
> > +#include <linux/overflow.h>
> >  #include <linux/types.h>
> >  #include <linux/uaccess.h>
> >  
> > @@ -392,6 +393,112 @@ int kstrtobool(const char *s, bool *res)
> >  }
> >  EXPORT_SYMBOL(kstrtobool);
> >  
> > +static int _kstrtoudec64(const char *s, unsigned int scale, u64 *res)
> > +{
> > +	u64 _res = 0, _frac = 0;
> > +	unsigned int rv;
> > +
> > +	if (scale > 19) /* log10(2^64) = 19.26 */
> > +		return -EINVAL;
> > +
> > +	if (*s != '.') {
> > +		rv = _parse_integer(s, 10, &_res);
> > +		if (rv & KSTRTOX_OVERFLOW)
> > +			return -ERANGE;
> > +		if (rv == 0)
> > +			return -EINVAL;
> > +		s += rv;
> > +	}
> > +
> > +	if (*s == '.' && scale) {

I havent really considered the scale == 0 case, I suppose that
one could rely on kstrtoull() instead. But as sashiko points
out, it deviates from the documented behavior. Also, I will
consider accepting "123." as a valid input, I see that others
parsers do that and should not be a problem. So I will add a
small change here. Also will make sure the test cases are ok.

> > +		s++; /* skip decimal point */
> > +		rv = _parse_integer_limit(s, 10, &_frac, scale);
> > +		if (rv & KSTRTOX_OVERFLOW)
> > +			return -ERANGE;
> > +		if (rv == 0)
> > +			return -EINVAL;
> > +		s += rv;
> > +		if (rv < scale)
> > +			_frac *= int_pow(10, scale - rv);
> > +		while (isdigit(*s)) /* truncate */
> > +			s++;
> > +	}
> > +
> > +	if (*s == '\n')
> > +		s++;
> > +	if (*s)
> > +		return -EINVAL;
> > +
> > +	if (check_mul_overflow(_res, int_pow(10, scale), &_res) ||
> > +	    check_add_overflow(_res, _frac, &_res))
> > +		return -ERANGE;
> > +
> > +	*res = _res;
> > +	return 0;
> > +}
> > +
> > +/**
> > + * kstrtoudec64() - Convert a string to an unsigned 64-bit value that represents
> > + *		    a scaled decimal number.
> > + * @s: The start of the string. The string must be null-terminated, and may also
> > + *  include a single newline before its terminating null. The first character
> > + *  may also be a plus sign, but not a minus sign. Digits beyond the specified
> > + *  scale are ignored.
> > + * @scale: The number of digits to the right of the decimal point. For example,
> > + *  a scale of 2 would mean the number is represented with two decimal places,
> > + *  so "123.45" would be represented as 12345.
> > + * @res: Where to write the result of the conversion on success.
> > + *
> > + * Return: 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
> > + */
> > +noinline
> > +int kstrtoudec64(const char *s, unsigned int scale, u64 *res)
> > +{
> > +	if (s[0] == '+')
> > +		s++;
> > +	return _kstrtoudec64(s, scale, res);
> > +}
> > +EXPORT_SYMBOL(kstrtoudec64);
> > +
> > +/**
> > + * kstrtodec64() - Convert a string to a signed 64-bit value that represents a
> > + *		   scaled decimal number.
> > + * @s: The start of the string. The string must be null-terminated, and may also
> > + *  include a single newline before its terminating null. The first character
> > + *  may also be a plus sign or a minus sign. Digits beyond the specified
> > + *  scale are ignored.
> > + * @scale: The number of digits to the right of the decimal point. For example,
> > + *  a scale of 5 would mean the number is represented with five decimal places,
> > + *  so "-3.141592" would be represented as -314159.
> > + * @res: Where to write the result of the conversion on success.
> > + *
> > + * Return: 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
> > + */
> > +noinline
> > +int kstrtodec64(const char *s, unsigned int scale, s64 *res)
> > +{
> > +	u64 tmp;
> > +	int rv;
> > +
> > +	if (s[0] == '-') {
> > +		rv = _kstrtoudec64(s + 1, scale, &tmp);
> > +		if (rv < 0)
> > +			return rv;
> > +		if ((s64)-tmp > 0)
> > +			return -ERANGE;
> > +		*res = -tmp;
> > +	} else {
> > +		rv = kstrtoudec64(s, scale, &tmp);
> > +		if (rv < 0)
> > +			return rv;
> > +		if ((s64)tmp < 0)
> > +			return -ERANGE;
> > +		*res = tmp;
> > +	}
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(kstrtodec64);
> > +
> >  /*
> >   * Since "base" would be a nonsense argument, this open-codes the
> >   * _from_user helper instead of using the helper macro below.
> > 
> 

-- 
Kind regards,

Rodrigo Alencar