[vlc-devel] [PATCH v2] compat: add qsort_r()

[Romain Vimont]

On Tue, Nov 20, 2018 at 06:02:26PM +0200, Rémi Denis-Courmont wrote:
> Le tiistaina 20. marraskuuta 2018, 16.36.41 EET Romain Vimont a écrit :
> > Add qsort_r() based on qsort() with Thread Local Storage to store the
> > comparison context.
> > ---
> >  compat/qsort_r.c     | 47 ++++++++++++++++++++++++++++++++++++++++++++
> >  configure.ac         |  2 +-
> >  include/vlc_fixups.h |  7 +++++++
> >  3 files changed, 55 insertions(+), 1 deletion(-)
> >  create mode 100644 compat/qsort_r.c
> > 
> > diff --git a/compat/qsort_r.c b/compat/qsort_r.c
> > new file mode 100644
> > index 0000000000..9519ddafc6
> > --- /dev/null
> > +++ b/compat/qsort_r.c
> > @@ -0,0 +1,47 @@
> > +/**
> > + * @file qsort_r.c
> > + * @brief replacement for GNU qsort_r()
> > + */
> > +/**************************************************************************
> > *** + * Copyright (C) 2018 VLC authors and VideoLAN
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU Lesser General Public License as published by
> > + * the Free Software Foundation; either version 2.1 of the License, or + *
> > (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public License
> > + * along with this program; if not, write to the Free Software Foundation,
> > + * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA. +
> > ***************************************************************************
> > **/ +
> > +#ifdef HAVE_CONFIG_H
> > +# include "config.h"
> > +#endif
> > +
> > +#include <stdlib.h>
> > +
> > +static thread_local struct qsort_r_context
> > +{
> > +    int (*compar)(const void *, const void *, void *);
> > +    void *arg;
> > +} qsort_r_ctx;
> > +
> > +static int qsort_cmp(const void *lhs, const void *rhs)
> > +{
> > +    return qsort_r_ctx.compar(lhs, rhs, qsort_r_ctx.arg);
> > +}
> > +
> > +void qsort_r(void *base, size_t nmemb, size_t size,
> > +             int (*compar)(const void *, const void *, void *),
> > +             void *arg)
> > +{
> > +    qsort_r_ctx.compar = compar;
> > +    qsort_r_ctx.arg = arg;
> > +    qsort(base, nmemb, size, qsort_cmp);
> > +}
> 
> The final r in qsort_r () stands for reentrant, and that there is not 
> reentrant.

Yes, I expected that feedback :)

In practice, I wanted thread-safety (two threads running qsort_r()
independently should work). Calling qsort_r() from the compar function
is "unusual", but I agree, it still breaks qsort_r().

Maybe I should just use a thread_local context locally in the playlist
(and use qsort()) instead?