Skip to content

Commit

Permalink
Use a stable sorting algorithm.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dag-erling committed Sep 13, 2024
1 parent 0abf238 commit 7ae97d3
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 16 deletions.
59 changes: 44 additions & 15 deletions bin/ptsort.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -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=""
Expand Down
2 changes: 1 addition & 1 deletion t/t_dp_red.to
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 7ae97d3

Please sign in to comment.