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

[Rodrigo Alencar]

On 26/05/12 11:16PM, Andy Shevchenko wrote:
> 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@xxxxxxxxx> 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@xxxxxxxxxx> 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)...

So that was the initial approach with strntoull() with integer and fractional parts
combined in iio core. At that time I realized that we ended up combining them anyways
with:

	val64 = (u64)val * MICRO + val2

so why not have val64 already! And all this made me realise that once leading 0s are ok,
scale can be even bigger, e.g.

scale = 20
	max = 0.09223372036854775807, min = -0.09223372036854775808
scale = 21
	max = 0.009223372036854775807, min = -0.009223372036854775808

It might be a sliding window of 19 digits, but here we trade range for scale, precision
is still fixed at 64-bit. I have a new idea to make thing simpler, actually
it would go back to what David pointed out in the past. Let me put this together...

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

-- 
Kind regards,

Rodrigo Alencar