All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Laight <david.laight.linux@gmail.com>
To: Rodrigo Alencar <455.rodrigo.alencar@gmail.com>
Cc: rodrigo.alencar@analog.com, linux-kernel@vger.kernel.org,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-doc@vger.kernel.org, Jonathan Cameron <jic23@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 v12 02/11] lib: kstrtox: add kstrtoudec64() and kstrtodec64()
Date: Fri, 15 May 2026 20:21:42 +0100	[thread overview]
Message-ID: <20260515202142.5dc561e0@pumpkin> (raw)
In-Reply-To: <ex6p5qpgsfvm5wzalpwo7whcj4m4uxzscpzxvb5ihfu2prx3fj@7skhmz3cbshw>

On Fri, 15 May 2026 17:05:06 +0100
Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote:

> On 26/05/13 10:41AM, Rodrigo Alencar wrote:
> > On 26/05/10 01:42PM, Rodrigo Alencar via B4 Relay wrote:  
> > > 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.  
> > 
> > Hi Andy,
> > 
> > I am starting over here, the other conversation is getting hard to follow.
> > This is my new proposal...  
> 
> +cc David

I just wouldn't do it this way :-)

You end up with more code than you would get if you just converted the digits.

-- David

>  
> > ...
> >   
> > > +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;
> > > +}  
> > 
> > This function now becomes:
> > 
> > 	static int _kstrtoudec64(const char *s, unsigned int scale, u64 *res)
> > 	{
> > 		u64 _res = 0;
> > 		unsigned int rv_int, rv_frac;
> > 
> > 		rv_int = _parse_integer(s, 10, &_res);
> > 		if (rv_int & KSTRTOX_OVERFLOW)
> > 			return -ERANGE;
> > 		s += rv_int;
> > 
> > 		if (*s == '.')
> > 			s++; /* skip decimal point */
> > 
> > 		rv_frac = _parse_integer_limit_init(s, 10, _res, &_res, scale);
> > 		if (rv_frac & KSTRTOX_OVERFLOW)
> > 			return -ERANGE;
> > 		s += rv_frac;
> > 
> > 		if (!rv_int && !rv_frac && !isdigit(*s))
> > 			return -EINVAL; /* no digits at all */
> > 
> > 		while (isdigit(*s)) /* truncate digits */
> > 			s++;
> > 
> > 		if (*s == '\n')
> > 			s++;
> > 		if (*s)
> > 			return -EINVAL;
> > 
> > 		if (_res && (scale > (19 + rv_frac) || /* log10(2^64) = 19.26 */
> > 		    check_mul_overflow(_res, int_pow(10, scale - rv_frac), &_res)))
> > 			return -ERANGE;
> > 
> > 		*res = _res;
> > 		return 0;
> > 	}
> > 
> > The new thing here is _parse_integer_limit_init(), which is a local modified
> > helper that accepts an init value, so _parse_integer_limit() becomes:
> > 
> > 	unsigned int _parse_integer_limit(const char *s, unsigned int base,
> > 					  unsigned long long *p, size_t max_chars)
> > 	{
> > 		return _parse_integer_limit_init(s, base, 0, p, max_chars);
> > 	}
> > 
> > with init = 0:
> > 
> > 	static unsigned int _parse_integer_limit_init(const char *s, unsigned int base,
> > 						      unsigned long long init,
> > 						      unsigned long long *p,
> > 						      size_t max_chars)
> > 	{
> > 		unsigned long long res;
> > 		unsigned int rv;
> > 
> > 		res = init;
> > 		/* ...
> > 		 * the rest is the same implementation as _parse_integer_limit()
> > 		 * ...
> > 		 */
> > 		return rv;
> > 	}
> > 
> > That allows to accumulate the final value into the same variable, which makes
> > things simpler and decreases the amount of overflow checks.
> > 
> > The scale can now be a bigger value, like 0.00000000000000000000000000000000423
> > can be parsed with scale = 35, resulting into 423.
> > 
> > The truncation loop is still there... I think this implementation is better,
> > and I am not sure what is the input limit that you would consider ok to allow
> > non-zero digits to be truncated once the scale can now be something bigger than 19.
> > As long as the output fits into a u64 variable, the parser still works.  
> 
> The truncation loop is at least stricting the input on digits!
> Any comments on that?
> 
> > 
> > I am also adding new test cases for that!  
> 
> I have a v13 ready with this. I'll give it a go soon...
> 


  reply	other threads:[~2026-05-15 19:21 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-10 12:42 [PATCH v12 00/11] ADF41513/ADF41510 PLL frequency synthesizers Rodrigo Alencar
2026-05-10 12:42 ` Rodrigo Alencar via B4 Relay
2026-05-10 12:42 ` [PATCH v12 01/11] dt-bindings: iio: frequency: add adf41513 Rodrigo Alencar
2026-05-10 12:42   ` Rodrigo Alencar via B4 Relay
2026-05-10 12:42 ` [PATCH v12 02/11] lib: kstrtox: add kstrtoudec64() and kstrtodec64() Rodrigo Alencar
2026-05-10 12:42   ` Rodrigo Alencar via B4 Relay
2026-05-11 21:14   ` sashiko-bot
2026-05-12 11:39   ` Jonathan Cameron
2026-05-12 11:52     ` Rodrigo Alencar
2026-05-12 13:12     ` Andy Shevchenko
2026-05-12 13:21       ` Rodrigo Alencar
2026-05-12 13:48         ` Andy Shevchenko
2026-05-12 14:12           ` Rodrigo Alencar
2026-05-12 14:43             ` Andy Shevchenko
2026-05-12 15:11               ` Rodrigo Alencar
2026-05-12 15:21                 ` Andy Shevchenko
2026-05-12 16:18                   ` David Laight
2026-05-12 17:08                     ` Andy Shevchenko
2026-05-12 16:35                   ` Rodrigo Alencar
2026-05-12 17:13                     ` Andy Shevchenko
2026-05-12 17:26                       ` Rodrigo Alencar
2026-05-12 17:46                         ` Andy Shevchenko
2026-05-12 18:15                           ` Rodrigo Alencar
2026-05-12 19:08                             ` Andy Shevchenko
2026-05-12 19:39                               ` Rodrigo Alencar
2026-05-12 20:16                                 ` Andy Shevchenko
2026-05-13  7:14                                   ` Rodrigo Alencar
2026-05-13 10:09                                     ` David Laight
2026-05-13  9:41   ` Rodrigo Alencar
2026-05-15 16:05     ` Rodrigo Alencar
2026-05-15 19:21       ` David Laight [this message]
2026-05-10 12:42 ` [PATCH v12 03/11] lib: test-kstrtox: tests for kstrtodec64() and kstrtoudec64() Rodrigo Alencar
2026-05-10 12:42   ` Rodrigo Alencar via B4 Relay
2026-05-12 13:51   ` Andy Shevchenko
2026-05-10 12:42 ` [PATCH v12 04/11] lib: math: div64: add div64_s64_rem() Rodrigo Alencar
2026-05-10 12:42   ` Rodrigo Alencar via B4 Relay
2026-05-12 13:50   ` Andy Shevchenko
2026-05-10 12:42 ` [PATCH v12 05/11] iio: core: add decimal value formatting into 64-bit value Rodrigo Alencar
2026-05-10 12:42   ` Rodrigo Alencar via B4 Relay
2026-05-11 21:59   ` sashiko-bot
2026-05-12 14:35   ` Andy Shevchenko
2026-05-12 16:09     ` Rodrigo Alencar
2026-05-12 17:49       ` Andy Shevchenko
2026-05-12 19:01         ` Rodrigo Alencar
2026-05-10 12:42 ` [PATCH v12 06/11] iio: test: iio-test-format: add test case for decimal format Rodrigo Alencar
2026-05-10 12:42   ` Rodrigo Alencar via B4 Relay
2026-05-12 14:36   ` Andy Shevchenko
2026-05-12 17:02     ` Rodrigo Alencar
2026-05-12 17:51       ` Andy Shevchenko
2026-05-10 12:42 ` [PATCH v12 07/11] iio: frequency: adf41513: driver implementation Rodrigo Alencar
2026-05-10 12:42   ` Rodrigo Alencar via B4 Relay
2026-05-11 22:43   ` sashiko-bot
2026-05-12 10:15     ` Rodrigo Alencar
2026-05-12 11:31       ` Jonathan Cameron
2026-05-10 12:42 ` [PATCH v12 08/11] iio: frequency: adf41513: handle LE synchronization feature Rodrigo Alencar
2026-05-10 12:42   ` Rodrigo Alencar via B4 Relay
2026-05-11 23:11   ` sashiko-bot
2026-05-10 12:42 ` [PATCH v12 09/11] iio: frequency: adf41513: features on frequency change Rodrigo Alencar
2026-05-10 12:42   ` Rodrigo Alencar via B4 Relay
2026-05-10 12:42 ` [PATCH v12 10/11] docs: iio: add documentation for adf41513 driver Rodrigo Alencar
2026-05-10 12:42   ` Rodrigo Alencar via B4 Relay
2026-05-11 23:44   ` sashiko-bot
2026-05-10 12:42 ` [PATCH v12 11/11] Documentation: ABI: testing: add common ABI file for iio/frequency Rodrigo Alencar
2026-05-10 12:42   ` Rodrigo Alencar via B4 Relay
2026-05-11 23:31   ` sashiko-bot
2026-05-12 11:36   ` Jonathan Cameron
2026-05-12 11:48 ` [PATCH v12 00/11] ADF41513/ADF41510 PLL frequency synthesizers Jonathan Cameron

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=20260515202142.5dc561e0@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.