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): switch cache for realpath from GNode t...
details: https://anonhg.NetBSD.org/src/rev/092d5140dfc9
branches: trunk
changeset: 942693:092d5140dfc9
user: rillig <rillig%NetBSD.org@localhost>
date: Sat Nov 14 23:03:08 2020 +0000
description:
make(1): switch cache for realpath from GNode to HashTable
An unintended side effect from the GNode implementation was that the
variable modifier :tA and the other places where cached_realpath are
used could be affected by setting a variable in the global scope,
thereby "redirecting" absolute paths to completely unrelated but
existing paths.
Another unintended side effect was that filenames containing a dollar
sign would not be resolved correctly since the dollar sign would be
expanded as a variable expression by Var_Set.
While here, the debugging output for the realpath cache has been
adjusted to the standard behavior. Previously, when a new entry was
added to the cache, this was logged for the module VAR, as a side effect
of calling Var_Set, but only if the preprocessor macro
DEBUG_REALPATH_CACHE was defined at compilation time. When relative
paths were purged from the cache because the current directory changed
and logging for the DIR module was active, the log output went directly
to stderr instead of the usual opts.debug_file. This deviation from the
standard behavior was probably not intended as well.
All logging concerning the realpath cache now goes into the standard
debug log file and is controlled by the -dd option, not -dv.
diffstat:
usr.bin/make/main.c | 66 ++++++++++++------------------
usr.bin/make/unit-tests/varmod-to-abs.exp | 5 +-
usr.bin/make/unit-tests/varmod-to-abs.mk | 24 +++++++---
3 files changed, 46 insertions(+), 49 deletions(-)
diffs (191 lines):
diff -r b6cd242d2ecf -r 092d5140dfc9 usr.bin/make/main.c
--- a/usr.bin/make/main.c Sat Nov 14 22:39:14 2020 +0000
+++ b/usr.bin/make/main.c Sat Nov 14 23:03:08 2020 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: main.c,v 1.468 2020/11/14 22:19:13 rillig Exp $ */
+/* $NetBSD: main.c,v 1.469 2020/11/14 23:03:08 rillig Exp $ */
/*
* Copyright (c) 1988, 1989, 1990, 1993
@@ -109,7 +109,7 @@
#include "trace.h"
/* "@(#)main.c 8.3 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: main.c,v 1.468 2020/11/14 22:19:13 rillig Exp $");
+MAKE_RCSID("$NetBSD: main.c,v 1.469 2020/11/14 23:03:08 rillig Exp $");
#if defined(MAKE_NATIVE) && !defined(lint)
__COPYRIGHT("@(#) Copyright (c) 1988, 1989, 1990, 1993 "
"The Regents of the University of California. "
@@ -135,7 +135,7 @@
static Boolean jobsRunning; /* TRUE if the jobs might be running */
static const char *tracefile;
static int ReadMakefile(const char *);
-static void purge_cached_realpaths(void);
+static void purge_relative_cached_realpaths(void);
static Boolean ignorePWD; /* if we use -C, PWD is meaningless */
static char objdir[MAXPATHLEN + 1]; /* where we chdir'ed to */
@@ -147,6 +147,7 @@
Boolean forceJobs = FALSE;
static int errors = 0;
+static HashTable cached_realpaths;
/*
* For compatibility with the POSIX version of MAKEFLAGS that includes
@@ -737,7 +738,7 @@
Var_Set(".OBJDIR", objdir, VAR_GLOBAL);
setenv("PWD", objdir, 1);
Dir_InitDot();
- purge_cached_realpaths();
+ purge_relative_cached_realpaths();
rc = TRUE;
if (opts.enterFlag && strcmp(objdir, curdir) != 0)
enterFlagObj = TRUE;
@@ -1341,6 +1342,8 @@
/* default to writing debug to stderr */
opts.debug_file = stderr;
+ HashTable_Init(&cached_realpaths);
+
#ifdef SIGINFO
(void)bmake_signal(SIGINFO, siginfo);
#endif
@@ -2006,42 +2009,22 @@
_exit(1);
}
-/*
- * realpath(3) can get expensive, cache results...
- */
-static GNode *cached_realpaths = NULL;
-
-static GNode *
-get_cached_realpaths(void)
-{
-
- if (cached_realpaths == NULL) {
- cached_realpaths = Targ_NewGN("Realpath");
-#ifndef DEBUG_REALPATH_CACHE
- cached_realpaths->flags = INTERNAL;
-#endif
- }
-
- return cached_realpaths;
-}
-
/* purge any relative paths */
static void
-purge_cached_realpaths(void)
+purge_relative_cached_realpaths(void)
{
- GNode *cache = get_cached_realpaths();
HashEntry *he, *nhe;
HashIter hi;
- HashIter_Init(&hi, &cache->context);
+ HashIter_Init(&hi, &cached_realpaths);
he = HashIter_Next(&hi);
while (he != NULL) {
nhe = HashIter_Next(&hi);
if (he->key[0] != '/') {
- if (DEBUG(DIR))
- fprintf(stderr, "cached_realpath: purging %s\n",
- he->key);
- HashTable_DeleteEntry(&cache->context, he);
+ DEBUG1(DIR, "cached_realpath: purging %s\n", he->key);
+ HashTable_DeleteEntry(&cached_realpaths, he);
+ /* XXX: What about the allocated he->value? Either
+ * free them or document why they cannot be freed. */
}
he = nhe;
}
@@ -2050,25 +2033,28 @@
char *
cached_realpath(const char *pathname, char *resolved)
{
- GNode *cache;
const char *rp;
- void *freeIt;
if (pathname == NULL || pathname[0] == '\0')
return NULL;
- cache = get_cached_realpaths();
-
- if ((rp = Var_Value(pathname, cache, &freeIt)) != NULL) {
+ rp = HashTable_FindValue(&cached_realpaths, pathname);
+ if (rp != NULL) {
/* a hit */
strncpy(resolved, rp, MAXPATHLEN);
resolved[MAXPATHLEN - 1] = '\0';
- } else if ((rp = realpath(pathname, resolved)) != NULL) {
- Var_Set(pathname, rp, cache);
- } /* else should we negative-cache? */
+ return resolved;
+ }
- bmake_free(freeIt);
- return rp ? resolved : NULL;
+ rp = realpath(pathname, resolved);
+ if (rp != NULL) {
+ HashTable_Set(&cached_realpaths, pathname, bmake_strdup(rp));
+ DEBUG2(DIR, "cached_realpath: %s -> %s\n", pathname, rp);
+ return resolved;
+ }
+
+ /* should we negative-cache? */
+ return NULL;
}
/*
diff -r b6cd242d2ecf -r 092d5140dfc9 usr.bin/make/unit-tests/varmod-to-abs.exp
--- a/usr.bin/make/unit-tests/varmod-to-abs.exp Sat Nov 14 22:39:14 2020 +0000
+++ b/usr.bin/make/unit-tests/varmod-to-abs.exp Sat Nov 14 23:03:08 2020 +0000
@@ -1,2 +1,5 @@
-make: "varmod-to-abs.mk" line 17: /dev/null
+make: "varmod-to-abs.mk" line 18: does-not-exist.c
+make: "varmod-to-abs.mk" line 19: does-not-exist.c
+cached_realpath: ./varmod-to-abs.mk -> varmod-to-abs.mk
+make: "varmod-to-abs.mk" line 23: varmod-to-abs.mk
exit status 0
diff -r b6cd242d2ecf -r 092d5140dfc9 usr.bin/make/unit-tests/varmod-to-abs.mk
--- a/usr.bin/make/unit-tests/varmod-to-abs.mk Sat Nov 14 22:39:14 2020 +0000
+++ b/usr.bin/make/unit-tests/varmod-to-abs.mk Sat Nov 14 23:03:08 2020 +0000
@@ -1,20 +1,28 @@
-# $NetBSD: varmod-to-abs.mk,v 1.3 2020/11/14 18:47:21 rillig Exp $
+# $NetBSD: varmod-to-abs.mk,v 1.4 2020/11/14 23:03:08 rillig Exp $
#
# Tests for the :tA variable modifier, which returns the absolute path for
# each of the words in the variable value.
# TODO: Implementation
-# Since 2016-06-03, it is possible to trick the :tA modifier into resolving
-# completely unrelated absolute paths by defining a global variable that has
-# the same name as the path that is to be resolved. There are a few
-# restrictions though: The "redirected" path must start with a slash, and it
-# must exist. (See ModifyWord_Realpath).
+# Between 2016-06-03 and 2020-11-14, it was possible to trick the :tA modifier
+# into resolving completely unrelated absolute paths by defining a global
+# variable with the same name as the path that is to be resolved. There were
+# a few restrictions though: The "redirected" path had to start with a slash,
+# and it had to exist (see ModifyWord_Realpath).
#
-# XXX: This is probably not intended. It is caused by cached_realpath using
-# a GNode for keeping the cache, instead of a simple HashTable.
+# This unintended behavior was caused by cached_realpath using a GNode for
+# keeping the cache, just like the GNode for global variables.
+.MAKEFLAGS: -dd
does-not-exist.c= /dev/null
.info ${does-not-exist.c:L:tA}
+.info ${does-not-exist.c:L:tA}
+
+# The output of the following line is modified by the global _SED_CMDS in
+# unit-tests/Makefile. See the .rawout file for the truth.
+.info ${MAKEFILE:S,^,./,:tA}
+
+.MAKEFLAGS: -d0
all:
@:;
Home |
Main Index |
Thread Index |
Old Index