linux-kernel - Re: [PATCH v2 4/4] iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family

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: <CAHp75VeP=G1Pa6=Q1gok5+mA7grDWrgAEM_m8FqjAwme==jt-A@mail.gmail.com>
Date: Sat, 30 Aug 2025 07:54:22 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: CAHp75VcGG_h+wpo7hHL=ERYqbrvvAaufwPAYBsEbRn3dB8-dfA@...l.gmail.com
Cc: Jonathan Santos <Jonathan.Santos@...log.com>, linux-kernel@...r.kernel.org, 
	linux-iio@...r.kernel.org, devicetree@...r.kernel.org, 
	Michael.Hennerich@...log.com, lars@...afoo.de, jic23@...nel.org, 
	dlechner@...libre.com, nuno.sa@...log.com, andy@...nel.org, robh@...nel.org, 
	krzk+dt@...nel.org, conor+dt@...nel.org, marcelo.schmitt1@...il.com
Subject: Re: [PATCH v2 4/4] iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family

On Fri, Aug 29, 2025 at 10:11 PM Jonathan Santos <jonath4nns@...il.com> wrote:
> On 08/24, Andy Shevchenko wrote:
> > On Sun, Aug 24, 2025 at 7:10 AM Jonathan Santos

...

> > > +       unsigned long denominator, numerator;
> >
> > struct u32_fract fract;
>
> Problem is that rational_best_approximation requires a unsigned long. I
> could do:
>
> rational_best_approximation(fract.numerator, fract.denominator,
>                                             INT_MAX, INT_MAX,
>                                             (unsigned long *)&fract.numerator,
>                                             (unsigned long *)&fract.denominator);

No, this is not good. And it will overflow. See below for a better alternative.

> But I don't know if this is the best practice.

The best practice is to add a patch to extend that to cover unsigned
long and long
https://elixir.bootlin.com/linux/v6.17-rc3/source/include/linux/math.h#L122
It's just a couple of lines which I review and acknowledge
immediately. (Just make them a precursor patch)

BUT, looking at the code, I don't see that people use the same
variables for input and output (otherwise it's also considered not the
best practice, for example, in the cases when input can be made a
const).

That said, the patching math.h is optional, either way, make sure your
input and output are different variables.

> One way around this would be to represent the gains in fractions instead of 1000x, example:
> then we could drop the rational approximation.

I like the use of rational best approximation. I think it gives a
really nice algo for simplifying fractions.

-- 
With Best Regards,
Andy Shevchenko