Module Name: src
Committed By: rillig
Date: Fri Jul 30 23:28:04 UTC 2021
Modified Files:
src/usr.bin/make: var.c
src/usr.bin/make/unit-tests: varmod-order-numeric.exp
varmod-order-numeric.mk
Log Message:
make: handle parse errors in ':O' uniformly
Previously, the error handling for the variable modifier ':O' differed
depending on the exact variant and in some cases led to misleading
or missing diagnostics.
To generate a diff of this commit:
cvs rdiff -u -r1.941 -r1.942 src/usr.bin/make/var.c
cvs rdiff -u -r1.2 -r1.3 src/usr.bin/make/unit-tests/varmod-order-numeric.exp \
src/usr.bin/make/unit-tests/varmod-order-numeric.mk
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/usr.bin/make/var.c
diff -u src/usr.bin/make/var.c:1.941 src/usr.bin/make/var.c:1.942
--- src/usr.bin/make/var.c:1.941 Fri Jul 30 22:19:51 2021
+++ src/usr.bin/make/var.c Fri Jul 30 23:28:04 2021
@@ -1,4 +1,4 @@
-/* $NetBSD: var.c,v 1.941 2021/07/30 22:19:51 rillig Exp $ */
+/* $NetBSD: var.c,v 1.942 2021/07/30 23:28:04 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.941 2021/07/30 22:19:51 rillig Exp $");
+MAKE_RCSID("$NetBSD: var.c,v 1.942 2021/07/30 23:28:04 rillig Exp $");
/*
* Variables are defined using one of the VAR=value assignments. Their
@@ -2045,7 +2045,7 @@ typedef struct Expr {
* Chain 2 ends at the ':' between ${IND1} and ${IND2}.
* Chain 3 starts with all modifiers from ${IND2}.
* Chain 3 ends at the ':' after ${IND2}.
- * Chain 1 continues with the the 2 modifiers ':O' and ':u'.
+ * Chain 1 continues with the 2 modifiers ':O' and ':u'.
* Chain 1 ends at the final '}' of the expression.
*
* After such a chain ends, its properties no longer have any effect.
@@ -3349,31 +3349,39 @@ ShuffleStrings(char **strs, size_t n)
static ApplyModifierResult
ApplyModifier_Order(const char **pp, ModChain *ch)
{
- const char *mod = (*pp)++; /* skip past the 'O' in any case */
+ const char *mod = *pp;
Words words;
enum SortMode {
- ASC, DESC, NUM_ASC, NUM_DESC, SHUFFLE
- } mode;
+ STR, NUM, SHUFFLE
+ } mode = STR;
+ enum SortDir {
+ ASC, DESC
+ } dir = ASC;
- if (IsDelimiter(mod[1], ch)) {
- mode = ASC;
- } else if (mod[1] == 'n') {
- mode = NUM_ASC;
- (*pp)++;
- if (!IsDelimiter(mod[2], ch)) {
- (*pp)++;
- if (mod[2] == 'r')
- mode = NUM_DESC;
- }
- } else if ((mod[1] == 'r' || mod[1] == 'x') &&
- IsDelimiter(mod[2], ch)) {
+ if (IsDelimiter(mod[1], ch) || mod[1] == '\0') {
+ mode = STR;
(*pp)++;
- mode = mod[1] == 'r' ? DESC : SHUFFLE;
- } else if (mod[1] == 'r' && mod[2] == 'n') {
- (*pp) += 2;
- mode = NUM_DESC;
- } else
- return AMR_BAD;
+ } else if (IsDelimiter(mod[2], ch) || mod[2] == '\0') {
+ if (mod[1] == 'n')
+ mode = NUM;
+ else if (mod[1] == 'r')
+ dir = DESC;
+ else if (mod[1] == 'x')
+ mode = SHUFFLE;
+ else
+ goto bad;
+ *pp += 2;
+ } else if (IsDelimiter(mod[3], ch) || mod[3] == '\0') {
+ if ((mod[1] == 'n' && mod[2] == 'r') ||
+ (mod[1] == 'r' && mod[2] == 'n')) {
+ mode = NUM;
+ dir = DESC;
+ } else
+ goto bad;
+ *pp += 3;
+ } else {
+ goto bad;
+ }
if (!ModChain_ShouldEval(ch))
return AMR_OK;
@@ -3381,15 +3389,19 @@ ApplyModifier_Order(const char **pp, Mod
words = Str_Words(ch->expr->value.str, false);
if (mode == SHUFFLE)
ShuffleStrings(words.words, words.len);
- else if (mode == NUM_ASC || mode == NUM_DESC)
+ else if (mode == NUM)
qsort(words.words, words.len, sizeof words.words[0],
- mode == NUM_ASC ? num_cmp_asc : num_cmp_desc);
+ dir == ASC ? num_cmp_asc : num_cmp_desc);
else
qsort(words.words, words.len, sizeof words.words[0],
- mode == ASC ? str_cmp_asc : str_cmp_desc);
+ dir == ASC ? str_cmp_asc : str_cmp_desc);
Expr_SetValueOwn(ch->expr, Words_JoinFree(words));
return AMR_OK;
+
+bad:
+ (*pp)++;
+ return AMR_BAD;
}
/* :? then : else */
Index: src/usr.bin/make/unit-tests/varmod-order-numeric.exp
diff -u src/usr.bin/make/unit-tests/varmod-order-numeric.exp:1.2 src/usr.bin/make/unit-tests/varmod-order-numeric.exp:1.3
--- src/usr.bin/make/unit-tests/varmod-order-numeric.exp:1.2 Fri Jul 30 22:16:09 2021
+++ src/usr.bin/make/unit-tests/varmod-order-numeric.exp Fri Jul 30 23:28:04 2021
@@ -1,16 +1,20 @@
make: Bad modifier ":Oxn" for variable "NUMBERS"
make: "varmod-order-numeric.mk" line 32: Malformed conditional (${NUMBERS:Oxn})
-make: Bad modifier ":typo" for variable "NUMBERS"
-make: "varmod-order-numeric.mk" line 45: Malformed conditional (${NUMBERS:On_typo})
-make: "varmod-order-numeric.mk" line 54: Unknown modifier "_typo"
-make: "varmod-order-numeric.mk" line 54: Malformed conditional (${NUMBERS:Onr_typo})
-make: "varmod-order-numeric.mk" line 63: Unknown modifier "_typo"
-make: "varmod-order-numeric.mk" line 63: Malformed conditional (${NUMBERS:Orn_typo})
-make: "varmod-order-numeric.mk" line 75: Missing argument for ".error"
-make: "varmod-order-numeric.mk" line 83: Unknown modifier "r"
-make: "varmod-order-numeric.mk" line 83: Malformed conditional (${NUMBERS:Onrr})
+make: Bad modifier ":On_typo" for variable "NUMBERS"
+make: "varmod-order-numeric.mk" line 42: Malformed conditional (${NUMBERS:On_typo})
+make: Bad modifier ":Onr_typo" for variable "NUMBERS"
+make: "varmod-order-numeric.mk" line 51: Malformed conditional (${NUMBERS:Onr_typo})
+make: Bad modifier ":Orn_typo" for variable "NUMBERS"
+make: "varmod-order-numeric.mk" line 60: Malformed conditional (${NUMBERS:Orn_typo})
+make: Bad modifier ":Onn" for variable "NUMBERS"
+make: "varmod-order-numeric.mk" line 71: Malformed conditional (${NUMBERS:Onn})
+make: Bad modifier ":Onrr" for variable "NUMBERS"
+make: "varmod-order-numeric.mk" line 80: Malformed conditional (${NUMBERS:Onrr})
make: Bad modifier ":Orrn" for variable "NUMBERS"
-make: "varmod-order-numeric.mk" line 94: Malformed conditional (${NUMBERS:Orrn})
+make: "varmod-order-numeric.mk" line 89: Malformed conditional (${NUMBERS:Orrn})
+make: Unclosed variable expression, expecting '}' for modifier "O" of variable "NUMBERS" with value "-2G -3m -42 1 1M 1k 3 42 5 5K 7"
+make: Unclosed variable expression, expecting '}' for modifier "On" of variable "NUMBERS" with value "-2G -3m -42 1 3 5 7 42 1k 5K 1M"
+make: Unclosed variable expression, expecting '}' for modifier "Onr" of variable "NUMBERS" with value "1M 5K 1k 42 7 5 3 1 -42 -3m -2G"
make: Fatal errors encountered -- cannot continue
make: stopped in unit-tests
exit status 1
Index: src/usr.bin/make/unit-tests/varmod-order-numeric.mk
diff -u src/usr.bin/make/unit-tests/varmod-order-numeric.mk:1.2 src/usr.bin/make/unit-tests/varmod-order-numeric.mk:1.3
--- src/usr.bin/make/unit-tests/varmod-order-numeric.mk:1.2 Fri Jul 30 22:16:09 2021
+++ src/usr.bin/make/unit-tests/varmod-order-numeric.mk Fri Jul 30 23:28:04 2021
@@ -1,4 +1,4 @@
-# $NetBSD: varmod-order-numeric.mk,v 1.2 2021/07/30 22:16:09 rillig Exp $
+# $NetBSD: varmod-order-numeric.mk,v 1.3 2021/07/30 23:28:04 rillig Exp $
#
# Tests for the :On variable modifier, which returns the words, sorted in
# ascending numeric order.
@@ -37,11 +37,8 @@ NUMBERS= 3 5 7 1 42 -42 5K -3m 1M 1k -2G
# Extra characters after ':On' are detected and diagnosed.
# TODO: Add line number information to the "Bad modifier" diagnostic.
-# TODO: Use uniform diagnostics for ':On' and ':Onr'.
-# TODO: Fix the misleading ':typo' in the diagnostic.
-# TODO: The '_' is already wrong but does not occur in the diagnostic.
#
-# expect-text: Bad modifier ":typo" for variable "NUMBERS"
+# expect-text: Bad modifier ":On_typo" for variable "NUMBERS"
.if ${NUMBERS:On_typo}
. error
.else
@@ -50,7 +47,7 @@ NUMBERS= 3 5 7 1 42 -42 5K -3m 1M 1k -2G
# Extra characters after ':Onr' are detected and diagnosed.
#
-# expect+1: Unknown modifier "_typo"
+# expect-text: Bad modifier ":Onr_typo" for variable "NUMBERS"
.if ${NUMBERS:Onr_typo}
. error
.else
@@ -59,7 +56,7 @@ NUMBERS= 3 5 7 1 42 -42 5K -3m 1M 1k -2G
# Extra characters after ':Orn' are detected and diagnosed.
#
-# expect+1: Unknown modifier "_typo"
+# expect+1: Bad modifier ":Orn_typo" for variable "NUMBERS"
.if ${NUMBERS:Orn_typo}
. error
.else
@@ -70,7 +67,7 @@ NUMBERS= 3 5 7 1 42 -42 5K -3m 1M 1k -2G
# criteria are fixed, not computed, therefore allowing this redundancy does
# not make sense.
#
-# TODO: This repetition is not diagnosed.
+# expect-text: Bad modifier ":Onn" for variable "NUMBERS"
.if ${NUMBERS:Onn}
. error
.else
@@ -79,7 +76,7 @@ NUMBERS= 3 5 7 1 42 -42 5K -3m 1M 1k -2G
# Repeating the 'r' is not supported as well, for the same reasons as above.
#
-# expect+1: Unknown modifier "r"
+# expect-text: Bad modifier ":Onrr" for variable "NUMBERS"
.if ${NUMBERS:Onrr}
. error
.else
@@ -88,8 +85,6 @@ NUMBERS= 3 5 7 1 42 -42 5K -3m 1M 1k -2G
# Repeating the 'r' is not supported as well, for the same reasons as above.
#
-# TODO: Use uniform diagnostics for ':Onrr' and ':Orrn'.
-#
# expect-text: Bad modifier ":Orrn" for variable "NUMBERS"
.if ${NUMBERS:Orrn}
. error
@@ -97,4 +92,9 @@ NUMBERS= 3 5 7 1 42 -42 5K -3m 1M 1k -2G
. error
.endif
+# Missing closing brace, to cover the error handling code.
+_:= ${NUMBERS:O
+_:= ${NUMBERS:On
+_:= ${NUMBERS:Onr
+
all:
