Am 01.12.2016 um 21:19 schrieb Jeff King:
On Thu, Dec 01, 2016 at 12:14:42PM -0800, Junio C Hamano wrote:
Jeff King <peff@xxxxxxxx> writes:
To make matters more fun, apparently[1] there are multiple variants of
qsort_r with different argument orders. _And_ apparently Microsoft
defines qsort_s, but it's not quite the same thing. But all of that can
be dealt with by having more specific flags (HAVE_GNU_QSORT_R, etc).
AFAIU it went like this:
// FreeBSD 5.0 (2003)
void qsort_r(void *base, size_t nmemb, size_t size,
void *context,
int (*compar)(void *context, const void *x, const void *y));
// Microsoft Visual Studio 2005
void qsort_s(void *base, size_t nmemb, size_t size,
int (*compar)(void *context, const void *x, const void *y),
void *context);
// glibc 2.8 (2008)
void qsort_r(void *base, size_t nmemb, size_t size,
int (*compar)(const void *x, const void *y, void *context),
void *context);
// C11 Annex K (2011)
errno_t qsort_s(void *base, rsize_t nmemb, rsize_t size,
int (*compar)(const void *x, const void *y, void *context),
void *context);
It just seems like we should be able to do a better job of using the
system qsort in many cases.
Sure, platform-specific implementations can be shorter.
If we were to go that route, perhaps we shouldn't have HAVE_QSORT_S
so that Microsoft folks won't define it by mistake (instead perhaps
call it HAVE_ISO_QSORT_S or something).
OK.
I like your suggestion in general. The body of git_qsort_s() on
systems without ISO_QSORT_S can do
- GNU qsort_r() without any change in the parameters,
- Microsoft qsort_s() with parameter reordered, or
- Apple/BSD qsort_r() with parameter reordered.
and that would cover the major platforms.
Yes.
However, for MSys INTERNAL_QSORT is defined for some reason, so the
platform's qsort(3) is not used there; I guess the same reason applies
to qsort_s(). If it doesn't then an implementation may want to convert
a call to the invalid parameter handler (which may show a dialog
offering to Retry, Continue or Abort) into a non-zero return value.
Eh, wait. BSD and Microsoft have paramters reordered in the
callback comparison function. I suspect that would not fly very
well.
You can hack around it by passing a wrapper callback that flips the
arguments. Since we have a "void *" data pointer, that would point to a
struct holding the "real" callback and chaining to the original data
pointer.
It does incur the cost of an extra level of indirection for each
comparison, though (not just for each qsort call).
Indeed. We'd need a perf test to measure that overhead before we could
determine if that's a problem, though.
You could do it as zero-cost if you were willing to turn the comparison
function definition into a macro.
Ugh. That either requires changing the signature of qsort_s() based on
the underlying native function as well, or using a void pointer to pass
the comparison function, no? Let's not do that, at least not without a
good reason.
René