[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260412152445.GZ1827@brightrain.aerifal.cx>
Date: Sun, 12 Apr 2026 11:24:45 -0400
From: Rich Felker <dalias@...c.org>
To: Luca Kellermann <mailto.luca.kellermann@...il.com>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH] scandir: fix qsort usage
On Sun, Apr 12, 2026 at 11:17:22AM -0400, Rich Felker wrote:
> On Sun, Apr 12, 2026 at 01:43:08PM +0200, Luca Kellermann wrote:
> > On Fri, Apr 10, 2026 at 01:30:27PM -0400, Rich Felker wrote:
> > > On Fri, Feb 27, 2026 at 09:17:15PM +0100, Luca Kellermann wrote:
> > > > calling qsort() with a pointer to a function whose type is not
> > > > compatible with int(const void *, const void *) results in UB because
> > > > qsort() would call this function with an incompatible type.
> > > >
> > > > avoid this by using qsort_r(). this is similar to how qsort() is
> > > > implemented on top of qsort_r().
> > > >
> > > > the casts to void * in wrapper_cmp() are required because scandir()
> > > > takes int (*)(const struct dirent **, const struct dirent **) and not
> > > > int (*)(const struct dirent *const *, const struct dirent *const *).
> > > > ---
> > > > Since you haven't posted a patch yet, I went ahead and fixed this. The
> > > > patch is based on the scandir() patch series I submitted in July.
> > > >
> > > > src/dirent/scandir.c | 12 +++++++++---
> > > > 1 file changed, 9 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/src/dirent/scandir.c b/src/dirent/scandir.c
> > > > index 9e4e623f..993e0c8b 100644
> > > > --- a/src/dirent/scandir.c
> > > > +++ b/src/dirent/scandir.c
> > > > @@ -7,9 +7,15 @@
> > > > #include <errno.h>
> > > > #include <stddef.h>
> > > >
> > > > +typedef int (*cmpfun)(const struct dirent **, const struct dirent **);
> > > > +
> > > > +static int wrapper_cmp(const void *de1, const void *de2, void *cmp)
> > > > +{
> > > > + return ((cmpfun)cmp)((void *)de1, (void *)de2);
> > > > +}
> > > > +
> > >
> > > I was going to ask you what the (void *) casts are for, but now I see
> > > it. The scandir comparison function was specified wrong and takes as
> > > const T ** rather than a T *const * like it should.
> >
> > I anticipated that this might raise some questions. That's why I tried
> > to explain it in the commit message :)
> >
> > > Unfortunately this also reveals an aliasing violation elsewhere. The
> > > lvalue type the caller's comparison function will be using to access
> > > elements of the names[] array are of type const struct dirent *, but
> > > the actual elements of the names[] array have type struct dirent *. I
> > > can prepare a separate patch to address this.
> >
> > I'm curious what fix you have in mind for this. The only thing I can
> > think of is to change the type of the names array to const struct
> > dirent ** and after sorting copy it back to a new array of type struct
> > dirent ** that can then be stored in *res.
>
> Uhg, I missed that the array returned in *res mismatches the type of
> the array to be sorted; I was thinking we could just adjust it to
> const. Indeed copying would be gratuitously awful. Maybe this is
> fixable just with some union punning. Or.. wrapper_cmp could just copy
> the pointer values into local vars and pass the addresses of those
> local vars to the cmp function. This is probably the most correct.
As in (untested but compiles without warnings):
static int wrapper_cmp(const void *de1, const void *de2, void *cmp)
{
const struct dirent *cd1 = *(struct dirent *const *)de1;
const struct dirent *cd2 = *(struct dirent *const *)de2;
return ((int (*)(const struct dirent **, const struct dirent **))cmp)(&cd1, &cd2);
}
This is fully const-correct and lacks aliasing violations.
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.
