Re: [PATCH RFC 3/3] iio: core: use kstrtodec64() to parse fixed-point values

[Andy Shevchenko]

On Thu, Oct 01, 2026 at 12:34:58PM -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 scale passed to kstrtodec64() comes from fract_mult, which
> is a power of ten, 10^n, so ffs(10^n) = n + 1. 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 truncated
> to 1, matching how kstrtodec64() drops digits beyond the requested
> scale.

...

> +	/* fract_mult = 10^n, so ffs(10^n) = ffs(2^n * 5^n) = n + 1 */
> +	unsigned int scale = ffs(fract_mult);
> +	s64 dec64;
> +	int ret;
> +
> +	ret = -EINVAL;
> +	if (!fract_mult) /* keep hex/octal support for integers */
> +		ret = kstrtoll(str, 0, &dec64);
> +	if (ret == -EINVAL)
> +		ret = kstrtodec64(str, scale, &dec64);
> +	if (ret)
> +		return ret;

Wouldn't be better to write as

	if (fract_mult) {
		ret = kstrtodec64(str, scale, &dec64);
	} else {
		/* keep hex/octal support for integers */
		ret = kstrtoll(str, 0, &dec64);
		if (ret == -EINVAL)
			ret = kstrtodec64(str, scale, &dec64);
	}
	if (ret)
		return ret;

> +	if (fract_mult > 0)
> +		dec64 = div_s64_rem(dec64, fract_mult * 10, fract);
> +	else
> +		*fract = 0;
> +
> +	if (dec64 > INT_MAX || dec64 < INT_MIN)
> +		return -ERANGE;
> +
> +	*integer = (int)dec64;
> +	/* only carry the sign in the fractional part if the integer is zero */
> +	if (*integer)
> +		*fract = abs(*fract);
> +

...

> +static int iio_str_to_fixpoint_units(const char *str, const char *units,
> +				     int fract_mult, int *integer, int *fract)
> +{
> +	size_t units_len = strlen(units);
> +	size_t num_len = strlen(str);
> +	const char *dec_str = str;
> +	char buf[64];
> +
> +	if (num_len && str[num_len - 1] == '\n')
> +		num_len--;
> +
> +	if (num_len > units_len &&
> +	    !strncmp(str + num_len - units_len, units, units_len)) {
> +		num_len -= units_len;
> +		if (str[num_len - 1] == ' ')
> +			num_len--;
> +		if (num_len >= sizeof(buf))
> +			return -EINVAL;
> +		memcpy(buf, str, num_len);
> +		buf[num_len] = '\0';
> +		dec_str = buf;
> +	}
> +
> +	return iio_str_to_fixpoint(dec_str, fract_mult, integer, fract);
> +}

This won't support cases when we have too many leading 0:s.
All these functions should also strip leading and unneeded 0:s.

-- 
With Best Regards,
Andy Shevchenko