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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aXdUvRZ9NmP5Nh95@smile.fi.intel.com>
Date: Mon, 26 Jan 2026 13:49:17 +0200
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: rodrigo.alencar@...log.com
Cc: linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org,
	devicetree@...r.kernel.org, linux-doc@...r.kernel.org,
	Jonathan Cameron <jic23@...nel.org>,
	David Lechner <dlechner@...libre.com>,
	Andy Shevchenko <andy@...nel.org>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Michael Hennerich <Michael.Hennerich@...log.com>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Jonathan Corbet <corbet@....net>
Subject: Re: [PATCH v5 2/8] iio: core: add fixed point parsing with 64-bit
 parts

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