[src/trunk]: src/usr.bin/make make(1): clean up Var_UnExport

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

details:   https://anonhg.NetBSD.org/src/rev/5232e8909492
branches:  trunk
changeset: 974665:5232e8909492
user:      rillig <rillig%NetBSD.org@localhost>
date:      Sat Aug 08 13:00:07 2020 +0000

description:
make(1): clean up Var_UnExport

Mark the parameter as constant since it is not modified.

Remove tests for '\n' since these can never succeed.

newenv can never be NULL since neither of bmake_malloc or bmake_realloc
returns NULL.

Improve variable names: vlist was too unexpressive.

Add debug logging since unexporting variables is an uncommon operation
that directly affects the observable environment of the child processes.

Fix CRLF line endings in a few unit tests.

diffstat:

 usr.bin/make/nonints.h                     |   6 +-
 usr.bin/make/unit-tests/export-variants.mk |  80 +++++++++++++++---------------
 usr.bin/make/unit-tests/export.mk          |   4 +-
 usr.bin/make/unit-tests/lint.mk            |  34 ++++++------
 usr.bin/make/unit-tests/unexport.mk        |  12 ++++-
 usr.bin/make/var.c                         |  58 ++++++++++++---------
 6 files changed, 105 insertions(+), 89 deletions(-)

diffs (truncated from 330 to 300 lines):

diff -r 983fcb99eb68 -r 5232e8909492 usr.bin/make/nonints.h
--- a/usr.bin/make/nonints.h    Sat Aug 08 12:43:06 2020 +0000
+++ b/usr.bin/make/nonints.h    Sat Aug 08 13:00:07 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: nonints.h,v 1.89 2020/08/01 18:02:37 rillig Exp $      */
+/*     $NetBSD: nonints.h,v 1.90 2020/08/08 13:00:07 rillig Exp $      */
 
 /*-
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -200,8 +200,8 @@
 void Var_Stats(void);
 void Var_Dump(GNode *);
 void Var_ExportVars(void);
-void Var_Export(char *, int);
-void Var_UnExport(char *);
+void Var_Export(const char *, int);
+void Var_UnExport(const char *);
 
 /* util.c */
 void (*bmake_signal(int, void (*)(int)))(int);
diff -r 983fcb99eb68 -r 5232e8909492 usr.bin/make/unit-tests/export-variants.mk
--- a/usr.bin/make/unit-tests/export-variants.mk        Sat Aug 08 12:43:06 2020 +0000
+++ b/usr.bin/make/unit-tests/export-variants.mk        Sat Aug 08 13:00:07 2020 +0000
@@ -1,40 +1,40 @@
-# $NetBSD: export-variants.mk,v 1.1 2020/08/07 19:24:27 rillig Exp $
-#
-# Test whether exported variables apply to each variant of running
-# external commands:
-#
-# The != assignments.
-# The :!cmd! modifier.
-# The :sh modifier.
-
-SHVAR!=        env | grep ^UT_ || true
-.if ${SHVAR} != ""
-.warning At this point, no variable should be exported.
-.endif
-
-.if ${:!env | grep ^UT_ || true!} != ""
-.warning At this point, no variable should be exported.
-.endif
-
-.if ${env | grep ^UT_ || true:L:sh} != ""
-.warning At this point, no variable should be exported.
-.endif
-
-UT_VAR=                value
-.export UT_VAR
-
-SHVAR!=        env | grep ^UT_ || true
-.if ${SHVAR} != "UT_VAR=value"
-.warning At this point, no variable should be exported.
-.endif
-
-.if ${:!env | grep ^UT_ || true!} != "UT_VAR=value"
-.warning At this point, some variables should be exported.
-.endif
-
-.if ${env | grep ^UT_ || true:L:sh} != "UT_VAR=value"
-.warning At this point, some variables should be exported.
-.endif
-
-all:
-       @:;
+# $NetBSD: export-variants.mk,v 1.2 2020/08/08 13:00:07 rillig Exp $
+#
+# Test whether exported variables apply to each variant of running
+# external commands:
+#
+# The != assignments.
+# The :!cmd! modifier.
+# The :sh modifier.
+
+SHVAR!=        env | grep ^UT_ || true
+.if ${SHVAR} != ""
+.warning At this point, no variable should be exported.
+.endif
+
+.if ${:!env | grep ^UT_ || true!} != ""
+.warning At this point, no variable should be exported.
+.endif
+
+.if ${env | grep ^UT_ || true:L:sh} != ""
+.warning At this point, no variable should be exported.
+.endif
+
+UT_VAR=                value
+.export UT_VAR
+
+SHVAR!=        env | grep ^UT_ || true
+.if ${SHVAR} != "UT_VAR=value"
+.warning At this point, no variable should be exported.
+.endif
+
+.if ${:!env | grep ^UT_ || true!} != "UT_VAR=value"
+.warning At this point, some variables should be exported.
+.endif
+
+.if ${env | grep ^UT_ || true:L:sh} != "UT_VAR=value"
+.warning At this point, some variables should be exported.
+.endif
+
+all:
+       @:;
diff -r 983fcb99eb68 -r 5232e8909492 usr.bin/make/unit-tests/export.mk
--- a/usr.bin/make/unit-tests/export.mk Sat Aug 08 12:43:06 2020 +0000
+++ b/usr.bin/make/unit-tests/export.mk Sat Aug 08 13:00:07 2020 +0000
@@ -1,11 +1,11 @@
-# $Id: export.mk,v 1.4 2020/07/28 18:53:07 sjg Exp $
+# $Id: export.mk,v 1.5 2020/08/08 13:00:07 rillig Exp $
 
 UT_TEST=export
 UT_FOO=foo${BAR}
 UT_FU=fubar
 UT_ZOO=hoopie
 UT_NO=all
