[src/trunk]: src/usr.bin/make make(1): fix the reference count of dotLast goi...

[rillig <rillig%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/4b4db0a232e7
branches:  trunk
changeset: 946520:4b4db0a232e7
user:      rillig <rillig%NetBSD.org@localhost>
date:      Sun Nov 29 16:37:10 2020 +0000

description:
make(1): fix the reference count of dotLast going negative

The memory management for dotLast is quite simple.  It is initialized
exactly once main_Init > Init_Objdir > Dir_InitDir and freed exactly
once in main_CleanUp > Dir_End.  Previously, dotLast was not freed at all.

The first call to CachedDir_Unref decremented the refCount to 0 but
didn't free anything.  Next, CachedDir_Destroy was called, which
decremented the reference count to -1, therefore skipping the actual
freeing.  This was probably an implementation mistake.

Since Dir_End is called at the very end of main_CleanUp, no code
accesses dotLast after it has been freed.

diffstat:

 usr.bin/make/dir.c |  27 ++++++++++++++++++++-------
 1 files changed, 20 insertions(+), 7 deletions(-)

diffs (69 lines):

diff -r 0b19e0c8778c -r 4b4db0a232e7 usr.bin/make/dir.c
--- a/usr.bin/make/dir.c        Sun Nov 29 16:04:34 2020 +0000
+++ b/usr.bin/make/dir.c        Sun Nov 29 16:37:10 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: dir.c,v 1.239 2020/11/29 16:04:34 rillig Exp $ */
+/*     $NetBSD: dir.c,v 1.240 2020/11/29 16:37:10 rillig Exp $ */
 
 /*
  * Copyright (c) 1988, 1989, 1990 The Regents of the University of California.
@@ -136,7 +136,7 @@
 #include "job.h"
 
 /*     "@(#)dir.c      8.2 (Berkeley) 1/2/94"  */
-MAKE_RCSID("$NetBSD: dir.c,v 1.239 2020/11/29 16:04:34 rillig Exp $");
+MAKE_RCSID("$NetBSD: dir.c,v 1.240 2020/11/29 16:37:10 rillig Exp $");
 
 #define DIR_DEBUG0(text) DEBUG0(DIR, text)
 #define DIR_DEBUG1(fmt, arg1) DEBUG1(DIR, fmt, arg1)
@@ -271,8 +271,8 @@
 
 static CachedDir *dot;         /* contents of current directory */
 static CachedDir *cur;         /* contents of current directory, if not dot */
-static CachedDir *dotLast;     /* a fake path entry indicating we need to
-                                * look for . last */
+/* A fake path entry indicating we need to look for '.' last. */
+static CachedDir *dotLast = NULL;
 
 /* Results of doing a last-resort stat in Dir_FindFile -- if we have to go to
  * the system to find the file, we might as well have its mtime on record.
@@ -341,6 +341,20 @@
                CachedDir_Free0(dir);
 }
 
+/* Update the value of the CachedDir variable, updating the reference counts. */
+static void
+CachedDir_Assign(CachedDir **var, CachedDir *dir)
+{
+       CachedDir *prev;
+
+       prev = *var;
+       *var = dir;
+       if (dir != NULL)
+               CachedDir_Ref(dir);
+       if (prev != NULL)
+               CachedDir_Destroy(prev);
+}
+
 static void
 OpenDirs_Init(OpenDirs *odirs)
 {
@@ -466,7 +480,7 @@
 {
        Dir_InitCur(cdname);
 
-       dotLast = CachedDir_Ref(CachedDir_New(".DOTLAST"));
+       CachedDir_Assign(&dotLast, CachedDir_New(".DOTLAST"));
 }
 
 /*
@@ -537,8 +551,7 @@
                CachedDir_Unref(cur);   /* XXX: why unref twice? */
                CachedDir_Destroy(cur);
        }
-       CachedDir_Unref(dotLast);       /* XXX: why unref twice? */
-       CachedDir_Destroy(dotLast);
+       CachedDir_Assign(&dotLast, NULL);
        CachedDir_Unref(dot);           /* XXX: why unref twice? */
        CachedDir_Destroy(dot);
        SearchPath_Clear(&dirSearchPath);