public inbox for linux-doc@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v10 02/11] lib: kstrtox: add kstrtoudec64() and kstrtodec64()
  2026-04-15  9:51 [PATCH v10 00/11] ADF41513/ADF41510 PLL frequency synthesizers Rodrigo Alencar via B4 Relay
@ 2026-04-15  9:51 ` Rodrigo Alencar via B4 Relay
  2026-04-17  8:36   ` Rodrigo Alencar
  0 siblings, 1 reply; 8+ messages in thread
From: Rodrigo Alencar via B4 Relay @ 2026-04-15  9:51 UTC (permalink / raw)
  To: linux-kernel, linux-iio, devicetree, linux-doc
  Cc: Jonathan Cameron, David Lechner, Andy Shevchenko,
	Lars-Peter Clausen, Michael Hennerich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Andrew Morton,
	Petr Mladek, Steven Rostedt, Andy Shevchenko, Rasmus Villemoes,
	Sergey Senozhatsky, Shuah Khan, Rodrigo Alencar

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

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@analog.com>
---
 include/linux/kstrtox.h |   3 ++
 lib/kstrtox.c           | 105 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 108 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..c7625ba4ac88 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,110 @@ 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) {
+		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 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.
+ *
+ * Returns 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 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.
+ *
+ * Returns 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.

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v10 02/11] lib: kstrtox: add kstrtoudec64() and kstrtodec64()
  2026-04-15  9:51 ` [PATCH v10 02/11] lib: kstrtox: add kstrtoudec64() and kstrtodec64() Rodrigo Alencar via B4 Relay
@ 2026-04-17  8:36   ` Rodrigo Alencar
  2026-04-25 15:40     ` Jonathan Cameron
  0 siblings, 1 reply; 8+ messages in thread
From: Rodrigo Alencar @ 2026-04-17  8:36 UTC (permalink / raw)
  To: Rodrigo Alencar, linux-kernel, linux-iio, devicetree, linux-doc
  Cc: Jonathan Cameron, David Lechner, Andy Shevchenko,
	Lars-Peter Clausen, Michael Hennerich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Andrew Morton,
	Petr Mladek, Steven Rostedt, Andy Shevchenko, Rasmus Villemoes,
	Sergey Senozhatsky, Shuah Khan

On 26/04/15 10:51AM, Rodrigo Alencar wrote:
> 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.

...

> +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) {
> +		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;
> +}

I have an alternative (slightly more complex) implementation of this function
that handles E notation. I find this particularly handy when writting big
values like 25 GHz when the ABI is defined in Hz, so instead of writing
25000000000, one can just use 25e9, or 2.5e10. I found that my python code
was printing big floating point values or really small ones using E notation
and that was giving me -EINVAL, so I had to adjust formatting when generating
the string input to the file. No big deal, and we would not need this here,
but if maintainers find this useful I could add it into a v11 of this series.

-- 
Kind regards,

Rodrigo Alencar

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v10 02/11] lib: kstrtox: add kstrtoudec64() and kstrtodec64()
  2026-04-17  8:36   ` Rodrigo Alencar
@ 2026-04-25 15:40     ` Jonathan Cameron
  2026-04-25 22:33       ` David Laight
  0 siblings, 1 reply; 8+ messages in thread
From: Jonathan Cameron @ 2026-04-25 15:40 UTC (permalink / raw)
  To: Rodrigo Alencar
  Cc: Rodrigo Alencar, linux-kernel, linux-iio, devicetree, linux-doc,
	David Lechner, Andy Shevchenko, Lars-Peter Clausen,
	Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet, Andrew Morton, Petr Mladek, Steven Rostedt,
	Andy Shevchenko, Rasmus Villemoes, Sergey Senozhatsky, Shuah Khan,
	David Laight

On Fri, 17 Apr 2026 09:36:20 +0100
Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote:

