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

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

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

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

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

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