Re: [PATCH v5 2/8] iio: core: add fixed point parsing with 64-bit parts

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

On Fri, Jan 23, 2026 at 03:53:07PM +0000, Rodrigo Alencar via B4 Relay wrote:

> Add iio_str_to_fixpoint64() function that leverages simple_strtoull()
> to parse numbers from a string.
> A helper function __iio_str_to_fixpoint64() replaces
> __iio_str_to_fixpoint() implementation, extending its usage for
> 64-bit fixed-point parsing.

...

> +/**
> + * __iio_str_to_fixpoint64() - Parse a fixed-point number from a string
> + * @str: The string to parse
> + * @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

Can we use struct s64_fract? (Yes, you would need to add a couple of lines into
math.h for that, but don't worry, I will Ack such a change immediately.)

> + * @scale_db: True if this should parse as dB
> + *
> + * This variant uses 64-bit integers for both integer and fractional parts.
> + *
> + * Returns:
> + * 0 on success, or a negative error code if the string could not be parsed.
> + */
> +static int __iio_str_to_fixpoint64(const char *str, u64 fract_mult,
> +				   s64 *integer, s64 *fract, bool scale_db)
> +{
> +	u64 i = 0, f = 0;
> +	char *end;
> +	int digit_count, precision = ffs(fract_mult);
> +	bool negative = false;
> +
> +	if (str[0] == '-') {
> +		negative = true;
> +		str++;
> +	} else if (str[0] == '+') {
> +		str++;
> +	}
> +
> +	i = simple_strtoull(str, &end, 10);
> +	digit_count = end - str;
> +	if (digit_count > 20)
> +		return -EINVAL;

Not really. If we are talking about decimal (only) cases we need to also count
leading 0:s.

0000000000000000000000000000000025 is still 25, no overflow.

That's why I recommend to have a helper, maybe for now locally here, like

int safe_strtoull(..., unsigned long long *res)
{
	...
}

that will do all necessary checks and returns -EINVAL, -ERANGE, et cetera.
In the below we would need check for the error codes respectively.

> +	if (precision && *end == '.') {
> +		str = end + 1;
> +		f = simple_strtoull(str, &end, 10);
> +		digit_count = end - str;
> +		if (!digit_count || digit_count > 20)
> +			return -EINVAL;
> +
> +		if (digit_count > precision) {
> +			digit_count -= precision;
> +			f = div64_u64(f, int_pow(10, digit_count));
> +		} else {
> +			digit_count = precision - digit_count;
> +			f *= int_pow(10, digit_count);
> +		}
> +	} else if (!digit_count) {
> +		return -EINVAL;
> +	}
> +
> +	if (scale_db) {

> +		/* Ignore the dB suffix */
> +		if (!strncmp(end, " dB", sizeof(" dB") - 1))
> +			end += sizeof(" dB") - 1;
> +		else if (!strncmp(end, "dB", sizeof("dB") - 1))
> +			end += sizeof("dB") - 1;

Now we have strends()

> +	}

And I'm not sure we need to go too deep with dB (I don't expect 64-bit values
for it ever), but for the sake of consistency and interoperability let's have
it be.

> +	if (*end == '\n')
> +		end++;
> +
> +	if (*end != '\0')
> +		return -EINVAL;

> +	*integer = (negative && i) ? -i : i;
> +	*fract = (negative && !i) ? -f : f;

> +	return 0;
> +}

...

> +	if (integer64 < INT_MIN || integer64 > INT_MAX ||
> +	    fract64 < INT_MIN || fract64 > INT_MAX)
> +		return -ERANGE;

This needs to account the sign, right?
It's fine to be UINT_MAX I believe. But I haven't checked the current
implementation.

-- 
With Best Regards,
Andy Shevchenko