From: Rodrigo Alencar <rodrigo.alencar@analog.com>
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.
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
drivers/iio/industrialio-core.c | 151 +++++++++++++++++++++++++++-------------
include/linux/iio/iio.h | 2 +
2 files changed, 103 insertions(+), 50 deletions(-)
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index f69deefcfb6f..4ad4f1b29421 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -881,6 +881,77 @@ static ssize_t iio_read_channel_info_avail(struct device *dev,
}
}
+/**
+ * __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
+ * @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;
+
+ 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;
+ }
+
+ if (*end == '\n')
+ end++;
+
+ if (*end != '\0')
+ return -EINVAL;
+
+ *integer = (negative && i) ? -i : i;
+ *fract = (negative && !i) ? -f : f;
+
+ return 0;
+}
+
/**
* __iio_str_to_fixpoint() - Parse a fixed-point number from a string
* @str: The string to parse
@@ -895,63 +966,43 @@ static ssize_t iio_read_channel_info_avail(struct device *dev,
static int __iio_str_to_fixpoint(const char *str, int fract_mult,
int *integer, int *fract, bool scale_db)
{
- int i = 0, f = 0;
- bool integer_part = true, negative = false;
+ s64 integer64, fract64;
+ int ret;
- if (fract_mult == 0) {
- *fract = 0;
+ ret = __iio_str_to_fixpoint64(str, fract_mult, &integer64, &fract64,
+ scale_db);
+ if (ret)
+ return ret;
- return kstrtoint(str, 0, integer);
- }
+ if (integer64 < INT_MIN || integer64 > INT_MAX ||
+ fract64 < INT_MIN || fract64 > INT_MAX)
+ return -ERANGE;
- if (str[0] == '-') {
- negative = true;
- str++;
- } else if (str[0] == '+') {
- str++;
- }
-
- while (*str) {
- if ('0' <= *str && *str <= '9') {
- if (integer_part) {
- i = i * 10 + *str - '0';
- } else {
- f += fract_mult * (*str - '0');
- fract_mult /= 10;
- }
- } else if (*str == '\n') {
- if (*(str + 1) == '\0')
- break;
- return -EINVAL;
- } else if (!strncmp(str, " dB", sizeof(" dB") - 1) && scale_db) {
- /* Ignore the dB suffix */
- str += sizeof(" dB") - 1;
- continue;
- } else if (!strncmp(str, "dB", sizeof("dB") - 1) && scale_db) {
- /* Ignore the dB suffix */
- str += sizeof("dB") - 1;
- continue;
- } else if (*str == '.' && integer_part) {
- integer_part = false;
- } else {
- return -EINVAL;
- }
- str++;
- }
-
- if (negative) {
- if (i)
- i = -i;
- else
- f = -f;
- }
-
- *integer = i;
- *fract = f;
+ *integer = integer64;
+ *fract = fract64;
return 0;
}
+/**
+ * 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
+ *
+ * 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.
+ */
+int iio_str_to_fixpoint64(const char *str, u64 fract_mult, s64 *integer,
+ s64 *fract)
+{
+ return __iio_str_to_fixpoint64(str, fract_mult, integer, fract, false);
+}
+EXPORT_SYMBOL_GPL(iio_str_to_fixpoint64);
+
/**
* iio_str_to_fixpoint() - Parse a fixed-point number from a string
* @str: The string to parse
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 872ebdf0dd77..432cd26fc9d0 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -940,6 +940,8 @@ int iio_active_scan_mask_index(struct iio_dev *indio_dev);
ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals);
+int iio_str_to_fixpoint64(const char *str, u64 fract_mult, s64 *integer,
+ s64 *fract);
int iio_str_to_fixpoint(const char *str, int fract_mult, int *integer,
int *fract);
--
2.43.0On 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
On 26/01/26 01:49PM, Andy Shevchenko wrote:
> 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.)
Sorry, I missed this. s64_fract would be declared as:
struct s64_fract {
__s64 numerator;
__s64 denominator;
};
and numerator and denominator is not really applicable here. This type seems to be
used to declare fractions.
kind regards,
Rodrigo Alencar
On Mon, Jan 26, 2026 at 02:56:34PM +0000, Rodrigo Alencar wrote:
> On 26/01/26 01:49PM, Andy Shevchenko wrote:
> > On Fri, Jan 23, 2026 at 03:53:07PM +0000, Rodrigo Alencar via B4 Relay wrote:
...
> > > + * @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.)
>
> Sorry, I missed this. s64_fract would be declared as:
>
> struct s64_fract {
> __s64 numerator;
> __s64 denominator;
> };
>
> and numerator and denominator is not really applicable here. This type seems to be
> used to declare fractions.
Ah, good point. My memory (on fraction part) made a joke, I truly believed this
is the same what we need.
--
With Best Regards,
Andy Shevchenko
On 26/01/26 01:49PM, Andy Shevchenko wrote:
> 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.
...
> > +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)
> {
> ...
> }
Are you suggesting to not use simple_strtoull then?
Understood, leading zeros can be ignored only when parsing the integer
part. Also, would be nice to have truncation of the fractional part
while doing the parsing. How about:
static int iio_safe_strtoull(const char *str, const char **end,
size_t max_chars, u64 *res)
- max_chars = 0: ignores leading 0's and process all digits
- max_chars > 0: process only initial max_chars digits and ignores the rest
on overflow of u64, the function would return -EOVERFLOW
> 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()
strends() would not account for the acceptable '\n' before the end.
I don't think we would need to test for " dB", " dB\n", "dB" and "dB\n"
...
> > + 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.
Yes, UINT_MAX should be valid, will add test case and adjust. Thanks.
Kind regards,
Rodrigo Alencar
On Mon, Jan 26, 2026 at 12:42:53PM +0000, Rodrigo Alencar wrote:
> On 26/01/26 01:49PM, Andy Shevchenko wrote:
> > On Fri, Jan 23, 2026 at 03:53:07PM +0000, Rodrigo Alencar via B4 Relay wrote:
...
> > > +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)
> > {
> > ...
> > }
>
> Are you suggesting to not use simple_strtoull then?
Nope, I suggest to do an additional step before checking for the range.
> Understood, leading zeros can be ignored only when parsing the integer
> part. Also, would be nice to have truncation of the fractional part
> while doing the parsing. How about:
>
> static int iio_safe_strtoull(const char *str, const char **end,
> size_t max_chars, u64 *res)
> - max_chars = 0: ignores leading 0's and process all digits
> - max_chars > 0: process only initial max_chars digits and ignores the rest
I'm not sure why we would need that. It should parse the whole line until the
first invalid character or overflow.
> on overflow of u64, the function would return -EOVERFLOW
>
> > 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()
>
> strends() would not account for the acceptable '\n' before the end.
Good point.
> I don't think we would need to test for " dB", " dB\n", "dB" and "dB\n"
Then you can try sysfs_eq() which does that check. But I think it requires
the (end of the) string to be exact, and not something like 'dB \n'.
--
With Best Regards,
Andy Shevchenko
On 26/01/26 03:35PM, Andy Shevchenko wrote:
> On Mon, Jan 26, 2026 at 12:42:53PM +0000, Rodrigo Alencar wrote:
> > On 26/01/26 01:49PM, Andy Shevchenko wrote:
> > > On Fri, Jan 23, 2026 at 03:53:07PM +0000, Rodrigo Alencar via B4 Relay wrote:
>
> ...
>
> > > > +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)
> > > {
> > > ...
> > > }
> >
> > Are you suggesting to not use simple_strtoull then?
>
> Nope, I suggest to do an additional step before checking for the range.
You mean, conditionally skip leading 0's when parsing the integer part?
e.g.
/*function entry and arg check */
while(*str == '\0')
str++;
/* then call simple_strtoull() */
simple_strtoull() is not overflow-safe, as it does not use
check_mul_overflow() or check_add_overflow(), only checking the
amount of digits is not enough. Previous implementation of fixpoint
parsing didn't care about that.
>
> > Understood, leading zeros can be ignored only when parsing the integer
> > part. Also, would be nice to have truncation of the fractional part
> > while doing the parsing. How about:
> >
> > static int iio_safe_strtoull(const char *str, const char **end,
> > size_t max_chars, u64 *res)
>
> > - max_chars = 0: ignores leading 0's and process all digits
> > - max_chars > 0: process only initial max_chars digits and ignores the rest
>
> I'm not sure why we would need that. It should parse the whole line until the
> first invalid character or overflow.
"process all digits" and "ignores the rest" would be for digits only, so it
would stop until the first invalid character is found. I suppose proper
overflow check is implemented with check_mul_overflow() and check_add_overflow(),
while iterating over the characters and accumulating the value.
Kind regards,
Rodrigo Alencar
On Mon, Jan 26, 2026 at 02:26:20PM +0000, Rodrigo Alencar wrote:
> On 26/01/26 03:35PM, Andy Shevchenko wrote:
> > On Mon, Jan 26, 2026 at 12:42:53PM +0000, Rodrigo Alencar wrote:
> > > On 26/01/26 01:49PM, Andy Shevchenko wrote:
> > > > On Fri, Jan 23, 2026 at 03:53:07PM +0000, Rodrigo Alencar via B4 Relay wrote:
...
> > > > > +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)
> > > > {
> > > > ...
> > > > }
> > >
> > > Are you suggesting to not use simple_strtoull then?
> >
> > Nope, I suggest to do an additional step before checking for the range.
>
> You mean, conditionally skip leading 0's when parsing the integer part?
> e.g.
>
> /*function entry and arg check */
> while(*str == '\0')
> str++;
> /* then call simple_strtoull() */
Not skipping, but counting them.
> simple_strtoull() is not overflow-safe,
Yes, I know. That's why all these additional checks are required,
> as it does not use
> check_mul_overflow() or check_add_overflow(), only checking the
> amount of digits is not enough.
Why? Can you elaborate how checking amount of digits is different to
check_mul_overflow()?
> Previous implementation of fixpoint parsing didn't care about that.
Do we have test cases for the current implementation?
> > > Understood, leading zeros can be ignored only when parsing the integer
> > > part. Also, would be nice to have truncation of the fractional part
> > > while doing the parsing. How about:
> > >
> > > static int iio_safe_strtoull(const char *str, const char **end,
> > > size_t max_chars, u64 *res)
> >
> > > - max_chars = 0: ignores leading 0's and process all digits
> > > - max_chars > 0: process only initial max_chars digits and ignores the rest
> >
> > I'm not sure why we would need that. It should parse the whole line until the
> > first invalid character or overflow.
>
> "process all digits" and "ignores the rest" would be for digits only, so it
> would stop until the first invalid character is found. I suppose proper
> overflow check is implemented with check_mul_overflow() and check_add_overflow(),
I don't see the need. Amount of digits defines the order of the number (in
power-of-ten).
> while iterating over the characters and accumulating the value.
The problem that you can refer to is the corner case when the first
(most significant digit(s)) are already give an overflow while being
inside the allowed length. But it also can be checked.
...
The benefit of simple_strto*() over kstrto*() that they do not require
a temporary buffer and work over constant data (always).
If you see a way how to refactor lib/kstrtox.c and lib/vsprintf.c to have
an implementation there directly that may operate over constant buffers,
I will be glad to help with it. That would be good for existing cases,
such as Intel QAT driver, and any newcomers. I actually don't know why
the heck kstrto*() were made against non-constant buffers. Perhaps to
avoid this 'end' parameter...
--
With Best Regards,
Andy Shevchenko
On 26/01/26 04:53PM, Andy Shevchenko wrote:
> On Mon, Jan 26, 2026 at 02:26:20PM +0000, Rodrigo Alencar wrote:
> > On 26/01/26 03:35PM, Andy Shevchenko wrote:
> > > On Mon, Jan 26, 2026 at 12:42:53PM +0000, Rodrigo Alencar wrote:
> > > > On 26/01/26 01:49PM, Andy Shevchenko wrote:
> > > > > On Fri, Jan 23, 2026 at 03:53:07PM +0000, Rodrigo Alencar via B4 Relay wrote:
>
> ...
>
> > > > > > +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)
> > > > > {
> > > > > ...
> > > > > }
> > > >
> > > > Are you suggesting to not use simple_strtoull then?
> > >
> > > Nope, I suggest to do an additional step before checking for the range.
> >
> > You mean, conditionally skip leading 0's when parsing the integer part?
> > e.g.
> >
> > /*function entry and arg check */
> > while(*str == '\0')
> > str++;
> > /* then call simple_strtoull() */
>
> Not skipping, but counting them.
>
> > simple_strtoull() is not overflow-safe,
>
> Yes, I know. That's why all these additional checks are required,
>
> > as it does not use
> > check_mul_overflow() or check_add_overflow(), only checking the
> > amount of digits is not enough.
>
> Why? Can you elaborate how checking amount of digits is different to
> check_mul_overflow()?
consider U64_MAX = 18_446_744_073_709_551_615 as the limit:
- 19_000_000_000_000_000_000 contains the same amount of digits but overflows.
- 18_446_744_073_710_000_000 contains the same amount of digits but overflows.
to catch those cases, we need to check for the overflow, everytime we read a
character and accumulate:
u64 acc;
while(isdigit(*str))
if (check_mul_overflow(acc, 10, &acc) ||
check_add_overflow(acc, *str - '0', &acc))
return -EOVERFLOW;
*res = acc;
acc can get weird results if not checked.
>
> > Previous implementation of fixpoint parsing didn't care about that.
>
> Do we have test cases for the current implementation?
No, I am adding a kunit test in this patch.
> > > > Understood, leading zeros can be ignored only when parsing the integer
> > > > part. Also, would be nice to have truncation of the fractional part
> > > > while doing the parsing. How about:
> > > >
> > > > static int iio_safe_strtoull(const char *str, const char **end,
> > > > size_t max_chars, u64 *res)
> > >
> > > > - max_chars = 0: ignores leading 0's and process all digits
> > > > - max_chars > 0: process only initial max_chars digits and ignores the rest
> > >
> > > I'm not sure why we would need that. It should parse the whole line until the
> > > first invalid character or overflow.
> >
> > "process all digits" and "ignores the rest" would be for digits only, so it
> > would stop until the first invalid character is found. I suppose proper
> > overflow check is implemented with check_mul_overflow() and check_add_overflow(),
>
> I don't see the need. Amount of digits defines the order of the number (in
> power-of-ten).
>
> > while iterating over the characters and accumulating the value.
>
> The problem that you can refer to is the corner case when the first
> (most significant digit(s)) are already give an overflow while being
> inside the allowed length. But it also can be checked.
>
Yes. Not sure how to do that without checking every digit again.
>
> The benefit of simple_strto*() over kstrto*() that they do not require
> a temporary buffer and work over constant data (always).
Agreed.
> If you see a way how to refactor lib/kstrtox.c and lib/vsprintf.c to have
> an implementation there directly that may operate over constant buffers,
> I will be glad to help with it. That would be good for existing cases,
> such as Intel QAT driver, and any newcomers. I actually don't know why
> the heck kstrto*() were made against non-constant buffers. Perhaps to
> avoid this 'end' parameter...
That would be out of the scope of this patch.
Kind regards,
Rodrigo Alencar
On Mon, Jan 26, 2026 at 03:20:03PM +0000, Rodrigo Alencar wrote:
> On 26/01/26 04:53PM, Andy Shevchenko wrote:
> > On Mon, Jan 26, 2026 at 02:26:20PM +0000, Rodrigo Alencar wrote:
> > > On 26/01/26 03:35PM, Andy Shevchenko wrote:
> > > > On Mon, Jan 26, 2026 at 12:42:53PM +0000, Rodrigo Alencar wrote:
> > > > > On 26/01/26 01:49PM, Andy Shevchenko wrote:
> > > > > > On Fri, Jan 23, 2026 at 03:53:07PM +0000, Rodrigo Alencar via B4 Relay wrote:
...
> > > > > > > +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)
> > > > > > {
> > > > > > ...
> > > > > > }
> > > > >
> > > > > Are you suggesting to not use simple_strtoull then?
> > > >
> > > > Nope, I suggest to do an additional step before checking for the range.
> > >
> > > You mean, conditionally skip leading 0's when parsing the integer part?
> > > e.g.
> > >
> > > /*function entry and arg check */
> > > while(*str == '\0')
> > > str++;
> > > /* then call simple_strtoull() */
> >
> > Not skipping, but counting them.
> >
> > > simple_strtoull() is not overflow-safe,
> >
> > Yes, I know. That's why all these additional checks are required,
> >
> > > as it does not use
> > > check_mul_overflow() or check_add_overflow(), only checking the
> > > amount of digits is not enough.
> >
> > Why? Can you elaborate how checking amount of digits is different to
> > check_mul_overflow()?
>
> consider U64_MAX = 18_446_744_073_709_551_615 as the limit:
> - 19_000_000_000_000_000_000 contains the same amount of digits but overflows.
> - 18_446_744_073_710_000_000 contains the same amount of digits but overflows.
Yes, the first digits give up to 3 additional bits on top of 64-bit.
> to catch those cases, we need to check for the overflow, everytime we read a
> character and accumulate:
>
> u64 acc;
>
> while(isdigit(*str))
> if (check_mul_overflow(acc, 10, &acc) ||
> check_add_overflow(acc, *str - '0', &acc))
> return -EOVERFLOW;
>
> *res = acc;
>
> acc can get weird results if not checked.
>
> >
> > > Previous implementation of fixpoint parsing didn't care about that.
> >
> > Do we have test cases for the current implementation?
>
> No, I am adding a kunit test in this patch.
>
> > > > > Understood, leading zeros can be ignored only when parsing the integer
> > > > > part. Also, would be nice to have truncation of the fractional part
> > > > > while doing the parsing. How about:
> > > > >
> > > > > static int iio_safe_strtoull(const char *str, const char **end,
> > > > > size_t max_chars, u64 *res)
> > > >
> > > > > - max_chars = 0: ignores leading 0's and process all digits
> > > > > - max_chars > 0: process only initial max_chars digits and ignores the rest
> > > >
> > > > I'm not sure why we would need that. It should parse the whole line until the
> > > > first invalid character or overflow.
> > >
> > > "process all digits" and "ignores the rest" would be for digits only, so it
> > > would stop until the first invalid character is found. I suppose proper
> > > overflow check is implemented with check_mul_overflow() and check_add_overflow(),
> >
> > I don't see the need. Amount of digits defines the order of the number (in
> > power-of-ten).
> >
> > > while iterating over the characters and accumulating the value.
> >
> > The problem that you can refer to is the corner case when the first
> > (most significant digit(s)) are already give an overflow while being
> > inside the allowed length. But it also can be checked.
> >
>
> Yes. Not sure how to do that without checking every digit again.
>
> >
> > The benefit of simple_strto*() over kstrto*() that they do not require
> > a temporary buffer and work over constant data (always).
>
> Agreed.
>
> > If you see a way how to refactor lib/kstrtox.c and lib/vsprintf.c to have
> > an implementation there directly that may operate over constant buffers,
> > I will be glad to help with it. That would be good for existing cases,
> > such as Intel QAT driver, and any newcomers. I actually don't know why
> > the heck kstrto*() were made against non-constant buffers. Perhaps to
> > avoid this 'end' parameter...
>
> That would be out of the scope of this patch.
Hmm... But if not doing that we will end up with a code duplication in one or
another form. The whole purpose of the patch is to avoid duplication.
--
With Best Regards,
Andy Shevchenko
On 26/01/26 03:20PM, Rodrigo Alencar wrote:
> On 26/01/26 04:53PM, Andy Shevchenko wrote:
> > On Mon, Jan 26, 2026 at 02:26:20PM +0000, Rodrigo Alencar wrote:
> > > On 26/01/26 03:35PM, Andy Shevchenko wrote:
> > > > On Mon, Jan 26, 2026 at 12:42:53PM +0000, Rodrigo Alencar wrote:
> > > > > On 26/01/26 01:49PM, Andy Shevchenko wrote:
> > > > > > On Fri, Jan 23, 2026 at 03:53:07PM +0000, Rodrigo Alencar via B4 Relay wrote:
> >
> > ...
> >
> > > > > > > +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)
> > > > > > {
> > > > > > ...
> > > > > > }
> > > > >
> > > > > Are you suggesting to not use simple_strtoull then?
> > > >
> > > > Nope, I suggest to do an additional step before checking for the range.
> > >
> > > You mean, conditionally skip leading 0's when parsing the integer part?
> > > e.g.
> > >
> > > /*function entry and arg check */
> > > while(*str == '\0')
> > > str++;
> > > /* then call simple_strtoull() */
> >
> > Not skipping, but counting them.
> >
> > > simple_strtoull() is not overflow-safe,
> >
> > Yes, I know. That's why all these additional checks are required,
> >
> > > as it does not use
> > > check_mul_overflow() or check_add_overflow(), only checking the
> > > amount of digits is not enough.
> >
> > Why? Can you elaborate how checking amount of digits is different to
> > check_mul_overflow()?
>
> consider U64_MAX = 18_446_744_073_709_551_615 as the limit:
> - 19_000_000_000_000_000_000 contains the same amount of digits but overflows.
> - 18_446_744_073_710_000_000 contains the same amount of digits but overflows.
>
> to catch those cases, we need to check for the overflow, everytime we read a
> character and accumulate:
>
> u64 acc;
>
> while(isdigit(*str))
> if (check_mul_overflow(acc, 10, &acc) ||
> check_add_overflow(acc, *str - '0', &acc))
> return -EOVERFLOW;
>
> *res = acc;
>
> acc can get weird results if not checked.
Thinking about it again, that check could be done only in the last step
(20th for u64)
Kind regards,
Rodrigo Alencar
On Mon, Jan 26, 2026 at 03:30:44PM +0000, Rodrigo Alencar wrote: > On 26/01/26 03:20PM, Rodrigo Alencar wrote: > > On 26/01/26 04:53PM, Andy Shevchenko wrote: > > > On Mon, Jan 26, 2026 at 02:26:20PM +0000, Rodrigo Alencar wrote: ... > > > Why? Can you elaborate how checking amount of digits is different to > > > check_mul_overflow()? > > > > consider U64_MAX = 18_446_744_073_709_551_615 as the limit: > > - 19_000_000_000_000_000_000 contains the same amount of digits but overflows. > > - 18_446_744_073_710_000_000 contains the same amount of digits but overflows. > > > > to catch those cases, we need to check for the overflow, everytime we read a > > character and accumulate: > > > > u64 acc; > > > > while(isdigit(*str)) > > if (check_mul_overflow(acc, 10, &acc) || > > check_add_overflow(acc, *str - '0', &acc)) > > return -EOVERFLOW; > > > > *res = acc; > > > > acc can get weird results if not checked. > > Thinking about it again, that check could be done only in the last step > (20th for u64) Does kstrto*() also perform only last check? I think they do for each iteration. -- With Best Regards, Andy Shevchenko
On 26/01/26 06:07PM, Andy Shevchenko wrote:
> On Mon, Jan 26, 2026 at 03:30:44PM +0000, Rodrigo Alencar wrote:
> > On 26/01/26 03:20PM, Rodrigo Alencar wrote:
> > > On 26/01/26 04:53PM, Andy Shevchenko wrote:
> > > > On Mon, Jan 26, 2026 at 02:26:20PM +0000, Rodrigo Alencar wrote:
>
> ...
>
> > > > Why? Can you elaborate how checking amount of digits is different to
> > > > check_mul_overflow()?
> > >
> > > consider U64_MAX = 18_446_744_073_709_551_615 as the limit:
> > > - 19_000_000_000_000_000_000 contains the same amount of digits but overflows.
> > > - 18_446_744_073_710_000_000 contains the same amount of digits but overflows.
> > >
> > > to catch those cases, we need to check for the overflow, everytime we read a
> > > character and accumulate:
> > >
> > > u64 acc;
> > >
> > > while(isdigit(*str))
> > > if (check_mul_overflow(acc, 10, &acc) ||
> > > check_add_overflow(acc, *str - '0', &acc))
> > > return -EOVERFLOW;
> > >
> > > *res = acc;
> > >
> > > acc can get weird results if not checked.
> >
> > Thinking about it again, that check could be done only in the last step
> > (20th for u64)
>
> Does kstrto*() also perform only last check? I think they do for each
> iteration.
It does the following:
...
if (unlikely(res & (~0ull << 60))) {
if (res > div_u64(ULLONG_MAX - val, base))
rv |= KSTRTOX_OVERFLOW;
}
...
so overflow is checked when either one of the 4 MSbits are set.
for now, I am thinking of something like:
static ssize_t iio_safe_strtou64(const char *str, const char **endp,
size_t max_chars, u64 *result)
{
u64 digit, acc = 0;
size_t idx = 0;
while (isdigit(*str) && idx < max_chars) {
digit = *str - '0';
if (unlikely(idx > 19)) {
if (check_mul_overflow(acc, 10, &acc) ||
check_add_overflow(acc, digit, &acc))
return -EOVERFLOW;
} else {
acc = acc * 10 + digit;
}
str++;
idx++;
}
*endp = str;
*result = acc;
return idx;
}
which would help the truncation when parsing the fractional part
with max_chars, avoiding a div64_u64() to adjust precision:
...
digit_count = iio_safe_strtou64(str, &end, SIZE_MAX, &i);
if (digit_count < 0)
return digit_count;
if (precision && *end == '.') {
str = end + 1;
digit_count = iio_safe_strtou64(str, &end, precision, &f);
if (digit_count < 0)
return digit_count;
if (digit_count < precision) /* scale up */
f *= int_pow(10, precision - digit_count);
while (isdigit(*end)) /* truncate */
end++;
}
...
but I understand you would not like this approach, because it does not use
simple_strtoull() or kstrtoull(). Problem is simple_strtoull() is not
overflow-safe and kstrtoull() does not allow to track a pointer to end
of the string.
Given that the current implementation of iio_str_to_fixpoint() is not using
simple_strtoull() I am not seeing an issue with this approach.
Kind regards,
Rodrigo Alencar
Remove DT related people and ML and add others based on the links I posted
below.
On Mon, Jan 26, 2026 at 04:55:13PM +0000, Rodrigo Alencar wrote:
> On 26/01/26 06:07PM, Andy Shevchenko wrote:
> > On Mon, Jan 26, 2026 at 03:30:44PM +0000, Rodrigo Alencar wrote:
> > > On 26/01/26 03:20PM, Rodrigo Alencar wrote:
> > > > On 26/01/26 04:53PM, Andy Shevchenko wrote:
> > > > > On Mon, Jan 26, 2026 at 02:26:20PM +0000, Rodrigo Alencar wrote:
...
> > > > > Why? Can you elaborate how checking amount of digits is different to
> > > > > check_mul_overflow()?
> > > >
> > > > consider U64_MAX = 18_446_744_073_709_551_615 as the limit:
> > > > - 19_000_000_000_000_000_000 contains the same amount of digits but overflows.
> > > > - 18_446_744_073_710_000_000 contains the same amount of digits but overflows.
> > > >
> > > > to catch those cases, we need to check for the overflow, everytime we read a
> > > > character and accumulate:
> > > >
> > > > u64 acc;
> > > >
> > > > while(isdigit(*str))
> > > > if (check_mul_overflow(acc, 10, &acc) ||
> > > > check_add_overflow(acc, *str - '0', &acc))
> > > > return -EOVERFLOW;
> > > >
> > > > *res = acc;
> > > >
> > > > acc can get weird results if not checked.
> > >
> > > Thinking about it again, that check could be done only in the last step
> > > (20th for u64)
> >
> > Does kstrto*() also perform only last check? I think they do for each
> > iteration.
>
> It does the following:
>
> ...
> if (unlikely(res & (~0ull << 60))) {
> if (res > div_u64(ULLONG_MAX - val, base))
> rv |= KSTRTOX_OVERFLOW;
> }
> ...
>
> so overflow is checked when either one of the 4 MSbits are set.
>
> for now, I am thinking of something like:
>
> static ssize_t iio_safe_strtou64(const char *str, const char **endp,
> size_t max_chars, u64 *result)
> {
> u64 digit, acc = 0;
> size_t idx = 0;
>
> while (isdigit(*str) && idx < max_chars) {
> digit = *str - '0';
> if (unlikely(idx > 19)) {
> if (check_mul_overflow(acc, 10, &acc) ||
> check_add_overflow(acc, digit, &acc))
> return -EOVERFLOW;
> } else {
> acc = acc * 10 + digit;
> }
> str++;
> idx++;
> }
>
> *endp = str;
> *result = acc;
> return idx;
> }
>
> which would help the truncation when parsing the fractional part
> with max_chars, avoiding a div64_u64() to adjust precision:
>
> ...
> digit_count = iio_safe_strtou64(str, &end, SIZE_MAX, &i);
> if (digit_count < 0)
> return digit_count;
>
> if (precision && *end == '.') {
> str = end + 1;
> digit_count = iio_safe_strtou64(str, &end, precision, &f);
> if (digit_count < 0)
> return digit_count;
>
> if (digit_count < precision) /* scale up */
> f *= int_pow(10, precision - digit_count);
>
> while (isdigit(*end)) /* truncate */
> end++;
> }
> ...
>
> but I understand you would not like this approach, because it does not use
> simple_strtoull() or kstrtoull(). Problem is simple_strtoull() is not
> overflow-safe and kstrtoull() does not allow to track a pointer to end
> of the string.
>
> Given that the current implementation of iio_str_to_fixpoint() is not using
> simple_strtoull() I am not seeing an issue with this approach.
I believe this is the goal, id est to get rid of the code duplication.
The idea is not exactly in _using_ simple_strto*() or kstrto*(), but
deriving the common parts that can be reused here. For simplicity, we
may leave iio_str_to_fixpoint() alone for now (as you mentioned it
doesn't share currently the code, so can be addressed later on)
and try to provide a treewide available safe_strto*().
Browsing through lore.kernel.org I found these
(in backward chronological order):
https://lore.kernel.org/linux-hardening/20260126162059.357467-1-dmantipov@yandex.ru/
https://lore.kernel.org/lkml/d6c3b369-9777-9986-f41f-3f3a4f85d64c@rasmusvillemoes.dk/
https://lore.kernel.org/lkml/CA+55aFyC7N4S65UVrp0Hcefb5AsMPqGn_bco6tFL+JZ0m3nh=A@mail.gmail.com/
which suggests that the problems are known and there are attempts to address them.
Perhaps we should consider what Rasmus started 6 years ago...
--
With Best Regards,
Andy Shevchenko
On 26/01/27 09:43AM, Andy Shevchenko wrote:
>
> Remove DT related people and ML and add others based on the links I posted
> below.
>
> On Mon, Jan 26, 2026 at 04:55:13PM +0000, Rodrigo Alencar wrote:
> > On 26/01/26 06:07PM, Andy Shevchenko wrote:
> > > On Mon, Jan 26, 2026 at 03:30:44PM +0000, Rodrigo Alencar wrote:
> > > > On 26/01/26 03:20PM, Rodrigo Alencar wrote:
> > > > > On 26/01/26 04:53PM, Andy Shevchenko wrote:
> > > > > > On Mon, Jan 26, 2026 at 02:26:20PM +0000, Rodrigo Alencar wrote:
>
> ...
>
> > > > > > Why? Can you elaborate how checking amount of digits is different to
> > > > > > check_mul_overflow()?
> > > > >
> > > > > consider U64_MAX = 18_446_744_073_709_551_615 as the limit:
> > > > > - 19_000_000_000_000_000_000 contains the same amount of digits but overflows.
> > > > > - 18_446_744_073_710_000_000 contains the same amount of digits but overflows.
> > > > >
> > > > > to catch those cases, we need to check for the overflow, everytime we read a
> > > > > character and accumulate:
> > > > >
> > > > > u64 acc;
> > > > >
> > > > > while(isdigit(*str))
> > > > > if (check_mul_overflow(acc, 10, &acc) ||
> > > > > check_add_overflow(acc, *str - '0', &acc))
> > > > > return -EOVERFLOW;
> > > > >
> > > > > *res = acc;
> > > > >
> > > > > acc can get weird results if not checked.
> > > >
> > > > Thinking about it again, that check could be done only in the last step
> > > > (20th for u64)
> > >
> > > Does kstrto*() also perform only last check? I think they do for each
> > > iteration.
> >
> > It does the following:
> >
> > ...
> > if (unlikely(res & (~0ull << 60))) {
> > if (res > div_u64(ULLONG_MAX - val, base))
> > rv |= KSTRTOX_OVERFLOW;
> > }
> > ...
> >
> > so overflow is checked when either one of the 4 MSbits are set.
> >
> > for now, I am thinking of something like:
> >
> > static ssize_t iio_safe_strtou64(const char *str, const char **endp,
> > size_t max_chars, u64 *result)
> > {
> > u64 digit, acc = 0;
> > size_t idx = 0;
> >
> > while (isdigit(*str) && idx < max_chars) {
> > digit = *str - '0';
> > if (unlikely(idx > 19)) {
> > if (check_mul_overflow(acc, 10, &acc) ||
> > check_add_overflow(acc, digit, &acc))
> > return -EOVERFLOW;
> > } else {
> > acc = acc * 10 + digit;
> > }
> > str++;
> > idx++;
> > }
> >
> > *endp = str;
> > *result = acc;
> > return idx;
> > }
> >
> > which would help the truncation when parsing the fractional part
> > with max_chars, avoiding a div64_u64() to adjust precision:
> >
> > ...
> > digit_count = iio_safe_strtou64(str, &end, SIZE_MAX, &i);
> > if (digit_count < 0)
> > return digit_count;
> >
> > if (precision && *end == '.') {
> > str = end + 1;
> > digit_count = iio_safe_strtou64(str, &end, precision, &f);
> > if (digit_count < 0)
> > return digit_count;
> >
> > if (digit_count < precision) /* scale up */
> > f *= int_pow(10, precision - digit_count);
> >
> > while (isdigit(*end)) /* truncate */
> > end++;
> > }
> > ...
> >
> > but I understand you would not like this approach, because it does not use
> > simple_strtoull() or kstrtoull(). Problem is simple_strtoull() is not
> > overflow-safe and kstrtoull() does not allow to track a pointer to end
> > of the string.
> >
> > Given that the current implementation of iio_str_to_fixpoint() is not using
> > simple_strtoull() I am not seeing an issue with this approach.
>
> I believe this is the goal, id est to get rid of the code duplication.
> The idea is not exactly in _using_ simple_strto*() or kstrto*(), but
> deriving the common parts that can be reused here.
Let's recap:
- The real goal here is the PLL driver implementation;
- Such driver requires 64-bit parsing because frequencies are big values
with sub-Hz resolutions.
- IIO helpers currently does not support that, so I added the code in driver;
- Duplication and reuse of code argument is valid, so I moved the code to IIO
core.
- Existing functions simple_strtoull() and kstrtoull() have their limitations.
About the current state of IIO core fixed-point parsing:
- Overflow-unsafe;
- Does not support 64-bit values;
- custom implementation, so does not reuse code;
> For simplicity, we
> may leave iio_str_to_fixpoint() alone for now (as you mentioned it
> doesn't share currently the code, so can be addressed later on)
> and try to provide a treewide available safe_strto*().
For simplicity, if possible, I still think the work here should be
bound to the IIO subsystem. I would like get other pepole's opinion here,
As you are increasing the scope of this patch, again.
The scope I would like to push for this work:
- Add 64-bit parsing support into IIO core;
- Add IIO kunit test for fixed point parsing;
- Get the code to be overflow-safe;
--
Kind regards,
Rodrigo Alencar
On Tue, 27 Jan 2026 10:17:33 +0000
Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote:
> On 26/01/27 09:43AM, Andy Shevchenko wrote:
> >
> > Remove DT related people and ML and add others based on the links I posted
> > below.
> >
> > On Mon, Jan 26, 2026 at 04:55:13PM +0000, Rodrigo Alencar wrote:
> > > On 26/01/26 06:07PM, Andy Shevchenko wrote:
> > > > On Mon, Jan 26, 2026 at 03:30:44PM +0000, Rodrigo Alencar wrote:
> > > > > On 26/01/26 03:20PM, Rodrigo Alencar wrote:
> > > > > > On 26/01/26 04:53PM, Andy Shevchenko wrote:
> > > > > > > On Mon, Jan 26, 2026 at 02:26:20PM +0000, Rodrigo Alencar wrote:
> >
> > ...
> >
> > > > > > > Why? Can you elaborate how checking amount of digits is different to
> > > > > > > check_mul_overflow()?
> > > > > >
> > > > > > consider U64_MAX = 18_446_744_073_709_551_615 as the limit:
> > > > > > - 19_000_000_000_000_000_000 contains the same amount of digits but overflows.
> > > > > > - 18_446_744_073_710_000_000 contains the same amount of digits but overflows.
> > > > > >
> > > > > > to catch those cases, we need to check for the overflow, everytime we read a
> > > > > > character and accumulate:
> > > > > >
> > > > > > u64 acc;
> > > > > >
> > > > > > while(isdigit(*str))
> > > > > > if (check_mul_overflow(acc, 10, &acc) ||
> > > > > > check_add_overflow(acc, *str - '0', &acc))
> > > > > > return -EOVERFLOW;
> > > > > >
> > > > > > *res = acc;
> > > > > >
> > > > > > acc can get weird results if not checked.
> > > > >
> > > > > Thinking about it again, that check could be done only in the last step
> > > > > (20th for u64)
> > > >
> > > > Does kstrto*() also perform only last check? I think they do for each
> > > > iteration.
> > >
> > > It does the following:
> > >
> > > ...
> > > if (unlikely(res & (~0ull << 60))) {
> > > if (res > div_u64(ULLONG_MAX - val, base))
> > > rv |= KSTRTOX_OVERFLOW;
> > > }
> > > ...
> > >
> > > so overflow is checked when either one of the 4 MSbits are set.
> > >
> > > for now, I am thinking of something like:
> > >
> > > static ssize_t iio_safe_strtou64(const char *str, const char **endp,
> > > size_t max_chars, u64 *result)
> > > {
> > > u64 digit, acc = 0;
> > > size_t idx = 0;
> > >
> > > while (isdigit(*str) && idx < max_chars) {
> > > digit = *str - '0';
> > > if (unlikely(idx > 19)) {
> > > if (check_mul_overflow(acc, 10, &acc) ||
> > > check_add_overflow(acc, digit, &acc))
> > > return -EOVERFLOW;
> > > } else {
> > > acc = acc * 10 + digit;
> > > }
> > > str++;
> > > idx++;
> > > }
> > >
> > > *endp = str;
> > > *result = acc;
> > > return idx;
> > > }
> > >
> > > which would help the truncation when parsing the fractional part
> > > with max_chars, avoiding a div64_u64() to adjust precision:
> > >
> > > ...
> > > digit_count = iio_safe_strtou64(str, &end, SIZE_MAX, &i);
> > > if (digit_count < 0)
> > > return digit_count;
> > >
> > > if (precision && *end == '.') {
> > > str = end + 1;
> > > digit_count = iio_safe_strtou64(str, &end, precision, &f);
> > > if (digit_count < 0)
> > > return digit_count;
> > >
> > > if (digit_count < precision) /* scale up */
> > > f *= int_pow(10, precision - digit_count);
> > >
> > > while (isdigit(*end)) /* truncate */
> > > end++;
> > > }
> > > ...
> > >
> > > but I understand you would not like this approach, because it does not use
> > > simple_strtoull() or kstrtoull(). Problem is simple_strtoull() is not
> > > overflow-safe and kstrtoull() does not allow to track a pointer to end
> > > of the string.
> > >
> > > Given that the current implementation of iio_str_to_fixpoint() is not using
> > > simple_strtoull() I am not seeing an issue with this approach.
> >
> > I believe this is the goal, id est to get rid of the code duplication.
> > The idea is not exactly in _using_ simple_strto*() or kstrto*(), but
> > deriving the common parts that can be reused here.
>
> Let's recap:
> - The real goal here is the PLL driver implementation;
> - Such driver requires 64-bit parsing because frequencies are big values
> with sub-Hz resolutions.
> - IIO helpers currently does not support that, so I added the code in driver;
> - Duplication and reuse of code argument is valid, so I moved the code to IIO
> core.
> - Existing functions simple_strtoull() and kstrtoull() have their limitations.
>
> About the current state of IIO core fixed-point parsing:
> - Overflow-unsafe;
> - Does not support 64-bit values;
> - custom implementation, so does not reuse code;
>
> > For simplicity, we
> > may leave iio_str_to_fixpoint() alone for now (as you mentioned it
> > doesn't share currently the code, so can be addressed later on)
> > and try to provide a treewide available safe_strto*().
>
> For simplicity, if possible, I still think the work here should be
> bound to the IIO subsystem. I would like get other pepole's opinion here,
> As you are increasing the scope of this patch, again.
I'd go with what Andy suggests for looking at global unification
but agree that might be split from what you need here.
>
> The scope I would like to push for this work:
> - Add 64-bit parsing support into IIO core;
Makes sense as it seems we have real users and if you improve the other
cases along the way (tests etc) all the better!
> - Add IIO kunit test for fixed point parsing;
That sounds particularly good to have.
> - Get the code to be overflow-safe;
Don't think anyone has really looked at that parser for a while
so good to test it better and fix the corner cases.
Jonathan
>
On 26/01/23 03:53PM, Rodrigo Alencar via B4 Relay wrote:
> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
>
> 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.
>
> Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
...
> +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);
I've just noted that I should have used ffs64() here.
kind regards,
Rodrigo Alencar
On 26/01/23 04:14PM, Rodrigo Alencar wrote:
> On 26/01/23 03:53PM, Rodrigo Alencar via B4 Relay wrote:
> > From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> >
> > 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.
> >
> > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
>
> ...
> > +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);
>
> I've just noted that I should have used ffs64() here.
The idea here is that powers of 10 are a bunch of 2 and 5 factors,
so the index of first non-zero bit (from lsb to msb) is the amount of
precision we are interested in. This is used down bellow because
simple_strtoull() does not stop at your will, so we need to adjust
the precision after the fractional part parsing.
simple_strntoull() would come to fix this with max_chars parameter,
but it is not function that is exposed.
Apparently the most correct would be:
(fract_mult)? __ffs64(fract_mult) + 1 : 0;
However, ffs() still works, because we would not have more than 20 digits
when parsing u64.
Kind regards,
Rodrigo Alencar
© 2016 - 2026 Red Hat, Inc.