From cc0354a49fcde0a6b433279d374ce6e26f70adff Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dag-Erling=20Sm=C3=B8rgrav?= <des@des.no>
Date: Fri, 13 Sep 2024 14:02:07 +0200
Subject: [PATCH] Use a stable sorting algorithm.

The documentation promises that the output is sorted first by either priority or depth, then by either depth or priority, and finally by name.  This derives from the fact that we store nodes in a name-indexed balanced search tree in the input phase.  However, in the output phase, after calculating the depths and priorities of all nodes, we sort them using quicksort, which is not stable, breaking our promise.  Moreover, different implementations of quicksort will choose different pivots, resulting in different outcomes for equally-ranked nodes.  This is illustrated by the `t_dp_red` test case, which passed on FreeBSD but failed on Ubuntu due to different internal ordering within each priority group.

To address this, replace `qsort(3)` with `mergesort(3)` if it is available, and our own implementation of insertion sort otherwise.  Also correct the expected output of the `t_dp_red` test case.
---
 bin/ptsort.c  | 59 ++++++++++++++++++++++++++++++++++++++-------------
 configure.ac  |  3 +++
 t/t_dp_red.to |  2 +-
 3 files changed, 48 insertions(+), 16 deletions(-)

diff --git a/bin/ptsort.c b/bin/ptsort.c
index 57e9c1a..21660fb 100644
--- a/bin/ptsort.c
+++ b/bin/ptsort.c
@@ -82,6 +82,11 @@ typedef struct pnode {
 	unsigned long	 prio;
 } pnode;
 
+/*
+ * Comparison function.
+ */
+typedef int (*pnode_cmp)(const pnode *, const pnode *);
+
 static aa_tree nodes;
 static unsigned long tnedges, tnnodes;
 
@@ -94,45 +99,43 @@ static aa_comparator pnode_namecmp = (aa_comparator)strcmp;
  * Compare two nodes by their depths first and priorities second.
  */
 static int
-pnode_depthcmp(const void *av, const void *bv)
+pnode_depthcmp(const pnode *a, const pnode *b)
 {
-	const pnode *a = av;
-	const pnode *b = bv;
-
 	return (a->depth > b->depth ? 1 : a->depth < b->depth ? -1 :
 	    a->prio > b->prio ? 1 : a->prio < b->prio ? -1 : 0);
 }
 
+#if HAVE_MERGESORT
 static int
 pnodep_depthcmp(const void *av, const void *bv)
 {
-	const pnode *const *a = av;
-	const pnode *const *b = bv;
+       const pnode *const *a = av;
+       const pnode *const *b = bv;
 
-	return (pnode_depthcmp(*a, *b));
+       return (pnode_depthcmp(*a, *b));
 }
+#endif
 
 /*
  * Compare two nodes by their priorities first and depths second.
  */
 static int
-pnode_priocmp(const void *av, const void *bv)
+pnode_priocmp(const pnode *a, const pnode *b)
 {
-	const pnode *a = av;
-	const pnode *b = bv;
-
 	return (a->prio > b->prio ? 1 : a->prio < b->prio ? -1 :
 	    a->depth > b->depth ? 1 : a->depth < b->depth ? -1 : 0);
 }
 
+#if HAVE_MERGESORT
 static int
 pnodep_priocmp(const void *av, const void *bv)
 {
-	const pnode *const *a = av;
-	const pnode *const *b = bv;
+       const pnode *const *a = av;
+       const pnode *const *b = bv;
 
-	return (pnode_priocmp(*a, *b));
+       return (pnode_priocmp(*a, *b));
 }
+#endif
 
 /*
  * Allocate and initialize a new node.
@@ -359,6 +362,28 @@ input(const char *fn)
 	tnnodes += nnodes;
 }
 
+#if !HAVE_MERGESORT
+/*
+ * Sort an array of nodes.
+ *
+ * Insertion sort is not the fastest, but it is stable and requires no
+ * additional memory.
+ */
+static void
+sort(pnode **array, unsigned long n, pnode_cmp cmp)
+{
+	pnode *t;
+	unsigned long i, j;
+
+	for (i = 1; i < n; i++) {
+		t = array[i];
+		for (j = i; j > 0 && cmp(array[j-1], t) == 1; j--)
+			array[j] = array[j-1];
+		array[j] = t;
+	}
+}
+#endif
+
 /*
  * Output a partial ordering of the nodes in the graph.  We form an array
  * of pointers to all of our notes, sort them by priority and print the
@@ -383,8 +408,12 @@ output(const char *fn)
 	/* p now points one past the end of the array */
 
 	/* sort by either priority or depth */
-	qsort(all, tnnodes, sizeof *all,
+#if HAVE_MERGESORT
+	mergesort(all, tnnodes, sizeof *all,
 	    bydepth ? pnodep_depthcmp : pnodep_priocmp);
+#else
+	sort(all, tnnodes, bydepth ? pnode_depthcmp : pnode_priocmp);
+#endif
 
 	/* output to file or stdout */
 	if (fn == NULL)
diff --git a/configure.ac b/configure.ac
index 97721a4..c6e0614 100644
--- a/configure.ac
+++ b/configure.ac
@@ -18,6 +18,9 @@ AC_C_VOLATILE
 AC_PROG_INSTALL
 AC_PROG_RANLIB
 
+# C library functions
+AC_CHECK_FUNC([mergesort])
+
 # libraries
 save_LIBS="${LIBS}"
 LIBS=""
diff --git a/t/t_dp_red.to b/t/t_dp_red.to
index c853225..50fdfc4 100644
--- a/t/t_dp_red.to
+++ b/t/t_dp_red.to
@@ -3,6 +3,6 @@
       2       2 a1
       1       1 b2
       1       1 a2
-      0       0 a3
       0       0 c1
       0       0 b3
+      0       0 a3