From: Salah Triki <salah.triki@gmail.com>
To: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: "Nuno Sá" <nuno.sa@analog.com>,
"Michael Hennerich" <Michael.Hennerich@analog.com>,
"Jonathan Cameron" <jic23@kernel.org>,
"David Lechner" <dlechner@baylibre.com>,
"Andy Shevchenko" <andy@kernel.org>,
linux@analog.com, linux-iio@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] iio: adc: ad4030: fix invalid oversampling_ratio validation
Date: Mon, 31 Aug 2026 12:01:12 +0100 [thread overview]
Message-ID: <apVe-Gcp6MT8E9Dw@pc> (raw)
In-Reply-To: <aoxbJzV5OQ839gAN@ashevche-desk.local>
Hi Andy,
Sorry for the confusion - my previous v3 email crossed paths with yours.
Regarding is_power_of_2(avg_val): following feedback from David and
Jonathan, we agreed to keep only the check for non-positive values
(avg_val <= 0) to avoid the invalid ilog2(0) call. Checking for non-powers
of two was dropped to preserve standard IIO attribute rounding behavior.
I will sent v4 addressing all your formatting and style requirements.
Thanks for your patience and thorough review.
Best regards,
Salah Triki
On Mon, Aug 24, 2026 at 05:54:31PM +0300, Andy Shevchenko wrote:
> On Mon, Aug 24, 2026 at 06:10:26AM +0100, Salah Triki wrote:
> > ad4030_set_avg_frame_len() computes avg_log2 = ilog2(avg_val) before
> > validating avg_val. The existing check (avg_val < 0) allows avg_val == 0
>
> Try to write the sentences straight to the point using plain English,
> Avoid using C language as substitute of plain English.
>
> > to pass through, resulting in ilog2(0) being called with undefined/garbage
> > behavior.
> >
> > Reject non-positive values (avg_val <= 0) before computing log2.
> >
> > This issue was identified with assistance from Claude AI and manually
> > verified against the code.
>
> > Changes since v1:
> > - Added note stating the issue was identified with assistance from
> > Claude AI and verified manually.
>
> Assisted-by?
>
> ...
>
> > static int ad4030_set_avg_frame_len(struct iio_dev *dev, int avg_val)
> > {
> > struct ad4030_state *st = iio_priv(dev);
> > - unsigned int avg_log2 = ilog2(avg_val);
> > + unsigned int avg_log2;
>
> Have you read my comments to the previous version?
>
> > unsigned int last_avg_idx = ARRAY_SIZE(ad4030_average_modes) - 1;
> > int freq_hz;
> > int ret;
> >
> > - if (avg_val < 0 || avg_val > ad4030_average_modes[last_avg_idx])
> > + if (avg_val <= 0 || avg_val > ad4030_average_modes[last_avg_idx])
> > return -EINVAL;
>
> Ditto.
>
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
prev parent reply other threads:[~2026-08-31 11:01 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-24 5:10 [PATCH v3] iio: adc: ad4030: fix invalid oversampling_ratio validation Salah Triki
2026-08-24 14:54 ` Andy Shevchenko
2026-08-31 11:01 ` Salah Triki [this message]
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=apVe-Gcp6MT8E9Dw@pc \
--to=salah.triki@gmail.com \
--cc=Michael.Hennerich@analog.com \
--cc=andriy.shevchenko@intel.com \
--cc=andy@kernel.org \
--cc=dlechner@baylibre.com \
--cc=jic23@kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@analog.com \
--cc=nuno.sa@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.