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

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

On Tue, Oct 20, 2026 at 09:27:02AM -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.

...

> +	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);
> +	} else {
> +		/* keep hex/octal support for integers */
> +		ret = kstrtoll(str, 0, &dec64);
> +		if (ret == -EINVAL)

The ret can be something else as well. Why only -EINVAL? Perhaps add a comment
on top of if ()?

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

Do you need casting?

...

> +/**
> + * __iio_str_to_fixpoint_units() - Parse a fixed-point number with units from a string
> + * @str: The string to parse
> + * @units: The units to expect at the end of the string
> + * @fract_mult: Multiplier for the first decimal place, should be a power of 10
> + * @integer: The integer part of the number
> + * @fract: The fractional part of the number
> + *

Add a description with the examples, what can be the input and what is expected
output.

> + * Returns:
> + * 0 on success, or a negative error code if the string could not be parsed.
> + */
> +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 s_len = strlen(str);
> +	const char *dec_str = str;
> +	char buf[64];
> +
> +	if (s_len && str[s_len - 1] == '\n')
> +		s_len--;
> +
> +	if (s_len > units_len &&
> +	    !strncmp(str + s_len - units_len, units, units_len)) {
> +		s_len -= units_len;
> +		if (str[s_len - 1] == ' ')
> +			s_len--;
> +		if (s_len >= sizeof(buf))
> +			return -EINVAL;
> +		memcpy(buf, str, s_len);
> +		buf[s_len] = '\0';
> +		dec_str = buf;
> +	}
> +
> +	return iio_str_to_fixpoint(dec_str, fract_mult, integer, fract);
> +}

...

> +		if (dec_scale) {
>  			ret = kstrtodec64(buf, dec_scale, &integer64);
> +		} else {
> +			/* keep hex/octal support for integers */
>  			ret = kstrtoll(buf, 0, &integer64);
> +			if (ret == -EINVAL)
> +				ret = kstrtodec64(buf, dec_scale, &integer64);
> +		}
>  		if (ret)
>  			return ret;

This is a dup with the above, can we have a common helper?

-- 
With Best Regards,
Andy Shevchenko