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): prepare Var_Parse for proper error han...



details:   https://anonhg.NetBSD.org/src/rev/e1eea6196e00
branches:  trunk
changeset: 954932:e1eea6196e00
user:      rillig <rillig%NetBSD.org@localhost>
date:      Sun Sep 13 18:27:39 2020 +0000

description:
make(1): prepare Var_Parse for proper error handling and reporting

Right now, Var_Parse swallows many errors during parsing and evaluation.
Ideally, these errors should propagate from the deeply nested
expressions where they occur up to the top-level expressions.  When such
an error occurs, the depending expressions should not be evaluated any
further.  They may still be parsed, but side effects should be
minimized.

The goal is to prevent incomplete expressions like the "xy}" in
moderrs.exp:106 from being evaluated and eventually passed to the shell
for execution.  This expression is a left-over from a parse error in the
mod-t-parse target in moderrs.mk:154.

This commit is a first step in analyzing and verifying the current state
of affairs. The modelling in VarParseErrors already looks complicated
but is expected to closely match reality.

diffstat:

 usr.bin/make/arch.c    |  14 ++++++-----
 usr.bin/make/cond.c    |  15 ++++++++----
 usr.bin/make/nonints.h |  58 ++++++++++++++++++++++++++++++++++++++++++++++++-
 usr.bin/make/parse.c   |  10 +++++---
 usr.bin/make/suff.c    |  10 +++++---
 usr.bin/make/var.c     |  56 +++++++++++++++++++++++++++++++----------------
 6 files changed, 123 insertions(+), 40 deletions(-)

diffs (truncated from 389 to 300 lines):

diff -r a80a146550a8 -r e1eea6196e00 usr.bin/make/arch.c
--- a/usr.bin/make/arch.c       Sun Sep 13 17:18:54 2020 +0000
+++ b/usr.bin/make/arch.c       Sun Sep 13 18:27:39 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: arch.c,v 1.114 2020/09/13 15:15:51 rillig Exp $        */
+/*     $NetBSD: arch.c,v 1.115 2020/09/13 18:27:39 rillig Exp $        */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -133,7 +133,7 @@
 #include    "config.h"
 
 /*     "@(#)arch.c     8.2 (Berkeley) 1/2/94"  */
-MAKE_RCSID("$NetBSD: arch.c,v 1.114 2020/09/13 15:15:51 rillig Exp $");
+MAKE_RCSID("$NetBSD: arch.c,v 1.115 2020/09/13 18:27:39 rillig Exp $");
 
 #ifdef TARGET_MACHINE
 #undef MAKE_MACHINE
@@ -228,8 +228,9 @@
            const char *result;
            Boolean isError;
 
-           result = Var_Parse(&nested_p, ctxt,
-                              VARE_UNDEFERR|VARE_WANTRES, &result_freeIt);
+           (void)Var_Parse(&nested_p, ctxt, VARE_UNDEFERR|VARE_WANTRES,
+                           &result, &result_freeIt);
+           /* TODO: handle errors */
            isError = result == var_Error;
            free(result_freeIt);
            if (isError)
@@ -270,8 +271,9 @@
                Boolean isError;
                const char *nested_p = cp;
 
-               result = Var_Parse(&nested_p, ctxt,
-                                  VARE_UNDEFERR|VARE_WANTRES, &freeIt);
+               (void)Var_Parse(&nested_p, ctxt, VARE_UNDEFERR|VARE_WANTRES,
+                               &result, &freeIt);
+               /* TODO: handle errors */
                isError = result == var_Error;
                free(freeIt);
 
diff -r a80a146550a8 -r e1eea6196e00 usr.bin/make/cond.c
--- a/usr.bin/make/cond.c       Sun Sep 13 17:18:54 2020 +0000
+++ b/usr.bin/make/cond.c       Sun Sep 13 18:27:39 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: cond.c,v 1.144 2020/09/13 15:15:51 rillig Exp $        */
+/*     $NetBSD: cond.c,v 1.145 2020/09/13 18:27:39 rillig Exp $        */
 
 /*
  * Copyright (c) 1988, 1989, 1990 The Regents of the University of California.
@@ -93,7 +93,7 @@
 #include "dir.h"
 
 /*     "@(#)cond.c     8.2 (Berkeley) 1/2/94"  */
-MAKE_RCSID("$NetBSD: cond.c,v 1.144 2020/09/13 15:15:51 rillig Exp $");
+MAKE_RCSID("$NetBSD: cond.c,v 1.145 2020/09/13 18:27:39 rillig Exp $");
 
 /*
  * The parsing of conditional expressions is based on this grammar:
@@ -248,7 +248,9 @@
             */
            void *freeIt;
            VarEvalFlags eflags = VARE_UNDEFERR | (doEval ? VARE_WANTRES : 0);
-           const char *cp2 = Var_Parse(&cp, VAR_CMD, eflags, &freeIt);
+           const char *cp2;
+           (void)Var_Parse(&cp, VAR_CMD, eflags, &cp2, &freeIt);
+           /* TODO: handle errors */
            Buf_AddStr(&buf, cp2);
            free(freeIt);
            continue;
