libc qsort(3): Eliminate ambiguous sign comparison

[cem]

Summary

The left side of the MIN() expression is the (signed) result of pointer
subtraction (ptrdiff_t). The right hand side is the also the (signed)
result of pointer subtraction, additionally subtracting the element size
('es'), which is unsigned size_t. This coerces the right-hand
expression into an unsigned value. MIN(signed, unsigned) triggers
-Wsign-compare.

Sorting elements of size greater than SSIZE_MAX is nonsensical, so we
can instead treat the element size as ssize_t, leaving the right-hand
result the same signedness as the left.

Test Plan

buildworld

[kib]

ptrdiff_t sounds arbitrary. Why did not you used e.g. ssize_t? Because it is not defined by C STD?

Also the cast needs a comment explaining it purpose.

[cem]

In D31292#704741, @kib wrote:

ptrdiff_t sounds arbitrary. Why did not you used e.g. ssize_t? Because it is not defined by C STD?

It is arbitrary. I agree ssize_t would be reasonable.

Also the cast needs a comment explaining it purpose.

Ok.

[cem]

Use ssize_t.

[arichardson]

Looks good to me

lib/libc/stdlib/Makefile.inc

21

Could we add this to all C files in this directory or are we not ready for that?

Unrelated, but do you know how close we are to bring able to increasing WARNS for libc?

lib/libc/stdlib/qsort.c

104

Can we change the type of es here instead of adding the cast?

[cem]

lib/libc/stdlib/Makefile.inc

21

Could we add this to all C files in this directory or are we not ready for that?

lib/libc/gen/getgrent.c:1146:28: error: comparison of integers of different signs: 'int' and 'size_t' (aka 'unsigned long') [-Werror,-Wsign-compare]
                            (unsigned long)gid) >= bufsize)
                            ~~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~

Looks like no.

Unrelated, but do you know how close we are to bring able to increasing WARNS for libc?

I don't.

lib/libc/stdlib/qsort.c

104

It's worse:

lib/libc/stdlib/qsort.c:181:10: error: comparison of integers of different signs: 'size_t' (aka 'unsigned long') and 'ssize_t' (aka 'long') [-Werror,-Wsign-compare]
                if (d1 > es) {
                    ~~ ^ ~~
lib/libc/stdlib/qsort.c:184:10: error: comparison of integers of different signs: 'size_t' (aka 'unsigned long') and 'ssize_t' (aka 'long') [-Werror,-Wsign-compare]
                if (d2 > es) {
                    ~~ ^ ~~
lib/libc/stdlib/qsort.c:193:10: error: comparison of integers of different signs: 'size_t' (aka 'unsigned long') and 'ssize_t' (aka 'long') [-Werror,-Wsign-compare]
                if (d2 > es) {
                    ~~ ^ ~~
lib/libc/stdlib/qsort.c:196:10: error: comparison of integers of different signs: 'size_t' (aka 'unsigned long') and 'ssize_t' (aka 'long') [-Werror,-Wsign-compare]
                if (d1 > es) {
                    ~~ ^ ~~

[arichardson]

lib/libc/stdlib/qsort.c

104

In that case, the current patch looks fine. Thanks

[cem]

Add comment explaining the cast.

[kib]

lib/libc/stdlib/qsort.c

176

Please use C89 comments, in style.

[cem]

Oops. Use C89-style comments per style(9).