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

[Rodrigo Alencar]

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.

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.

-- 
Kind regards,

Rodrigo Alencar