Re: [PATCH v4 5/6] iio: core: use kstrtodec64() to parse fixed-point values

[Andy Shevchenko]

On Mon, Oct 26, 2026 at 08:54:09AM -0300, Rodrigo Alencar via B4 Relay wrote:

> Replace the open-coded __iio_str_to_fixpoint() parser with kstrtodec64(),
> so sysfs writes of fixed-point values also accept E notation.
> 
> The fractional part only keeps the sign when the integer part is zero,
> as before. The dB suffix for scale attributes is now handled by a separate
> __iio_str_to_fixpoint_units() helper. It strips an optional trailing
> newline and the "dB" or " dB" suffix before parsing.
> 
> When there is no fractional part (fract_mult == 0, or dec_scale == 0 for
> 64-bit values), try base-autodetecting kstrtoll() first so hexadecimal
> and octal input keeps its meaning ("010" is still 8). Fall back to
> kstrtodec64() on -EINVAL to accept E notation.

> As a side effect, inputs that used to be rejected for integer-only
> attributes are now accepted. An invalid octal number such as "08" is
> parsed as decimal 8, and a fractional value such as "1.5" is rounded
> to 2.

The main question here is: Do we want these side effects?
And follow up question is: Where is it going to be documented?
I consider this paragraph is very important for the users.

...

> +	/*
> +	 * Keep hex/octal support for integers. E notation and decimal values
> +	 * result in -EINVAL from kstrtoll, due to unexpected termination on '.'

kstrtoll()

> +	 * or 'e'. It falls back to kstrtodec64 for decimal fixed-point numbers
> +	 * with zero scale.
> +	 */
> +	ret = kstrtoll(str, 0, val);
> +	if (ret == -EINVAL)

> +		ret = kstrtodec64(str, 0, val);

		return kstrtodec64(str, 0, val);

?

> +	return ret;

...

>  int iio_str_to_fixpoint(const char *str, int fract_mult,
>  			int *integer, int *fract)
>  {
> -	return __iio_str_to_fixpoint(str, fract_mult, integer, fract, false);
> +	s64 dec64;
> +	int ret;
> +
> +	if (fract_mult) {
> +		/* scale = ffs(fract_mult) = ffs(10^n) = ffs(2^n * 5^n) = n + 1 */
> +		ret = kstrtodec64(str, ffs(fract_mult), &dec64);
> +		if (ret)
> +			return ret;
> +
> +		dec64 = div_s64_rem(dec64, fract_mult * 10, fract);
> +		/* only carry the sign in fract if the integer is zero */
> +		if (dec64)
> +			*fract = abs(*fract);

Should we actually switch to UB-free abs() implementation?

> +	} else {
> +		ret = iio_str_to_s64(str, &dec64);
> +		if (ret)
> +			return ret;
> +
> +		*fract = 0;
> +	}
> +
> +	if (dec64 > INT_MAX || dec64 < INT_MIN)
> +		return -ERANGE;
> +
> +	*integer = dec64;
> +
> +	return 0;
>  }

...

> +		if (scale_db)
> +			ret = iio_str_to_fixpoint_units(buf, "dB", fract_mult,
> +							&integer, &fract);

Can we actually make units to be the last argument? In this case we might join
two calls together, it also might be beneficial from code generation point of
view (as the units version calls the basic one at the end).

> +		else
> +			ret = iio_str_to_fixpoint(buf, fract_mult, &integer,
> +						  &fract);

-- 
With Best Regards,
Andy Shevchenko