> On 26/04/15 10:51AM, Rodrigo Alencar wrote:
> > 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.  
> 
> ...
> 
> > +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) {
> > +		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;
> > +}  
> 
> I have an alternative (slightly more complex) implementation of this function
> that handles E notation. I find this particularly handy when writting big
> values like 25 GHz when the ABI is defined in Hz, so instead of writing
> 25000000000, one can just use 25e9, or 2.5e10. I found that my python code
> was printing big floating point values or really small ones using E notation
> and that was giving me -EINVAL, so I had to adjust formatting when generating
> the string input to the file. No big deal, and we would not need this here,
> but if maintainers find this useful I could add it into a v11 of this series.
> 

I'd rather we didn't slow this one down. However I'm waiting on some tags
on this patch from folk who are more familiar with these parsers than
I am.  Given discussion, Andy or David Laight perhaps?
+CC David - please make sure to include folk who have been active
in discussion of earlier versions to decrease chance they miss the new
one.

Maybe start a discussion about whether adding e notation as a separate
thread after this has merged?

Jonathan



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v10 02/11] lib: kstrtox: add kstrtoudec64() and kstrtodec64()
@ 2026-04-25 18:57 Alexey Dobriyan
  2026-04-25 21:44 ` Rodrigo Alencar
  0 siblings, 1 reply; 8+ messages in thread
From: Alexey Dobriyan @ 2026-04-25 18:57 UTC (permalink / raw)
  To: Rodrigo Alencar; +Cc: linux-kernel, linux-iio, devicetree, linux-doc

Rodrigo Alencar wrote:

> kstrtoudec64
> kstrtodec64

The only comment I have is to maybe sneak in "fixed point" into names
somehow. Or change to kstrtou64_scaled() because return type is not real
fixed point type.

	A.lexey

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v10 02/11] lib: kstrtox: add kstrtoudec64() and kstrtodec64()
  2026-04-25 18:57 [PATCH v10 02/11] lib: kstrtox: add kstrtoudec64() and kstrtodec64() Alexey Dobriyan
@ 2026-04-25 21:44 ` Rodrigo Alencar
  0 siblings, 0 replies; 8+ messages in thread
From: Rodrigo Alencar @ 2026-04-25 21:44 UTC (permalink / raw)
  To: Alexey Dobriyan, Rodrigo Alencar
  Cc: linux-kernel, linux-iio, devicetree, linux-doc

On 26/04/25 09:57PM, Alexey Dobriyan wrote:
> Rodrigo Alencar wrote:
> 
> > kstrtoudec64
> > kstrtodec64
> 
> The only comment I have is to maybe sneak in "fixed point" into names
> somehow. Or change to kstrtou64_scaled() because return type is not real
> fixed point type.
> 
> 	A.lexey

I understand that a decimal number is inherently a fixed precision number
with pre-defined scale. The "64" in "dec64" tells that we are storing it in
a 64-bit variable.

kstrtou64_scaled() does not tell me anything about a decimal point handling,
or that the base 10 is taken for granted. Maybe a typedef on u64/s64 to udec64
or dec64 can make things clearer?
I suppose the documentation header of the function would be enough for that.
As it is clear, the function intent is not to return a "fixed point type",
but its scaled representation in u64/s64.

-- 
Kind regards,

