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

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

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.

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.

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