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(1): clean up make.h, meta.c, parse.c, str.c



details:   https://anonhg.NetBSD.org/src/rev/ea00ae126e9e
branches:  trunk
changeset: 1016319:ea00ae126e9e
user:      rillig <rillig%NetBSD.org@localhost>
date:      Sun Nov 15 12:02:44 2020 +0000

description:
make(1): clean up make.h, meta.c, parse.c, str.c

The main changes are in the comments, which have been shortened and
corrected.

Some local variables changed their names.

In ParseErrorInternal, the scope of va_start is now narrower.

In ParseDoDependency, the type of tOp has been fixed.

ParseGetLine doesn't take flags anymore but instead a parsing mode.
Previously, the flags had not been combined anyway.

At the beginning of Parse_File, fatals is already guaranteed to be 0, and
even if not, it would be wrong to just discard the fatal errors.

diffstat:

 usr.bin/make/make.h  |    4 +-
 usr.bin/make/meta.c  |    3 +-
 usr.bin/make/parse.c |  239 +++++++++++++++++++++++---------------------------
 usr.bin/make/str.c   |   31 ++---
 4 files changed, 129 insertions(+), 148 deletions(-)

diffs (truncated from 730 to 300 lines):

diff -r 0f3d65b68a8a -r ea00ae126e9e usr.bin/make/make.h
--- a/usr.bin/make/make.h       Sun Nov 15 11:57:00 2020 +0000
+++ b/usr.bin/make/make.h       Sun Nov 15 12:02:44 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: make.h,v 1.208 2020/11/14 17:39:59 rillig Exp $        */
+/*     $NetBSD: make.h,v 1.209 2020/11/15 12:02:44 rillig Exp $        */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -193,6 +193,8 @@
  *
  * Some of the OP_ constants can be combined, others cannot. */
 typedef enum GNodeType {
+    OP_NONE            = 0,
+
     /* The dependency operator ':' is the most common one.  The commands of
      * this node are executed if any child is out-of-date. */
     OP_DEPENDS         = 1 << 0,
diff -r 0f3d65b68a8a -r ea00ae126e9e usr.bin/make/meta.c
--- a/usr.bin/make/meta.c       Sun Nov 15 11:57:00 2020 +0000
+++ b/usr.bin/make/meta.c       Sun Nov 15 12:02:44 2020 +0000
@@ -1,4 +1,4 @@
-/*      $NetBSD: meta.c,v 1.143 2020/11/14 19:24:24 rillig Exp $ */
+/*      $NetBSD: meta.c,v 1.144 2020/11/15 12:02:44 rillig Exp $ */
 
 /*
  * Implement 'meta' mode.
@@ -1379,6 +1379,7 @@
                        break;
 
                    /* ignore anything containing the string "tmp" */
+                   /* XXX: The arguments to strstr must be swapped. */
                    if ((strstr("tmp", p)))
                        break;
 
diff -r 0f3d65b68a8a -r ea00ae126e9e usr.bin/make/parse.c
--- a/usr.bin/make/parse.c      Sun Nov 15 11:57:00 2020 +0000
+++ b/usr.bin/make/parse.c      Sun Nov 15 12:02:44 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: parse.c,v 1.440 2020/11/14 16:09:08 rillig Exp $       */
+/*     $NetBSD: parse.c,v 1.441 2020/11/15 12:02:44 rillig Exp $       */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -117,7 +117,7 @@
 #include "pathnames.h"
 
 /*     "@(#)parse.c    8.3 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: parse.c,v 1.440 2020/11/14 16:09:08 rillig Exp $");
+MAKE_RCSID("$NetBSD: parse.c,v 1.441 2020/11/15 12:02:44 rillig Exp $");
 
 /* types and constants */
 
@@ -348,6 +348,7 @@
 /* file loader */
 
 struct loadedfile {
+       /* XXX: What is the lifetime of this path? Who manages the memory? */
        const char *path;               /* name, for error reports */
        char *buf;                      /* contents buffer */
        size_t len;                     /* length of contents */
@@ -355,6 +356,7 @@
        Boolean used;                   /* XXX: have we used the data yet */
 };
 
+/* XXX: What is the lifetime of the path? Who manages the memory? */
 static struct loadedfile *
 loadedfile_create(const char *path)
 {
@@ -607,14 +609,14 @@
 }
 
 static void
-PrintLocation(FILE *f, const char *filename, size_t lineno)
+PrintLocation(FILE *f, const char *fname, size_t lineno)
 {
        char dirbuf[MAXPATHLEN+1];
        const char *dir, *base;
        void *dir_freeIt, *base_freeIt;
 
-       if (*filename == '/' || strcmp(filename, "(stdin)") == 0) {
-               (void)fprintf(f, "\"%s\" line %zu: ", filename, lineno);
+       if (*fname == '/' || strcmp(fname, "(stdin)") == 0) {
+               (void)fprintf(f, "\"%s\" line %zu: ", fname, lineno);
                return;
        }
 
@@ -629,8 +631,8 @@
 
        base = Var_Value(".PARSEFILE", VAR_GLOBAL, &base_freeIt);
        if (base == NULL) {
-               const char *slash = strrchr(filename, '/');
-               base = slash != NULL ? slash + 1 : filename;
+               const char *slash = strrchr(fname, '/');
+               base = slash != NULL ? slash + 1 : fname;
        }
 
        (void)fprintf(f, "\"%s/%s\" line %zu: ", dir, base, lineno);
@@ -638,21 +640,16 @@
        bmake_free(dir_freeIt);
 }
 
-/* Print a parse error message, including location information.
- *
- * Increment "fatals" if the level is PARSE_FATAL, and continue parsing
- * until the end of the current top-level makefile, then exit (see
- * Parse_File). */
 static void
-ParseVErrorInternal(FILE *f, const char *cfname, size_t clineno,
+ParseVErrorInternal(FILE *f, const char *fname, size_t lineno,
                    ParseErrorLevel type, const char *fmt, va_list ap)
 {
        static Boolean fatal_warning_error_printed = FALSE;
 
        (void)fprintf(f, "%s: ", progname);
 
-       if (cfname != NULL)
-               PrintLocation(f, cfname, clineno);
+       if (fname != NULL)
+               PrintLocation(f, fname, lineno);
        if (type == PARSE_WARNING)
                (void)fprintf(f, "warning: ");
        (void)vfprintf(f, fmt, ap);
@@ -670,26 +667,28 @@
 }
 
 static void
-ParseErrorInternal(const char *cfname, size_t clineno, ParseErrorLevel type,
-                  const char *fmt, ...)
+ParseErrorInternal(const char *fname, size_t lineno,
+                  ParseErrorLevel type, const char *fmt, ...)
 {
        va_list ap;
 
+       (void)fflush(stdout);
        va_start(ap, fmt);
-       (void)fflush(stdout);
-       ParseVErrorInternal(stderr, cfname, clineno, type, fmt, ap);
+       ParseVErrorInternal(stderr, fname, lineno, type, fmt, ap);
        va_end(ap);
 
        if (opts.debug_file != stderr && opts.debug_file != stdout) {
                va_start(ap, fmt);
-               ParseVErrorInternal(opts.debug_file, cfname, clineno, type,
+               ParseVErrorInternal(opts.debug_file, fname, lineno, type,
                                    fmt, ap);
                va_end(ap);
        }
 }
 
-/* External interface to ParseErrorInternal; uses the default filename and
- * line number.
+/* Print a parse error 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).
  *
  * Fmt is given without a trailing newline. */
 void
@@ -722,12 +721,8 @@
 }
 
 
-/* Parse a .info .warning or .error directive.
- *
- * The input is the line minus the ".".  We substitute variables, print the
- * message and exit(1) (for .error) or just print a warning if the directive
- * is malformed.
- */
+/* Parse and handle a .info, .warning or .error directive.
+ * For an .error directive, immediately exit. */
 static Boolean
 ParseMessage(const char *directive)
 {
@@ -806,9 +801,12 @@
 
     if (op == OP_DOUBLEDEP && (gn->type & OP_OPMASK) == OP_DOUBLEDEP) {
        /*
-        * If the node was the object of a :: operator, we need to create a
-        * new instance of it for the children and commands on this dependency
-        * line. The new instance is placed on the 'cohorts' list of the
+        * If the node was of 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.
+        *
+        * The new instance is placed on the 'cohorts' list of the
         * initial one (note the initial one is not on its own cohorts list)
         * and the new instance is linked to all parents of the initial
         * instance.
@@ -840,7 +838,7 @@
     } else {
        /*
         * We don't want to nuke any previous flags (whatever they were) so we
-        * just OR the new operator into the old
+        * just OR the new operator into the old.
         */
        gn->type |= op;
     }
@@ -876,11 +874,12 @@
                /*
                 * We add a .WAIT node in the dependency list.
                 * After any dynamic dependencies (and filename globbing)
-                * have happened, it is given a dependency on the each
-                * previous child back to and previous .WAIT node.
+                * 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 diag).
+                * We give each .WAIT node a unique name (mainly for
+                * diagnostics).
                 */
                snprintf(wait_src, sizeof wait_src, ".WAIT_%u", ++wait_number);
                gn = Targ_NewInternalNode(wait_src);
@@ -899,12 +898,13 @@
 ParseDoSrcMain(const char *src)
 {
     /*
-     * If we have noted the existence of a .MAIN, it means we need
-     * to add the sources of said target to the list of things
-     * to create. The string 'src' is likely to be free, so we
-     * must make a new copy of it. Note that this will only be
-     * invoked if the user didn't specify a target on the command
-     * line. This is to allow #ifmake's to succeed, or something...
+     * In a line like ".MAIN: source1 source2", it means we need to add
+     * the sources of said target to the list of things to create.
+     *
+     * Note that this will only be invoked if the user didn't specify a
+     * target on the command line. This is to allow .ifmake to succeed.
+     *
+     * XXX: Double-check all of the above comment.
      */
     Lst_Append(opts.create, bmake_strdup(src));
     /*
@@ -962,7 +962,7 @@
     gn = Targ_GetNode(src);
     if (doing_depend)
        ParseMark(gn);
-    if (tOp)
+    if (tOp != OP_NONE)
        gn->type |= tOp;
     else
        LinkToTargets(gn, specType != SP_NOT);
@@ -1255,7 +1255,8 @@
     switch (specType) {
     default:
        Parse_Error(PARSE_WARNING,
-                   "Special and mundane targets don't mix. Mundane ones ignored");
+                   "Special and mundane targets don't mix. "
+                   "Mundane ones ignored");
        break;
     case SP_DEFAULT:
     case SP_STALE:
@@ -1264,13 +1265,11 @@
     case SP_ERROR:
     case SP_INTERRUPT:
        /*
-        * These four create nodes on which to hang commands, so
-        * targets shouldn't be empty...
+        * These create nodes on which to hang commands, so targets
+        * shouldn't be empty...
         */
     case SP_NOT:
-       /*
-        * Nothing special here -- targets can be empty if it wants.
-        */
+       /* Nothing special here -- targets can be empty if it wants. */
        break;
     }
 }
@@ -1588,9 +1587,8 @@
  *
  * The sources are parsed in much the same way as the targets, except
  * that they are expanded using the wildcarding scheme of the C-Shell,
- * and all instances of the resulting words in the list of all targets
- * are found. Each of the resulting nodes is then linked to each of the
- * targets as one of its children.
+ * and a target is created for each expanded word. Each of the resulting
+ * nodes is then linked to each of the targets as one of its children.
  *
  * Certain targets and sources such as .PHONY or .PRECIOUS are handled
  * specially. These are the ones detailed by the specType variable.
@@ -1600,6 +1598,8 @@
  * Suff_IsTransform. If it is a transformation rule, its node is gotten
  * from the suffix module via Suff_AddTransform rather than the standard
  * Targ_FindNode in the target module.
+ *
+ * Upon return, the value of the line is unspecified.
  */
 static void
 ParseDoDependency(char *line)
@@ -1608,7 +1608,7 @@
     GNodeType op;              /* the operator on the line */
     SearchPathList *paths;     /* search paths to alter when parsing
                                 * a list of .PATH targets */
-    int tOp;                   /* operator from special target */
+    GNodeType tOp;             /* operator from special target */
     StringList *curTargs;      /* target names to be found and added
                                 * to the targets list */



Home | Main Index | Thread Index | Old Index