@@ -452,7 +454,8 @@
                     (doEval ? VARE_WANTRES : 0);
            nested_p = par->p;
            atStart = nested_p == start;
-           str = Var_Parse(&nested_p, VAR_CMD, eflags, freeIt);
+           (void)Var_Parse(&nested_p, VAR_CMD, eflags, &str, freeIt);
+           /* TODO: handle errors */
            if (str == var_Error) {
                if (*freeIt) {
                    free(*freeIt);
@@ -693,7 +696,9 @@
     *argPtr = NULL;
 
     (*linePtr)--;              /* Make (*linePtr)[1] point to the '('. */
-    val = Var_Parse(linePtr, VAR_CMD, doEval ? VARE_WANTRES : 0, &val_freeIt);
+    (void)Var_Parse(linePtr, VAR_CMD, doEval ? VARE_WANTRES : 0,
+                   &val, &val_freeIt);
+    /* TODO: handle errors */
     /* If successful, *linePtr points beyond the closing ')' now. */
 
     if (val == var_Error) {
diff -r a80a146550a8 -r e1eea6196e00 usr.bin/make/nonints.h
--- a/usr.bin/make/nonints.h    Sun Sep 13 17:18:54 2020 +0000
+++ b/usr.bin/make/nonints.h    Sun Sep 13 18:27:39 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: nonints.h,v 1.118 2020/09/13 15:27:25 rillig Exp $     */
+/*     $NetBSD: nonints.h,v 1.119 2020/09/13 18:27:39 rillig Exp $     */
 
 /*-
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -204,6 +204,59 @@
     VAR_SET_READONLY   = 0x02
 } VarSet_Flags;
 
+/* The state of error handling returned by Var_Parse.
+ *
+ * As of 2020-09-13, this bitset looks quite bloated,
+ * with all the constants doubled.
+ *
+ * Its purpose is to first document the existing behavior,
+ * and then migrate away from the SILENT constants, step by step,
+ * as these are not suited for reliable, consistent error handling
+ * and reporting. */
+typedef enum {
+
+    /* Both parsing and evaluation succeeded. */
+    VPE_OK             = 0x0000,
+
+    /* Parsing failed.
+     * An error message has already been printed. */
+    VPE_PARSE_MSG      = 0x0001,
+
+    /* Parsing failed.
+     * No error message has been printed yet.
+     *
+     * This should never happen since it is impossible to say where
+     * the parsing error occurred. */
+    VPE_PARSE_SILENT   = 0x0002,
+
+    /* Parsing succeeded.
+     * During evaluation, VARE_UNDEFERR was set and there was an undefined
+     * variable.
+     * An error message has already been printed. */
+    VPE_UNDEF_MSG      = 0x0010,
+
+    /* Parsing succeeded.
+     * During evaluation, VARE_UNDEFERR was set and there was an undefined
+     * variable.
+     * No error message has been printed yet.
+     *
+     * This should never happen since it is impossible to say which of
+     * the variables was undefined. */
+    VPE_UNDEF_SILENT   = 0x0020,
+
+    /* Parsing succeeded.
+     * Evaluation failed.
+     * An error message has already been printed. */
+    VPE_EVAL_MSG       = 0x0100,
+
+    /* Parsing succeeded.
+     * Evaluation failed.
+     * No error message has been printed yet.
+     *
+     * This should never happens since it is impossible to say where
+     * exactly the evaluation error occurred. */
+    VPE_EVAL_SILENT    = 0x0200
+} VarParseErrors;
 
 void Var_Delete(const char *, GNode *);
 void Var_Set(const char *, const char *, GNode *);
@@ -211,7 +264,8 @@
 void Var_Append(const char *, const char *, GNode *);
 Boolean Var_Exists(const char *, GNode *);
 const char *Var_Value(const char *, GNode *, char **);
-const char *Var_Parse(const char **, GNode *, VarEvalFlags, void **);
+VarParseErrors Var_Parse(const char **, GNode *, VarEvalFlags,
+                        const char **, void **);
 char *Var_Subst(const char *, GNode *, VarEvalFlags);
 void Var_Init(void);
 void Var_End(void);
diff -r a80a146550a8 -r e1eea6196e00 usr.bin/make/parse.c
--- a/usr.bin/make/parse.c      Sun Sep 13 17:18:54 2020 +0000
+++ b/usr.bin/make/parse.c      Sun Sep 13 18:27:39 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: parse.c,v 1.303 2020/09/13 15:15:51 rillig Exp $       */
+/*     $NetBSD: parse.c,v 1.304 2020/09/13 18:27:39 rillig Exp $       */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -131,7 +131,7 @@
 #include "pathnames.h"
 
 /*     "@(#)parse.c    8.3 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: parse.c,v 1.303 2020/09/13 15:15:51 rillig Exp $");
+MAKE_RCSID("$NetBSD: parse.c,v 1.304 2020/09/13 18:27:39 rillig Exp $");
 
 /* types and constants */
 
@@ -1164,10 +1164,12 @@
                 * in the initial Var_Subst and we wouldn't be here.
                 */
                const char *nested_p = cp;
+               const char *nested_val;
                void    *freeIt;
 
-               (void)Var_Parse(&nested_p, VAR_CMD,
-                               VARE_UNDEFERR|VARE_WANTRES, &freeIt);
+               (void)Var_Parse(&nested_p, VAR_CMD, VARE_UNDEFERR|VARE_WANTRES,
+                               &nested_val, &freeIt);
+               /* TODO: handle errors */
                free(freeIt);
                cp += nested_p - cp;
            } else
diff -r a80a146550a8 -r e1eea6196e00 usr.bin/make/suff.c
--- a/usr.bin/make/suff.c       Sun Sep 13 17:18:54 2020 +0000
+++ b/usr.bin/make/suff.c       Sun Sep 13 18:27:39 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: suff.c,v 1.155 2020/09/13 15:15:51 rillig Exp $        */
+/*     $NetBSD: suff.c,v 1.156 2020/09/13 18:27:39 rillig Exp $        */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -126,7 +126,7 @@
 #include         "dir.h"
 
 /*     "@(#)suff.c     8.4 (Berkeley) 3/21/94" */
-MAKE_RCSID("$NetBSD: suff.c,v 1.155 2020/09/13 15:15:51 rillig Exp $");
+MAKE_RCSID("$NetBSD: suff.c,v 1.156 2020/09/13 18:27:39 rillig Exp $");
 
 #define SUFF_DEBUG0(fmt) \
     if (!DEBUG(SUFF)) (void) 0; else fprintf(debug_file, fmt)
@@ -1318,8 +1318,10 @@
                    void        *freeIt;
 
                    /* XXX: Why VARE_WANTRES when the result is not used? */
-                   junk = Var_Parse(&nested_p, pgn,
-                                    VARE_UNDEFERR|VARE_WANTRES, &freeIt);
+                   (void)Var_Parse(&nested_p, pgn,
+                                   VARE_UNDEFERR|VARE_WANTRES,
+                                   &junk, &freeIt);
+                   /* TODO: handle errors */
                    if (junk == var_Error) {
                        Parse_Error(PARSE_FATAL,
                                    "Malformed variable expression at \"%s\"",
diff -r a80a146550a8 -r e1eea6196e00 usr.bin/make/var.c
--- a/usr.bin/make/var.c        Sun Sep 13 17:18:54 2020 +0000
+++ b/usr.bin/make/var.c        Sun Sep 13 18:27:39 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: var.c,v 1.513 2020/09/13 16:47:24 rillig Exp $ */
+/*     $NetBSD: var.c,v 1.514 2020/09/13 18:27:39 rillig Exp $ */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -121,7 +121,7 @@
 #include    "metachar.h"
 
 /*     "@(#)var.c      8.3 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: var.c,v 1.513 2020/09/13 16:47:24 rillig Exp $");
+MAKE_RCSID("$NetBSD: var.c,v 1.514 2020/09/13 18:27:39 rillig Exp $");
 
 #define VAR_DEBUG_IF(cond, fmt, ...)   \
     if (!(DEBUG(VAR) && (cond)))       \
@@ -1673,8 +1673,9 @@
            void *nested_val_freeIt;
            VarEvalFlags nested_eflags = eflags & ~(unsigned)VARE_ASSIGN;
 
-           nested_val = Var_Parse(&nested_p, ctxt, nested_eflags,
-                                  &nested_val_freeIt);
+           (void)Var_Parse(&nested_p, ctxt, nested_eflags,
+                           &nested_val, &nested_val_freeIt);
+           /* TODO: handle errors */
            Buf_AddStr(&buf, nested_val);
            free(nested_val_freeIt);
            p += nested_p - p;
@@ -2021,7 +2022,9 @@
            const char *nested_val;
            void *nested_val_freeIt;
 
-           nested_val = Var_Parse(&p, st->ctxt, eflags, &nested_val_freeIt);
+           (void)Var_Parse(&p, st->ctxt, eflags,
+                           &nested_val, &nested_val_freeIt);
+           /* TODO: handle errors */
            Buf_AddStr(&buf, nested_val);
            free(nested_val_freeIt);
            continue;
@@ -3008,14 +3011,16 @@
             */
            const char *nested_p = p;
            void *freeIt;
-           const char *rval = Var_Parse(&nested_p, st.ctxt, st.eflags,
-                                        &freeIt);
+           const char *rval;
+           int c;
+
+           (void)Var_Parse(&nested_p, st.ctxt, st.eflags, &rval, &freeIt);
+           /* TODO: handle errors */
 
            /*
             * If we have not parsed up to st.endc or ':',
             * we are not interested.
             */
-           int c;
            if (rval[0] != '\0' &&
                (c = *nested_p) != '\0' && c != ':' && c != st.endc) {
                free(freeIt);
@@ -3344,7 +3349,9 @@
        /* A variable inside a variable, expand. */
        if (*p == '$') {
            void *freeIt;



Home | Main Index | Thread Index | Old Index