Rodrigo Alencar

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v10 02/11] lib: kstrtox: add kstrtoudec64() and kstrtodec64()
  2026-04-25 15:40     ` Jonathan Cameron
@ 2026-04-25 22:33       ` David Laight
  2026-04-26  8:02         ` Rodrigo Alencar
  0 siblings, 1 reply; 8+ messages in thread
From: David Laight @ 2026-04-25 22:33 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Rodrigo Alencar, Rodrigo Alencar, linux-kernel, linux-iio,
	devicetree, linux-doc, David Lechner, Andy Shevchenko,
	Lars-Peter Clausen, Michael Hennerich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Andrew Morton,
	Petr Mladek, Steven Rostedt, Andy Shevchenko, Rasmus Villemoes,
	Sergey Senozhatsky, Shuah Khan

On Sat, 25 Apr 2026 16:40:06 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Fri, 17 Apr 2026 09:36:20 +0100
> Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote:
> 
> > On 26/04/15 10:51AM, Rodrigo Alencar wrote:  
> > > 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.    
> > 
> > ...
> >   
> > > +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) {
> > > +		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;
> > > +}    
> > 
> > I have an alternative (slightly more complex) implementation of this function
> > that handles E notation. I find this particularly handy when writting big
> > values like 25 GHz when the ABI is defined in Hz, so instead of writing
> > 25000000000, one can just use 25e9, or 2.5e10. I found that my python code
> > was printing big floating point values or really small ones using E notation
> > and that was giving me -EINVAL, so I had to adjust formatting when generating
> > the string input to the file. No big deal, and we would not need this here,
> > but if maintainers find this useful I could add it into a v11 of this series.
> >   
> 
> I'd rather we didn't slow this one down. However I'm waiting on some tags
> on this patch from folk who are more familiar with these parsers than
> I am.  Given discussion, Andy or David Laight perhaps?
> +CC David - please make sure to include folk who have been active
> in discussion of earlier versions to decrease chance they miss the new
> one.

I can't help feeling this code would be smaller if it didn't try to use
the existing conversion functions.
Something like:
	u64 r = 0;
	unsigned int n = ~0;
	while (*s == ' ' || *s == '\n')
		s++;
	for (;;) {
		unsigned int dig = *s++ - '0';
		if (dig <= 9) {
			if (!n)
				continue;
			n--;
			r = r * 10 + dig;
			continue;
		}
		switch (s[-1]) {
		case '.':
			if (n <= scale)
				return -EINVAL;
			n = scale;
			continue;
		case '\n':
			if (*s)
				return -EINVAL;
			break;
		case 0:
			break;
		default:
			return -EIVAL;
		}
		break;
	}
	if (n > scale)
		n = scale;
	while (n--)
		r *= 10;
	*res = r;
	return 0;
}

That is missing the overflow detect for the multiply and add.
While check_add_overflow() hopefully looks at the carry flag (on non-mips
style cpu), I don't know how the 'mul' variant works - it might be horrid.
A bound check against ~0ull/10 might generate better code.

But I really prefer functions that return the terminating character to
the caller - they are more useful for parsing compound parameters.

	David

> 
> Maybe start a discussion about whether adding e notation as a separate
> thread after this has merged?
> 
> Jonathan
> 
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v10 02/11] lib: kstrtox: add kstrtoudec64() and kstrtodec64()
  2026-04-25 22:33       ` David Laight
@ 2026-04-26  8:02         ` Rodrigo Alencar
  2026-04-27  6:56           ` Andy Shevchenko
  0 siblings, 1 reply; 8+ messages in thread
From: Rodrigo Alencar @ 2026-04-26  8:02 UTC (permalink / raw)
  To: David Laight, Jonathan Cameron
  Cc: Rodrigo Alencar, Rodrigo Alencar, linux-kernel, linux-iio,
	devicetree, linux-doc, David Lechner, Andy Shevchenko,
	Lars-Peter Clausen, Michael Hennerich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Andrew Morton,
	Petr Mladek, Steven Rostedt, Andy Shevchenko, Rasmus Villemoes,
	Sergey Senozhatsky, Shuah Khan

On 26/04/25 11:33PM, David Laight wrote:
> On Sat, 25 Apr 2026 16:40:06 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > On Fri, 17 Apr 2026 09:36:20 +0100
> > Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote:
> > 
> > > On 26/04/15 10:51AM, Rodrigo Alencar wrote:  
> > > > 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.    

...

