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(1): group the condition parsing state into...



details:   https://anonhg.NetBSD.org/src/rev/4d983bd9b7ba
branches:  trunk
changeset: 954707:4d983bd9b7ba
user:      rillig <rillig%NetBSD.org@localhost>
date:      Tue Sep 08 17:55:23 2020 +0000

description:
make(1): group the condition parsing state into a struct

Instead of having 3 global variables, the struct clearly communicates
that the 3 variables belong together. During debugging, it's easy to
just "p *lex" instead of remembering the names of the 3 former global
variables.

Converting the global variables into a local variable makes it
immediately clear that the functions in this file operate on this
struct.  Keeping the global variables in mind is more difficult.  Having
a local variable also gets rid of the 3 sv_* variables in
Cond_EvalExpression, which were also a sign that these "global
variables" were not that global at all.

This commit only contains the minimal code changes for converting the
variables into a local struct.  It was tempting to add functions like
CondLexer_SkipWhitespace, but this is better left for a follow-up
commit.

diffstat:

 usr.bin/make/cond.c |  204 +++++++++++++++++++++++++--------------------------
 1 files changed, 101 insertions(+), 103 deletions(-)

diffs (truncated from 538 to 300 lines):

diff -r e055bcb8940b -r 4d983bd9b7ba usr.bin/make/cond.c
--- a/usr.bin/make/cond.c       Tue Sep 08 17:39:04 2020 +0000
+++ b/usr.bin/make/cond.c       Tue Sep 08 17:55:23 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: cond.c,v 1.113 2020/09/08 14:51:43 rillig Exp $        */
+/*     $NetBSD: cond.c,v 1.114 2020/09/08 17:55:23 rillig Exp $        */
 
 /*
  * Copyright (c) 1988, 1989, 1990 The Regents of the University of California.
@@ -70,14 +70,14 @@
  */
 
 #ifndef MAKE_NATIVE
-static char rcsid[] = "$NetBSD: cond.c,v 1.113 2020/09/08 14:51:43 rillig Exp $";
+static char rcsid[] = "$NetBSD: cond.c,v 1.114 2020/09/08 17:55:23 rillig Exp $";
 #else
 #include <sys/cdefs.h>
 #ifndef lint
 #if 0
 static char sccsid[] = "@(#)cond.c     8.2 (Berkeley) 1/2/94";
 #else
-__RCSID("$NetBSD: cond.c,v 1.113 2020/09/08 14:51:43 rillig Exp $");
+__RCSID("$NetBSD: cond.c,v 1.114 2020/09/08 17:55:23 rillig Exp $");
 #endif
 #endif /* not lint */
 #endif
@@ -136,13 +136,15 @@
     TOK_LPAREN, TOK_RPAREN, TOK_EOF, TOK_NONE, TOK_ERROR
 } Token;
 
-static Token CondE(Boolean);
-static CondEvalResult do_Cond_EvalExpression(Boolean *);
+typedef struct {
+    const struct If *if_info;  /* Info for current statement */
+    const char *condExpr;      /* The expression to parse */
+    Token condPushBack;                /* Single push-back token used in
+                                * parsing */
+} CondLexer;
 
-static const struct If *if_info;       /* Info for current statement */
-static const char *condExpr;           /* The expression to parse */
-static Token condPushBack = TOK_NONE;  /* Single push-back token used in
-                                        * parsing */
+static Token CondE(CondLexer *lex, Boolean);
+static CondEvalResult do_Cond_EvalExpression(CondLexer *lex, Boolean *);
 
 static unsigned int cond_depth = 0;    /* current .if nesting level */
 static unsigned int cond_min_depth = 0;        /* depth at makefile open */
@@ -165,9 +167,9 @@
 /* Push back the most recent token read. We only need one level of
  * this, so the thing is just stored in 'condPushback'. */
 static void
-CondPushBack(Token t)
+CondPushBack(CondLexer *lex, Token t)
 {
-    condPushBack = t;
+    lex->condPushBack = t;
 }
 
 /* Parse the argument of a built-in function.
@@ -383,7 +385,8 @@
  */
 /* coverity:[+alloc : arg-*2] */
 static const char *
