From: Rodrigo Alencar <rodrigo.alencar@analog.com>
Expose simple_strntoull(), by addressing its FIXME, i.e. its prototype is
slightly changed so that -ERANGE or -EINVAL can be evaluated by the user.
Flow of the function is not changed and error value is returned in the
end. Unsafe internal wrapper is created to reduce amount of changes.
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
include/linux/kstrtox.h | 4 ++++
lib/vsprintf.c | 59 +++++++++++++++++++++++++++++++++++++++++--------
2 files changed, 54 insertions(+), 9 deletions(-)
diff --git a/include/linux/kstrtox.h b/include/linux/kstrtox.h
index 6ea897222af1..5e161073121f 100644
--- a/include/linux/kstrtox.h
+++ b/include/linux/kstrtox.h
@@ -148,4 +148,8 @@ extern long simple_strtol(const char *,char **,unsigned int);
extern unsigned long long simple_strtoull(const char *,char **,unsigned int);
extern long long simple_strtoll(const char *,char **,unsigned int);
+extern ssize_t __must_check simple_strntoull(const char *startp, const char **endp,
+ unsigned int base, size_t max_chars,
+ unsigned long long *res);
+
#endif /* _LINUX_KSTRTOX_H */
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 800b8ac49f53..6fb880f4013b 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -75,25 +75,66 @@ enum hash_pointers_policy {
};
static enum hash_pointers_policy hash_pointers_mode __initdata;
+/**
+ * simple_strntoull - convert a string to an unsigned long long with a character limit
+ *
+ * @startp: The start of the string
+ * @endp: A pointer to the end of the parsed string will be placed here
+ * @base: The number base to use
+ * @max_chars: The maximum number of characters to parse
+ * @res: Where to write the result of the conversion on success
+ *
+ * Returns amount of processed characters on success, -ERANGE on overflow and
+ * -EINVAL on parsing error.
+ */
noinline
-static unsigned long long simple_strntoull(const char *startp, char **endp, unsigned int base, size_t max_chars)
+ssize_t simple_strntoull(const char *startp, const char **endp,
+ unsigned int base, size_t max_chars,
+ unsigned long long *res)
{
const char *cp;
- unsigned long long result = 0ULL;
size_t prefix_chars;
unsigned int rv;
+ ssize_t ret;
cp = _parse_integer_fixup_radix(startp, &base);
prefix_chars = cp - startp;
if (prefix_chars < max_chars) {
- rv = _parse_integer_limit(cp, base, &result, max_chars - prefix_chars);
- /* FIXME */
+ rv = _parse_integer_limit(cp, base, res, max_chars - prefix_chars);
+ if (rv & KSTRTOX_OVERFLOW)
+ ret = -ERANGE;
+ else if (rv == 0)
+ ret = -EINVAL;
+ else
+ ret = rv + prefix_chars;
cp += (rv & ~KSTRTOX_OVERFLOW);
} else {
/* Field too short for prefix + digit, skip over without converting */
cp = startp + max_chars;
+ ret = -EINVAL;
+ *res = 0ULL;
}
+ if (endp)
+ *endp = cp;
+
+ return ret;
+}
+EXPORT_SYMBOL(simple_strntoull);
+
+/* unsafe_strntoull ignores simple_strntoull() return value and endp const qualifier */
+inline
+static unsigned long long unsafe_strntoull(const char *startp, char **endp,
+ unsigned int base, size_t max_chars)
+{
+ unsigned long long result;
+ const char *cp;
+
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wunused-result"
+ simple_strntoull(startp, &cp, base, max_chars, &result);
+#pragma GCC diagnostic pop
+
if (endp)
*endp = (char *)cp;
@@ -111,7 +152,7 @@ static unsigned long long simple_strntoull(const char *startp, char **endp, unsi
noinline
unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int base)
{
- return simple_strntoull(cp, endp, base, INT_MAX);
+ return unsafe_strntoull(cp, endp, base, INT_MAX);
}
EXPORT_SYMBOL(simple_strtoull);
@@ -132,7 +173,7 @@ EXPORT_SYMBOL(simple_strtoul);
unsigned long simple_strntoul(const char *cp, char **endp, unsigned int base,
size_t max_chars)
{
- return simple_strntoull(cp, endp, base, max_chars);
+ return unsafe_strntoull(cp, endp, base, max_chars);
}
EXPORT_SYMBOL(simple_strntoul);
@@ -163,9 +204,9 @@ static long long simple_strntoll(const char *cp, char **endp, unsigned int base,
* and the content of *cp is irrelevant.
*/
if (*cp == '-' && max_chars > 0)
- return -simple_strntoull(cp + 1, endp, base, max_chars - 1);
+ return -unsafe_strntoull(cp + 1, endp, base, max_chars - 1);
- return simple_strntoull(cp, endp, base, max_chars);
+ return unsafe_strntoull(cp, endp, base, max_chars);
}
/**
@@ -3670,7 +3711,7 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
val.s = simple_strntoll(str, &next, base,
field_width >= 0 ? field_width : INT_MAX);
else
- val.u = simple_strntoull(str, &next, base,
+ val.u = unsafe_strntoull(str, &next, base,
field_width >= 0 ? field_width : INT_MAX);
switch (qualifier) {
--
2.43.0On Fri 2026-03-20 16:27:27, Rodrigo Alencar via B4 Relay wrote:
> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
>
> Expose simple_strntoull(), by addressing its FIXME, i.e. its prototype is
> slightly changed so that -ERANGE or -EINVAL can be evaluated by the user.
> Flow of the function is not changed and error value is returned in the
> end. Unsafe internal wrapper is created to reduce amount of changes.
>
> --- a/include/linux/kstrtox.h
> +++ b/include/linux/kstrtox.h
> @@ -148,4 +148,8 @@ extern long simple_strtol(const char *,char **,unsigned int);
> extern unsigned long long simple_strtoull(const char *,char **,unsigned int);
> extern long long simple_strtoll(const char *,char **,unsigned int);
>
> +extern ssize_t __must_check simple_strntoull(const char *startp, const char **endp,
> + unsigned int base, size_t max_chars,
> + unsigned long long *res);
Sigh, naming is hard. I personally find it a bit confusing that the
name is too similar to the unsafe API.
IMHO, the semantic of the new API is closer to kstrtoull().
It just limits the size, so I would call it kstrntoull().
Also I would use int as the return parameter, see below.
> #endif /* _LINUX_KSTRTOX_H */
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 800b8ac49f53..6fb880f4013b 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -75,25 +75,66 @@ enum hash_pointers_policy {
> };
> static enum hash_pointers_policy hash_pointers_mode __initdata;
>
> +/**
> + * simple_strntoull - convert a string to an unsigned long long with a character limit
> + *
> + * @startp: The start of the string
> + * @endp: A pointer to the end of the parsed string will be placed here
I would write:
* @endp: A pointer to the end of the parsed string (output)
> + * @base: The number base to use
> + * @max_chars: The maximum number of characters to parse
> + * @res: Where to write the result of the conversion on success
Nit: I would omit "on success" *res value is set to 0 on failure.
Instead, I would write:
* @res: Result of the conversion (output)
> + *
> + * Returns amount of processed characters on success, -ERANGE on overflow and
> + * -EINVAL on parsing error.
> + */
> noinline
> -static unsigned long long simple_strntoull(const char *startp, char **endp, unsigned int base, size_t max_chars)
> +ssize_t simple_strntoull(const char *startp, const char **endp,
> + unsigned int base, size_t max_chars,
> + unsigned long long *res)
It might be enoungh to use "int" for the return value. The number
of proceed characters is pretty limited by definition. And it
would be similar to vsnprintf(), kstrtoull(), ...
I guess that you wanted to match the "size_t max_chars" parameter.
It makes some sense as well.
Please, use "int" especially if we agreed to call the new API
kstrntoull().
> {
> const char *cp;
> - unsigned long long result = 0ULL;
> size_t prefix_chars;
> unsigned int rv;
> + ssize_t ret;
>
> cp = _parse_integer_fixup_radix(startp, &base);
> prefix_chars = cp - startp;
> if (prefix_chars < max_chars) {
> - rv = _parse_integer_limit(cp, base, &result, max_chars - prefix_chars);
> - /* FIXME */
> + rv = _parse_integer_limit(cp, base, res, max_chars - prefix_chars);
> + if (rv & KSTRTOX_OVERFLOW)
> + ret = -ERANGE;
> + else if (rv == 0)
> + ret = -EINVAL;
> + else
> + ret = rv + prefix_chars;
> cp += (rv & ~KSTRTOX_OVERFLOW);
> } else {
> /* Field too short for prefix + digit, skip over without converting */
> cp = startp + max_chars;
> + ret = -EINVAL;
> + *res = 0ULL;
> }
>
> + if (endp)
> + *endp = cp;
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(simple_strntoull);
> +
> +/* unsafe_strntoull ignores simple_strntoull() return value and endp const qualifier */
> +inline
> +static unsigned long long unsafe_strntoull(const char *startp, char **endp,
> + unsigned int base, size_t max_chars)
> +{
> + unsigned long long result;
> + const char *cp;
> +
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wunused-result"
> + simple_strntoull(startp, &cp, base, max_chars, &result);
> +#pragma GCC diagnostic pop
> +
> if (endp)
> *endp = (char *)cp;
IMHO, we do not need local "cp". We could simply pass the endp
to the new simple_strntoull. Or do I miss anything?
Best Regards,
Petr
On 26/03/27 09:45AM, Petr Mladek wrote:
> On Fri 2026-03-20 16:27:27, Rodrigo Alencar via B4 Relay wrote:
> > From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> >
> > Expose simple_strntoull(), by addressing its FIXME, i.e. its prototype is
> > slightly changed so that -ERANGE or -EINVAL can be evaluated by the user.
> > Flow of the function is not changed and error value is returned in the
> > end. Unsafe internal wrapper is created to reduce amount of changes.
> >
> > --- a/include/linux/kstrtox.h
> > +++ b/include/linux/kstrtox.h
> > @@ -148,4 +148,8 @@ extern long simple_strtol(const char *,char **,unsigned int);
> > extern unsigned long long simple_strtoull(const char *,char **,unsigned int);
> > extern long long simple_strtoll(const char *,char **,unsigned int);
> >
> > +extern ssize_t __must_check simple_strntoull(const char *startp, const char **endp,
> > + unsigned int base, size_t max_chars,
> > + unsigned long long *res);
>
> Sigh, naming is hard. I personally find it a bit confusing that the
> name is too similar to the unsafe API.
>
> IMHO, the semantic of the new API is closer to kstrtoull().
> It just limits the size, so I would call it kstrntoull().
>
> Also I would use int as the return parameter, see below.
Thanks for look into this one.
kstrntoull() was what I used in v8:
https://lore.kernel.org/r/20260303-adf41513-iio-driver-v8-0-8dd2417cc465@analog.com
There was a discussion around the naming:
https://lore.kernel.org/all/4mtdzxfj656sjr66npabfvrr7yd7q26l2unhsihjtniz4ossfj@g3qnzonoary6/
please suggest how the function prototype should look like.
...
> > +/* unsafe_strntoull ignores simple_strntoull() return value and endp const qualifier */
> > +inline
> > +static unsigned long long unsafe_strntoull(const char *startp, char **endp,
> > + unsigned int base, size_t max_chars)
> > +{
> > + unsigned long long result;
> > + const char *cp;
> > +
> > +#pragma GCC diagnostic push
> > +#pragma GCC diagnostic ignored "-Wunused-result"
> > + simple_strntoull(startp, &cp, base, max_chars, &result);
> > +#pragma GCC diagnostic pop
> > +
> > if (endp)
> > *endp = (char *)cp;
>
> IMHO, we do not need local "cp". We could simply pass the endp
> to the new simple_strntoull. Or do I miss anything?
Basically the unsafe version drops the const qualifier and compiler
complains that pointer types do not match. Maybe an extra warning can
be suppressed there.
--
Kind regards,
Rodrigo Alencar
On Fri, Mar 27, 2026 at 09:45:17AM +0100, Petr Mladek wrote:
> On Fri 2026-03-20 16:27:27, Rodrigo Alencar via B4 Relay wrote:
...
> > +extern ssize_t __must_check simple_strntoull(const char *startp, const char **endp,
> > + unsigned int base, size_t max_chars,
> > + unsigned long long *res);
>
> Sigh, naming is hard. I personally find it a bit confusing that the
> name is too similar to the unsafe API.
>
> IMHO, the semantic of the new API is closer to kstrtoull().
> It just limits the size, so I would call it kstrntoull().
It's not. kstrto*() quite strict about the input, this one is actually relaxed
variant, so I wouldn't mix these two groups.
> Also I would use int as the return parameter, see below.
...
TBH, I am skeptical about this approach. My main objection is max_chars
parameter. If we want to limit the input strictly to the given number of
characters, we have to copy the string and then just use kstrto*() in a normal
way. The whole idea of that parameter is to be able to parse the fractional
part of the float number as 'iiiii.fffff', where 'i' is for integer part, and
'f' for the fractional. Since we have *endp, we may simply check that.
In case if we want to parse only, say, 6 digits and input is longer there are
a few options (in my personal preferences, the first is the better):
- consider the input invalid
- parse it as is up to the maximum and then do ceil() or floor() on top of that
- copy only necessary amount of the (sub)string and parse that.
The problem with precision is that we need to also consider floor() or ceil()
and I don't think this should be burden of the library as it's individual
preference of each of the callers (users). At least for the starter, we will
see if it's only one approach is used, we may incorporate it into the library
code.
The easiest way out is to just consider the input invalid if it overflows the
given type (s32 or s64).
But we need to have an agreement what will be the representation of the
fixed-width float numbers in the kernel? Currently IIO uses
struct float // name is crafted for simplicity
{
int integer;
int fraction;
}
This parser wants AFAIU to have at the end of the day something like
struct float
{
s64 integer;
s64 fraction;
}
but also wants to have the fraction part be limited in some cases to s32
or so:
struct float
{
s64 integer;
s32 fraction; // precision may be lost if input is longer
}
Maybe we want to have kstrtof32() and kstrtof64() for these two cases?
With that we will always consider the fraction part as 32- or 64-bit,
imply floor() on the fraction for the sake of simplicity and require
it to be NUL-terminated with possible trailing '\n'.
--
With Best Regards,
Andy Shevchenko
On Fri, 27 Mar 2026 11:17:16 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> On Fri, Mar 27, 2026 at 09:45:17AM +0100, Petr Mladek wrote:
> > On Fri 2026-03-20 16:27:27, Rodrigo Alencar via B4 Relay wrote:
>
> ...
>
> > > +extern ssize_t __must_check simple_strntoull(const char *startp, const char **endp,
> > > + unsigned int base, size_t max_chars,
> > > + unsigned long long *res);
> >
> > Sigh, naming is hard. I personally find it a bit confusing that the
> > name is too similar to the unsafe API.
> >
> > IMHO, the semantic of the new API is closer to kstrtoull().
> > It just limits the size, so I would call it kstrntoull().
>
> It's not. kstrto*() quite strict about the input, this one is actually relaxed
> variant, so I wouldn't mix these two groups.
>
> > Also I would use int as the return parameter, see below.
>
> ...
>
> TBH, I am skeptical about this approach. My main objection is max_chars
> parameter. If we want to limit the input strictly to the given number of
> characters, we have to copy the string and then just use kstrto*() in a normal
> way. The whole idea of that parameter is to be able to parse the fractional
> part of the float number as 'iiiii.fffff', where 'i' is for integer part, and
> 'f' for the fractional. Since we have *endp, we may simply check that.
>
> In case if we want to parse only, say, 6 digits and input is longer there are
> a few options (in my personal preferences, the first is the better):
> - consider the input invalid
> - parse it as is up to the maximum and then do ceil() or floor() on top of that
> - copy only necessary amount of the (sub)string and parse that.
Isn't there a bigger problem?
If you want a max of 6 digits you need to correctly parse 3.1 3.159265
3.159256358979 3.0001 3.000159 3.00015926535 3.000100 (etc).
That seems to always require checking the length and then multiply/divide
by 10.
Then there is 'round to even' which rounds these two in opposite directions:
4.500000000000000000000000000000000000000000000000000
4.500000000000000000000000000000000000000000000000001
I suspect you really want a completely different function for reading
fractional parts of floating point numbers.
It isn't as though the actual digit conversion is hard.
>
> The problem with precision is that we need to also consider floor() or ceil()
> and I don't think this should be burden of the library as it's individual
> preference of each of the callers (users). At least for the starter, we will
> see if it's only one approach is used, we may incorporate it into the library
> code.
>
> The easiest way out is to just consider the input invalid if it overflows the
> given type (s32 or s64).
>
> But we need to have an agreement what will be the representation of the
> fixed-width float numbers in the kernel? Currently IIO uses
> struct float // name is crafted for simplicity
> {
> int integer;
> int fraction;
> }
>
> This parser wants AFAIU to have at the end of the day something like
>
> struct float
> {
> s64 integer;
> s64 fraction;
> }
>
> but also wants to have the fraction part be limited in some cases to s32
> or so:
>
> struct float
> {
> s64 integer;
> s32 fraction; // precision may be lost if input is longer
> }
Are those 'fraction' counts of (say) 10^-6 (like times in seconds+usecs)
or true binary values where the value could be treated as a u64 (or u128)
for addition and subtraction.
So parse the latter you don't need to know the length
(and it can be converted the to former by multiplying by 10^6).
David
>
> Maybe we want to have kstrtof32() and kstrtof64() for these two cases?
>
> With that we will always consider the fraction part as 32- or 64-bit,
> imply floor() on the fraction for the sake of simplicity and require
> it to be NUL-terminated with possible trailing '\n'.
>
On Fri, Mar 27, 2026 at 10:44:40AM +0000, David Laight wrote:
> On Fri, 27 Mar 2026 11:17:16 +0200
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
...
> > TBH, I am skeptical about this approach. My main objection is max_chars
> > parameter. If we want to limit the input strictly to the given number of
> > characters, we have to copy the string and then just use kstrto*() in a normal
> > way. The whole idea of that parameter is to be able to parse the fractional
> > part of the float number as 'iiiii.fffff', where 'i' is for integer part, and
> > 'f' for the fractional. Since we have *endp, we may simply check that.
> >
> > In case if we want to parse only, say, 6 digits and input is longer there are
> > a few options (in my personal preferences, the first is the better):
> > - consider the input invalid
> > - parse it as is up to the maximum and then do ceil() or floor() on top of that
> > - copy only necessary amount of the (sub)string and parse that.
>
> Isn't there a bigger problem?
> If you want a max of 6 digits you need to correctly parse 3.1 3.159265
> 3.159256358979 3.0001 3.000159 3.00015926535 3.000100 (etc).
> That seems to always require checking the length and then multiply/divide
> by 10.
Yep.
> Then there is 'round to even' which rounds these two in opposite directions:
> 4.500000000000000000000000000000000000000000000000000
> 4.500000000000000000000000000000000000000000000000001
These are wrong inputs and if we want to have them cut, it will be just a cut.
(Yeah, which will have different result for negative numbers.)
> I suspect you really want a completely different function for reading
> fractional parts of floating point numbers.
> It isn't as though the actual digit conversion is hard.
>
> > The problem with precision is that we need to also consider floor() or ceil()
> > and I don't think this should be burden of the library as it's individual
> > preference of each of the callers (users). At least for the starter, we will
> > see if it's only one approach is used, we may incorporate it into the library
> > code.
> >
> > The easiest way out is to just consider the input invalid if it overflows the
> > given type (s32 or s64).
> >
> > But we need to have an agreement what will be the representation of the
> > fixed-width float numbers in the kernel? Currently IIO uses
> > struct float // name is crafted for simplicity
> > {
> > int integer;
> > int fraction;
> > }
> >
> > This parser wants AFAIU to have at the end of the day something like
> >
> > struct float
> > {
> > s64 integer;
> > s64 fraction;
> > }
> >
> > but also wants to have the fraction part be limited in some cases to s32
> > or so:
> >
> > struct float
> > {
> > s64 integer;
> > s32 fraction; // precision may be lost if input is longer
> > }
>
> Are those 'fraction' counts of (say) 10^-6 (like times in seconds+usecs)
> or true binary values where the value could be treated as a u64 (or u128)
> for addition and subtraction.
It depends. IIO has scale on top of that, so the fraction part can be 10⁻³,
10⁻⁶, 10⁻⁹. I don't remember by heart if the ABI requires all digits to be
placed, I think we don't require that.
> So parse the latter you don't need to know the length
> (and it can be converted the to former by multiplying by 10^6).
>
> > Maybe we want to have kstrtof32() and kstrtof64() for these two cases?
> >
> > With that we will always consider the fraction part as 32- or 64-bit,
> > imply floor() on the fraction for the sake of simplicity and require
> > it to be NUL-terminated with possible trailing '\n'.
--
With Best Regards,
Andy Shevchenko
On Fri, 27 Mar 2026 12:57:56 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> On Fri, Mar 27, 2026 at 10:44:40AM +0000, David Laight wrote:
,,,
> > > but also wants to have the fraction part be limited in some cases to s32
> > > or so:
> > >
> > > struct float
> > > {
> > > s64 integer;
> > > s32 fraction; // precision may be lost if input is longer
> > > }
> >
> > Are those 'fraction' counts of (say) 10^-6 (like times in seconds+usecs)
> > or true binary values where the value could be treated as a u64 (or u128)
> > for addition and subtraction.
>
> It depends. IIO has scale on top of that, so the fraction part can be 10⁻³,
> 10⁻⁶, 10⁻⁹. I don't remember by heart if the ABI requires all digits to be
> placed, I think we don't require that.
Seems like you want this function (untested):
u64 strtofrac(const char *buf, const char **end, unsigned int len)
{
u64 val = 0;
unsigned int digit;
while (len--) {
digit = *buf - '0';
if (digit <= 9) {
buf++;
val += digit;
}
val *= 10;
}
while (*buf - '0' <= 9u)
buf++;
*end = buf;
return val;
}
David
On 26/03/27 11:17AM, Andy Shevchenko wrote:
> On Fri, Mar 27, 2026 at 09:45:17AM +0100, Petr Mladek wrote:
> > On Fri 2026-03-20 16:27:27, Rodrigo Alencar via B4 Relay wrote:
>
> ...
>
> > > +extern ssize_t __must_check simple_strntoull(const char *startp, const char **endp,
> > > + unsigned int base, size_t max_chars,
> > > + unsigned long long *res);
> >
> > Sigh, naming is hard. I personally find it a bit confusing that the
> > name is too similar to the unsafe API.
> >
> > IMHO, the semantic of the new API is closer to kstrtoull().
> > It just limits the size, so I would call it kstrntoull().
>
> It's not. kstrto*() quite strict about the input, this one is actually relaxed
> variant, so I wouldn't mix these two groups.
>
> > Also I would use int as the return parameter, see below.
>
> ...
>
> TBH, I am skeptical about this approach. My main objection is max_chars
> parameter. If we want to limit the input strictly to the given number of
> characters, we have to copy the string and then just use kstrto*() in a normal
> way. The whole idea of that parameter is to be able to parse the fractional
> part of the float number as 'iiiii.fffff', where 'i' is for integer part, and
> 'f' for the fractional. Since we have *endp, we may simply check that.
A max_chars would not be only useful for that. It can prevent out-of-bounds
reads when the input isn't NUL-terminated (like buffers, file chunks,
network packets, memory-mapped data, ....). Even if there is a NUL later in
memory, a regular strtoull() function may consume characters that are outside
the field one intends to parse.
> In case if we want to parse only, say, 6 digits and input is longer there are
> a few options (in my personal preferences, the first is the better):
> - consider the input invalid
> - parse it as is up to the maximum and then do ceil() or floor() on top of that
> - copy only necessary amount of the (sub)string and parse that.
Yes, my use case is the fixed point parsing, but I suppose we are implementing
things here for reuse. Also, the default behavior of the previous fixed point
parsing in IIO is flooring the result, which leads to the same result as
ignoring further digits.
> The problem with precision is that we need to also consider floor() or ceil()
> and I don't think this should be burden of the library as it's individual
> preference of each of the callers (users). At least for the starter, we will
> see if it's only one approach is used, we may incorporate it into the library
> code.
>
> The easiest way out is to just consider the input invalid if it overflows the
> given type (s32 or s64).
>
> But we need to have an agreement what will be the representation of the
> fixed-width float numbers in the kernel? Currently IIO uses
> struct float // name is crafted for simplicity
> {
> int integer;
> int fraction;
> }
Yes, but to represent things like that, an assumption is made to the precision that
"fraction" carries.
>
> This parser wants AFAIU to have at the end of the day something like
>
> struct float
> {
> s64 integer;
> s64 fraction;
> }
>
> but also wants to have the fraction part be limited in some cases to s32
> or so:
>
> struct float
> {
> s64 integer;
> s32 fraction; // precision may be lost if input is longer
> }
>
> Maybe we want to have kstrtof32() and kstrtof64() for these two cases?
>
> With that we will always consider the fraction part as 32- or 64-bit,
> imply floor() on the fraction for the sake of simplicity and require
> it to be NUL-terminated with possible trailing '\n'.
I think this is a good idea, but calling it float or fixed point itself
is a bit confusing as float often refers to the IEEE 754 standard and
fixed point types is often expressed in Q-format.
--
Kind regards,
Rodrigo Alencar
On Fri, Mar 27, 2026 at 10:11:56AM +0000, Rodrigo Alencar wrote:
> On 26/03/27 11:17AM, Andy Shevchenko wrote:
> > On Fri, Mar 27, 2026 at 09:45:17AM +0100, Petr Mladek wrote:
> > > On Fri 2026-03-20 16:27:27, Rodrigo Alencar via B4 Relay wrote:
...
> > > > +extern ssize_t __must_check simple_strntoull(const char *startp, const char **endp,
> > > > + unsigned int base, size_t max_chars,
> > > > + unsigned long long *res);
> > >
> > > Sigh, naming is hard. I personally find it a bit confusing that the
> > > name is too similar to the unsafe API.
> > >
> > > IMHO, the semantic of the new API is closer to kstrtoull().
> > > It just limits the size, so I would call it kstrntoull().
> >
> > It's not. kstrto*() quite strict about the input, this one is actually relaxed
> > variant, so I wouldn't mix these two groups.
> >
> > > Also I would use int as the return parameter, see below.
...
> > TBH, I am skeptical about this approach. My main objection is max_chars
> > parameter. If we want to limit the input strictly to the given number of
> > characters, we have to copy the string and then just use kstrto*() in a normal
> > way. The whole idea of that parameter is to be able to parse the fractional
> > part of the float number as 'iiiii.fffff', where 'i' is for integer part, and
> > 'f' for the fractional. Since we have *endp, we may simply check that.
>
> A max_chars would not be only useful for that. It can prevent out-of-bounds
> reads when the input isn't NUL-terminated (like buffers, file chunks,
> network packets, memory-mapped data, ....). Even if there is a NUL later in
> memory, a regular strtoull() function may consume characters that are outside
> the field one intends to parse.
Okay, but is it the current case or just an attempt to solve the problem that
doesn't exist (yet)?
> > In case if we want to parse only, say, 6 digits and input is longer there are
> > a few options (in my personal preferences, the first is the better):
> > - consider the input invalid
> > - parse it as is up to the maximum and then do ceil() or floor() on top of that
> > - copy only necessary amount of the (sub)string and parse that.
>
> Yes, my use case is the fixed point parsing, but I suppose we are implementing
> things here for reuse.
Yes, I'm full for reuse, but I want to have it balanced between complexity,
existing use cases and possible reuse in the future.
> Also, the default behavior of the previous fixed point
> parsing in IIO is flooring the result, which leads to the same result as
> ignoring further digits.
Correct, I also lean to implying floor() (as you can read below).
> > The problem with precision is that we need to also consider floor() or ceil()
> > and I don't think this should be burden of the library as it's individual
> > preference of each of the callers (users). At least for the starter, we will
> > see if it's only one approach is used, we may incorporate it into the library
> > code.
> >
> > The easiest way out is to just consider the input invalid if it overflows the
> > given type (s32 or s64).
> >
> > But we need to have an agreement what will be the representation of the
> > fixed-width float numbers in the kernel? Currently IIO uses
> > struct float // name is crafted for simplicity
> > {
> > int integer;
> > int fraction;
> > }
>
> Yes, but to represent things like that, an assumption is made to the precision that
> "fraction" carries.
Correct.
> > This parser wants AFAIU to have at the end of the day something like
> >
> > struct float
> > {
> > s64 integer;
> > s64 fraction;
> > }
> >
> > but also wants to have the fraction part be limited in some cases to s32
> > or so:
> >
> > struct float
> > {
> > s64 integer;
> > s32 fraction; // precision may be lost if input is longer
> > }
> >
> > Maybe we want to have kstrtof32() and kstrtof64() for these two cases?
> >
> > With that we will always consider the fraction part as 32- or 64-bit,
> > imply floor() on the fraction for the sake of simplicity and require
> > it to be NUL-terminated with possible trailing '\n'.
>
> I think this is a good idea, but calling it float or fixed point itself
> is a bit confusing as float often refers to the IEEE 754 standard and
> fixed point types is often expressed in Q-format.
Yeah... I am lack of better naming.
--
With Best Regards,
Andy Shevchenko
On 26/03/27 12:21PM, Andy Shevchenko wrote:
> On Fri, Mar 27, 2026 at 10:11:56AM +0000, Rodrigo Alencar wrote:
> > On 26/03/27 11:17AM, Andy Shevchenko wrote:
> > > On Fri, Mar 27, 2026 at 09:45:17AM +0100, Petr Mladek wrote:
> > > > On Fri 2026-03-20 16:27:27, Rodrigo Alencar via B4 Relay wrote:
>
> ...
>
> > > > > +extern ssize_t __must_check simple_strntoull(const char *startp, const char **endp,
> > > > > + unsigned int base, size_t max_chars,
> > > > > + unsigned long long *res);
> > > >
> > > > Sigh, naming is hard. I personally find it a bit confusing that the
> > > > name is too similar to the unsafe API.
> > > >
> > > > IMHO, the semantic of the new API is closer to kstrtoull().
> > > > It just limits the size, so I would call it kstrntoull().
> > >
> > > It's not. kstrto*() quite strict about the input, this one is actually relaxed
> > > variant, so I wouldn't mix these two groups.
> > >
> > > > Also I would use int as the return parameter, see below.
>
> ...
>
> > > TBH, I am skeptical about this approach. My main objection is max_chars
> > > parameter. If we want to limit the input strictly to the given number of
> > > characters, we have to copy the string and then just use kstrto*() in a normal
> > > way. The whole idea of that parameter is to be able to parse the fractional
> > > part of the float number as 'iiiii.fffff', where 'i' is for integer part, and
> > > 'f' for the fractional. Since we have *endp, we may simply check that.
> >
> > A max_chars would not be only useful for that. It can prevent out-of-bounds
> > reads when the input isn't NUL-terminated (like buffers, file chunks,
> > network packets, memory-mapped data, ....). Even if there is a NUL later in
> > memory, a regular strtoull() function may consume characters that are outside
> > the field one intends to parse.
>
> Okay, but is it the current case or just an attempt to solve the problem that
> doesn't exist (yet)?
The current case can be seen as such. Copying the string and use regular ksrto*()
requires an unecessary scan of string from the user side, which is something that
_parse_integer_limit() already does, mostly because it checks for digits and stops
at any non-digit character. In the IIO case, we also want control over the consumed
characters because there are weird terminations like "dB", so having an implementation
like this ends up with a cleaner sequence of steps.
> > > In case if we want to parse only, say, 6 digits and input is longer there are
> > > a few options (in my personal preferences, the first is the better):
> > > - consider the input invalid
> > > - parse it as is up to the maximum and then do ceil() or floor() on top of that
> > > - copy only necessary amount of the (sub)string and parse that.
> >
> > Yes, my use case is the fixed point parsing, but I suppose we are implementing
> > things here for reuse.
>
> Yes, I'm full for reuse, but I want to have it balanced between complexity,
> existing use cases and possible reuse in the future.
Not seeing complexity here as in this case I am just exposing something
that already exists! No need for a completely different implementation.
I just want to get an agreement on the naming and interface prototype.
Bringing back the discussion again just because I suppose Petr havent even
seen the v8 of this patch series. If kstrtox.h is the right place for this,
kstrntoull() sounds like ideal. Specially because simple_strto*() is already
labeled as unsafe and kstrnto*() != kstrto*().
> > Also, the default behavior of the previous fixed point
> > parsing in IIO is flooring the result, which leads to the same result as
> > ignoring further digits.
>
> Correct, I also lean to implying floor() (as you can read below).
>
> > > The problem with precision is that we need to also consider floor() or ceil()
> > > and I don't think this should be burden of the library as it's individual
> > > preference of each of the callers (users). At least for the starter, we will
> > > see if it's only one approach is used, we may incorporate it into the library
> > > code.
> > >
> > > The easiest way out is to just consider the input invalid if it overflows the
> > > given type (s32 or s64).
> > >
> > > But we need to have an agreement what will be the representation of the
> > > fixed-width float numbers in the kernel? Currently IIO uses
> > > struct float // name is crafted for simplicity
> > > {
> > > int integer;
> > > int fraction;
> > > }
> >
> > Yes, but to represent things like that, an assumption is made to the precision that
> > "fraction" carries.
>
> Correct.
>
> > > This parser wants AFAIU to have at the end of the day something like
> > >
> > > struct float
> > > {
> > > s64 integer;
> > > s64 fraction;
> > > }
> > >
> > > but also wants to have the fraction part be limited in some cases to s32
> > > or so:
> > >
> > > struct float
> > > {
> > > s64 integer;
> > > s32 fraction; // precision may be lost if input is longer
> > > }
> > >
> > > Maybe we want to have kstrtof32() and kstrtof64() for these two cases?
> > >
> > > With that we will always consider the fraction part as 32- or 64-bit,
> > > imply floor() on the fraction for the sake of simplicity and require
> > > it to be NUL-terminated with possible trailing '\n'.
> >
> > I think this is a good idea, but calling it float or fixed point itself
> > is a bit confusing as float often refers to the IEEE 754 standard and
> > fixed point types is often expressed in Q-format.
>
> Yeah... I am lack of better naming.
decimals is the name, but they are often represented as:
DECIMAL = INT * 10^X + FRAC
in a single 64-bit number, which would be fine for my end use case.
However IIO decimal fixed point parsing is out there for quite some time a
lot of drivers use that. The interface often relies on breaking parsed values
into an integer array (for standard attributes int val and int val2 are expected).
--
Kind regards,
Rodrigo Alencar
On 26/03/27 03:17PM, Rodrigo Alencar wrote:
> On 26/03/27 12:21PM, Andy Shevchenko wrote:
> > On Fri, Mar 27, 2026 at 10:11:56AM +0000, Rodrigo Alencar wrote:
> > > On 26/03/27 11:17AM, Andy Shevchenko wrote:
> > > > On Fri, Mar 27, 2026 at 09:45:17AM +0100, Petr Mladek wrote:
> > > > > On Fri 2026-03-20 16:27:27, Rodrigo Alencar via B4 Relay wrote:
...
> > > > Maybe we want to have kstrtof32() and kstrtof64() for these two cases?
> > > >
> > > > With that we will always consider the fraction part as 32- or 64-bit,
> > > > imply floor() on the fraction for the sake of simplicity and require
> > > > it to be NUL-terminated with possible trailing '\n'.
> > >
> > > I think this is a good idea, but calling it float or fixed point itself
> > > is a bit confusing as float often refers to the IEEE 754 standard and
> > > fixed point types is often expressed in Q-format.
> >
> > Yeah... I am lack of better naming.
>
> decimals is the name, but they are often represented as:
>
> DECIMAL = INT * 10^X + FRAC
>
> in a single 64-bit number, which would be fine for my end use case.
> However IIO decimal fixed point parsing is out there for quite some time a
> lot of drivers use that. The interface often relies on breaking parsed values
> into an integer array (for standard attributes int val and int val2 are expected).
Thinking about this again and in IIO drivers we end up doing something like:
val64 = (u64)val * MICRO + val2;
so that drivers often work with scaled versions of the decimal value.
then, would it make sense to have a function that already outputs such value?
That would allow to have more freedom over the 64-bit split between integer
and fractional parts.
As a draft:
static int _kstrtodec64(const char *s, unsigned int scale, u64 *res)
{
u64 _res = 0, _frac = 0;
unsigned int rv;
if (*s != '.') {
rv = _parse_integer(s, 10, &_res);
if (rv & KSTRTOX_OVERFLOW)
return -ERANGE;
if (rv == 0)
return -EINVAL;
s += rv;
}
if (*s == '.') {
s++;
rv = _parse_integer_limit(s, 10, &_frac, scale);
if (rv & KSTRTOX_OVERFLOW)
return -ERANGE;
if (rv == 0)
return -EINVAL;
s += rv;
if (rv < scale)
_frac *= int_pow(10, scale - rv);
while (isdigit(*s)) /* truncate */
s++;
}
if (*s == '\n')
s++;
if (*s)
return -EINVAL;
if (check_mul_overflow(_res, int_pow(10, scale), &_res) ||
check_add_overflow(_res, _frac, &_res))
return -ERANGE;
*res = _res;
return 0;
}
noinline
int kstrtoudec64(const char *s, unsigned int scale, u64 *res)
{
if (s[0] == '+')
s++;
return _kstrtodec64(s, scale, res);
}
EXPORT_SYMBOL(kstrtoudec64);
noinline
int kstrtosdec64(const char *s, unsigned int scale, s64 *res)
{
u64 tmp;
int rv;
if (s[0] == '-') {
rv = _kstrtodec64(s + 1, scale, &tmp);
if (rv < 0)
return rv;
if ((s64)-tmp > 0)
return -ERANGE;
*res = -tmp;
} else {
rv = kstrtoudec64(s, scale, &tmp);
if (rv < 0)
return rv;
if ((s64)tmp < 0)
return -ERANGE;
*res = tmp;
}
return 0;
}
EXPORT_SYMBOL(kstrtosdec64);
e.g., kstrtosdec64() or kstrtoudec64() parses "3.1415" with scale 3 into 3141
--
Kind regards,
Rodrigo Alencar
On Mon 2026-03-30 13:49:48, Rodrigo Alencar wrote:
> On 26/03/27 03:17PM, Rodrigo Alencar wrote:
> > On 26/03/27 12:21PM, Andy Shevchenko wrote:
> > > On Fri, Mar 27, 2026 at 10:11:56AM +0000, Rodrigo Alencar wrote:
> > > > On 26/03/27 11:17AM, Andy Shevchenko wrote:
> > > > > On Fri, Mar 27, 2026 at 09:45:17AM +0100, Petr Mladek wrote:
> > > > > > On Fri 2026-03-20 16:27:27, Rodrigo Alencar via B4 Relay wrote:
>
> ...
>
> > > > > Maybe we want to have kstrtof32() and kstrtof64() for these two cases?
> > > > >
> > > > > With that we will always consider the fraction part as 32- or 64-bit,
> > > > > imply floor() on the fraction for the sake of simplicity and require
> > > > > it to be NUL-terminated with possible trailing '\n'.
> > > >
> > > > I think this is a good idea, but calling it float or fixed point itself
> > > > is a bit confusing as float often refers to the IEEE 754 standard and
> > > > fixed point types is often expressed in Q-format.
> > >
> > > Yeah... I am lack of better naming.
> >
> > decimals is the name, but they are often represented as:
> >
> > DECIMAL = INT * 10^X + FRAC
> >
> > in a single 64-bit number, which would be fine for my end use case.
> > However IIO decimal fixed point parsing is out there for quite some time a
> > lot of drivers use that. The interface often relies on breaking parsed values
> > into an integer array (for standard attributes int val and int val2 are expected).
>
> Thinking about this again and in IIO drivers we end up doing something like:
>
> val64 = (u64)val * MICRO + val2;
>
> so that drivers often work with scaled versions of the decimal value.
> then, would it make sense to have a function that already outputs such value?
> That would allow to have more freedom over the 64-bit split between integer
> and fractional parts.
> As a draft:
My understanding is that you want to allow parsing frequencies
in the range from microHz to GHz.
So, you might want to support input in simple float numbers
with some precision, for example, 1.2GHz, 0.345Hz, ...
By simple, I mean that there is no x10^3 or so.
> static int _kstrtodec64(const char *s, unsigned int scale, u64 *res)
I would personally change this to something like:
static int _unit_float_ktstrtodec64(const char *s, unsigned int precision, u64 *res, char **unit)
It would allow to read float number in the the format XXXX.YYYYunit,
for example 1.2Ghz
, where:
+ _unit_ means that it might set @unit pointer which point to the unit
string right after the number part.
+ _float_ means that it will be able to read float numbers
+ @precisions parameter defines the number of digits accepted
after the radix point. It is also used as multiplier for scaling
the output number.
+ @res is pointer to the read number multiplied by the given
@precision
+ @unit will be set to string after the number
For example:
+ s="1.2GHz", precision=3 will result in *res=1200, *unit="GHz"
+ s="0.0100004", precision=3 will result in *res=10, *unit=""
+ s=1.234567GHz, precision=3 will result in *res=1235, *unit="GHz"
Note that the result is rounded in the last example.
The function might be used like simple_strtoull() in memparse(),
see lib/cmdline.c. Which is able to read the given size in B
and handle various units like kB, GB, ...
> {
> u64 _res = 0, _frac = 0;
> unsigned int rv;
>
> if (*s != '.') {
> rv = _parse_integer(s, 10, &_res);
> if (rv & KSTRTOX_OVERFLOW)
> return -ERANGE;
> if (rv == 0)
> return -EINVAL;
> s += rv;
> }
>
> if (*s == '.') {
> s++;
> rv = _parse_integer_limit(s, 10, &_frac, scale);
> if (rv & KSTRTOX_OVERFLOW)
> return -ERANGE;
> if (rv == 0)
> return -EINVAL;
> s += rv;
> if (rv < scale)
> _frac *= int_pow(10, scale - rv);
> while (isdigit(*s)) /* truncate */
> s++;
We might/should use the first digit to round the _frac.
> }
>
> if (*s == '\n')
> s++;
> if (*s)
> return -EINVAL;
I would omit this. Instead I would set @unit pointer so that the
caller might handle units defined after the number.
> if (check_mul_overflow(_res, int_pow(10, scale), &_res) ||
> check_add_overflow(_res, _frac, &_res))
> return -ERANGE;
>
> *res = _res;
> return 0;
> }
Otherwise, this approach looks sensible to me. IMHO, some generic
API for reading numbers with misc units should be usable in more
situations. And it would make the kernel interface more user
friendly.
Of course, we must not over-engineer it. But the above does not
look much more complex than we already have.
Best Regards,
Petr
PS: I am sorry if I used some mathematical terms a wrong way.
I am not a native speaker. And it is a long time since
I worked with float numbers.
On 26/04/01 02:22PM, Petr Mladek wrote:
> On Mon 2026-03-30 13:49:48, Rodrigo Alencar wrote:
> > On 26/03/27 03:17PM, Rodrigo Alencar wrote:
> > > On 26/03/27 12:21PM, Andy Shevchenko wrote:
> > > > On Fri, Mar 27, 2026 at 10:11:56AM +0000, Rodrigo Alencar wrote:
> > > > > On 26/03/27 11:17AM, Andy Shevchenko wrote:
> > > > > > On Fri, Mar 27, 2026 at 09:45:17AM +0100, Petr Mladek wrote:
> > > > > > > On Fri 2026-03-20 16:27:27, Rodrigo Alencar via B4 Relay wrote:
> >
> > ...
> >
> > > > > > Maybe we want to have kstrtof32() and kstrtof64() for these two cases?
> > > > > >
> > > > > > With that we will always consider the fraction part as 32- or 64-bit,
> > > > > > imply floor() on the fraction for the sake of simplicity and require
> > > > > > it to be NUL-terminated with possible trailing '\n'.
> > > > >
> > > > > I think this is a good idea, but calling it float or fixed point itself
> > > > > is a bit confusing as float often refers to the IEEE 754 standard and
> > > > > fixed point types is often expressed in Q-format.
> > > >
> > > > Yeah... I am lack of better naming.
> > >
> > > decimals is the name, but they are often represented as:
> > >
> > > DECIMAL = INT * 10^X + FRAC
> > >
> > > in a single 64-bit number, which would be fine for my end use case.
> > > However IIO decimal fixed point parsing is out there for quite some time a
> > > lot of drivers use that. The interface often relies on breaking parsed values
> > > into an integer array (for standard attributes int val and int val2 are expected).
> >
> > Thinking about this again and in IIO drivers we end up doing something like:
> >
> > val64 = (u64)val * MICRO + val2;
> >
> > so that drivers often work with scaled versions of the decimal value.
> > then, would it make sense to have a function that already outputs such value?
> > That would allow to have more freedom over the 64-bit split between integer
> > and fractional parts.
> > As a draft:
>
> My understanding is that you want to allow parsing frequencies
> in the range from microHz to GHz.
Correct, the ABI requires the values in Hz, and I would like to support
micro Hz resolution, so that 10 GHz can be represensted as:
10000000000.000000
> So, you might want to support input in simple float numbers
> with some precision, for example, 1.2GHz, 0.345Hz, ...
>
> By simple, I mean that there is no x10^3 or so.
>
> > static int _kstrtodec64(const char *s, unsigned int scale, u64 *res)
>
> I would personally change this to something like:
>
> static int _unit_float_ktstrtodec64(const char *s, unsigned int precision, u64 *res, char **unit)
I don't really need "unit" for my specific use case, IIUC this pattern is not
something to be handled by kstrto*(), because those function should requires NUL
termination. I am not sure why is that, but I like the idea of returning a
const char* pointer to the end of the conversion (that was the whole point of
having something like kstrntoull())
> It would allow to read float number in the the format XXXX.YYYYunit,
> for example 1.2Ghz
>
> , where:
>
> + _unit_ means that it might set @unit pointer which point to the unit
> string right after the number part.
as mentioned, the units is defined in the ABI, so this part is not really needed.
> + _float_ means that it will be able to read float numbers
its a decimal fixed precision, decimal point should not float.
>
> + @precisions parameter defines the number of digits accepted
> after the radix point. It is also used as multiplier for scaling
> the output number.
precision != scale, for this case we have a fixed precision of 64-bits.
while scale is passed as parameter.
Reference:
https://www.ibm.com/docs/ro/informix-servers/12.10.0?topic=types-decimalps-data
>
> + @res is pointer to the read number multiplied by the given
> @precision
>
> + @unit will be set to string after the number
>
> For example:
>
> + s="1.2GHz", precision=3 will result in *res=1200, *unit="GHz"
> + s="0.0100004", precision=3 will result in *res=10, *unit=""
> + s=1.234567GHz, precision=3 will result in *res=1235, *unit="GHz"
>
> Note that the result is rounded in the last example.
>
> The function might be used like simple_strtoull() in memparse(),
> see lib/cmdline.c. Which is able to read the given size in B
> and handle various units like kB, GB, ...
As dicussed above, scaling the value based on the units is not my use case.
>
> > {
> > u64 _res = 0, _frac = 0;
> > unsigned int rv;
> >
> > if (*s != '.') {
> > rv = _parse_integer(s, 10, &_res);
> > if (rv & KSTRTOX_OVERFLOW)
> > return -ERANGE;
> > if (rv == 0)
> > return -EINVAL;
> > s += rv;
> > }
> >
> > if (*s == '.') {
> > s++;
> > rv = _parse_integer_limit(s, 10, &_frac, scale);
> > if (rv & KSTRTOX_OVERFLOW)
> > return -ERANGE;
> > if (rv == 0)
> > return -EINVAL;
> > s += rv;
> > if (rv < scale)
> > _frac *= int_pow(10, scale - rv);
> > while (isdigit(*s)) /* truncate */
> > s++;
>
> We might/should use the first digit to round the _frac.
flooring should not be a problem if it is documented like that.
I suppose we cannot afford to carry over all roundings from
subsequent digits. If so, we would be parsing it all and use
div64 which I would like avoid.
> > }
> >
> > if (*s == '\n')
> > s++;
> > if (*s)
> > return -EINVAL;
>
> I would omit this. Instead I would set @unit pointer so that the
> caller might handle units defined after the number.
I understand that this is the whole point of creating a kstrto*()
function.
> > if (check_mul_overflow(_res, int_pow(10, scale), &_res) ||
> > check_add_overflow(_res, _frac, &_res))
> > return -ERANGE;
> >
> > *res = _res;
> > return 0;
> > }
>
> Otherwise, this approach looks sensible to me. IMHO, some generic
> API for reading numbers with misc units should be usable in more
> situations. And it would make the kernel interface more user
> friendly.
>
> Of course, we must not over-engineer it. But the above does not
> look much more complex than we already have.
I really appreciate your time looking into this, thanks.
--
Kind regards,
Rodrigo Alencar
On 26/03/30 01:49PM, Rodrigo Alencar wrote:
> On 26/03/27 03:17PM, Rodrigo Alencar wrote:
> > On 26/03/27 12:21PM, Andy Shevchenko wrote:
> > > On Fri, Mar 27, 2026 at 10:11:56AM +0000, Rodrigo Alencar wrote:
> > > > On 26/03/27 11:17AM, Andy Shevchenko wrote:
> > > > > On Fri, Mar 27, 2026 at 09:45:17AM +0100, Petr Mladek wrote:
> > > > > > On Fri 2026-03-20 16:27:27, Rodrigo Alencar via B4 Relay wrote:
>
> ...
>
> > > > > Maybe we want to have kstrtof32() and kstrtof64() for these two cases?
> > > > >
> > > > > With that we will always consider the fraction part as 32- or 64-bit,
> > > > > imply floor() on the fraction for the sake of simplicity and require
> > > > > it to be NUL-terminated with possible trailing '\n'.
> > > >
> > > > I think this is a good idea, but calling it float or fixed point itself
> > > > is a bit confusing as float often refers to the IEEE 754 standard and
> > > > fixed point types is often expressed in Q-format.
> > >
> > > Yeah... I am lack of better naming.
> >
> > decimals is the name, but they are often represented as:
> >
> > DECIMAL = INT * 10^X + FRAC
> >
> > in a single 64-bit number, which would be fine for my end use case.
> > However IIO decimal fixed point parsing is out there for quite some time a
> > lot of drivers use that. The interface often relies on breaking parsed values
> > into an integer array (for standard attributes int val and int val2 are expected).
>
> Thinking about this again and in IIO drivers we end up doing something like:
>
> val64 = (u64)val * MICRO + val2;
>
> so that drivers often work with scaled versions of the decimal value.
> then, would it make sense to have a function that already outputs such value?
> That would allow to have more freedom over the 64-bit split between integer
> and fractional parts.
> As a draft:
>
> static int _kstrtodec64(const char *s, unsigned int scale, u64 *res)
> {
> u64 _res = 0, _frac = 0;
> unsigned int rv;
>
> if (*s != '.') {
> rv = _parse_integer(s, 10, &_res);
> if (rv & KSTRTOX_OVERFLOW)
> return -ERANGE;
> if (rv == 0)
> return -EINVAL;
> s += rv;
> }
>
> if (*s == '.') {
> s++;
> rv = _parse_integer_limit(s, 10, &_frac, scale);
> if (rv & KSTRTOX_OVERFLOW)
> return -ERANGE;
> if (rv == 0)
> return -EINVAL;
> s += rv;
> if (rv < scale)
> _frac *= int_pow(10, scale - rv);
> while (isdigit(*s)) /* truncate */
> s++;
> }
>
> if (*s == '\n')
> s++;
> if (*s)
> return -EINVAL;
>
> if (check_mul_overflow(_res, int_pow(10, scale), &_res) ||
> check_add_overflow(_res, _frac, &_res))
> return -ERANGE;
>
> *res = _res;
> return 0;
> }
>
> noinline
> int kstrtoudec64(const char *s, unsigned int scale, u64 *res)
> {
> if (s[0] == '+')
> s++;
> return _kstrtodec64(s, scale, res);
> }
> EXPORT_SYMBOL(kstrtoudec64);
>
> noinline
> int kstrtosdec64(const char *s, unsigned int scale, s64 *res)
> {
> u64 tmp;
> int rv;
>
> if (s[0] == '-') {
> rv = _kstrtodec64(s + 1, scale, &tmp);
> if (rv < 0)
> return rv;
> if ((s64)-tmp > 0)
> return -ERANGE;
> *res = -tmp;
> } else {
> rv = kstrtoudec64(s, scale, &tmp);
> if (rv < 0)
> return rv;
> if ((s64)tmp < 0)
> return -ERANGE;
> *res = tmp;
> }
> return 0;
> }
> EXPORT_SYMBOL(kstrtosdec64);
>
> e.g., kstrtosdec64() or kstrtoudec64() parses "3.1415" with scale 3 into 3141
Hi Jonathan,
developing more on that, I wouldn't need to create a iio_str_to_fixpoint64(),
what do you think on new format types:
#define IIO_VAL_DECIMAL64_1 101
#define IIO_VAL_DECIMAL64_2 102
#define IIO_VAL_DECIMAL64_3 103
#define IIO_VAL_DECIMAL64_4 104
#define IIO_VAL_DECIMAL64_5 105
#define IIO_VAL_DECIMAL64_6 106
#define IIO_VAL_DECIMAL64_7 107
#define IIO_VAL_DECIMAL64_8 108
#define IIO_VAL_DECIMAL64_9 109
#define IIO_VAL_DECIMAL64_10 110
#define IIO_VAL_DECIMAL64_11 111
#define IIO_VAL_DECIMAL64_12 112
#define IIO_VAL_DECIMAL64_13 113
#define IIO_VAL_DECIMAL64_14 114
#define IIO_VAL_DECIMAL64_15 115
#define IIO_VAL_DECIMAL64_MILLI IIO_VAL_DECIMAL64_3
#define IIO_VAL_DECIMAL64_MICRO IIO_VAL_DECIMAL64_6
#define IIO_VAL_DECIMAL64_NANO IIO_VAL_DECIMAL64_9
#define IIO_VAL_DECIMAL64_PICO IIO_VAL_DECIMAL64_12
#define IIO_VAL_DECIMAL64_FEMTO IIO_VAL_DECIMAL64_15
which gets stored as 64-bit, and represent the decimal scaled value.
That would also work for the PLL driver (using IIO_VAL_DECIMAL64_MICRO):
- It supports frequency range from 1 to 26 GHz with micro Hz resolution
- In the driver a 64-bit value: (val * MICRO + val2) is already created
anyways.
I would leverage something like kstrtodec64() in iio_write_channel_info().
That way, I would drop the changes on the iio fixpoint parse, which I think
it would do better with something like kstrntoull() to be able to handle that
"dB" suffix.
So for now, I may have the following approaches:
- new kstrntoull() function: to have control over the parsing, whithout
requiring NUL-termination, avoiding unecessary string scanning or copying.
covered in v8.
- expose a "safe" simple_strntoull(): minimal changes to vsprintf.c, this
is covered by this patch series (v9), and it similar solution to kstrntoull().
- new kstrtodec64() function: parse decimal numbers as 64-bit with NUL-termination.
Might be covered in a v10, if it is a good idea.
let me know your thoughts.
--
Kind regards,
Rodrigo Alencar
© 2016 - 2026 Red Hat, Inc.