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: replace enum bit-set with struct bit-fields



details:   https://anonhg.NetBSD.org/src/rev/7297c695c579
branches:  trunk
changeset: 960957:7297c695c579
user:      rillig <rillig%NetBSD.org@localhost>
date:      Sat Apr 03 23:08:30 2021 +0000

description:
make: replace enum bit-set with struct bit-fields

This makes the code easier to read, especially when setting one of the
flags to false.

No functional change.

diffstat:

 usr.bin/make/unit-tests/directive-export-impl.exp |   8 +-
 usr.bin/make/unit-tests/directive-export-impl.mk  |  23 +++--
 usr.bin/make/var.c                                |  88 +++++++++++-----------
 3 files changed, 61 insertions(+), 58 deletions(-)

diffs (truncated from 390 to 300 lines):

diff -r c1a4c870dfb7 -r 7297c695c579 usr.bin/make/unit-tests/directive-export-impl.exp
--- a/usr.bin/make/unit-tests/directive-export-impl.exp Sat Apr 03 22:44:43 2021 +0000
+++ b/usr.bin/make/unit-tests/directive-export-impl.exp Sat Apr 03 23:08:30 2021 +0000
@@ -23,7 +23,7 @@
 Var_Parse: ${REF}> (eval)
 Result of ${:!echo "\$UT_VAR"!} is "<>" (eval-defined, defined)
 lhs = "<>", rhs = "<>", op = !=
-ParseReadLine (49): ': ${UT_VAR:N*}'
+ParseReadLine (50): ': ${UT_VAR:N*}'
 Var_Parse: ${UT_VAR:N*} (eval-defined)
 Var_Parse: ${REF}> (eval-defined)
 Applying ${UT_VAR:N...} to "<>" (eval-defined, regular)
@@ -31,7 +31,7 @@
 ModifyWords: split "<>" into 1 words
 Result of ${UT_VAR:N*} is "" (eval-defined, regular)
 ParseDoDependency(: )
-ParseReadLine (53): 'REF=              defined'
+ParseReadLine (54): 'REF=              defined'
 Global:REF = defined
 CondParser_Eval: ${:!echo "\$UT_VAR"!} != "<defined>"
 Var_Parse: ${:!echo "\$UT_VAR"!} != "<defined>" (eval-defined)
@@ -46,10 +46,10 @@
 Var_Parse: ${REF}> (eval)
 Result of ${:!echo "\$UT_VAR"!} is "<defined>" (eval-defined, defined)
 lhs = "<defined>", rhs = "<defined>", op = !=
-ParseReadLine (61): 'all:'
+ParseReadLine (62): 'all:'
 ParseDoDependency(all:)
 Global:.ALLTARGETS =  all
-ParseReadLine (62): '.MAKEFLAGS: -d0'
+ParseReadLine (63): '.MAKEFLAGS: -d0'
 ParseDoDependency(.MAKEFLAGS: -d0)
 Global:.MAKEFLAGS =  -r -k -d cpv -d
 Global:.MAKEFLAGS =  -r -k -d cpv -d 0
diff -r c1a4c870dfb7 -r 7297c695c579 usr.bin/make/unit-tests/directive-export-impl.mk
--- a/usr.bin/make/unit-tests/directive-export-impl.mk  Sat Apr 03 22:44:43 2021 +0000
+++ b/usr.bin/make/unit-tests/directive-export-impl.mk  Sat Apr 03 23:08:30 2021 +0000
@@ -1,4 +1,4 @@
-# $NetBSD: directive-export-impl.mk,v 1.2 2021/02/16 16:28:41 rillig Exp $
+# $NetBSD: directive-export-impl.mk,v 1.3 2021/04/03 23:08:30 rillig Exp $
 #
 # Test for the implementation of exporting variables to child processes.
 # This involves marking variables for export, actually exporting them,
@@ -8,8 +8,8 @@
 #      Var_Export
 #      ExportVar
 #      VarExportedMode (global)
-#      VFL_EXPORTED (per variable)
-#      VFL_REEXPORT (per variable)
+#      VarFlags.exported (per variable)
+#      VarFlags.reexport (per variable)
 #      VarExportMode (per call of Var_Export and ExportVar)
 
 : ${:U:sh}                     # side effect: initialize .SHELL
@@ -22,13 +22,13 @@
 
 # At this point, ExportVar("UT_VAR", VEM_PLAIN) is called.  Since the
 # variable value refers to another variable, ExportVar does not actually