-CondGetString(Boolean doEval, Boolean *quoted, void **freeIt, Boolean strictLHS)
+CondGetString(CondLexer *lex, Boolean doEval, Boolean *quoted, void **freeIt,
+             Boolean strictLHS)
 {
     Buffer buf;
     const char *str;
@@ -395,23 +398,24 @@
     Buf_Init(&buf, 0);
     str = NULL;
     *freeIt = NULL;
-    *quoted = qt = *condExpr == '"' ? 1 : 0;
+    *quoted = qt = *lex->condExpr == '"' ? 1 : 0;
     if (qt)
-       condExpr++;
-    for (start = condExpr; *condExpr && str == NULL; condExpr++) {
-       switch (*condExpr) {
+       lex->condExpr++;
+    for (start = lex->condExpr;
+        *lex->condExpr && str == NULL; lex->condExpr++) {
+       switch (*lex->condExpr) {
        case '\\':
-           if (condExpr[1] != '\0') {
-               condExpr++;
-               Buf_AddByte(&buf, *condExpr);
+           if (lex->condExpr[1] != '\0') {
+               lex->condExpr++;
+               Buf_AddByte(&buf, *lex->condExpr);
            }
            break;
        case '"':
            if (qt) {
-               condExpr++;             /* we don't want the quotes */
+               lex->condExpr++;        /* we don't want the quotes */
                goto got_str;
            } else
-               Buf_AddByte(&buf, *condExpr); /* likely? */
+               Buf_AddByte(&buf, *lex->condExpr); /* likely? */
            break;
        case ')':
        case '!':
@@ -423,13 +427,13 @@
            if (!qt)
                goto got_str;
            else
-               Buf_AddByte(&buf, *condExpr);
+               Buf_AddByte(&buf, *lex->condExpr);
            break;
        case '$':
            /* if we are in quotes, then an undefined variable is ok */
            eflags = ((!qt && doEval) ? VARE_UNDEFERR : 0) |
                     (doEval ? VARE_WANTRES : 0);
-           str = Var_Parse(condExpr, VAR_CMD, eflags, &len, freeIt);
+           str = Var_Parse(lex->condExpr, VAR_CMD, eflags, &len, freeIt);
            if (str == var_Error) {
                if (*freeIt) {
                    free(*freeIt);
@@ -442,16 +446,16 @@
                str = NULL;
                goto cleanup;
            }
-           condExpr += len;
+           lex->condExpr += len;
            /*
             * If the '$' was first char (no quotes), and we are
             * followed by space, the operator or end of expression,
             * we are done.
             */
-           if ((condExpr == start + len) &&
-               (*condExpr == '\0' ||
-                isspace((unsigned char)*condExpr) ||
-                strchr("!=><)", *condExpr))) {
+           if ((lex->condExpr == start + len) &&
+               (*lex->condExpr == '\0' ||
+                isspace((unsigned char)*lex->condExpr) ||
+                strchr("!=><)", *lex->condExpr))) {
                goto cleanup;
            }
 
@@ -461,7 +465,7 @@
                *freeIt = NULL;
            }
            str = NULL;         /* not finished yet */
-           condExpr--;         /* don't skip over next char */
+           lex->condExpr--;    /* don't skip over next char */
            break;
        default:
            if (strictLHS && !qt && *start != '$' &&
@@ -474,7 +478,7 @@
                str = NULL;
                goto cleanup;
            }
-           Buf_AddByte(&buf, *condExpr);
+           Buf_AddByte(&buf, *lex->condExpr);
            break;
        }
     }
@@ -508,7 +512,7 @@
  *     condPushback will be set back to TOK_NONE if it is used.
  */
 static Token
-compare_expression(Boolean doEval)
+compare_expression(CondLexer *lex, Boolean doEval)
 {
     Token t;
     const char *lhs;
@@ -529,31 +533,31 @@
      * Parse the variable spec and skip over it, saving its
      * value in lhs.
      */
-    lhs = CondGetString(doEval, &lhsQuoted, &lhsFree, lhsStrict);
+    lhs = CondGetString(lex, doEval, &lhsQuoted, &lhsFree, lhsStrict);
     if (!lhs)
        goto done;
 
     /*
      * Skip whitespace to get to the operator
      */
-    while (isspace((unsigned char)*condExpr))
-       condExpr++;
+    while (isspace((unsigned char)*lex->condExpr))
+       lex->condExpr++;
 
     /*
      * Make sure the operator is a valid one. If it isn't a
      * known relational operator, pretend we got a
      * != 0 comparison.
      */
-    op = condExpr;
-    switch (*condExpr) {
+    op = lex->condExpr;
+    switch (*lex->condExpr) {
     case '!':
     case '=':
     case '<':
     case '>':
-       if (condExpr[1] == '=') {
-           condExpr += 2;
+       if (lex->condExpr[1] == '=') {
+           lex->condExpr += 2;
        } else {
-           condExpr += 1;
+           lex->condExpr += 1;
        }
        break;
     default:
@@ -572,25 +576,25 @@
            goto done;
        }
        /* For .if ${...} check for non-empty string (defProc is ifdef). */
-       if (if_info->form[0] == 0) {
+       if (lex->if_info->form[0] == 0) {
            t = lhs[0] != 0;
            goto done;
        }
        /* Otherwise action default test ... */
-       t = if_info->defProc(strlen(lhs), lhs) != if_info->doNot;
+       t = lex->if_info->defProc(strlen(lhs), lhs) != lex->if_info->doNot;
        goto done;
     }
 
-    while (isspace((unsigned char)*condExpr))
-       condExpr++;
+    while (isspace((unsigned char)*lex->condExpr))
+       lex->condExpr++;
 
-    if (*condExpr == '\0') {
+    if (*lex->condExpr == '\0') {
        Parse_Error(PARSE_WARNING,
                    "Missing right-hand-side of operator");
        goto done;
     }
 
-    rhs = CondGetString(doEval, &rhsQuoted, &rhsFree, FALSE);
+    rhs = CondGetString(lex, doEval, &rhsQuoted, &rhsFree, FALSE);
     if (!rhs)
        goto done;
 
@@ -714,7 +718,7 @@
 }
 
 static Token
-compare_function(Boolean doEval)
+compare_function(CondLexer *lex, Boolean doEval)
 {
     static const struct fn_def {
        const char *fn_name;
@@ -734,7 +738,7 @@
     Token t;
     char *arg = NULL;
     int arglen;
-    const char *cp = condExpr;
+    const char *cp = lex->condExpr;
     const char *cp1;
 
     for (fn_def = fn_defs; fn_def->fn_name != NULL; fn_def++) {
@@ -749,20 +753,20 @@
 
        arglen = fn_def->fn_getarg(doEval, &cp, &arg, fn_def->fn_name);
        if (arglen <= 0) {
-           condExpr = cp;
+           lex->condExpr = cp;
            return arglen < 0 ? TOK_ERROR : TOK_FALSE;
        }
        /* Evaluate the argument using the required function. */
        t = !doEval || fn_def->fn_proc(arglen, arg);
        free(arg);
-       condExpr = cp;
+       lex->condExpr = cp;
        return t;
     }
 
     /* Push anything numeric through the compare expression */
-    cp = condExpr;
+    cp = lex->condExpr;
     if (isdigit((unsigned char)cp[0]) || strchr("+-", cp[0]))
-       return compare_expression(doEval);
+       return compare_expression(lex, doEval);
 
     /*
      * Most likely we have a naked token to apply the default function to.
@@ -776,8 +780,8 @@
     for (cp1 = cp; isspace((unsigned char)*cp1); cp1++)
        continue;
     if (*cp1 == '=' || *cp1 == '!')
-       return compare_expression(doEval);
-    condExpr = cp;
+       return compare_expression(lex, doEval);
+    lex->condExpr = cp;
 
     /*
      * Evaluate the argument using the default function.
@@ -785,52 +789,52 @@
      * after .if must have been taken literally, so the argument cannot
      * be empty - even if it contained a variable expansion.
      */
-    t = !doEval || if_info->defProc(arglen, arg) != if_info->doNot;



Home | Main Index | Thread Index | Old Index