[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260414161003.GC1827@brightrain.aerifal.cx>
Date: Tue, 14 Apr 2026 12:10:03 -0400
From: "dalias@...ifal.cx" <dalias@...ifal.cx>
To: David Sparks <sparks05@...ton.me>
Cc: "musl@...ts.openwall.com" <musl@...ts.openwall.com>,
"mailto.luca.kellermann@...il.com" <mailto.luca.kellermann@...il.com>
Subject: Re: Some additional qsort patches
On Tue, Apr 14, 2026 at 03:24:32AM +0000, David Sparks wrote:
> The recent qsort patches caused conflicts with some local changes
> I've had sitting around for a long time. Since the code is fresh
This is exactly why we don't introduce gratuitous churn in the form of
whitespace changes, minor refactorings, etc. If we'd applied these
patches, the CVE fixes would not have applied cleanly to previous
versions, and we would had had to backport them manually. On top of
that, churn puts a further load on anyone who is actually reviewing
all changes between versions to make sure they trust them, for anyone
maintaining their own out-of-tree functional patchsets, etc.
I'm happy to look at and probably apply patches in this series that
eliminate wasteful compares or make other actual improvements, but I'm
not going to apply the "style uniformity" ones.
> The first several are cosmetic, but the two big wins are the merger
> of the pntz() and shr() helper functions, and the saving one
> one compare (from 3 to 2) per level in sift(). (I could try to
> do the same for trinkle(), but it's messier.)
I know the compare has come up before, and just never been acted on.
We should probably go ahead and do it.
I don't see how the refactoring of pntz/shr is an improvement. It
makes some call points "prettier", others "uglier", but it's entirely
a cosmetic change.
> Tested, but I have almost certainly failed to create acceptable
> patches on my first attempt. Still, Cunningham's law suggests
> that it's better to post an incorrect patch than to ask a lot
> of questions.
>
> TODO: Clean out redundant header files. Neither stdint.h nor stdlib.h
> are actually needed. (Noticed as I was composing this mail.)
stdlib.h is needed because it's the public declaration of qsort. We
always include the public declaration of the interface being
implemented. This ensures the compiler checks that the signatures
match.
I don't see any reason for stdint.h to be included. It should be fine
to remove.
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