-# belive it or not, we expect this one to come out with $UT_FU unexpanded.
+# believe it or not, we expect this one to come out with $UT_FU unexpanded.
 UT_DOLLAR= This is $$UT_FU
 
 .export UT_FU UT_FOO
diff -r 983fcb99eb68 -r 5232e8909492 usr.bin/make/unit-tests/lint.mk
--- a/usr.bin/make/unit-tests/lint.mk   Sat Aug 08 12:43:06 2020 +0000
+++ b/usr.bin/make/unit-tests/lint.mk   Sat Aug 08 13:00:07 2020 +0000
@@ -1,17 +1,17 @@
-# $NetBSD: lint.mk,v 1.1 2020/08/03 15:43:32 rillig Exp $
-#
-# Demonstrates stricter checks that are only enabled in the lint mode,
-# using the -dL option.
-
-# Ouch: as of 2020-08-03, make exits successfully even though the error
-# message has been issued as PARSE_FATAL.
-
-# Ouch: as of 2020-08-03, the variable is malformed and parsing stops
-# for a moment, but is continued after the wrongly-guessed end of the
-# variable, which echoes "y@:Q}".
-
-all: mod-loop-varname
-
-mod-loop-varname:
-       @echo ${VAR:Uvalue:@${:Ubar:S,b,v,}@x${var}y@:Q}
-       @echo ${VAR:Uvalue:@!@x$!y@:Q}  # surprisingly allowed
+# $NetBSD: lint.mk,v 1.2 2020/08/08 13:00:07 rillig Exp $
+#
+# Demonstrates stricter checks that are only enabled in the lint mode,
+# using the -dL option.
+
+# Ouch: as of 2020-08-03, make exits successfully even though the error
+# message has been issued as PARSE_FATAL.
+
+# Ouch: as of 2020-08-03, the variable is malformed and parsing stops
+# for a moment, but is continued after the wrongly-guessed end of the
+# variable, which echoes "y@:Q}".
+
+all: mod-loop-varname
+
+mod-loop-varname:
+       @echo ${VAR:Uvalue:@${:Ubar:S,b,v,}@x${var}y@:Q}
+       @echo ${VAR:Uvalue:@!@x$!y@:Q}  # surprisingly allowed
diff -r 983fcb99eb68 -r 5232e8909492 usr.bin/make/unit-tests/unexport.mk
--- a/usr.bin/make/unit-tests/unexport.mk       Sat Aug 08 12:43:06 2020 +0000
+++ b/usr.bin/make/unit-tests/unexport.mk       Sat Aug 08 13:00:07 2020 +0000
@@ -1,4 +1,4 @@
-# $Id: unexport.mk,v 1.2 2020/07/27 19:45:56 rillig Exp $
+# $Id: unexport.mk,v 1.3 2020/08/08 13:00:07 rillig Exp $
 
 # pick up a bunch of exported vars
 FILTER_CMD=    grep ^UT_
@@ -7,3 +7,13 @@
 .unexport UT_ZOO UT_FOO
 
 UT_TEST = unexport