> > > I have an alternative (slightly more complex) implementation of this function
> > > that handles E notation. I find this particularly handy when writting big
> > > values like 25 GHz when the ABI is defined in Hz, so instead of writing
> > > 25000000000, one can just use 25e9, or 2.5e10. I found that my python code
> > > was printing big floating point values or really small ones using E notation
> > > and that was giving me -EINVAL, so I had to adjust formatting when generating
> > > the string input to the file. No big deal, and we would not need this here,
> > > but if maintainers find this useful I could add it into a v11 of this series.
> > >   
> > 
> > I'd rather we didn't slow this one down. However I'm waiting on some tags
> > on this patch from folk who are more familiar with these parsers than
> > I am.  Given discussion, Andy or David Laight perhaps?
> > +CC David - please make sure to include folk who have been active
> > in discussion of earlier versions to decrease chance they miss the new
> > one.

From Andy's message "We still have several weeks time", I thought we would have
time to discuss this e-notation thing, but that's just a nice-to-have indeed.
It fits well in the decimal context, as it is widely used and works in powers of 10!

> 
> I can't help feeling this code would be smaller if it didn't try to use
> the existing conversion functions.
> Something like:
> 	u64 r = 0;
> 	unsigned int n = ~0;
> 	while (*s == ' ' || *s == '\n')
> 		s++;
> 	for (;;) {
> 		unsigned int dig = *s++ - '0';
> 		if (dig <= 9) {
> 			if (!n)
> 				continue;
> 			n--;
> 			r = r * 10 + dig;
> 			continue;
> 		}
> 		switch (s[-1]) {
> 		case '.':
> 			if (n <= scale)
> 				return -EINVAL;
> 			n = scale;
> 			continue;
> 		case '\n':
> 			if (*s)
> 				return -EINVAL;
> 			break;
> 		case 0:
> 			break;
> 		default:
> 			return -EIVAL;
> 		}
> 		break;
> 	}
> 	if (n > scale)
> 		n = scale;
> 	while (n--)
> 		r *= 10;
> 	*res = r;
> 	return 0;
> }
> 
> That is missing the overflow detect for the multiply and add.
> While check_add_overflow() hopefully looks at the carry flag (on non-mips
> style cpu), I don't know how the 'mul' variant works - it might be horrid.
> A bound check against ~0ull/10 might generate better code.

It may be a compact parsing but aside from bugs or typos, there is a readability
tradeoff. For the context, yes, it would be better to accumulate interger and
fractional parts to the same variable, rather than separate ones.
_parse_integer_limit() would have to allow for a custom init value, so we could
just skip the decimal point and resume. The proposed implementation reuses tested
infrastructure and follows kstrto* conventions. I suppose the most important thing
to review here is the new interface with its function prototypes.

> But I really prefer functions that return the terminating character to
> the caller - they are more useful for parsing compound parameters.

I agree, I tried something like this with the kstrntoull() approach.

> > Maybe start a discussion about whether adding e notation as a separate
> > thread after this has merged?

Agreed.

-- 
Kind regards,

Rodrigo Alencar

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v10 02/11] lib: kstrtox: add kstrtoudec64() and kstrtodec64()
  2026-04-26  8:02         ` Rodrigo Alencar
@ 2026-04-27  6:56           ` Andy Shevchenko
  0 siblings, 0 replies; 8+ messages in thread
From: Andy Shevchenko @ 2026-04-27  6:56 UTC (permalink / raw)
  To: Rodrigo Alencar
  Cc: David Laight, Jonathan Cameron, Rodrigo Alencar, linux-kernel,
	linux-iio, devicetree, linux-doc, David Lechner, Andy Shevchenko,
	Lars-Peter Clausen, Michael Hennerich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Andrew Morton,
	Petr Mladek, Steven Rostedt, Rasmus Villemoes, Sergey Senozhatsky,
	Shuah Khan

