Sun Mar 14 15:04:13 2021 UTC ()
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.


(rillig)
diff -r1.857 -r1.858 src/usr.bin/make/var.c
cvs diff -r1.857 -r1.858 src/usr.bin/make/var.c (expand / switch to unified diff)

--- src/usr.bin/make/var.c 2021/03/14 11:15:37 1.857
+++ src/usr.bin/make/var.c 2021/03/14 15:04:13 1.858
@@ -1,14 +1,14 @@ @@ -1,14 +1,14 @@
1/* $NetBSD: var.c,v 1.857 2021/03/14 11:15:37 rillig Exp $ */ 1/* $NetBSD: var.c,v 1.858 2021/03/14 15:04:13 rillig Exp $ */
2 2
3/* 3/*
4 * Copyright (c) 1988, 1989, 1990, 1993 4 * Copyright (c) 1988, 1989, 1990, 1993
5 * The Regents of the University of California. All rights reserved. 5 * The Regents of the University of California. All rights reserved.
6 * 6 *
7 * This code is derived from software contributed to Berkeley by 7 * This code is derived from software contributed to Berkeley by
8 * Adam de Boor. 8 * Adam de Boor.
9 * 9 *
10 * Redistribution and use in source and binary forms, with or without 10 * Redistribution and use in source and binary forms, with or without
11 * modification, are permitted provided that the following conditions 11 * modification, are permitted provided that the following conditions
12 * are met: 12 * are met:
13 * 1. Redistributions of source code must retain the above copyright 13 * 1. Redistributions of source code must retain the above copyright
14 * notice, this list of conditions and the following disclaimer. 14 * notice, this list of conditions and the following disclaimer.
@@ -130,27 +130,27 @@ @@ -130,27 +130,27 @@
130#include <regex.h> 130#include <regex.h>
131#endif 131#endif
132#include <errno.h> 132#include <errno.h>
133#include <inttypes.h> 133#include <inttypes.h>
134#include <limits.h> 134#include <limits.h>
135#include <time.h> 135#include <time.h>
136 136
137#include "make.h" 137#include "make.h"
138#include "dir.h" 138#include "dir.h"
139#include "job.h" 139#include "job.h"
140#include "metachar.h" 140#include "metachar.h"
141 141
142/* "@(#)var.c 8.3 (Berkeley) 3/19/94" */ 142/* "@(#)var.c 8.3 (Berkeley) 3/19/94" */
143MAKE_RCSID("$NetBSD: var.c,v 1.857 2021/03/14 11:15:37 rillig Exp $"); 143MAKE_RCSID("$NetBSD: var.c,v 1.858 2021/03/14 15:04:13 rillig Exp $");
144 144
145typedef enum VarFlags { 145typedef enum VarFlags {
146 VFL_NONE = 0, 146 VFL_NONE = 0,
147 147
148 /* 148 /*
149 * The variable's value is currently being used by Var_Parse or 149 * The variable's value is currently being used by Var_Parse or
150 * Var_Subst. This marker is used to avoid endless recursion. 150 * Var_Subst. This marker is used to avoid endless recursion.
151 */ 151 */
152 VFL_IN_USE = 1 << 0, 152 VFL_IN_USE = 1 << 0,
153 153
154 /* 154 /*
155 * The variable comes from the environment. 155 * The variable comes from the environment.
156 * These variables are not registered in any GNode, therefore they 156 * These variables are not registered in any GNode, therefore they
@@ -3827,26 +3827,36 @@ ApplyModifiers( @@ -3827,26 +3827,36 @@ ApplyModifiers(
3827 goto bad_modifier; 3827 goto bad_modifier;
3828 } 3828 }
3829 3829
3830 *pp = p; 3830 *pp = p;
3831 assert(expr->value.str != NULL); /* Use var_Error or varUndefined. */ 3831 assert(expr->value.str != NULL); /* Use var_Error or varUndefined. */
3832 return; 3832 return;
3833 3833
3834bad_modifier: 3834bad_modifier:
3835 /* XXX: The modifier end is only guessed. */ 3835 /* XXX: The modifier end is only guessed. */
3836 Error("Bad modifier \":%.*s\" for variable \"%s\"", 3836 Error("Bad modifier \":%.*s\" for variable \"%s\"",
3837 (int)strcspn(mod, ":)}"), mod, expr->var->name.str); 3837 (int)strcspn(mod, ":)}"), mod, expr->var->name.str);
3838 3838
3839cleanup: 3839cleanup:
 3840 /*
 3841 * TODO: Use p + strlen(p) instead, to stop parsing immediately.
 3842 *
 3843 * In the unit tests, this generates a few unterminated strings in the
 3844 * shell commands though. Instead of producing these unfinished
 3845 * strings, commands with evaluation errors should not be run at all.
 3846 *
 3847 * To make that happen, Var_Subst must report the actual errors
 3848 * instead of returning VPR_OK unconditionally.
 3849 */
3840 *pp = p; 3850 *pp = p;
3841 Expr_SetValueRefer(expr, var_Error); 3851 Expr_SetValueRefer(expr, var_Error);
3842} 3852}
3843 3853
3844/* 3854/*
3845 * Only four of the local variables are treated specially as they are the 3855 * Only four of the local variables are treated specially as they are the
3846 * only four that will be set when dynamic sources are expanded. 3856 * only four that will be set when dynamic sources are expanded.
3847 */ 3857 */
3848static Boolean 3858static Boolean
3849VarnameIsDynamic(const char *name, size_t len) 3859VarnameIsDynamic(const char *name, size_t len)
3850{ 3860{
3851 if (len == 1 || (len == 2 && (name[1] == 'F' || name[1] == 'D'))) { 3861 if (len == 1 || (len == 2 && (name[1] == 'F' || name[1] == 'D'))) {
3852 switch (name[0]) { 3862 switch (name[0]) {