Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/usr.bin/make make: handle parse errors in ':O' uniformly



details:   https://anonhg.NetBSD.org/src/rev/38c4dc788a1f
branches:  trunk
changeset: 984909:38c4dc788a1f
user:      rillig <rillig%NetBSD.org@localhost>
date:      Fri Jul 30 23:28:04 2021 +0000

description:
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.

diffstat:

 usr.bin/make/unit-tests/varmod-order-numeric.exp |  24 ++++---
 usr.bin/make/unit-tests/varmod-order-numeric.mk  |  22 +++---
 usr.bin/make/var.c                               |  68 ++++++++++++++---------
 3 files changed, 65 insertions(+), 49 deletions(-)

diffs (223 lines):

diff -r 96ff9cdb2d0e -r 38c4dc788a1f usr.bin/make/unit-tests/varmod-order-numeric.exp
--- a/usr.bin/make/unit-tests/varmod-order-numeric.exp  Fri Jul 30 22:19:51 2021 +0000
+++ b/usr.bin/make/unit-tests/varmod-order-numeric.exp  Fri Jul 30 23:28:04 2021 +0000
@@ -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
diff -r 96ff9cdb2d0e -r 38c4dc788a1f usr.bin/make/unit-tests/varmod-order-numeric.mk
--- a/usr.bin/make/unit-tests/varmod-order-numeric.mk   Fri Jul 30 22:19:51 2021 +0000
+++ b/usr.bin/make/unit-tests/varmod-order-numeric.mk   Fri Jul 30 23:28:04 2021 +0000
@@ -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 @@
 
 # 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 @@
 
 # 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 @@
 
 # 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 @@
 # 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 @@
 
 # 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 @@
 
 # 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 @@
 .  error
 .endif
 
+# Missing closing brace, to cover the error handling code.
+_:=    ${NUMBERS:O
+_:=    ${NUMBERS:On
+_:=    ${NUMBERS:Onr
+
 all:
diff -r 96ff9cdb2d0e -r 38c4dc788a1f usr.bin/make/var.c
--- a/usr.bin/make/var.c        Fri Jul 30 22:19:51 2021 +0000
+++ b/usr.bin/make/var.c        Fri Jul 30 23:28:04 2021 +0000
@@ -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 @@
  *       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 @@
 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;
-
-       if (IsDelimiter(mod[1], ch)) {
-               mode = ASC;
-       } else if (mod[1] == 'n') {
-               mode = NUM_ASC;
+               STR, NUM, SHUFFLE
+       } mode = STR;
+       enum SortDir {
+               ASC, DESC
+       } dir = ASC;
+
+       if (IsDelimiter(mod[1], ch) || mod[1] == '\0') {
+               mode = STR;
                (*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)) {
-               (*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 @@
        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 */



Home | Main Index | Thread Index | Old Index