+
+# Until 2020-08-08, Var_UnExport had special handling for '\n', that code
+# was not reachable though.  At that point, backslash-newline has already
+# been replaced with a simple space, and variables are not yet expanded.
+UT_BEFORE_NL=  before
+UT_AFTER_NL=   after
+.export UT_BEFORE_NL UT_AFTER_NL
+.unexport \
+  UT_BEFORE_NL
+.unexport ${.newline} UT_AFTER_NL
diff -r 983fcb99eb68 -r 5232e8909492 usr.bin/make/var.c
--- a/usr.bin/make/var.c        Sat Aug 08 12:43:06 2020 +0000
+++ b/usr.bin/make/var.c        Sat Aug 08 13:00:07 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: var.c,v 1.422 2020/08/08 12:43:06 rillig Exp $ */
+/*     $NetBSD: var.c,v 1.423 2020/08/08 13:00:07 rillig Exp $ */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -69,14 +69,14 @@
  */
 
 #ifndef MAKE_NATIVE
-static char rcsid[] = "$NetBSD: var.c,v 1.422 2020/08/08 12:43:06 rillig Exp $";
+static char rcsid[] = "$NetBSD: var.c,v 1.423 2020/08/08 13:00:07 rillig Exp $";
 #else
 #include <sys/cdefs.h>
 #ifndef lint
 #if 0
 static char sccsid[] = "@(#)var.c      8.3 (Berkeley) 3/19/94";
 #else
-__RCSID("$NetBSD: var.c,v 1.422 2020/08/08 12:43:06 rillig Exp $");
+__RCSID("$NetBSD: var.c,v 1.423 2020/08/08 13:00:07 rillig Exp $");
 #endif
 #endif /* not lint */
 #endif
@@ -603,7 +603,7 @@
  * It is also called when any exported variable is modified.
  */
 void
-Var_Export(char *str, int isExport)
+Var_Export(const char *str, int isExport)
 {
     VarExportFlags flags;
     char *val;
@@ -655,19 +655,21 @@
  * str must have the form "unexport[-env] varname...".
  */
 void
-Var_UnExport(char *str)
+Var_UnExport(const char *str)
 {
     char tmp[BUFSIZ];
-    char *vlist;
-    char *cp;
+    const char *varnames;
+    char *varnames_freeIt;
     int n;
     Boolean unexport_env;
 
-    vlist = NULL;
+    varnames = NULL;
+    varnames_freeIt = NULL;
 
     str += strlen("unexport");
     unexport_env = strncmp(str, "-env", 4) == 0;
     if (unexport_env) {
+       const char *cp;
        char **newenv;
 
        cp = getenv(MAKE_LEVEL_ENV);    /* we should preserve this */
@@ -681,8 +683,7 @@
            }
            newenv = bmake_malloc(2 * sizeof(char *));
        }
-       if (!newenv)
-           return;
+
        /* Note: we cannot safely free() the original environ. */
        environ = savedEnv = newenv;
        newenv[0] = NULL;
@@ -690,45 +691,50 @@
        if (cp && *cp)
            setenv(MAKE_LEVEL_ENV, cp, 1);
     } else {
-       for (; *str != '\n' && isspace((unsigned char)*str); str++)
+       for (; isspace((unsigned char)*str); str++)
            continue;
-       if (str[0] && str[0] != '\n') {
-           vlist = str;
-       }
+       if (str[0] != '\0')
+           varnames = str;
     }
 
-    if (!vlist) {
+    if (varnames == NULL) {
        /* Using .MAKE.EXPORTED */
-       vlist = Var_Subst("${" MAKE_EXPORTED ":O:u}", VAR_GLOBAL,
-                         VARE_WANTRES);
+       varnames = varnames_freeIt = Var_Subst("${" MAKE_EXPORTED ":O:u}",
+                                              VAR_GLOBAL, VARE_WANTRES);
     }
-    if (vlist) {
+
+    if (TRUE) {
        Var *v;
        char **av;
        char *as;
        int ac;
        int i;
 
-       av = brk_string(vlist, &ac, FALSE, &as);
+       av = brk_string(varnames, &ac, FALSE, &as);
        for (i = 0; i < ac; i++) {
            v = VarFind(av[i], VAR_GLOBAL, 0);
-           if (!v)
+           if (v == NULL) {
+               VAR_DEBUG("Not unexporting \"%s\" (not found)\n", av[i]);
                continue;
-           if (!unexport_env &&
-               (v->flags & (VAR_EXPORTED | VAR_REEXPORT)) == VAR_EXPORTED)
+           }
+
+           VAR_DEBUG("Unexporting \"%s\"\n", av[i]);
+           if (!unexport_env && (v->flags & VAR_EXPORTED) &&
+               !(v->flags & VAR_REEXPORT))
                unsetenv(v->name);