lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAHp75VdobsXLQXsopyi1Ms3AHxL=wWZMjfLQjRRNWyhr6Q7q7Q@mail.gmail.com>
Date: Fri, 8 Nov 2024 15:33:14 +0200
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Matteo Martelli <matteomartelli3@...il.com>
Cc: victor.duicu@...rochip.com, jic23@...nel.org, lars@...afoo.de, 
	marius.cristea@...rochip.com, linux-iio@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v8] iio: adc: pac1921: Add ACPI support to Microchip pac1921

On Fri, Nov 8, 2024 at 11:56 AM Matteo Martelli
<matteomartelli3@...il.com> wrote:
> On Fri, 8 Nov 2024 11:23:18 +0200, Andy Shevchenko <andy.shevchenko@...il.com> wrote:
> > On Fri, Nov 8, 2024 at 10:52 AM <victor.duicu@...rochip.com> wrote:

...

> > > +static inline bool pac1921_shunt_is_valid(u32 shunt_val)
> > > +{
> > > +       return shunt_val > 0 && shunt_val <= INT_MAX;
> > > +}
> >
> > This basically is the (shunt_val - 1) & BIT(31) which can be used
> > inline in the caller. Hence, drop this function and use the check
> > inline. See also below.
>
> I think the current comparison check is more clear. Also my suggestion
> to move the check in a seperate function was to keep it consistent in
> different places since such check can change in future and one might
> change it only in one place, as it was happening during the first
> iterations of this series. However I am fine to remove the function and
> move the check back inline in the caller as the check is now only in two
> places and it shouldn't be a big deal.

Up to you. But then drop the comment (which kinda useless) in the
caller and add in the callee, i.e. in this helper to explain the range
of valid values more clearly, ideally with reference to datasheet text
or so.

-- 
With Best Regards,
Andy Shevchenko

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