Add qsort_s(3)

[trasz]

Summary

Add qsort_s(3). Apart from the constraints, it also makes it easier
to port software written for Linux variant of qsort_r(3).

[trasz]

(Note that the tests are still TBD.)

[kib]

include/stdlib.h

343 ↗

(On Diff #66743)

Prototype is wrong. Take a look at the K.3.6.3.2.

344 ↗

(On Diff #66743)

Indent is wrong.

lib/libc/stdlib/Symbol.map

123 ↗

(On Diff #66743)

Why did you put the symbol under 1.5 version ?

[rpokala]

lib/libc/stdlib/qsort.3

270 ↗

(On Diff #66743)

May suggest some wordsmithing, for English readability?

The qsort_s function behaves the same as qsort_r, except that

• The order of arguments to compar is different
• If nmemb or size are greater than RSIZE_MAX, or nmemb is non-zero and compar is NULL, then the runtime-constraint handler is called, and qsort_s returns an error. Note that the handler is called before qsort_s returns the error, and that the handler itself does not return.

(Assuming I interpreted the existing wording correctly; it's a little hard to follow, which is why I'm suggesting the re-wording.)

328 ↗

(On Diff #66743)

If the glibc version of qsort_r() is using non-standard ordering, perhaps note that? i.e.

and the non-standard GNU libc implementation of

[arichardson]

lib/libc/stdlib/qsort.3

328 ↗

(On Diff #66743)

I wouldn't necessarily write that GNU is non-standard since the real problem is that there is no standard for qsort_r... I guess glibc just did whatever seemed easiest for their qsort.

lib/libc/stdlib/qsort.c

98 ↗

(On Diff #66743)

void* context is the last argument for qsort_s

[trasz]

Fix stuff, add tests.

[trasz]

lib/libc/stdlib/Symbol.map

123 ↗

(On Diff #66743)

Where should it go instead?

[arichardson]

Looks fine to me but someone else should give the final approval.

include/stdlib.h

327 ↗

(On Diff #66855)

Unrelated to this patch, but all other types use _FOO_DECLARED. Would be nice if we could be consistent.
Annoyingly, the clang builtin headers use _RSIZE_T and _SIZE_T, etc. so they are not compatible.

lib/libc/tests/stdlib/qsort_s_test.c

76 ↗

(On Diff #66855)

Should this check for == EINVAL instead of != 0? And the same for the other tests.

[trasz]

Undo the mess caused by 'svn cp'.

[trasz]

lib/libc/tests/stdlib/qsort_s_test.c

76 ↗

(On Diff #66855)

I don't think so - the standard doesn't seem to define the actual error to return?

[arichardson]

lib/libc/stdlib/Makefile.inc

14 ↗

(On Diff #66896)

qsort_r.c is missing now.

[arichardson]

lib/libc/tests/stdlib/qsort_s_test.c

76 ↗

(On Diff #66855)

True, I don't think we need to be testing the specific implementation here so we could theoretically reuse the test for other implementations.

[trasz]

Sigh, removed qsort_r from the wrong makefile.

[arichardson]

lib/libc/stdlib/Symbol.map

123 ↗

(On Diff #66743)

I think looking at the other symbol.map files 1.6 is the version for 13?

[kib]

lib/libc/stdlib/Makefile.inc

16 ↗

(On Diff #66905)

Why not merge this and previous line ?

lib/libc/stdlib/Symbol.map

123 ↗

(On Diff #66743)

To the current version for CURRENT-13, which is 1.6.

[trasz]

Fix symbol version, and formatting in the makefile.

[trasz]

lib/libc/stdlib/Symbol.map

123 ↗

(On Diff #66743)

Ah, now I get it. I only looked at this particular Symbol.map.

[jilles]

Hmm, https://reviews.freebsd.org/D17083 proposes to change qsort_r() argument order to glibc's since that is likely to become POSIX standard and is slightly better than our order.

[trasz]

In D23174#509448, @jilles wrote:

Hmm, https://reviews.freebsd.org/D17083 proposes to change qsort_r() argument order to glibc's since that is likely to become POSIX standard and is slightly better than our order.

True. Implementing qsort_s(3) is much easier compatibility-wise, though.