Thanks, Pádraig, for fixing that. And thanks, Larry, for reporting that.
The existing tests are sufficient to catch this.
Yes, evidently I forgot to run 'make check', which I usually do. I'll
try to not forget next time....
I installed the attached further patches to (1) coalesce duplicate code
and explain why it's needed and (2) tweak performance a tiny bit.
From 15627794459933d293547c2bf7d77ab196ae73a3 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Wed, 25 May 2022 11:19:08 -0700
Subject: [PATCH 1/2] sort: refactor tricky diff reversal
* src/sort.c (diff_reversed): New function, to make the intent clearer.
(keycompare, compare): Use it.
---
src/sort.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/src/sort.c b/src/sort.c
index dbe456038..0cd22f931 100644
--- a/src/sort.c
+++ b/src/sort.c
@@ -2601,6 +2601,15 @@ key_warnings (struct keyfield const *gkey, bool gkey_only)
error (0, 0, _("option '-r' only applies to last-resort comparison"));
}
+/* Return either the sense of DIFF or its reverse, depnding on REVERSED.
+ If REVERSED, do not simply negate DIFF as that can mishandle INT_MIN. */
+
+static int
+diff_reversed (int diff, bool reversed)
+{
+ return reversed ? (diff < 0 ? 1 : -diff) : diff;
+}
+
/* Compare two lines A and B trying every key in sequence until there
are no more keys or a difference is found. */
@@ -2793,9 +2802,7 @@ keycompare (struct line const *a, struct line const *b)
}
}
- if (key->reverse)
- diff = diff < 0 ? 1 : -diff;
- return diff;
+ return diff_reversed (diff, key->reverse);
}
/* Compare two lines A and B, returning negative, zero, or positive
@@ -2840,9 +2847,7 @@ compare (struct line const *a, struct line const *b)
diff = (alen > blen) - (alen < blen);
}
- if (reverse)
- diff = diff < 0 ? 1 : -diff;
- return diff;
+ return diff_reversed (diff, reverse);
}
/* Write LINE to output stream FP; the output file's name is
--
2.34.1
From 85ddde23116e578768f90bad6899340da5394b75 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Wed, 25 May 2022 11:23:39 -0700
Subject: [PATCH 2/2] sort: tune diff_reversed
* src/sort.c (diff_reversed): Tune. On x86-64 with GCC, this
saves a conditional branch and shortens the generated machine code.
---
src/sort.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/sort.c b/src/sort.c
index 0cd22f931..0a6b557ac 100644
--- a/src/sort.c
+++ b/src/sort.c
@@ -2607,7 +2607,7 @@ key_warnings (struct keyfield const *gkey, bool gkey_only)
static int
diff_reversed (int diff, bool reversed)
{
- return reversed ? (diff < 0 ? 1 : -diff) : diff;
+ return reversed ? (diff < 0) - (diff > 0) : diff;
}
/* Compare two lines A and B trying every key in sequence until there
--
2.34.1