-# export the variable but only marks it as VFL_EXPORTED and VFL_REEXPORT.
-# After that, ExportVars registers the variable name in .MAKE.EXPORTED.
-# That's all for now.
+# export the variable but only marks it as VarFlags.exported and
+# VarFlags.reexport.  After that, ExportVars registers the variable name in
+# .MAKE.EXPORTED.  That's all for now.
 .export UT_VAR
 
-# Evaluating this expression shows the variable flags in the debug log,
-# which are VFL_EXPORTED|VFL_REEXPORT.
+# The following expression has both flags 'exported' and 'reexport' set.
+# These flags do not show up anywhere, not even in the debug log.
 : ${UT_VAR:N*}
 
 # At the last moment before actually forking off the child process for the
@@ -43,9 +43,10 @@
 .  error
 .endif
 
-# Evaluating this expression shows the variable flags in the debug log,
-# which are still VFL_EXPORTED|VFL_REEXPORT, which means that the variable
-# is still marked as being re-exported for each child process.
+# The following expression still has 'exported' and 'reexport' set.
+# These flags do not show up anywhere though, not even in the debug log.
+# These flags means that the variable is still marked as being re-exported
+# for each child process.
 : ${UT_VAR:N*}
 
 # Now the referenced variable gets defined.  This does not influence anything
diff -r c1a4c870dfb7 -r 7297c695c579 usr.bin/make/var.c
--- a/usr.bin/make/var.c        Sat Apr 03 22:44:43 2021 +0000
+++ b/usr.bin/make/var.c        Sat Apr 03 23:08:30 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: var.c,v 1.900 2021/04/03 22:06:23 rillig Exp $ */
+/*     $NetBSD: var.c,v 1.901 2021/04/03 23:08:30 rillig Exp $ */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -140,29 +140,27 @@
 #include "metachar.h"
 
 /*     "@(#)var.c      8.3 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: var.c,v 1.900 2021/04/03 22:06:23 rillig Exp $");
-
-typedef enum VarFlags {
-       VFL_NONE        = 0,
-
+MAKE_RCSID("$NetBSD: var.c,v 1.901 2021/04/03 23:08:30 rillig Exp $");
+
+typedef struct VarFlags {
        /*
         * The variable's value is currently being used by Var_Parse or
         * Var_Subst.  This marker is used to avoid endless recursion.
         */
-       VFL_IN_USE      = 1 << 0,
+       bool inUse: 1;
 
        /*
         * The variable comes from the environment.
         * These variables are not registered in any GNode, therefore they
         * must be freed as soon as they are not used anymore.
         */
-       VFL_FROM_ENV    = 1 << 1,
+       bool fromEnv: 1;
 
        /*
         * The variable is exported to the environment, to be used by child
         * processes.
         */
-       VFL_EXPORTED    = 1 << 2,
+       bool exported: 1;
 
        /*
         * At the point where this variable was exported, it contained an
@@ -170,10 +168,10 @@
         * process is started, it needs to be exported again, in the hope
         * that the referenced variable can then be resolved.
         */
-       VFL_REEXPORT    = 1 << 3,
+       bool reexport: 1;
 
        /* The variable came from the command line. */
-       VFL_FROM_CMD    = 1 << 4,
+       bool fromCmd: 1;
 
        /*
         * The variable value cannot be changed anymore, and the variable
@@ -182,7 +180,7 @@
         *
         * See VAR_SET_READONLY.
         */
-       VFL_READONLY    = 1 << 5
+       bool readOnly: 1;
 } VarFlags;
 
 /*
@@ -351,14 +349,17 @@
 
 
 static Var *
-VarNew(FStr name, const char *value, VarFlags flags)
+VarNew(FStr name, const char *value, bool fromEnv, bool readOnly)
 {
        size_t value_len = strlen(value);
+       VarFlags vflags = { false, false, false, false, false, false };
        Var *var = bmake_malloc(sizeof *var);
        var->name = name;
        Buf_InitSize(&var->val, value_len + 1);
        Buf_AddBytes(&var->val, value, value_len);
-       var->flags = flags;
+       vflags.fromEnv = fromEnv;
+       vflags.readOnly = readOnly;
+       var->flags = vflags;
        return var;
 }
 
@@ -456,7 +457,7 @@
 
                if ((env = getenv(name)) != NULL) {
                        char *varname = bmake_strdup(name);
-                       return VarNew(FStr_InitOwn(varname), env, VFL_FROM_ENV);
+                       return VarNew(FStr_InitOwn(varname), env, true, false);
                }
 
                if (opts.checkEnvFirst && scope != SCOPE_GLOBAL) {
@@ -477,7 +478,7 @@
 static void
 VarFreeEnv(Var *v)
 {
-       if (!(v->flags & VFL_FROM_ENV))
+       if (!v->flags.fromEnv)
                return;
 
        FStr_Done(&v->name);
@@ -491,7 +492,7 @@
 {
        HashEntry *he = HashTable_CreateEntry(&scope->vars, name, NULL);
        Var *v = VarNew(FStr_InitRefer(/* aliased to */ he->key), value,
