On Wed, 6 Nov 2024 16:03:36 +0200 Alexandru Ardelean <aardelean@xxxxxxxxxxxx> wrote: > On Wed, Nov 6, 2024 at 1:08 AM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> 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.
