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: