Re: [PATCH v9 2/9] lib: vsprintf: export simple_strntoull() in a safe prototype

[Petr Mladek]

On Mon 2026-03-30 13:49:48, Rodrigo Alencar wrote:
> On 26/03/27 03:17PM, Rodrigo Alencar wrote:
> > On 26/03/27 12:21PM, Andy Shevchenko wrote:
> > > On Fri, Mar 27, 2026 at 10:11:56AM +0000, Rodrigo Alencar wrote:
> > > > On 26/03/27 11:17AM, Andy Shevchenko wrote:
> > > > > On Fri, Mar 27, 2026 at 09:45:17AM +0100, Petr Mladek wrote:
> > > > > > On Fri 2026-03-20 16:27:27, Rodrigo Alencar via B4 Relay wrote:
> 
> ...
> 
> > > > > Maybe we want to have kstrtof32() and kstrtof64() for these two cases?
> > > > > 
> > > > > With that we will always consider the fraction part as 32- or 64-bit,
> > > > > imply floor() on the fraction for the sake of simplicity and require
> > > > > it to be NUL-terminated with possible trailing '\n'.
> > > > 
> > > > I think this is a good idea, but calling it float or fixed point itself
> > > > is a bit confusing as float often refers to the IEEE 754 standard and
> > > > fixed point types is often expressed in Q-format.
> > > 
> > > Yeah... I am lack of better naming.
> > 
> > decimals is the name, but they are often represented as:
> > 
> > 	DECIMAL = INT * 10^X + FRAC
> > 
> > in a single 64-bit number, which would be fine for my end use case.
> > However IIO decimal fixed point parsing is out there for quite some time a
> > lot of drivers use that. The interface often relies on breaking parsed values
> > into an integer array (for standard attributes int val and int val2 are expected).
> 
> Thinking about this again and in IIO drivers we end up doing something like:
> 
> val64 = (u64)val * MICRO + val2;
> 
> so that drivers often work with scaled versions of the decimal value.
> then, would it make sense to have a function that already outputs such value?
> That would allow to have more freedom over the 64-bit split between integer
> and fractional parts.
> As a draft:

My understanding is that you want to allow parsing frequencies
in the range from microHz to GHz.

So, you might want to support input in simple float numbers
with some precision, for example, 1.2GHz, 0.345Hz, ...

By simple, I mean that there is no x10^3 or so.

> static int _kstrtodec64(const char *s, unsigned int scale, u64 *res)

I would personally change this to something like:

  static int _unit_float_ktstrtodec64(const char *s, unsigned int precision, u64 *res, char **unit)

It would allow to read float number in the the format XXXX.YYYYunit,
for example 1.2Ghz

, where:

  + _unit_ means that it might set @unit pointer which point to the unit
    string right after the number part.

  + _float_ means that it will be able to read float numbers

  + @precisions parameter defines the number of digits accepted
    after the radix point. It is also used as multiplier for scaling
    the output number.

  + @res is pointer to the read number multiplied by the given
    @precision

  + @unit will be set to string after the number

For example:

  + s="1.2GHz", precision=3 will result in *res=1200, *unit="GHz"
  + s="0.0100004", precision=3 will result in *res=10, *unit=""
  + s=1.234567GHz, precision=3 will result in *res=1235, *unit="GHz"

Note that the result is rounded in the last example.

The function might be used like simple_strtoull() in memparse(),
see lib/cmdline.c. Which is able to read the given size in B
and handle various units like kB, GB, ...

> {
> 	u64 _res = 0, _frac = 0;
> 	unsigned int rv;
> 
> 	if (*s != '.') {
> 		rv = _parse_integer(s, 10, &_res);
> 		if (rv & KSTRTOX_OVERFLOW)
> 			return -ERANGE;
> 		if (rv == 0)
> 			return -EINVAL;
> 		s += rv;
> 	}
> 
> 	if (*s == '.') {
> 		s++;
> 		rv = _parse_integer_limit(s, 10, &_frac, scale);
> 		if (rv & KSTRTOX_OVERFLOW)
> 			return -ERANGE;
> 		if (rv == 0)
> 			return -EINVAL;
> 		s += rv;
> 		if (rv < scale)
> 			_frac *= int_pow(10, scale - rv);
> 		while (isdigit(*s)) /* truncate */
> 			s++;

We might/should use the first digit to round the _frac.

> 	}
> 
> 	if (*s == '\n')
> 		s++;
> 	if (*s)
> 		return -EINVAL;

I would omit this. Instead I would set @unit pointer so that the
caller might handle units defined after the number.

> 	if (check_mul_overflow(_res, int_pow(10, scale), &_res) ||
> 	    check_add_overflow(_res, _frac, &_res))
> 		return -ERANGE;
> 
> 	*res = _res;
> 	return 0;
> }

Otherwise, this approach looks sensible to me. IMHO, some generic
API for reading numbers with misc units should be usable in more
situations. And it would make the kernel interface more user
friendly.

Of course, we must not over-engineer it. But the above does not
look much more complex than we already have.

Best Regards,
Petr

PS: I am sorry if I used some mathematical terms a wrong way.
    I am not a native speaker. And it is a long time since
    I worked with float numbers.