Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/usr.bin/make Fix the bug addressed in revision 1.27 properly...



details:   https://anonhg.NetBSD.org/src/rev/efa7dd578980
branches:  trunk
changeset: 521473:efa7dd578980
user:      pk <pk%NetBSD.org@localhost>
date:      Thu Jan 31 12:38:34 2002 +0000

description:
Fix the bug addressed in revision 1.27 properly. Analysis of the problem
(see also PR#15179):

  When looking up names which directory components (i.e. having slashes,
  except when of the form `./name'), FindFile()/DirLookup() first looks
  the final filename component in the cache for each directory on the search
  path and then proceeds to match the prefixed directory components by
  comparing them to the trailing directory components of the the search
  path being probed.

  This is not correct. When looking for `bar/target' in a path `.../src/foo',
  you want it to come up with `.../src/foo/bar/target' (if it exists). There's
  no point in comparing the the `bar' prefix on the target to the `foo' suffix
  on the search path. Indeed, this will cause a false match if those prefix
  and suffix components are actually equal and search path itself also has a
  file called `target'. For example, looking for `foo/target' in `.../src/foo'
  will spuriously match `.../src/foo/target', not `.../src/foo/foo/target'.

  This last bug prompted the change in dir.c, rev 1.27, which happens
  to partially workaround it by avoiding the above matching code in the
  case of the `curdir' search path entry (at the cost of incurring an
  exorbitant amount of cache misses). The situation is unchanged however,
  when processing other entries on the search path (e.g. those other than
  `dot' and `cur').

Drop the prefix matching code in DirLookup() entirely and use DirFindDot()
and DirLookup() only for names without proper directory components (i.e.
`target' and `./target). Otherwise, non-absolute names are dealt with by
DirLookupSubdir(), while absolute names can be checked for an exact match
of the directory components prefix against the directories on the current
search path. This allows for the use of the file cache to check the
existence of the file and additionally, provides a shortcut out of
Dir_FindFile() if we have the prefix match but not a cache entry (this
is especially beneficial for searches in .CURDIR when it's not equal
to `dot').

diffstat:

 usr.bin/make/dir.c |  266 ++++++++++++++++++++++++++++++++--------------------
 1 files changed, 164 insertions(+), 102 deletions(-)

diffs (truncated from 380 to 300 lines):

diff -r aa9ef174550f -r efa7dd578980 usr.bin/make/dir.c
--- a/usr.bin/make/dir.c        Thu Jan 31 11:51:25 2002 +0000
+++ b/usr.bin/make/dir.c        Thu Jan 31 12:38:34 2002 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: dir.c,v 1.31 2002/01/27 01:50:54 reinoud Exp $ */
+/*     $NetBSD: dir.c,v 1.32 2002/01/31 12:38:34 pk Exp $      */
 
 /*
  * Copyright (c) 1988, 1989, 1990 The Regents of the University of California.
@@ -39,14 +39,14 @@
  */
 
 #ifdef MAKE_BOOTSTRAP
-static char rcsid[] = "$NetBSD: dir.c,v 1.31 2002/01/27 01:50:54 reinoud Exp $";
+static char rcsid[] = "$NetBSD: dir.c,v 1.32 2002/01/31 12:38:34 pk Exp $";
 #else
 #include <sys/cdefs.h>
 #ifndef lint
 #if 0
 static char sccsid[] = "@(#)dir.c      8.2 (Berkeley) 1/2/94";
 #else
-__RCSID("$NetBSD: dir.c,v 1.31 2002/01/27 01:50:54 reinoud Exp $");
+__RCSID("$NetBSD: dir.c,v 1.32 2002/01/31 12:38:34 pk Exp $");
 #endif
 #endif /* not lint */
 #endif
@@ -210,6 +210,7 @@
 static char *DirLookup __P((Path *, char *, char *, Boolean));
 static char *DirLookupSubdir __P((Path *, char *));
 static char *DirFindDot __P((Boolean, char *, char *));
+static char *DirLookupAbs __P((Path *, char *, char *));
 
 /*-
  *-----------------------------------------------------------------------
@@ -729,9 +730,7 @@
  *     Find if the file with the given name exists in the given path.
  *
  * Results:
- *     The path to the file, the empty string or NULL. If the file is
- *     the empty string, the search should be terminated.
- *     This path is guaranteed to be in a
+ *     The path to the file or NULL. This path is guaranteed to be in a
  *     different part of memory than name and so may be safely free'd.
  *
  * Side Effects:
@@ -745,62 +744,25 @@
     char *cp;
     Boolean hasSlash;
 {
-    char *p1;          /* pointer into p->name */
-    char *p2;          /* pointer into name */
     char *file;                /* the current filename to check */
 
     if (DEBUG(DIR)) {
        printf("%s...", p->name);
     }
-    if (Hash_FindEntry (&p->files, cp) != (Hash_Entry *)NULL) {
-       if (DEBUG(DIR)) {
-           printf("here...");
-       }
-       if (hasSlash) {
-           /*
-            * If the name had a slash, its initial components and p's
-            * final components must match. This is false if a mismatch
-            * is encountered before all of the initial components
-            * have been checked (p2 > name at the end of the loop), or
-            * we matched only part of one of the components of p
-            * along with all the rest of them (*p1 != '/').
-            */
-           p1 = p->name + strlen (p->name) - 1;
-           p2 = cp - 2;
-           while (p2 >= name && p1 >= p->name && *p1 == *p2) {
-               p1 -= 1; p2 -= 1;
-           }
-           if (p2 >= name || (p1 >= p->name && *p1 != '/')) {
-               if (DEBUG(DIR)) {
-                   printf("component mismatch -- continuing...");
-               }
-               return NULL;
-           }
-       }
-       file = str_concat (p->name, cp, STR_ADDSLASH);
-       if (DEBUG(DIR)) {
-           printf("returning %s\n", file);
-       }
-       p->hits += 1;
-       hits += 1;
-       return file;
-    } else if (hasSlash) {
-       /*
-        * If the file has a leading path component and that component
-        * exactly matches the entire name of the current search
-        * directory, we assume the file doesn't exist and return NULL.
-        */
-       for (p1 = p->name, p2 = name; *p1 && *p1 == *p2; p1++, p2++) {
-           continue;
-       }
-       if (*p1 == '\0' && p2 == cp - 1) {
-           if (DEBUG(DIR)) {
-               printf("must be here but isn't -- returing\n");
-           }
-           return "";
-       }
+
+    if (Hash_FindEntry (&p->files, cp) == (Hash_Entry *)NULL)
+       return NULL;
+
+    if (DEBUG(DIR)) {
+       printf("here...");
     }
-    return NULL;
+    file = str_concat (p->name, cp, STR_ADDSLASH);
+    if (DEBUG(DIR)) {
+       printf("returning %s\n", file);
+    }
+    p->hits += 1;
+    hits += 1;
+    return file;
 }
 
 
@@ -865,6 +827,66 @@
 
 /*-
  *-----------------------------------------------------------------------
+ * DirLookupAbs  --
+ *     Find if the file with the given name exists in the given path.
+ *
+ * Results:
+ *     The path to the file, the empty string or NULL. If the file is
+ *     the empty string, the search should be terminated.
+ *     This path is guaranteed to be in a different part of memory
+ *     than name and so may be safely free'd.
+ *
+ * Side Effects:
+ *     None.
+ *-----------------------------------------------------------------------
+ */
+static char *
+DirLookupAbs(p, name, cp)
+       Path *p;
+       char *name;
+       char *cp;
+{
+       char *p1;               /* pointer into p->name */
+       char *p2;               /* pointer into name */
+
+       if (DEBUG(DIR)) {
+               printf("%s...", p->name);
+       }
+
+       /*
+        * If the file has a leading path component and that component
+        * exactly matches the entire name of the current search
+        * directory, we can attempt another cache lookup. And if we don't
+        * have a hit, we can safely assume the file does not exist at all.
+        */
+       for (p1 = p->name, p2 = name; *p1 && *p1 == *p2; p1++, p2++) {
+               continue;
+       }
+       if (*p1 != '\0' || p2 != cp - 1) {
+               return NULL;
+       }
+
+       if (Hash_FindEntry (&p->files, cp) == (Hash_Entry *)NULL) {
+               if (DEBUG(DIR)) {
+                       printf("must be here but isn't -- returning\n");
+               }
+               /* Return empty string: terminates search */
+               return "";
+       }
+
+       if (DEBUG(DIR)) {
+               printf("here...");
+       }
+       p->hits += 1;
+       hits += 1;
+       if (DEBUG(DIR)) {
+               printf("returning %s\n", name);
+       }
+       return (estrdup (name));
+}
+
+/*-
+ *-----------------------------------------------------------------------
  * DirFindDot  --
  *     Find the file given on "." or curdir
  *
@@ -882,9 +904,7 @@
     char *name;
     char *cp;
 {
-    char *file;
 
-    if ((!hasSlash || (cp - name == 2 && *name == '.'))) {
        if (Hash_FindEntry (&dot->files, cp) != (Hash_Entry *)NULL) {
            if (DEBUG(DIR)) {
                printf("in '.'\n");
@@ -902,15 +922,8 @@
            cur->hits += 1;
            return str_concat (cur->name, cp, STR_ADDSLASH);
        }
-    }
 
-    if (cur && (file = DirLookupSubdir(cur, name)) != NULL) {
-       if (*file)
-           return file;
-       else
-           return NULL;
-    }
-    return NULL;
+       return NULL;
 }
 
 /*-
@@ -974,58 +987,68 @@
        p = (Path *) Lst_Datum (ln);
        if (p == dotLast) {
            hasLastDot = TRUE;
-            if (DEBUG(DIR)) printf("[dot last]...");
+            if (DEBUG(DIR))
+               printf("[dot last]...");
        }
     }
 
     /*
-     * No matter what, we always look for the file in the current directory
-     * before anywhere else and we *do not* add the ./ to it if it exists.
-     * This is so there are no conflicts between what the user specifies
-     * (fish.c) and what pmake finds (./fish.c).
-     * Unless we found the magic DOTLAST path...
+     * If there's no leading directory components or if the leading
+     * directory component is exactly `./', consult the cached contents
+     * of each of the directories on the search path.
      */
-    if (!hasLastDot && name[0] != '/')
-       if ((file = DirFindDot(hasSlash, name, cp)) != NULL)
-           return file;
+    if ((!hasSlash || (cp - name == 2 && *name == '.'))) {
+           /*
+            * We look through all the directories on the path seeking one which
+            * contains the final component of the given name.  If such a beast
+            * is found, we concatenate the directory name and the final
+            * component and return the resulting string. If we don't find any
+            * such thing, we go on to phase two...
+            * 
+            * No matter what, we always look for the file in the current
+            * directory before anywhere else (unless we found the magic
+            * DOTLAST path, in which case we search it last) and we *do not*
+            * add the ./ to it if it exists.
+            * This is so there are no conflicts between what the user
+            * specifies (fish.c) and what pmake finds (./fish.c).
+            */
+           if (!hasLastDot &&
+                       (file = DirFindDot(hasSlash, name, cp)) != NULL) {
+                   Lst_Close (path);
+                   return file;
+           }
 
-    /*
-     * We look through all the directories on the path seeking one which
-     * contains the final component of the given name and whose final
-     * component(s) match the name's initial component(s). If such a beast
-     * is found, we concatenate the directory name and the final component
-     * and return the resulting string. If we don't find any such thing,
-     * we go on to phase two...
-     */
-    while ((ln = Lst_Next (path)) != NILLNODE) {
-       p = (Path *) Lst_Datum (ln);
-       if (p == dotLast)
-           continue;
-        if ((file = DirLookup(p, name, cp, hasSlash)) != NULL) {
-           Lst_Close (path);
-           if (*file)
-               return file;
-           else
-               return NULL;
-       }
+           while ((ln = Lst_Next (path)) != NILLNODE) {
+               p = (Path *) Lst_Datum (ln);
+               if (p == dotLast)
+                   continue;
+               if ((file = DirLookup(p, name, cp, hasSlash)) != NULL) {
+                   Lst_Close (path);
+                       return file;
+               }
+           }
+
+           if (hasLastDot &&
+                       (file = DirFindDot(hasSlash, name, cp)) != NULL) {
+                   Lst_Close (path);
+                   return file;
+           }
     }
     Lst_Close (path);
 
-    if (hasLastDot && name[0] != '/')
-       if ((file = DirFindDot(hasSlash, name, cp)) != NULL)
-           return file;
-



Home | Main Index | Thread Index | Old Index