-           flags & VAR_SET_READONLY ? VFL_READONLY : VFL_NONE);
+           false, (flags & VAR_SET_READONLY) != 0);
        HashEntry_Set(he, v);
        DEBUG3(VAR, "%s:%s = %s\n", scope->name, name, value);
        return v;
@@ -514,7 +515,7 @@
 
        DEBUG2(VAR, "%s:delete %s\n", scope->name, varname);
        v = he->value;
-       if (v->flags & VFL_EXPORTED)
+       if (v->flags.exported)
                unsetenv(v->name.str);
        if (strcmp(v->name.str, MAKE_EXPORTED) == 0)
                var_exportedVars = VAR_EXPORTED_NONE;
@@ -615,16 +616,16 @@
        char *val = v->val.data;
        char *expr;
 
-       if ((v->flags & VFL_EXPORTED) && !(v->flags & VFL_REEXPORT))
+       if (v->flags.exported && !v->flags.reexport)
                return false;   /* nothing to do */
 
        if (strchr(val, '$') == NULL) {
-               if (!(v->flags & VFL_EXPORTED))
+               if (!v->flags.exported)
                        setenv(name, val, 1);
                return true;
        }
 
-       if (v->flags & VFL_IN_USE) {
+       if (v->flags.inUse) {
                /*
                 * We recursed while exporting in a child.
                 * This isn't going to end well, just skip it.
@@ -647,8 +648,8 @@
 {
        if (strchr(v->val.data, '$') == NULL) {
                setenv(v->name.str, v->val.data, 1);
-               v->flags |= VFL_EXPORTED;
-               v->flags &= ~(unsigned)VFL_REEXPORT;
+               v->flags.exported = true;
+               v->flags.reexport = false;
                return true;
        }
 
@@ -658,17 +659,18 @@
         * the child process can do it at the last minute.
         * Avoid calling setenv more often than necessary since it can leak.
         */
-       v->flags |= VFL_EXPORTED | VFL_REEXPORT;
+       v->flags.exported = true;
+       v->flags.reexport = true;
        return true;
 }
 
 static bool
 ExportVarLiteral(Var *v)
 {
-       if ((v->flags & VFL_EXPORTED) && !(v->flags & VFL_REEXPORT))
+       if (v->flags.exported && !v->flags.reexport)
                return false;
 
-       if (!(v->flags & VFL_EXPORTED))
+       if (!v->flags.exported)
                setenv(v->name.str, v->val.data, 1);
 
        return true;
@@ -874,10 +876,10 @@
        }
 
        DEBUG1(VAR, "Unexporting \"%s\"\n", varname);
-       if (what != UNEXPORT_ENV &&
-           (v->flags & VFL_EXPORTED) && !(v->flags & VFL_REEXPORT))
+       if (what != UNEXPORT_ENV && v->flags.exported && !v->flags.reexport)
                unsetenv(v->name.str);
-       v->flags &= ~(unsigned)(VFL_EXPORTED | VFL_REEXPORT);
+       v->flags.exported = false;
+       v->flags.reexport = false;
 
        if (what == UNEXPORT_NAMED) {
                /* Remove the variable names from .MAKE.EXPORTED. */
@@ -945,7 +947,7 @@
        if (v == NULL)
                return false;
 
-       if (v->flags & VFL_FROM_CMD) {
+       if (v->flags.fromCmd) {
                DEBUG3(VAR, "%s:%s = %s ignored!\n",
                    SCOPE_GLOBAL->name, name, val);
                return true;
@@ -990,7 +992,7 @@
                }
                v = VarAdd(name, val, scope, flags);
        } else {
-               if ((v->flags & VFL_READONLY) && !(flags & VAR_SET_READONLY)) {
+               if (v->flags.readOnly && !(flags & VAR_SET_READONLY)) {
                        DEBUG3(VAR, "%s:%s = %s ignored (read-only)\n",
                            scope->name, name, val);
                        return;
@@ -999,7 +1001,7 @@
                Buf_AddStr(&v->val, val);
 
                DEBUG3(VAR, "%s:%s = %s\n", scope->name, name, val);
-               if (v->flags & VFL_EXPORTED)



Home | Main Index | Thread Index | Old Index