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

[Andy Shevchenko]

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.
> > > > > > > > > > >
> > > > > > > > > > > Whilst Rodrigo has already replied to say there will be another version
> > > > > > > > > > > I'd like to request final feedback from those who were involved in the parser
> > > > > > > > > > > discussions.
> > > > > > > > > > >
> > > > > > > > > > > They got very involved and I'm far from an expert in the right way to do
> > > > > > > > > > > this stuff.
> > > > > > > > > > >
> > > > > > > > > > > I don't think David Laight was +CC so I've added that.
> > > > > > > > > > > David, Andy - I think you two were most involved in that discussion:
> > > > > > > > > > > Any objections to the end result?
> > > > > > > > > >
> > > > > > > > > > I already said a few times about the naming. I do not like the kstrto*()
> > > > > > > > > > be semantically different on how they treat the input. Second point is
> > > > > > > > > > to avoid code duplication, but this one is less of a concern since the
> > > > > > > > > > new code is in the library close to the other potentially duplicate code
> > > > > > > > > > piece and hence can be addressed later.
> > > > > > > > >
> > > > > > > > > I suppose I reached into kstrtodec64() and kstrtoudec64() because it aligns
> > > > > > > > > with your expectations for kstrto*() semantics, no? Those include:
> > > > > > > > >  - overflow check;
> > > > > > > > >  - extensive input validation;
> > > > > > > > >  - optional '\n' in the end;
> > > > > > > > >  - mandatory nul-termination.
> > > > > > > > >
> > > > > > > > > am I missing anything?
> > > > > > > >
> > > > > > > > When we add scale we basically make that not true. Moreover the code in this
> > > > > > > > patch makes scale == number_of_characters which I think a bit fragile, however
> > > > > > > > it's about the fractional part when the amount of digits is equal to scale.
> > > > > > >
> > > > > > > That is not really the case. It is being set as a limit, so it does check for
> > > > > > > truncation and zero-padding.
> > > > > >
> > > > > > I do not see it happens in _parse_integer_limit(). It doesn't try to parse more
> > > > > > characters than it's requested in max_chars. It doesn't check if there are more
> > > > > > character nor their converted values.
> > > > > >
> > > > > > > > To make this work as expected we need to add an additional call like
> > > > > > > > kstrtoull() (and perhaps drop that \n and NUL-terminator checks) and see
> > > > > > > > if that overflows or not. Since it's a fractional part it must have less
> > > > > > > > than 20 (decimal) digits there, so we check the rv (or how many digits
> > > > > > > > were parsed successfully) and compare to 20. If it's more, we got too many
> > > > > > > > decimal digits.
> > > > > > >
> > > > > > > For overflow it checks the KSTRTOX_OVERFLOW flag and leverages check_mul_overflow()
> > > > > > > and check_add_overflow() when combining fractional and integer parts. The amount
> > > > > > > of characters is not really important there. The scale cannot be bigger than 19 and
> > > > > > > that makes sure that int_pow() does not overflow. The code uses _parse_integer_limit()
> > > > > > > due to the nature of input and to avoid 64-bit division, kstrtoull() at any point
> > > > > > > (parsing integer or fractional parts) does not make much sense.
> > > > > >
> > > > > > Under 'like kstrotoull()' I meant something that repeats needed functionality.
> > > > > > I believe it's parse_integer() (without limit).
> > > > >
> > > > > 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++;

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

> > > > > - check for input termination
> > > > > - combination of integer and fractional parts with check_mul_overflow() and check_add_overflow()
> > > > >
> > > > > > > > Maybe I'm missing these checks already performed?
> > > > > > > >
> > > > > > > > > > Having the test cases is a big benefit, and that part I like the most.

-- 
With Best Regards,
Andy Shevchenko