[src/trunk]: src/usr.bin/make make: document how error handling should be don...

[rillig <rillig%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/cdbbd9c3b1e3
branches:  trunk
changeset: 953617:cdbbd9c3b1e3
user:      rillig <rillig%NetBSD.org@localhost>
date:      Sun Mar 14 15:04:13 2021 +0000

description:
make: document how error handling should be done correctly

Right now, when a variable expression cannot be parsed, the result of
calling Var_Subst is a string containing garbage, and no error is
reported.  In addition, there are some silent errors that are not
reported at all.  This combination makes it difficult to change the
error handling without introducing subtle breakage in some edge cases.

An example for garbage output is in varmod-subst-regex.mk, in target
mod-regex-compile-error.

No functional change.

diffstat:

 usr.bin/make/var.c |  14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diffs (35 lines):

diff -r d39cc74a5f55 -r cdbbd9c3b1e3 usr.bin/make/var.c
--- a/usr.bin/make/var.c        Sun Mar 14 13:11:59 2021 +0000
+++ b/usr.bin/make/var.c        Sun Mar 14 15:04:13 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: var.c,v 1.857 2021/03/14 11:15:37 rillig Exp $ */
+/*     $NetBSD: var.c,v 1.858 2021/03/14 15:04:13 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.857 2021/03/14 11:15:37 rillig Exp $");
+MAKE_RCSID("$NetBSD: var.c,v 1.858 2021/03/14 15:04:13 rillig Exp $");
 
 typedef enum VarFlags {
        VFL_NONE        = 0,
@@ -3837,6 +3837,16 @@
            (int)strcspn(mod, ":)}"), mod, expr->var->name.str);
 
 cleanup:
+       /*
+        * TODO: Use p + strlen(p) instead, to stop parsing immediately.
+        *
+        * In the unit tests, this generates a few unterminated strings in the
+        * shell commands though.  Instead of producing these unfinished
+        * strings, commands with evaluation errors should not be run at all.
+        *
+        * To make that happen, Var_Subst must report the actual errors
+        * instead of returning VPR_OK unconditionally.
+        */
        *pp = p;
        Expr_SetValueRefer(expr, var_Error);
 }