On Sun, Apr 26, 2026 at 09:02:26AM +0100, Rodrigo Alencar wrote:
> On 26/04/25 11:33PM, David Laight wrote:
> > On Sat, 25 Apr 2026 16:40:06 +0100
> > Jonathan Cameron <jic23@kernel.org> wrote:
> > > On Fri, 17 Apr 2026 09:36:20 +0100
> > > Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote:
> > > > On 26/04/15 10:51AM, Rodrigo Alencar wrote:  

...

> > > > I have an alternative (slightly more complex) implementation of this function
> > > > that handles E notation. I find this particularly handy when writting big
> > > > values like 25 GHz when the ABI is defined in Hz, so instead of writing
> > > > 25000000000, one can just use 25e9, or 2.5e10. I found that my python code
> > > > was printing big floating point values or really small ones using E notation
> > > > and that was giving me -EINVAL, so I had to adjust formatting when generating
> > > > the string input to the file. No big deal, and we would not need this here,
> > > > but if maintainers find this useful I could add it into a v11 of this series.
> > > 
> > > I'd rather we didn't slow this one down. However I'm waiting on some tags
> > > on this patch from folk who are more familiar with these parsers than
> > > I am.  Given discussion, Andy or David Laight perhaps?
> > > +CC David - please make sure to include folk who have been active
> > > in discussion of earlier versions to decrease chance they miss the new
> > > one.
> 
> From Andy's message "We still have several weeks time", I thought we would have
> time to discuss this e-notation thing, but that's just a nice-to-have indeed.
> It fits well in the decimal context, as it is widely used and works in powers of 10!

My main concern is to have one source of implementation.

Yes, we can have specific parser for the specific need in the certain subsystem,
but as I already said (and maybe not once) the problem is that if any bugs or
features is desired it may diverse for the 2+ implementations of the same, putting
bug spreading and double effort.

Having e-notation supported is fine, but can we do it separately?

> > I can't help feeling this code would be smaller if it didn't try to use
> > the existing conversion functions.
> > Something like:

> > That is missing the overflow detect for the multiply and add.
> > While check_add_overflow() hopefully looks at the carry flag (on non-mips
> > style cpu), I don't know how the 'mul' variant works - it might be horrid.
> > A bound check against ~0ull/10 might generate better code.
> 
> It may be a compact parsing but aside from bugs or typos, there is a readability
> tradeoff. For the context, yes, it would be better to accumulate interger and
> fractional parts to the same variable, rather than separate ones.
> _parse_integer_limit() would have to allow for a custom init value, so we could
> just skip the decimal point and resume. The proposed implementation reuses tested
> infrastructure and follows kstrto* conventions. I suppose the most important thing
> to review here is the new interface with its function prototypes.
> 
> > But I really prefer functions that return the terminating character to
> > the caller - they are more useful for parsing compound parameters.
> 
> I agree, I tried something like this with the kstrntoull() approach.

With the wrong naming. kstrn* is oxymoron. Semantically the kstrto* are about
strict input checks, including overflow.

> > > Maybe start a discussion about whether adding e notation as a separate
> > > thread after this has merged?
> 
> Agreed.

Yep, sounds like an additional feature that may be added later on.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-04-27  6:56 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-25 18:57 [PATCH v10 02/11] lib: kstrtox: add kstrtoudec64() and kstrtodec64() Alexey Dobriyan
2026-04-25 21:44 ` Rodrigo Alencar
  -- strict thread matches above, loose matches on Subject: below --
2026-04-15  9:51 [PATCH v10 00/11] ADF41513/ADF41510 PLL frequency synthesizers Rodrigo Alencar via B4 Relay
2026-04-15  9:51 ` [PATCH v10 02/11] lib: kstrtox: add kstrtoudec64() and kstrtodec64() Rodrigo Alencar via B4 Relay
2026-04-17  8:36   ` Rodrigo Alencar
2026-04-25 15:40     ` Jonathan Cameron
2026-04-25 22:33       ` David Laight
2026-04-26  8:02         ` Rodrigo Alencar
2026-04-27  6:56           ` Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox