Linux IIO development
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Rodrigo Alencar <455.rodrigo.alencar@gmail.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>,
	Jonathan Cameron <jic23@kernel.org>,
	Rodrigo Alencar via B4 Relay
	<devnull+rodrigo.alencar.analog.com@kernel.org>,
	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>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	Shuah Khan <skhan@linuxfoundation.org>,
	David Laight <david.laight.linux@gmail.com>
Subject: Re: [PATCH v12 02/11] lib: kstrtox: add kstrtoudec64() and kstrtodec64()
Date: Tue, 12 May 2026 23:16:43 +0300	[thread overview]
Message-ID: <agOKq0iH2CHQ3TIg@ashevche-desk.local> (raw)
In-Reply-To: <hvwyrb7g3ar7hzesj32zoxzqvjmdtwybamy4zxepqdbu37qvog@xnmgqhfya34f>

On Tue, May 12, 2026 at 08:39:21PM +0100, Rodrigo Alencar wrote:
> On 26/05/12 10:08PM, Andy Shevchenko wrote:
> > On Tue, May 12, 2026 at 07:15:17PM +0100, Rodrigo Alencar wrote:
> > > On 26/05/12 08:46PM, Andy Shevchenko wrote:
> > > > On Tue, May 12, 2026 at 06:26:12PM +0100, Rodrigo Alencar wrote:
> > > > > On 26/05/12 08:13PM, Andy Shevchenko wrote:
> > > > > > On Tue, May 12, 2026 at 05:35:59PM +0100, Rodrigo Alencar wrote:
> > > > > > > On 26/05/12 06:21PM, Andy Shevchenko wrote:
> > > > > > > > On Tue, May 12, 2026 at 6:11 PM Rodrigo Alencar
> > > > > > > > <455.rodrigo.alencar@gmail.com> wrote:
> > > > > > > > > On 26/05/12 05:43PM, Andy Shevchenko wrote:
> > > > > > > > > > On Tue, May 12, 2026 at 03:12:24PM +0100, Rodrigo Alencar wrote:
> > > > > > > > > > > On 26/05/12 04:48PM, Andy Shevchenko wrote:
> > > > > > > > > > > > On Tue, May 12, 2026 at 02:21:14PM +0100, Rodrigo Alencar wrote:
> > > > > > > > > > > > > On 26/05/12 04:12PM, Andy Shevchenko wrote:
> > > > > > > > > > > > > > On Tue, May 12, 2026 at 12:39:53PM +0100, Jonathan Cameron wrote:
> > > > > > > > > > > > > > > On Sun, 10 May 2026 13:42:20 +0100
> > > > > > > > > > > > > > > Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> 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 think we are going in circles here and we could look at the code instead:
> > > > > > > > > - integer parsing with _parse_integer()
> > > > > > > > >         - overflow check and validation of the return value
> > > > > > > > > - fractional parsing with _parse_integer_limit()
> > > > > > > > >         - overflow check and validation of the return value
> > > > > > > > 
> > > > > > > > No, this is not fully true. That's what my whole point is about. The
> > > > > > > > max_chars parameter limits the input check, then it skips an arbitrary
> > > > > > > > number of digits and only *then* it checks for \n and \0. What will be
> > > > > > > > the result of the
> > > > > > > > 0.00000000000000000000000000000000423 in your case? Whatever scale you
> > > > > > > > gave it will return 0 without checking on how many digits were
> > > > > > > > supplied.
> > > > > > > 
> > > > > > > I suppose that is a valid input and 0 is the expected result there.
> > > > > > > 
> > > > > > > > All the same for 0.9999999999999999999999999999999000423. My
> > > > > > > > point is that we should limit this by 19 digits.
> > > > > > > 
> > > > > > > why we need to limit by 19? Digits beyond the scale carry no value...
> > > > > > 
> > > > > > ...only if they are all 0:s.
> > > > > 
> > > > > I thought your concern was on input length.
> > > > 
> > > > One of, since I think you rose the topic of leading 0:s for integers and
> > > > I agreed with that which makes sense to have mirrored in fractional part.
> > > > 
> > > > > > > just like leading zeros to the integer part (which is also accepted by
> > > > > > > kstrtoull() when parsing with base 10). Not sure why this is invalid input.
> > > > > > 
> > > > > > See above. I agree on truncating trailing 0:s as it's done for leading ones
> > > > > > in integer part, but if any of the digit behind 19th is not 0, it's an overflow
> > > > > > condition (or bad input, depending how strict the rules are).
> > > > > 
> > > > > stating in the documentation that digits beyond the scale are ignored is not
> > > > > enough?
> > > > 
> > > > It's in case we are not for kstrto*() family. My understanding that kstrto*()
> > > > use strict rules on the input in overflow check.
> > > > 
> > > > > > > > On top of that, what about -0.9(19 times) ? the fraction should be u64
> > > > > > > > in this case and it's fine. The sign applies to the combined value.
> > > > > > > 
> > > > > > > yes, range for signed values are verified later.
> > > > > > 
> > > > > > > > >         - extra scaling and truncation happening outside if needed.
> > > > > > > > 
> > > > > > > > Right, but the given input may be way too long and still needs more validation.
> > > > > > > 
> > > > > > > What is the problem with a long input of digits?
> > > > > > > C compiler does not complain about this when parsing a float value,
> > > > > > > python does not
> > > > > > > complain about this when parsing floats or decimals either.
> > > > > > 
> > > > > > Because there is an exponent limit and for double it's something like 1e307
> > > > > > IIRC, meaning, try 1024 digits to be sure.
> > > > > > 
> > > > > > Python most likely uses the library for big numbers, you can't compare it at all with this.
> > > > > 
> > > > > You would be fine if the truncation loop:
> > > > > 
> > > > > while (isdigit(*s)) /* truncate */
> > > > > 	s++;
> > > > > 
> > > > > is bounded by (19-scale) iteration count? or it should keep iterating if those are zero?
> > > > 
> > > > Ideally both.
> > > > 
> > > > We don't care about the digits in the range of 19-scale and skip all 0:s after
> > > > that.
> > > > 
> > > > 	/* truncate unrequired digits within type limit, i.e. 19 decimal digits */
> > > > 	while (isdigit(*s) && "(s - pos_of_dot) is less than 19")
> > > > 		s++;
> > > > 	while (s == '0') /* truncate trailing 0:s, it's not a bad input nor overflow */
> > > > 		s++;
> > > 
> > > We could have agreed on something like that since the beginning!
> > 
> > Yes, but who knew that we go to have this agreement?
> > 
> > > And I think that changing the logic to something like this would not change a
> > > thing on the kind of inputs we expect, it will just complicate the code.
> > > I suppose that kind of kstrto*() rules were never stated anywhere.
> > > 
> > >                            |> 20th digit 
> > > Also, 0.00000000000000000001 still sounds like a valid decimal number to me, even
> > > though it is going to be parsed as 0!
> > 
> > Hmm... It would mean that testing for 19th/20th digits is not enough... :-(
> > 
> > > > 
> > > > 	// Now if it's not \0 nor \n and
> > > > 	//	a) still a digit consider either overflow or bad input,
> > > > 	//	b) if not a digit, consider as bad input.
> > > > 
> > > > In a) I tend to be on par with the other k*() and consider that as overflow.
> > > > 
> > > > > is that the only concern? Again, the usage of _parse_integer_limit(s, 10, &_frac, scale)
> > > > > avoids a 64-bit division when checking the rv.
> > > > 
> > > > I'm not against usage of _parse_integer_limit(), I'm for stricter rules on the input.
> > > > With the above addressed, I have no more concerns.
> > > 
> > > Thanks! I will proceed with the requested adjustments.
> > 
> > But it seems it's not enough as you pointed out!
> > 
> > So the biggest fraction we may consume in 64-bit (unsigned) value is
> > 0.18446744073709551615. If we go with one digit less, the whole value
> > can be
> > 
> > In [3]: hex(9999999999999999999)
> > Out[3]: '0x8ac7230489e7ffff'
> > 
> > So, I don't know how we are supposed to represent values between
> > -0.9223372036854775808
> > -0.9999999999999999999
> > in a signed type as they have bit 63 set.
> > 
> > The easiest way out is to limit scale to 18 (but still accept 19th digit, and
> > with check for overflow even 20th up to 0.18446744073709551615). This will need
> > to run _parse_integer_limit() twice (with given scale and with 20).
> > 
> > Can you add the respective test cases and see what is currently going on with
> > them?
> 
> I can add test cases, but for the signed case the situation is:
> 
> scale = 0
> 	max = 9223372036854775807, min = -9223372036854775808
> scale = 1
> 	max = 922337203685477580.7, min = -922337203685477580.8
> scale = 2
> 	max = 92233720368547758.07, min = -92233720368547758.08
> ...
> scale = 18
> 	max = 9.223372036854775807, min = -9.223372036854775808
> scake = 19
> 	max = 0.9223372036854775807, min = -0.9223372036854775808
> 
> anything outside those ranges will give you -ERANGE. Then it depends on the scale used.

