linux-kernel - Re: [PATCH v2 1/2] util_macros.h: fix/rework find

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: <20241106120848.63b6665af42264a70bba1621@linux-foundation.org>
Date: Wed, 6 Nov 2024 12:08:48 -0800
From: Andrew Morton <akpm@...ux-foundation.org>
To: Alexandru Ardelean <aardelean@...libre.com>
Cc: linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
 jic23@...nel.org, bartosz.golaszewski@...aro.org,
 gregkh@...uxfoundation.org
Subject: Re: [PATCH v2 1/2] util_macros.h: fix/rework find_closest() macros

On Wed, 6 Nov 2024 16:03:36 +0200 Alexandru Ardelean <aardelean@...libre.com> wrote:

> On Wed, Nov 6, 2024 at 1:08 AM Andrew Morton <akpm@...ux-foundation.org> wrote:
> > Can we fix both issues by just giving up on the macro approach and
> > reimplement them in out-of-line C code?  All the sites I looked at are
> > using 32-bit quantities - a mix of signed and unsigned.
> >
> 
> Converting this to a static-inline was my other thought, rather than
> keeping the macros.

Non-inline, I think.  It's big.

> But I'm not sure where to draw the line between too much rework vs a bug-fix.
> Just fixing the bug was done in V1 of this patch, but then the kunit
> exposed a bunch more.

Sure, just the minimum for a bugfix.

> > It's separate from this bugfix of course, but would it be feasible for
> > someone to go switch all callers to use u32's then reimplement these in
> > lib/find_closest.c?
> >
> 
> That would work.
> How would a rework be preferred?
> As a continuation to this patchset? Or a V3 to this patchset?

A new and separate patchset.  A low-priority cleanup from whoever has
the time and motivation ;)

> But, moving forward: what would some preferences be?
> - have variants of find_closest() for unsigned/signed arrays? (
> find_closest_u32() or find_closest_i32() ?)
>    - AFAICT so far, there aren't any values in the arrays that get
> close to INT32_MAX, so int32 may work for now
>    - maybe later some 64-bit variants could be added if needed
> - should the variables X, mid, left & right be the same signedness as the array
>
> The only preference (towards which I'm leaning) is just making sure
> that X (and friends) are signed.

Yes, I guess int32 would be best.  I agree that unsigned values greater
than INT_MAX are unlikely.

I suggest a series of patches which convert individual callers to int32
and the final patch introduces lib/find_closest.c.