From: Andy Shevchenko <andriy.shevchenko@intel.com>
To: Rodrigo Alencar <455.rodrigo.alencar@gmail.com>
Cc: rodrigo.alencar@analog.com, linux-kernel@vger.kernel.org,
linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
linux-doc@vger.kernel.org, Jonathan Cameron <jic23@kernel.org>,
David Lechner <dlechner@baylibre.com>,
Andy Shevchenko <andy@kernel.org>,
Lars-Peter Clausen <lars@metafoo.de>,
Michael Hennerich <Michael.Hennerich@analog.com>,
Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
Jonathan Corbet <corbet@lwn.net>
Subject: Re: [PATCH v5 2/8] iio: core: add fixed point parsing with 64-bit parts
Date: Mon, 26 Jan 2026 15:35:34 +0200 [thread overview]
Message-ID: <aXdtpkL5QUhhB_hh@smile.fi.intel.com> (raw)
In-Reply-To: <byitgzjli5gsq5v66topve7ip3inkk2udwhuihjdp6bknnkmos@tv226l7tek7s>
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
next prev parent reply other threads:[~2026-01-26 13:35 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-23 15:53 [PATCH v5 0/8] ADF41513/ADF41510 PLL frequency synthesizers Rodrigo Alencar via B4 Relay
2026-01-23 15:53 ` [PATCH v5 1/8] dt-bindings: iio: frequency: add adf41513 Rodrigo Alencar via B4 Relay
2026-01-23 15:53 ` [PATCH v5 2/8] iio: core: add fixed point parsing with 64-bit parts Rodrigo Alencar via B4 Relay
2026-01-23 16:14 ` Rodrigo Alencar
2026-01-23 16:29 ` Rodrigo Alencar
2026-01-26 11:49 ` Andy Shevchenko
2026-01-26 12:42 ` Rodrigo Alencar
2026-01-26 13:35 ` Andy Shevchenko [this message]
2026-01-26 14:26 ` Rodrigo Alencar
2026-01-26 14:53 ` Andy Shevchenko
2026-01-26 15:20 ` Rodrigo Alencar
2026-01-26 15:30 ` Rodrigo Alencar
2026-01-26 16:07 ` Andy Shevchenko
2026-01-26 16:55 ` Rodrigo Alencar
2026-01-26 16:05 ` Andy Shevchenko
2026-01-26 14:56 ` Rodrigo Alencar
2026-01-26 15:06 ` Andy Shevchenko
2026-01-23 15:53 ` [PATCH v5 3/8] iio: test: add kunit test for fixed-point parsing Rodrigo Alencar via B4 Relay
2026-01-23 15:53 ` [PATCH v5 4/8] iio: frequency: adf41513: driver implementation Rodrigo Alencar via B4 Relay
2026-01-23 15:53 ` [PATCH v5 5/8] iio: frequency: adf41513: handle LE synchronization feature Rodrigo Alencar via B4 Relay
2026-01-23 15:53 ` [PATCH v5 6/8] iio: frequency: adf41513: features on frequency change Rodrigo Alencar via B4 Relay
2026-01-23 15:53 ` [PATCH v5 7/8] docs: iio: add documentation for adf41513 driver Rodrigo Alencar via B4 Relay
2026-01-23 15:53 ` [PATCH v5 8/8] Documentation: ABI: testing: add common ABI file for iio/frequency Rodrigo Alencar via B4 Relay
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aXdtpkL5QUhhB_hh@smile.fi.intel.com \
--to=andriy.shevchenko@intel.com \
--cc=455.rodrigo.alencar@gmail.com \
--cc=Michael.Hennerich@analog.com \
--cc=andy@kernel.org \
--cc=conor+dt@kernel.org \
--cc=corbet@lwn.net \
--cc=devicetree@vger.kernel.org \
--cc=dlechner@baylibre.com \
--cc=jic23@kernel.org \
--cc=krzk+dt@kernel.org \
--cc=lars@metafoo.de \
--cc=linux-doc@vger.kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=robh@kernel.org \
--cc=rodrigo.alencar@analog.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox