On Wed, 22 Jan 2020, Stefan Schulze Frielinghaus wrote:
> Hi David,
>
> In function `tree_cmp` an invariant [1] is assumed which does not necessarily
> exist. In case both input trees are finally compared via `strcmp`, then
>
> tree_cmp (t1, t2) == -tree_cmp (t2, t1)
>
> does not hold in general, since function `strcmp (x, y)` guarantees only that
> a
> negative integer is returned in case x<y or a positive integer in case x>y.
> Currently this breaks s390x where, for example, for certain inputs x,y
> `tree_cmp (x, y) == 1` and `tree_cmp (y, x) == -2` hold. The attached patch
> normalizes the output from `strcmp` to -1, 0, or 1 while using an auxiliary
> function `sign` (stolen from the Hacker's Delight book ;-)).
>
> Bootstrapped and tested on s390x. Any thoughts?
It's more appropriate to fix the assert rather than the comparator, like
gcc_assert (sign (reversed) == -sign (result));
But qsort_chk already checks that, and more, so why is the assert there?
Shouldn't it be simply removed?
Alexander
