Module Name: src Committed By: rillig Date: Thu Nov 2 05:40:49 UTC 2023
Modified Files: src/usr.bin/make: main.c parse.c var.c src/usr.bin/make/unit-tests: varmod-defined.mk Log Message: make: clean up comments No functional change. To generate a diff of this commit: cvs rdiff -u -r1.601 -r1.602 src/usr.bin/make/main.c cvs rdiff -u -r1.707 -r1.708 src/usr.bin/make/parse.c cvs rdiff -u -r1.1065 -r1.1066 src/usr.bin/make/var.c cvs rdiff -u -r1.14 -r1.15 src/usr.bin/make/unit-tests/varmod-defined.mk Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/usr.bin/make/main.c diff -u src/usr.bin/make/main.c:1.601 src/usr.bin/make/main.c:1.602 --- src/usr.bin/make/main.c:1.601 Thu Nov 2 04:50:44 2023 +++ src/usr.bin/make/main.c Thu Nov 2 05:40:49 2023 @@ -1,4 +1,4 @@ -/* $NetBSD: main.c,v 1.601 2023/11/02 04:50:44 rillig Exp $ */ +/* $NetBSD: main.c,v 1.602 2023/11/02 05:40:49 rillig Exp $ */ /* * Copyright (c) 1988, 1989, 1990, 1993 @@ -111,7 +111,7 @@ #include "trace.h" /* "@(#)main.c 8.3 (Berkeley) 3/19/94" */ -MAKE_RCSID("$NetBSD: main.c,v 1.601 2023/11/02 04:50:44 rillig Exp $"); +MAKE_RCSID("$NetBSD: main.c,v 1.602 2023/11/02 05:40:49 rillig Exp $"); #if defined(MAKE_NATIVE) && !defined(lint) __COPYRIGHT("@(#) Copyright (c) 1988, 1989, 1990, 1993 " "The Regents of the University of California. " @@ -121,7 +121,7 @@ __COPYRIGHT("@(#) Copyright (c) 1988, 19 CmdOpts opts; time_t now; /* Time at start of make */ GNode *defaultNode; /* .DEFAULT node */ -bool allPrecious; /* .PRECIOUS given on line by itself */ +bool allPrecious; /* .PRECIOUS given on a line by itself */ bool deleteOnError; /* .DELETE_ON_ERROR: set */ static int maxJobTokens; /* -j argument */ @@ -148,7 +148,7 @@ static HashTable cached_realpaths; /* * For compatibility with the POSIX version of MAKEFLAGS that includes - * all the options without '-', convert 'flags' to '-f -l -a -g -s'. + * all the options without '-', convert 'flags' to '-f -l -a -g -s '. */ static char * explode(const char *flags) @@ -439,7 +439,6 @@ MainParseArgJobs(const char *arg) static void MainParseArgSysInc(const char *argvalue) { - /* look for magic parent directory search string */ if (strncmp(".../", argvalue, 4) == 0) { char *found_path = Dir_FindHereOrAbove(curdir, argvalue + 4); if (found_path == NULL) @@ -1015,20 +1014,14 @@ InitVarMachineArch(void) #ifndef NO_PWD_OVERRIDE /* - * All this code is so that we know where we are when we start up - * on a different machine with pmake. - * - * XXX: Make no longer has "local" and "remote" mode. Is this code still - * necessary? - * * Overriding getcwd() with $PWD totally breaks MAKEOBJDIRPREFIX * since the value of curdir can vary depending on how we got - * here. Ie sitting at a shell prompt (shell that provides $PWD) - * or via subdir.mk in which case its likely a shell which does + * here. That is, sitting at a shell prompt (shell that provides $PWD) + * or via subdir.mk, in which case it's likely a shell which does * not provide it. * * So, to stop it breaking this case only, we ignore PWD if - * MAKEOBJDIRPREFIX is set or MAKEOBJDIR contains a variable expression. + * MAKEOBJDIRPREFIX is set or MAKEOBJDIR contains an expression. */ static void HandlePWD(const struct stat *curdir_st) @@ -1346,22 +1339,12 @@ main_Init(int argc, char **argv) exit(2); } - /* - * Get the name of this type of MACHINE from utsname - * so we can share an executable for similar machines. - * (i.e. m68k: amiga hp300, mac68k, sun3, ...) - * - * Note that both MACHINE and MACHINE_ARCH are decided at - * run-time. - */ machine = InitVarMachine(&utsname); machine_arch = InitVarMachineArch(); myPid = getpid(); /* remember this for vFork() */ - /* - * Just in case MAKEOBJDIR wants us to do something tricky. - */ + /* Just in case MAKEOBJDIR wants us to do something tricky. */ Targ_Init(); Var_Init(); Global_Set_ReadOnly(".MAKE.OS", utsname.sysname); @@ -1370,7 +1353,7 @@ main_Init(int argc, char **argv) #ifdef MAKE_VERSION Global_Set("MAKE_VERSION", MAKE_VERSION); #endif - Global_Set_ReadOnly(".newline", "\n"); /* handy for :@ loops */ + Global_Set_ReadOnly(".newline", "\n"); #ifndef MAKEFILE_PREFERENCE_LIST /* This is the traditional preference for makefiles. */ # define MAKEFILE_PREFERENCE_LIST "makefile Makefile" Index: src/usr.bin/make/parse.c diff -u src/usr.bin/make/parse.c:1.707 src/usr.bin/make/parse.c:1.708 --- src/usr.bin/make/parse.c:1.707 Thu Nov 2 04:50:44 2023 +++ src/usr.bin/make/parse.c Thu Nov 2 05:40:49 2023 @@ -1,4 +1,4 @@ -/* $NetBSD: parse.c,v 1.707 2023/11/02 04:50:44 rillig Exp $ */ +/* $NetBSD: parse.c,v 1.708 2023/11/02 05:40:49 rillig Exp $ */ /* * Copyright (c) 1988, 1989, 1990, 1993 @@ -105,7 +105,7 @@ #include "pathnames.h" /* "@(#)parse.c 8.3 (Berkeley) 3/19/94" */ -MAKE_RCSID("$NetBSD: parse.c,v 1.707 2023/11/02 04:50:44 rillig Exp $"); +MAKE_RCSID("$NetBSD: parse.c,v 1.708 2023/11/02 05:40:49 rillig Exp $"); /* Detects a multiple-inclusion guard in a makefile. */ typedef enum { @@ -550,7 +550,7 @@ ParseErrorInternal(const GNode *gn, } /* - * Print a parse error message, including location information. + * Print a message, including location information. * * If the level is PARSE_FATAL, continue parsing until the end of the * current top-level makefile, then exit (see Parse_File). @@ -605,8 +605,7 @@ HandleMessage(ParseErrorLevel level, con /* * Add the child to the parent's children, and for non-special targets, vice - * versa. Special targets such as .END do not need to be informed once the - * child target has been made. + * versa. */ static void LinkSource(GNode *pgn, GNode *cgn, bool isSpecial) @@ -617,7 +616,10 @@ LinkSource(GNode *pgn, GNode *cgn, bool Lst_Append(&pgn->children, cgn); pgn->unmade++; - /* Special targets like .END don't need any children. */ + /* + * Special targets like .END do not need to be informed once the child + * target has been made. + */ if (!isSpecial) Lst_Append(&cgn->parents, pgn); @@ -656,10 +658,9 @@ TryApplyDependencyOperator(GNode *gn, GN if (op == OP_DOUBLEDEP && (gn->type & OP_OPMASK) == OP_DOUBLEDEP) { /* * If the node was on the left-hand side of a '::' operator, - * we need to create a new instance of it for the children - * and commands on this dependency line since each of these - * dependency groups has its own attributes and commands, - * separate from the others. + * create a new node for the children and commands on this + * dependency line, since each of these dependency groups has + * its own attributes and commands, separate from the others. * * The new instance is placed on the 'cohorts' list of the * initial one (note the initial one is not on its own @@ -678,7 +679,7 @@ TryApplyDependencyOperator(GNode *gn, GN if (doing_depend) RememberLocation(cohort); /* - * Make the cohort invisible as well to avoid duplicating it + * Make the cohort invisible to avoid duplicating it * into other variables. True, parents of this target won't * tend to do anything with their local variables, but better * safe than sorry. @@ -693,11 +694,7 @@ TryApplyDependencyOperator(GNode *gn, GN snprintf(cohort->cohort_num, sizeof cohort->cohort_num, "#%d", (unsigned int)gn->unmade_cohorts % 1000000); } else { - /* - * We don't want to nuke any previous flags (whatever they - * were) so we just OR the new operator into the old. - */ - gn->type |= op; + gn->type |= op; /* preserve any previous flags */ } return true; @@ -714,12 +711,12 @@ ApplyDependencyOperator(GNodeType op) } /* - * We add a .WAIT node in the dependency list. After any dynamic dependencies + * Add a .WAIT node in the dependency list. After any dynamic dependencies * (and filename globbing) have happened, it is given a dependency on each * previous child, back until the previous .WAIT node. The next child won't * be scheduled until the .WAIT node is built. * - * We give each .WAIT node a unique name (mainly for diagnostics). + * Give each .WAIT node a unique name (mainly for diagnostics). */ static void ApplyDependencySourceWait(bool isSpecial) @@ -803,9 +800,7 @@ ApplyDependencySourceOrder(const char *s Targ_PrintNode(gn, 0); } } - /* - * The current source now becomes the predecessor for the next one. - */ + /* The current source now becomes the predecessor for the next one. */ order_pred = gn; } @@ -901,16 +896,9 @@ ParseDependencyTargetWord(char **pp, con break; if (*cp == '$') { - /* - * Must be a dynamic source (would have been expanded - * otherwise). - * - * There should be no errors in this, as they would - * have been discovered in the initial Var_Subst and - * we wouldn't be here. - */ FStr val = Var_Parse(&cp, SCOPE_CMDLINE, VARE_PARSE_ONLY); + /* TODO: handle errors */ FStr_Done(&val); } else cp++; @@ -1578,7 +1566,6 @@ ParseDependencySources(char *p, GNodeTyp return; } - /* Now go for the sources. */ switch (special) { case SP_INCLUDES: case SP_LIBS: @@ -1825,7 +1812,7 @@ VarAssign_EvalSubst(GNode *scope, const char *evalue; /* - * make sure that we set the variable the first time to nothing + * Make sure that we set the variable the first time to nothing * so that it gets substituted. * * TODO: Add a test that demonstrates why this code is needed, @@ -1975,16 +1962,10 @@ MaybeSubMake(const char *cmd) return false; } -/* - * Append the command to the target node. - * - * The node may be marked as a submake node if the command is determined to - * be that. - */ +/* Append the command to the target node. */ static void GNode_AddCommand(GNode *gn, char *cmd) { - /* Add to last (ie current) cohort for :: targets */ if ((gn->type & OP_DOUBLEDEP) && gn->cohorts.last != NULL) gn = gn->cohorts.last->datum; @@ -2042,7 +2023,6 @@ ParseInclude(char *directive) endc = '"'; file = FStr_InitRefer(p); - /* Skip to matching delimiter */ while (*p != '\0' && *p != endc) p++; @@ -2318,9 +2298,8 @@ ParseGmakeExport(char *line) #endif /* - * Called when EOF is reached in the current file. If we were reading an - * include file or a .for loop, the includes stack is popped and things set - * up to go back to reading the previous file at the previous location. + * When the end of the current file or .for loop is reached, continue reading + * the previous file at the previous location. * * Results: * true to continue parsing, i.e. it had only reached the end of an @@ -2583,23 +2562,9 @@ SkipIrrelevantBranches(void) { const char *line; - while ((line = ReadLowLevelLine(LK_DOT)) != NULL) { + while ((line = ReadLowLevelLine(LK_DOT)) != NULL) if (Cond_EvalLine(line) == CR_TRUE) return true; - /* - * TODO: Check for typos in .elif directives such as .elsif - * or .elseif. - * - * This check will probably duplicate some of the code in - * ParseLine. Most of the code there cannot apply, only - * ParseVarassign and ParseDependencyLine can, and to prevent - * code duplication, these would need to be called with a - * flag called onlyCheckSyntax. - * - * See directive-elif.mk for details. - */ - } - return false; } @@ -2638,9 +2603,9 @@ ParseForLoop(const char *line) /* * Read an entire line from the input file. * - * Empty lines, .if and .for are completely handled by this function, - * leaving only variable assignments, other directives, dependency lines - * and shell commands to the caller. + * Empty lines, .if and .for are handled by this function, while variable + * assignments, other directives, dependency lines and shell commands are + * handled by the caller. * * Return a line without trailing whitespace, or NULL for EOF. The returned * string will be freed at the end of including the file. @@ -2871,10 +2836,6 @@ FindSemicolon(char *p) return p; } -/* - * dependency -> [target...] op [source...] [';' command] - * op -> ':' | '::' | '!' - */ static void ParseDependencyLine(char *line) { @@ -2882,11 +2843,6 @@ ParseDependencyLine(char *line) char *expanded_line; const char *shellcmd = NULL; - /* - * For some reason - probably to make the parser impossible - - * a ';' can be used to separate commands from dependencies. - * Attempt to skip over ';' inside substitution patterns. - */ { char *semicolon = FindSemicolon(line); if (*semicolon != '\0') { @@ -2897,7 +2853,7 @@ ParseDependencyLine(char *line) } /* - * We now know it's a dependency line so it needs to have all + * We now know it's a dependency line, so it needs to have all * variables expanded before being parsed. * * XXX: Ideally the dependency line would first be split into @@ -2965,9 +2921,6 @@ ParseLine(char *line) #ifdef SYSVINCLUDE if (IsSysVInclude(line)) { - /* - * It's an S3/S5-style "include". - */ ParseTraditionalInclude(line); return; } @@ -2976,9 +2929,6 @@ ParseLine(char *line) #ifdef GMAKEEXPORT if (strncmp(line, "export", 6) == 0 && ch_isspace(line[6]) && strchr(line, ':') == NULL) { - /* - * It's a Gmake "export". - */ ParseGmakeExport(line); return; } @@ -2992,10 +2942,7 @@ ParseLine(char *line) ParseDependencyLine(line); } -/* - * Parse a top-level makefile, incorporating its content into the global - * dependency graph. - */ +/* Interpret a top-level makefile. */ void Parse_File(const char *name, int fd) { @@ -3016,7 +2963,6 @@ Parse_File(const char *name, int fd) CurFile()->lineno, line); ParseLine(line); } - /* Reached EOF, but it may be just EOF of an include file. */ } while (ParseEOF()); FinishDependencyGroup(); Index: src/usr.bin/make/var.c diff -u src/usr.bin/make/var.c:1.1065 src/usr.bin/make/var.c:1.1066 --- src/usr.bin/make/var.c:1.1065 Thu Nov 2 05:14:58 2023 +++ src/usr.bin/make/var.c Thu Nov 2 05:40:49 2023 @@ -1,4 +1,4 @@ -/* $NetBSD: var.c,v 1.1065 2023/11/02 05:14:58 rillig Exp $ */ +/* $NetBSD: var.c,v 1.1066 2023/11/02 05:40:49 rillig Exp $ */ /* * Copyright (c) 1988, 1989, 1990, 1993 @@ -139,7 +139,7 @@ #include "metachar.h" /* "@(#)var.c 8.3 (Berkeley) 3/19/94" */ -MAKE_RCSID("$NetBSD: var.c,v 1.1065 2023/11/02 05:14:58 rillig Exp $"); +MAKE_RCSID("$NetBSD: var.c,v 1.1066 2023/11/02 05:40:49 rillig Exp $"); /* * Variables are defined using one of the VAR=value assignments. Their @@ -213,8 +213,8 @@ typedef struct Var { /* * At the point where this variable was exported, it contained an * unresolved reference to another variable. Before any child - * process is started, it needs to be exported again, in the hope - * that the referenced variable can then be resolved. + * process is started, it needs to be actually exported, resolving + * the referenced variable just in time. */ bool reexport:1; } Var; @@ -1198,11 +1198,11 @@ Var_ExistsExpand(GNode *scope, const cha /* * Return the unexpanded value of the given variable in the given scope, - * or the usual scopes. + * falling back to the command, global and environment scopes, in this order, + * but see the -e option. * * Input: - * scope scope in which to search for it - * name name to find, is not expanded any further + * name the name to find, is not expanded any further * * Results: * The value if the variable exists, NULL if it doesn't. @@ -1227,9 +1227,7 @@ Var_Value(GNode *scope, const char *name return FStr_InitOwn(value); } -/* - * set readOnly attribute of specified var if it exists - */ +/* Set or clear the read-only attribute of the specified var if it exists. */ void Var_ReadOnly(const char *name, bool bf) { @@ -1932,8 +1930,8 @@ FormatTime(const char *fmt, time_t t, bo * parsing position is well-known in ApplyModifiers.) * * If parsing fails and the SysV modifier ${VAR:from=to} should not be used - * as a fallback, either issue an error message using Error or Parse_Error - * and then return AMR_CLEANUP, or return AMR_BAD for the default error + * as a fallback, issue an error message using Parse_Error (preferred over + * Error) and then return AMR_CLEANUP, or return AMR_BAD for the default error * message. Both of these return values will stop processing the variable * expression. (XXX: As of 2020-08-23, evaluation of the whole string * continues nevertheless after skipping a few bytes, which essentially is @@ -1950,19 +1948,19 @@ FormatTime(const char *fmt, time_t t, bo * * Evaluating the modifier usually takes the current value of the variable * expression from ch->expr->value, or the variable name from ch->var->name - * and stores the result back in expr->value via Expr_SetValueOwn or + * and stores the result back in ch->expr->value via Expr_SetValueOwn or * Expr_SetValueRefer. * * If evaluating fails (as of 2020-08-23), an error message is printed using - * Error. This function has no side-effects, it really just prints the error + * Error. This function has no side effects, it really just prints the error * message. Processing the expression continues as if everything were ok. - * XXX: This should be fixed by adding proper error handling to Var_Subst, + * TODO: This should be fixed by adding proper error handling to Var_Subst, * Var_Parse, ApplyModifiers and ModifyWords. * * Housekeeping * * Some modifiers such as :D and :U turn undefined expressions into defined - * expressions (see Expr_Define). + * expressions using Expr_Define. * * Some modifiers need to free some memory. */ @@ -2033,10 +2031,10 @@ typedef struct ModChain { char const_member startc; /* '\0' or '}' or ')' */ char const_member endc; - /* Word separator in expansions (see the :ts modifier). */ + /* Word separator when joining words (see the :ts modifier). */ char sep; /* - * True if some modifiers that otherwise split the variable value + * Whether some modifiers that otherwise split the variable value * into words, like :S and :C, treat the variable value as a single * big word, possibly containing spaces. */ @@ -2143,8 +2141,8 @@ ParseModifierPartExpr(const char **pp, L * In a part of a modifier, parse some text that looks like a subexpression. * If the text starts with '$(', any '(' and ')' must be balanced. * If the text starts with '${', any '{' and '}' must be balanced. - * If the text starts with '$', that '$' is copied, it is not parsed as a - * short-name variable expression. + * If the text starts with '$', that '$' is copied verbatim, it is not parsed + * as a short-name variable expression. */ static void ParseModifierPartBalanced(const char **pp, LazyBuf *part) @@ -2184,13 +2182,13 @@ ParseModifierPartSubst( ModChain *ch, LazyBuf *part, /* - * For the first part of the modifier ':S', set anchorEnd if the last + * For the first part of the ':S' modifier, set anchorEnd if the last * character of the pattern is a $. */ PatternFlags *out_pflags, /* - * For the second part of the :S modifier, allow ampersands to be escaped - * and replace unescaped ampersands with subst->lhs. + * For the second part of the ':S' modifier, allow ampersands to be + * escaped and replace unescaped ampersands with subst->lhs. */ struct ModifyWord_SubstArgs *subst ) @@ -2260,7 +2258,7 @@ ParseModifierPart( const char **pp, /* Parsing stops at this delimiter */ char delim, - /* Mode for evaluating nested variables. */ + /* Mode for evaluating nested expressions. */ VarEvalMode emode, ModChain *ch, LazyBuf *part @@ -2461,8 +2459,7 @@ ParseModifier_Defined(const char **pp, M * ParseModifierPart. */ - /* Escaped delimiter or other special character */ - /* See Buf_AddEscaped in for.c. */ + /* See Buf_AddEscaped in for.c for the counterpart. */ if (*p == '\\') { char c = p[1]; if ((IsDelimiter(c, ch) && c != '\0') || @@ -2474,7 +2471,6 @@ ParseModifier_Defined(const char **pp, M } } - /* Nested variable expression */ if (*p == '$') { FStr val = Var_Parse(&p, ch->expr->scope, shouldEval ? ch->expr->emode : VARE_PARSE_ONLY); @@ -2485,7 +2481,6 @@ ParseModifier_Defined(const char **pp, M continue; } - /* Ordinary text */ if (shouldEval) LazyBuf_Add(buf, *p); p++; @@ -3105,7 +3100,7 @@ ApplyModifier_ToSep(const char **pp, Mod goto ok; } - /* ":ts<unrecognised><unrecognised>". */ + /* ":ts<unrecognized><unrecognized>". */ if (sep[0] != '\\') { (*pp)++; /* just for backwards compatibility */ return AMR_BAD; @@ -3135,7 +3130,7 @@ ApplyModifier_ToSep(const char **pp, Mod p++; } else if (!ch_isdigit(sep[1])) { (*pp)++; /* just for backwards compatibility */ - return AMR_BAD; /* ":ts<backslash><unrecognised>". */ + return AMR_BAD; /* ":ts<backslash><unrecognized>". */ } if (!TryParseChar(&p, base, &ch->sep)) { @@ -3231,7 +3226,7 @@ ApplyModifier_To(const char **pp, ModCha return AMR_OK; } - /* Found ":t<unrecognised>:" or ":t<unrecognised><endc>". */ + /* Found ":t<unrecognized>:" or ":t<unrecognized><endc>". */ *pp = mod + 1; /* XXX: unnecessary but observable */ return AMR_BAD; } @@ -3562,7 +3557,7 @@ ApplyModifier_Assign(const char **pp, Mo goto found_op; if ((op[0] == '+' || op[0] == '?' || op[0] == '!') && op[1] == '=') goto found_op; - return AMR_UNKNOWN; /* "::<unrecognised>" */ + return AMR_UNKNOWN; /* "::<unrecognized>" */ found_op: if (expr->name[0] == '\0') { @@ -3632,7 +3627,7 @@ ApplyModifier_Remember(const char **pp, /* * XXX: This ad-hoc call to strcspn deviates from the usual * behavior defined in ParseModifierPart. This creates an - * unnecessary, undocumented inconsistency in make. + * unnecessary and undocumented inconsistency in make. */ const char *arg = mod + 2; size_t argLen = strcspn(arg, ":)}"); @@ -4218,7 +4213,7 @@ ParseVarname(const char **pp, char start LazyBuf *buf) { const char *p = *pp; - int depth = 0; /* Track depth so we can spot parse errors. */ + int depth = 0; LazyBuf_Init(buf, p); @@ -4230,7 +4225,7 @@ ParseVarname(const char **pp, char start if (*p == endc) depth--; - /* A variable inside a variable, expand. */ + /* An expression inside an expression, expand. */ if (*p == '$') { FStr nested_val = Var_Parse(&p, scope, emode); /* TODO: handle errors */ Index: src/usr.bin/make/unit-tests/varmod-defined.mk diff -u src/usr.bin/make/unit-tests/varmod-defined.mk:1.14 src/usr.bin/make/unit-tests/varmod-defined.mk:1.15 --- src/usr.bin/make/unit-tests/varmod-defined.mk:1.14 Thu Nov 2 05:14:58 2023 +++ src/usr.bin/make/unit-tests/varmod-defined.mk Thu Nov 2 05:40:49 2023 @@ -1,4 +1,4 @@ -# $NetBSD: varmod-defined.mk,v 1.14 2023/11/02 05:14:58 rillig Exp $ +# $NetBSD: varmod-defined.mk,v 1.15 2023/11/02 05:40:49 rillig Exp $ # # Tests for the :D variable modifier, which returns the given string # if the variable is defined. It is closely related to the :U modifier. @@ -106,7 +106,7 @@ VAR:= ${VAR:@var@${8_DOLLARS}@} # Before var.c 1.1030 from 2022-08-24, the following expression caused an -# out-of-bounds read when parsing the indirect ':D' modifier. +# out-of-bounds read when parsing the indirect ':U' modifier. M_U_backslash:= ${:UU\\} .if ${:${M_U_backslash}} != "\\" . error