Sat Nov 21 15:32:52 2020 UTC ()

make(1): clean up freeing of environment variables in Var_Parse

The previous code with the extra boolean variable was a brain-twister
since the responsibility of freeing the memory was distributed over 3
different functions.


(rillig)

diff -r1.691 -r1.692 src/usr.bin/make/var.c

--- src/usr.bin/make/var.c 2020/11/21 15:28:44	1.691
+++ src/usr.bin/make/var.c 2020/11/21 15:32:52	1.692

 @@ -1,14 +1,14 @@
-/*	$NetBSD: var.c,v 1.691 2020/11/21 15:28:44 rillig Exp $	*/
+/*	$NetBSD: var.c,v 1.692 2020/11/21 15:32:52 rillig Exp $	*/
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
  *	The Regents of the University of California.  All rights reserved.
+ *
  * This code is derived from software contributed to Berkeley by
  * Adam de Boor.
+ *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
 @@ -120,27 +120,27 @@
 #include <regex.h>
 #endif
 #include <errno.h>
 #include <inttypes.h>
 #include <limits.h>
 #include <time.h>
 #include "make.h"
 #include "dir.h"
 #include "job.h"
 #include "metachar.h"
 /*	"@(#)var.c	8.3 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: var.c,v 1.691 2020/11/21 15:28:44 rillig Exp $");
+MAKE_RCSID("$NetBSD: var.c,v 1.692 2020/11/21 15:32:52 rillig Exp $");
 #define VAR_DEBUG1(fmt, arg1) DEBUG1(VAR, fmt, arg1)
 #define VAR_DEBUG2(fmt, arg1, arg2) DEBUG2(VAR, fmt, arg1, arg2)
 #define VAR_DEBUG3(fmt, arg1, arg2, arg3) DEBUG3(VAR, fmt, arg1, arg2, arg3)
 #define VAR_DEBUG4(fmt, arg1, arg2, arg3, arg4) DEBUG4(VAR, fmt, arg1, arg2, arg3, arg4)
 ENUM_FLAGS_RTTI_3(VarEvalFlags,
 		  VARE_UNDEFERR, VARE_WANTRES, VARE_KEEP_DOLLAR);
 /*
  * This lets us tell if we have replaced the original environ
  * (which we cannot free).
  */
 @@ -3915,32 +3915,37 @@ Var_Parse(const char **pp, GNode *ctxt,
 				   v, &exprFlags, ctxt, eflags, out_val_freeIt);
 	    free(extraFree);
 	} else {
 	    *out_val_freeIt = extraFree;
+	}
+    }
     if (*p != '\0')		/* Skip past endc if possible. */
 	p++;
     *pp = p;
     if (v->flags & VAR_FROM_ENV) {
-	/* Free the environment variable now since we own it,
+	/* Free the environment variable now since we own it. */
 	 * but don't free the variable value if it will be returned. */
-	Boolean keepValue = value == Buf_GetAll(&v->val, NULL);
+	char *varValue = Buf_Destroy(&v->val, FALSE);
-	if (keepValue)
+	if (value == varValue) {
-	    *out_val_freeIt = value;
+	    /* Don't free the variable value since it will be returned. */
-	(void)VarFreeEnv(v, !keepValue);
+	    *out_val_freeIt = varValue;
 	} else
 	    free(varValue);
 	free(v->name_freeIt);
 	free(v);
     } else if (exprFlags & VEF_UNDEF) {
 	if (!(exprFlags & VEF_DEF)) {
 	    /* TODO: Use a local variable instead of out_val_freeIt.
 	     * Variables named out_* must only be written to. */
 	    if (*out_val_freeIt != NULL) {
 		free(*out_val_freeIt);
 		*out_val_freeIt = NULL;
+	    }
 	    if (dynamic) {
 		value = bmake_strsedup(start, p);
 		*out_val_freeIt = value;
 	    } else {