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