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

[Rodrigo Alencar]

On 26/03/30 01:49PM, 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:
> 
> static int _kstrtodec64(const char *s, unsigned int scale, u64 *res)
> {
> 	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++;
> 	}
> 
> 	if (*s == '\n')
> 		s++;
> 	if (*s)
> 		return -EINVAL;
> 
> 	if (check_mul_overflow(_res, int_pow(10, scale), &_res) ||
> 	    check_add_overflow(_res, _frac, &_res))
> 		return -ERANGE;
> 
> 	*res = _res;
> 	return 0;
> }
> 
> noinline
> int kstrtoudec64(const char *s, unsigned int scale, u64 *res)
> {
> 	if (s[0] == '+')
> 		s++;
> 	return _kstrtodec64(s, scale, res);
> }
> EXPORT_SYMBOL(kstrtoudec64);
> 
> noinline
> int kstrtosdec64(const char *s, unsigned int scale, s64 *res)
> {
> 	u64 tmp;
> 	int rv;
> 
> 	if (s[0] == '-') {
> 		rv = _kstrtodec64(s + 1, scale, &tmp);
> 		if (rv < 0)
> 			return rv;
> 		if ((s64)-tmp > 0)
> 			return -ERANGE;
> 		*res = -tmp;
> 	} else {
> 		rv = kstrtoudec64(s, scale, &tmp);
> 		if (rv < 0)
> 			return rv;
> 		if ((s64)tmp < 0)
> 			return -ERANGE;
> 		*res = tmp;
> 	}
> 	return 0;
> }
> EXPORT_SYMBOL(kstrtosdec64);
> 
> e.g., kstrtosdec64() or kstrtoudec64() parses "3.1415" with scale 3 into 3141

Hi Jonathan,

developing more on that, I wouldn't need to create a iio_str_to_fixpoint64(),
what do you think on new format types:

#define IIO_VAL_DECIMAL64_1 101
#define IIO_VAL_DECIMAL64_2 102
#define IIO_VAL_DECIMAL64_3 103
#define IIO_VAL_DECIMAL64_4 104
#define IIO_VAL_DECIMAL64_5 105
#define IIO_VAL_DECIMAL64_6 106
#define IIO_VAL_DECIMAL64_7 107
#define IIO_VAL_DECIMAL64_8 108
#define IIO_VAL_DECIMAL64_9 109
#define IIO_VAL_DECIMAL64_10 110
#define IIO_VAL_DECIMAL64_11 111
#define IIO_VAL_DECIMAL64_12 112
#define IIO_VAL_DECIMAL64_13 113
#define IIO_VAL_DECIMAL64_14 114
#define IIO_VAL_DECIMAL64_15 115

#define IIO_VAL_DECIMAL64_MILLI IIO_VAL_DECIMAL64_3
#define IIO_VAL_DECIMAL64_MICRO IIO_VAL_DECIMAL64_6
#define IIO_VAL_DECIMAL64_NANO IIO_VAL_DECIMAL64_9
#define IIO_VAL_DECIMAL64_PICO IIO_VAL_DECIMAL64_12
#define IIO_VAL_DECIMAL64_FEMTO IIO_VAL_DECIMAL64_15

which gets stored as 64-bit, and represent the decimal scaled value.
That would also work for the PLL driver (using IIO_VAL_DECIMAL64_MICRO):
  - It supports frequency range from 1 to 26 GHz with micro Hz resolution
  - In the driver a 64-bit value: (val * MICRO + val2) is already created
  anyways.
I would leverage something like kstrtodec64() in iio_write_channel_info().

That way, I would drop the changes on the iio fixpoint parse, which I think
it would do better with something like kstrntoull() to be able to handle that
"dB" suffix.

So for now, I may have the following approaches:
- new kstrntoull() function: to have control over the parsing, whithout
  requiring NUL-termination, avoiding unecessary string scanning or copying.
  covered in v8.
- expose a "safe" simple_strntoull(): minimal changes to vsprintf.c, this
  is covered by this patch series (v9), and it similar solution to kstrntoull().
- new kstrtodec64() function: parse decimal numbers as 64-bit with NUL-termination.
  Might be covered in a v10, if it is a good idea.

let me know your thoughts.

-- 
Kind regards,

Rodrigo Alencar