All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Laight <david.laight.linux@gmail.com>
To: Jonathan Cameron <jic23@kernel.org>
Cc: Rodrigo Alencar <455.rodrigo.alencar@gmail.com>,
	Rodrigo Alencar <rodrigo.alencar@analog.com>,
	linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-doc@vger.kernel.org,
	David Lechner <dlechner@baylibre.com>,
	Andy Shevchenko <andy@kernel.org>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Michael Hennerich <Michael.Hennerich@analog.com>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Jonathan Corbet <corbet@lwn.net>,
	Andrew Morton <akpm@linux-foundation.org>,
	Petr Mladek <pmladek@suse.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	Shuah Khan <skhan@linuxfoundation.org>
Subject: Re: [PATCH v10 02/11] lib: kstrtox: add kstrtoudec64() and kstrtodec64()
Date: Sat, 25 Apr 2026 23:33:16 +0100	[thread overview]
Message-ID: <20260425233316.0a2e2abd@pumpkin> (raw)
In-Reply-To: <20260425164006.17b75faf@jic23-huawei>

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
> 
> 


  reply	other threads:[~2026-04-25 22:33 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-15  9:51 [PATCH v10 00/11] ADF41513/ADF41510 PLL frequency synthesizers Rodrigo Alencar
2026-04-15  9:51 ` Rodrigo Alencar via B4 Relay
2026-04-15  9:51 ` [PATCH v10 01/11] dt-bindings: iio: frequency: add adf41513 Rodrigo Alencar
2026-04-15  9:51   ` Rodrigo Alencar via B4 Relay
2026-04-15  9:51 ` [PATCH v10 02/11] lib: kstrtox: add kstrtoudec64() and kstrtodec64() Rodrigo Alencar
2026-04-15  9:51   ` 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 [this message]
2026-04-26  8:02         ` Rodrigo Alencar
2026-04-27  6:56           ` Andy Shevchenko
2026-04-15  9:51 ` [PATCH v10 03/11] lib: test-kstrtox: tests for kstrtodec64() and kstrtoudec64() Rodrigo Alencar
2026-04-15  9:51   ` Rodrigo Alencar via B4 Relay
2026-04-15  9:51 ` [PATCH v10 04/11] lib: math: div64: add div64_s64_rem() Rodrigo Alencar
2026-04-15  9:51   ` Rodrigo Alencar via B4 Relay
2026-04-15  9:51 ` [PATCH v10 05/11] iio: core: add decimal value formatting into 64-bit value Rodrigo Alencar
2026-04-15  9:51   ` Rodrigo Alencar via B4 Relay
2026-05-04 10:42   ` Rodrigo Alencar
2026-05-05 16:42     ` Jonathan Cameron
2026-04-15  9:51 ` [PATCH v10 06/11] iio: test: iio-test-format: add test case for decimal format Rodrigo Alencar
2026-04-15  9:51   ` Rodrigo Alencar via B4 Relay
2026-04-15  9:51 ` [PATCH v10 07/11] iio: frequency: adf41513: driver implementation Rodrigo Alencar
2026-04-15  9:51   ` Rodrigo Alencar via B4 Relay
2026-04-25 16:33   ` Jonathan Cameron
2026-04-15  9:51 ` [PATCH v10 08/11] iio: frequency: adf41513: handle LE synchronization feature Rodrigo Alencar
2026-04-15  9:51   ` Rodrigo Alencar via B4 Relay
2026-04-15  9:51 ` [PATCH v10 09/11] iio: frequency: adf41513: features on frequency change Rodrigo Alencar
2026-04-15  9:51   ` Rodrigo Alencar via B4 Relay
2026-04-15  9:51 ` [PATCH v10 10/11] docs: iio: add documentation for adf41513 driver Rodrigo Alencar
2026-04-15  9:51   ` Rodrigo Alencar via B4 Relay
2026-04-25 16:40   ` Jonathan Cameron
2026-04-15  9:51 ` [PATCH v10 11/11] Documentation: ABI: testing: add common ABI file for iio/frequency Rodrigo Alencar
2026-04-15  9:51   ` Rodrigo Alencar via B4 Relay
2026-04-15 10:24 ` [PATCH v10 00/11] ADF41513/ADF41510 PLL frequency synthesizers Andy Shevchenko
  -- strict thread matches above, loose matches on Subject: below --
2026-04-25 18:57 [PATCH v10 02/11] lib: kstrtox: add kstrtoudec64() and kstrtodec64() Alexey Dobriyan
2026-04-25 21:44 ` Rodrigo Alencar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260425233316.0a2e2abd@pumpkin \
    --to=david.laight.linux@gmail.com \
    --cc=455.rodrigo.alencar@gmail.com \
    --cc=Michael.Hennerich@analog.com \
    --cc=akpm@linux-foundation.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=andy@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=corbet@lwn.net \
    --cc=devicetree@vger.kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=jic23@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=lars@metafoo.de \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=pmladek@suse.com \
    --cc=robh@kernel.org \
    --cc=rodrigo.alencar@analog.com \
    --cc=rostedt@goodmis.org \
    --cc=senozhatsky@chromium.org \
    --cc=skhan@linuxfoundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.