From 6dc5a3c0cc338ce0053da6509f56b11fc2b6ec89 Mon Sep 17 00:00:00 2001 From: riastradh Date: Fri, 5 Apr 2024 00:43:42 +0000 Subject: [PATCH] config(1): Make sort order deterministic. Ensure we break ties in every case. This way, even though we use the unstable qsort(3) library routine, the output is reproducible, no matter what algorithm is behind qsort(3). It would be nice if we could just use a stable sort function here, but mergesort(3) is nonstandard, so we'd have to add it to tools/compat, which is a big pain. Instead, put a tie-breaking rule in every comparison function we use with qsort, and abort() in the event of ties -- that way, we noisily refuse to rely on unstable sort order. While here, dispense with any question of integer overflow, and sprinkle comments. PR bin/58115 --- usr.bin/config/defs.h | 4 +++- usr.bin/config/files.c | 8 ++++---- usr.bin/config/mkioconf.c | 11 ++++++++--- usr.bin/config/mkmakefile.c | 21 ++++++++++++++++++--- usr.bin/config/pack.c | 29 +++++++++++++++++++++++------ 5 files changed, 56 insertions(+), 17 deletions(-) diff --git a/usr.bin/config/defs.h b/usr.bin/config/defs.h index b45d31ff377c..abeb9bb0485d 100644 --- a/usr.bin/config/defs.h +++ b/usr.bin/config/defs.h @@ -1,4 +1,4 @@ -/* $NetBSD: defs.h,v 1.108 2024/01/18 05:41:38 thorpej Exp $ */ +/* $NetBSD: defs.h,v 1.109 2024/04/05 00:43:42 riastradh Exp $ */ /* * Copyright (c) 1992, 1993 @@ -207,6 +207,8 @@ struct attr { /* "device class" */ const char *a_devclass; /* device class described */ struct where a_where; + + size_t a_idx; /* index to break sorting ties */ }; /* diff --git a/usr.bin/config/files.c b/usr.bin/config/files.c index ae7126d2ee68..6ff47181d569 100644 --- a/usr.bin/config/files.c +++ b/usr.bin/config/files.c @@ -1,4 +1,4 @@ -/* $NetBSD: files.c,v 1.37 2020/03/07 19:26:13 christos Exp $ */ +/* $NetBSD: files.c,v 1.38 2024/04/05 00:43:42 riastradh Exp $ */ /* * Copyright (c) 1992, 1993 @@ -45,7 +45,7 @@ #endif #include -__RCSID("$NetBSD: files.c,v 1.37 2020/03/07 19:26:13 christos Exp $"); +__RCSID("$NetBSD: files.c,v 1.38 2024/04/05 00:43:42 riastradh Exp $"); #include #include @@ -281,9 +281,9 @@ cmpfiles(const void *a, const void *b) if (sa < sb) return -1; else if (sa > sb) - return 1; + return +1; else - return 0; + abort(); /* no ties possible */ } /* diff --git a/usr.bin/config/mkioconf.c b/usr.bin/config/mkioconf.c index ae39a55b8613..8ae454228450 100644 --- a/usr.bin/config/mkioconf.c +++ b/usr.bin/config/mkioconf.c @@ -1,4 +1,4 @@ -/* $NetBSD: mkioconf.c,v 1.35 2017/11/19 01:46:29 christos Exp $ */ +/* $NetBSD: mkioconf.c,v 1.36 2024/04/05 00:43:42 riastradh Exp $ */ /* * Copyright (c) 1992, 1993 @@ -45,7 +45,7 @@ #endif #include -__RCSID("$NetBSD: mkioconf.c,v 1.35 2017/11/19 01:46:29 christos Exp $"); +__RCSID("$NetBSD: mkioconf.c,v 1.36 2024/04/05 00:43:42 riastradh Exp $"); #include #include @@ -136,7 +136,12 @@ cforder(const void *a, const void *b) n1 = (*(const struct devi * const *)a)->i_cfindex; n2 = (*(const struct devi * const *)b)->i_cfindex; - return (n1 - n2); + if (n1 < n2) + return -1; + else if (n1 > n2) + return +1; + else + abort(); /* no ties possible */ } static void diff --git a/usr.bin/config/mkmakefile.c b/usr.bin/config/mkmakefile.c index fe5232eaeb57..32ca947c49ae 100644 --- a/usr.bin/config/mkmakefile.c +++ b/usr.bin/config/mkmakefile.c @@ -1,4 +1,4 @@ -/* $NetBSD: mkmakefile.c,v 1.72 2024/01/18 04:41:37 thorpej Exp $ */ +/* $NetBSD: mkmakefile.c,v 1.73 2024/04/05 00:43:42 riastradh Exp $ */ /* * Copyright (c) 1992, 1993 @@ -45,7 +45,7 @@ #endif #include -__RCSID("$NetBSD: mkmakefile.c,v 1.72 2024/01/18 04:41:37 thorpej Exp $"); +__RCSID("$NetBSD: mkmakefile.c,v 1.73 2024/04/05 00:43:42 riastradh Exp $"); #include #include @@ -413,6 +413,7 @@ emitallkobjscb(const char *name, void *v, void *arg) return 0; if (TAILQ_EMPTY(&a->a_files)) return 0; + a->a_idx = attridx; attrbuf[attridx++] = a; /* XXX nattrs tracking is not exact yet */ if (attridx == nattrs) { @@ -447,7 +448,21 @@ attrcmp(const void *l, const void *r) { const struct attr * const *a = l, * const *b = r; const int wa = (*a)->a_weight, wb = (*b)->a_weight; - return (wa > wb) ? -1 : (wa < wb) ? 1 : 0; + + /* + * Higher-weight first; then, among equal weights, earlier + * index first. + */ + if (wa > wb) + return -1; + else if (wa < wb) + return +1; + else if ((*a)->a_idx < (*b)->a_idx) + return -1; + else if ((*a)->a_idx > (*b)->a_idx) + return +1; + else + abort(); /* no ties possible */ } static void diff --git a/usr.bin/config/pack.c b/usr.bin/config/pack.c index f227aa3b3539..d48edc360eb8 100644 --- a/usr.bin/config/pack.c +++ b/usr.bin/config/pack.c @@ -1,4 +1,4 @@ -/* $NetBSD: pack.c,v 1.10 2015/09/12 19:11:13 joerg Exp $ */ +/* $NetBSD: pack.c,v 1.11 2024/04/05 00:43:42 riastradh Exp $ */ /* * Copyright (c) 1992, 1993 @@ -45,7 +45,7 @@ #endif #include -__RCSID("$NetBSD: pack.c,v 1.10 2015/09/12 19:11:13 joerg Exp $"); +__RCSID("$NetBSD: pack.c,v 1.11 2024/04/05 00:43:42 riastradh Exp $"); #include #include @@ -319,21 +319,38 @@ addlocs(const char **locs, int len) /* * Comparison function for qsort-by-locator-length, longest first. - * We rashly assume that subtraction of these lengths does not overflow. + * + * In the event of equality, break ties by i_cfindex, which should be + * unique, obviating the need for a stable sort. */ static int loclencmp(const void *a, const void *b) { + const struct devi *const *d1p = a, *d1 = *d1p; + const struct devi *const *d2p = b, *d2 = *d2p; const struct pspec *p1, *p2; int l1, l2; - p1 = (*(const struct devi * const *)a)->i_pspec; + p1 = d1->i_pspec; l1 = p1 != NULL ? p1->p_iattr->a_loclen : 0; - p2 = (*(const struct devi * const *)b)->i_pspec; + p2 = d2->i_pspec; l2 = p2 != NULL ? p2->p_iattr->a_loclen : 0; - return (l2 - l1); + /* + * Longer locators first; among equal locator lengths, earliest + * index first. + */ + if (l2 < l1) + return -1; + else if (l2 > l1) + return +1; + else if (d2->i_cfindex > d1->i_cfindex) + return -1; + else if (d2->i_cfindex < d1->i_cfindex) + return +1; + else + abort(); /* no ties possible */ } static void