Re: [PATCH 1/3] compat: add qsort_s()

From: Jeff King <peff@peff.net>
Date: Dec 1, 2016, 19:35 UTC
Message-ID: <20161201193556.j2odwy3sepaxxq5a@sigill.intra.peff.net>
In-Reply-To: <fc602a66-a06c-203e-b50b-55fd7b258b54@web.de>

On Thu, Dec 01, 2016 at 05:26:43PM +0100, René Scharfe wrote:

Show 11 quoted lines

> The function qsort_s() was introduced with C11 Annex K; it provides the
> ability to pass a context pointer to the comparison function, supports
> the convention of using a NULL pointer for an empty array and performs a
> few safety checks.
> 
> Add an implementation based on compat/qsort.c for platforms that lack a
> native standards-compliant qsort_s() (i.e. basically everyone).  It
> doesn't perform the full range of possible checks: It uses size_t
> instead of rsize_t and doesn't check nmemb and size against RSIZE_MAX
> because we probably don't have the restricted size type defined.  For
> the same reason it returns int instead of errno_t.

Hmm. So it sounds like qsort_r(), but with the NULL-is-empty magic. But we already are OK without the latter (and can emulate it easily). Would it make sense to do:

  #if defined(HAVE_QSORT_S)
  /* huzzah, use the system-native qsort_s */

  #elif defined(HAVE_QSORT_R)
  int git_qsort_s(void *b, size_t n, size_t s,
		   int (*cmp)(const void *, const void *, void *), void *ctx)
  {
	if (!n)
		return 0;
	if (!b || !cmp)
		return -1;
	qsort_r(b, n, s, cmp, ctx);
	return 0;
  }

  #else
  /* fallback implementation as your patch does */
  #endif

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).

It just seems like we should be able to do a better job of using the system qsort in many cases.

-Peff

[1] https://stackoverflow.com/questions/39560773/different-declarations-of-qsort-r-on-mac-and-linux/39561369