From f707c76a62b15f6bbaf0909332b6c1d6ebff225d Mon Sep 17 00:00:00 2001
From: rillig <rillig@NetBSD.org>
Date: Sun, 5 Dec 2021 17:00:02 +0000
Subject: [PATCH] make: fix types for modifier ':O' (since today)

In var.c 1.962 from 2021-12-05, the type of the array to be sorted
changed from 'const char *' to 'Substring'.  The code still worked since
the first member of Substring is 'const char *start', but that was pure
luck.

Fix the type assumptions in the comparison functions.

Add an assertion that the words must be null-terminated since the
comparison functions currently assume this.

This kind of inconsistency may be possible to detect using static
analysis, but NetBSD's lint is currently too limited for that.  To
detect this, it would have to analyze each possible comparison function
and remember the assumed type.  This type could then be compared to the
argument type of a function call to qsort or bsearch.

No functional change.
---
 usr.bin/make/var.c | 51 ++++++++++++++++++++++++++++------------------
 1 file changed, 31 insertions(+), 20 deletions(-)

diff --git a/usr.bin/make/var.c b/usr.bin/make/var.c
index 4af23e0de465..68733cc745ba 100644
--- a/usr.bin/make/var.c
+++ b/usr.bin/make/var.c
@@ -1,4 +1,4 @@
-/*	$NetBSD: var.c,v 1.963 2021/12/05 15:20:13 rillig Exp $	*/
+/*	$NetBSD: var.c,v 1.964 2021/12/05 17:00:02 rillig Exp $	*/
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -140,7 +140,7 @@
 #include "metachar.h"
 
 /*	"@(#)var.c	8.3 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: var.c,v 1.963 2021/12/05 15:20:13 rillig Exp $");
+MAKE_RCSID("$NetBSD: var.c,v 1.964 2021/12/05 17:00:02 rillig Exp $");
 
 /*
  * Variables are defined using one of the VAR=value assignments.  Their
@@ -3285,13 +3285,13 @@ bad_modifier:
 #endif
 
 static NUM_TYPE
-num_val(const char *s)
+num_val(Substring s)
 {
 	NUM_TYPE val;
 	char *ep;
 
-	val = strtoll(s, &ep, 0);
-	if (ep != s) {
+	val = strtoll(s.start, &ep, 0);
+	if (ep != s.start) {
 		switch (*ep) {
 		case 'K':
 		case 'k':
@@ -3311,31 +3311,32 @@ num_val(const char *s)
 }
 
 static int
-num_cmp_asc(const void *sa, const void *sb)
+SubNumAsc(const void *sa, const void *sb)
 {
 	NUM_TYPE a, b;
 
-	a = num_val(*(const char *const *)sa);
-	b = num_val(*(const char *const *)sb);
+	a = num_val(*((const Substring *)sa));
+	b = num_val(*((const Substring *)sb));
 	return (a > b) ? 1 : (b > a) ? -1 : 0;
 }
 
 static int
-num_cmp_desc(const void *sa, const void *sb)
+SubNumDesc(const void *sa, const void *sb)
 {
-	return num_cmp_asc(sb, sa);
+	return SubNumAsc(sb, sa);
 }
 
 static int
-str_cmp_asc(const void *a, const void *b)
+SubStrAsc(const void *sa, const void *sb)
 {
-	return strcmp(*(const char *const *)a, *(const char *const *)b);
+	return strcmp(
+	    ((const Substring *)sa)->start, ((const Substring *)sb)->start);
 }
 
 static int
-str_cmp_desc(const void *a, const void *b)
+SubStrDesc(const void *sa, const void *sb)
 {
-	return str_cmp_asc(b, a);
+	return SubStrAsc(sb, sa);
 }
 
 static void
@@ -3366,13 +3367,13 @@ ApplyModifier_Order(const char **pp, ModChain *ch)
 	int (*cmp)(const void *, const void *);
 
 	if (IsDelimiter(mod[1], ch) || mod[1] == '\0') {
-		cmp = str_cmp_asc;
+		cmp = SubStrAsc;
 		(*pp)++;
 	} else if (IsDelimiter(mod[2], ch) || mod[2] == '\0') {
 		if (mod[1] == 'n')
-			cmp = num_cmp_asc;
+			cmp = SubNumAsc;
 		else if (mod[1] == 'r')
-			cmp = str_cmp_desc;
+			cmp = SubStrDesc;
 		else if (mod[1] == 'x')
 			cmp = NULL;
 		else
@@ -3381,7 +3382,7 @@ ApplyModifier_Order(const char **pp, ModChain *ch)
 	} else if (IsDelimiter(mod[3], ch) || mod[3] == '\0') {
 		if ((mod[1] == 'n' && mod[2] == 'r') ||
 		    (mod[1] == 'r' && mod[2] == 'n'))
-			cmp = num_cmp_desc;
+			cmp = SubNumDesc;
 		else
 			goto bad;
 		*pp += 3;
@@ -3395,8 +3396,10 @@ ApplyModifier_Order(const char **pp, ModChain *ch)
 	words = Substring_Words(ch->expr->value.str, false);
 	if (cmp == NULL)
 		ShuffleSubstrings(words.words, words.len);
-	else
+	else {
+		assert(words.words[0].end[0] == '\0');
 		qsort(words.words, words.len, sizeof(words.words[0]), cmp);
+	}
 	Expr_SetValueOwn(ch->expr, SubstringWords_JoinFree(words));
 
 	return AMR_OK;
@@ -4803,6 +4806,14 @@ Var_Stats(void)
 	HashTable_DebugStats(&SCOPE_GLOBAL->vars, "Global variables");
 }
 
+static int
+StrAsc(const void *sa, const void *sb)
+{
+	return strcmp(
+	    *((const char *const *)sa), *((const char *const *)sb));
+}
+
+
 /* Print all variables in a scope, sorted by name. */
 void
 Var_Dump(GNode *scope)
@@ -4819,7 +4830,7 @@ Var_Dump(GNode *scope)
 		*(const char **)Vector_Push(&vec) = hi.entry->key;
 	varnames = vec.items;
 
-	qsort(varnames, vec.len, sizeof varnames[0], str_cmp_asc);
+	qsort(varnames, vec.len, sizeof varnames[0], StrAsc);
 
 	for (i = 0; i < vec.len; i++) {
 		const char *varname = varnames[i];