Oh, I only now realised that this is sliding window for a single 64-bit signed value!
I was under impression that you wanted implementation that covers 128-bit signed value
(with 64 + 64)...

> I am not representing -0.9999999999999999999 as is. The desired scale will have this
> truncated. It may be -0.9999 or -0.999999 or -0.9. And this is practical for a
> reasonable scale value... for pico and femto precision you still get a decent range.

-- 
With Best Regards,
Andy Shevchenko



  reply	other threads:[~2026-05-12 20:16 UTC|newest]

Thread overview: 47+ 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 via B4 Relay
2026-05-10 12:42 ` [PATCH v12 01/11] dt-bindings: iio: frequency: add adf41513 Rodrigo Alencar via B4 Relay
2026-05-10 12:42 ` [PATCH v12 02/11] lib: kstrtox: add kstrtoudec64() and kstrtodec64() Rodrigo Alencar via B4 Relay
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 [this message]
2026-05-13  7:14                                   ` Rodrigo Alencar
2026-05-13 10:09                                     ` David Laight
2026-05-13  9:41   ` Rodrigo Alencar
2026-05-10 12:42 ` [PATCH v12 03/11] lib: test-kstrtox: tests for kstrtodec64() and kstrtoudec64() 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 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 via B4 Relay
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 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 via B4 Relay
     [not found]   ` <20260511224355.6ED49C2BCB0@smtp.kernel.org>
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 via B4 Relay
2026-05-10 12:42 ` [PATCH v12 09/11] iio: frequency: adf41513: features on frequency change Rodrigo Alencar via B4 Relay
2026-05-10 12:42 ` [PATCH v12 10/11] docs: iio: add documentation for adf41513 driver Rodrigo Alencar via B4 Relay
2026-05-10 12:42 ` [PATCH v12 11/11] Documentation: ABI: testing: add common ABI file for iio/frequency Rodrigo Alencar via B4 Relay
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=agOKq0iH2CHQ3TIg@ashevche-desk.local \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=455.rodrigo.alencar@gmail.com \
    --cc=Michael.Hennerich@analog.com \
    --cc=akpm@linux-foundation.org \
    --cc=andy.shevchenko@gmail.com \
    --cc=andy@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=corbet@lwn.net \
    --cc=david.laight.linux@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=devnull+rodrigo.alencar.analog.com@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox