Merge branch 'master' into feature/fix-ns-memleak

12 files changed, 124 insertions, 162 deletions

diff --git a/ChangeLog b/ChangeLog
index 7b0b3710..5879c2f4 100755
--- a/ChangeLog
+++ b/ChangeLog
@@ -38,6 +38,45 @@
 	assigning to it.
 	* profile.c (pp_namespace): Ditto. (This may not be necessary.)
 
+2019-01-18         Arnold D. Robbins     <arnold@skeeve.com>
+
+	* debug.c (do_set_var): Add comments before calls to assoc_set.
+	* interpret.h (r_interpret): For Op_sub_array, deref the subscript
+	appropriately. Thanks to Andy Schorr for the catch.
+
+2019-01-15         Arnold D. Robbins     <arnold@skeeve.com>
+
+	* array.c (asort_actual): Use assoc_set in 2 places.
+	* awk.h (assoc_set): Improve leading comment.
+	* debug.c (do_set_var): Use assoc_set in 2 places.
+	* field.c (set_element, update_PROCINFO_str, update_PROCINFO_num):
+	Use assoc_set in each.
+	* gawkapi.c (api_set_array_element): Use assoc_set.
+	* interpret.h (r_interpret): Use assoc_set.
+	* main.c (init_args, load_environ, load_procinfo_argv):
+	Use assoc_set in each.
+	* mpfr.c (do_mpfr_intdiv): Use assoc_set in 2 places.
+	* str_array.c (str_lookup): Fix leading comment.
+	* symbol.c (install): Use assoc_set.
+
+2019-01-15         Andrew J. Schorr      <aschorr@telemetry-investments.com>
+
+	* builtin.c (do_match, do_intdiv): Remove unused `sub' and `lhs'
+	variables, since assoc_set is now doing all of the work.
+
+2019-01-15         Andrew J. Schorr      <aschorr@telemetry-investments.com>
+
+	* awk.h (assoc_set): Move the definition lower down because it
+	needs to be after unref is declared.
+	* builtin.c (do_match): Use assoc_set in 3 places.
+	(do_intdiv): Use assoc_set in 2 places.
+	(do_typeof): Use assoc_set in 2 places.
+
+2019-01-14         Andrew J. Schorr      <aschorr@telemetry-investments.com>
+
+	* awk.h (assoc_set): Add new inline function to set an array element
+	to eliminate code duplication and reduce the chance of memory leaks.
+
 2019-01-14         Andrew J. Schorr      <aschorr@telemetry-investments.com>
 
 	* builtin.c (do_typeof): Fix memory leak when populating the
diff --git a/array.c b/array.c
index 34b7076a..2cc76743 100644
--- a/array.c
+++ b/array.c
@@ -3,7 +3,7 @@
  */
 
 /*
- * Copyright (C) 1986, 1988, 1989, 1991-2014, 2016, 2018,
+ * Copyright (C) 1986, 1988, 1989, 1991-2014, 2016, 2018, 2019,
  * the Free Software Foundation, Inc.
  *
  * This file is part of GAWK, the GNU implementation of the
@@ -796,7 +796,7 @@ asort_actual(int nargs, sort_context_t ctxt)
 {
 	NODE *array, *dest = NULL, *result;
 	NODE *r, *subs, *s;
-	NODE **list = NULL, **ptr, **lhs;
+	NODE **list = NULL, **ptr;
 	unsigned long num_elems, i;
 	const char *sort_str;
 	char save;
@@ -884,12 +884,7 @@ asort_actual(int nargs, sort_context_t ctxt)
 
 		for (i = 1, ptr = list; i <= num_elems; i++, ptr += 2) {
 			subs = make_number(i);
-			lhs = assoc_lookup(result, subs);
-			unref(*lhs);
-			*lhs = *ptr;
-			if (result->astore != NULL)
-				(*result->astore)(result, subs);
-			unref(subs);
+			assoc_set(result, subs, *ptr);
 		}
 	} else {
 		/* We want the values of the source array. */
@@ -904,11 +899,11 @@ asort_actual(int nargs, sort_context_t ctxt)
 			/* value node */
 			r = *ptr++;
 
-			if (r->type == Node_val) {
-				lhs = assoc_lookup(result, subs);
-				unref(*lhs);
-				*lhs = dupnode(r);
-			} else {
+			NODE *value;
+
+			if (r->type == Node_val)
+				value = dupnode(r);
+			else {
 				NODE *arr;
 				arr = make_array();
 				subs = force_string(subs);
@@ -917,13 +912,10 @@ asort_actual(int nargs, sort_context_t ctxt)
 				subs->stptr = NULL;
 				subs->flags &= ~STRCUR;
 				arr->parent_array = array; /* actual parent, not the temporary one. */
-				lhs = assoc_lookup(result, subs);
-				unref(*lhs);
-				*lhs = assoc_copy(r, arr);
+
+				value = assoc_copy(r, arr);
 			}
-			if (result->astore != NULL)
-				(*result->astore)(result, subs);
-			unref(subs);
+			assoc_set(result, subs, value);
 		}
 	}
 
diff --git a/awk.h b/awk.h
index a18cf522..3ba5f1fc 100644
--- a/awk.h
+++ b/awk.h
@@ -1362,6 +1362,7 @@ extern int fatal_tag_valid;
 /* assoc_remove --- remove an index from symbol[] */
 #define assoc_remove(a, s) ((a)->aremove(a, s) != NULL)
 
+
 /* ------------- Function prototypes or defs (as appropriate) ------------- */
 /* array.c */
 typedef enum { SORTED_IN = 1, ASORT, ASORTI } sort_context_t;
@@ -2024,6 +2025,20 @@ make_number_node(unsigned int flags)
 	return r;
 }
 
+/* assoc_set -- set an element in an array. Does unref(sub)! */
+
+static inline void
+assoc_set(NODE *array, NODE *sub, NODE *value)
+{
+
+	NODE **lhs = assoc_lookup(array, sub);
+	unref(*lhs);
+	*lhs = value;
+	if (array->astore != NULL)
+		(*array->astore)(array, sub);
+	unref(sub);
+}
+
 /*
  * str_terminate_f, str_terminate, str_restore: function and macros to
  * reduce chances of typos when terminating and restoring strings.
diff --git a/builtin.c b/builtin.c
index 9a64dde0..491a96b3 100644
--- a/builtin.c
+++ b/builtin.c
@@ -2706,8 +2706,6 @@ do_match(int nargs)
 				if ((s = SUBPATSTART(rp, t1->stptr, ii)) != -1) {
 					size_t subpat_start;
 					size_t subpat_len;
-					NODE **lhs;
-					NODE *sub;
 
 					start = t1->stptr + s;
 					subpat_start = s;
@@ -2719,15 +2717,7 @@ do_match(int nargs)
 
 					it = make_string(start, len);
 					it->flags |= USER_INPUT;
-
-					sub = make_number((AWKNUM) (ii));
-					lhs = assoc_lookup(dest, sub);
-					unref(*lhs);
-					*lhs = it;
-					/* execute post-assignment routine if any */
-					if (dest->astore != NULL)
-						(*dest->astore)(dest, sub);
-					unref(sub);
+					assoc_set(dest, make_number((AWKNUM) (ii)), it);;
 
 					sprintf(buff, "%d", ii);
 					ilen = strlen(buff);
@@ -2745,14 +2735,7 @@ do_match(int nargs)
 
 					slen = ilen + subseplen + 5;
 
-					it = make_number((AWKNUM) subpat_start + 1);
-					sub = make_string(buf, slen);
-					lhs = assoc_lookup(dest, sub);
-					unref(*lhs);
-					*lhs = it;
-					if (dest->astore != NULL)
-						(*dest->astore)(dest, sub);
-					unref(sub);
+					assoc_set(dest, make_string(buf, slen), make_number((AWKNUM) subpat_start + 1));
 
 					memcpy(buf, buff, ilen);
 					memcpy(buf + ilen, subsepstr, subseplen);
@@ -2760,14 +2743,7 @@ do_match(int nargs)
 
 					slen = ilen + subseplen + 6;
 
-					it = make_number((AWKNUM) subpat_len);
-					sub = make_string(buf, slen);
-					lhs = assoc_lookup(dest, sub);
-					unref(*lhs);
-					*lhs = it;
-					if (dest->astore != NULL)
-						(*dest->astore)(dest, sub);
-					unref(sub);
+					assoc_set(dest, make_string(buf, slen), make_number((AWKNUM) subpat_len));
 				}
 			}
 
@@ -4006,7 +3982,6 @@ do_intdiv(int nargs)
 {
 	NODE *numerator, *denominator, *result;
 	double num, denom, quotient, remainder;
-	NODE *sub, **lhs;
 
 	result = POP_PARAM();
 	if (result->type != Node_var_array)
@@ -4044,17 +4019,9 @@ do_intdiv(int nargs)
 #endif	/* ! HAVE_FMOD */
 	remainder = double_to_int(remainder);
 
-	sub = make_string("quotient", 8);
-	lhs = assoc_lookup(result, sub);
-	unref(*lhs);
-	*lhs = make_number((AWKNUM) quotient);
-	unref(sub);
+	assoc_set(result, make_string("quotient", 8), make_number((AWKNUM) quotient));
 
-	sub = make_string("remainder", 9);
-	lhs = assoc_lookup(result, sub);
-	unref(*lhs);
-	*lhs = make_number((AWKNUM) remainder);
-	unref(sub);
+	assoc_set(result, make_string("remainder", 9), make_number((AWKNUM) remainder));
 
 	DEREF(denominator);
 	DEREF(numerator);
@@ -4087,15 +4054,8 @@ do_typeof(int nargs)
 		/* Node_var_array is never UPREF'ed */
 		res = "array";
 		deref = false;
-		if (dbg) {
-			NODE *sub = make_string("array_type", 10);
-			NODE **lhs = assoc_lookup(dbg, sub);
-			unref(*lhs);
-			*lhs = make_string(arg->array_funcs->name, strlen(arg->array_funcs->name));
-			if (dbg->astore != NULL)
-				(*dbg->astore)(dbg, sub);
-			unref(sub);
-		}
+		if (dbg)
+			assoc_set(dbg, make_string("array_type", 10), make_string(arg->array_funcs->name, strlen(arg->array_funcs->name)));
 		break;
 	case Node_val:
 		switch (fixtype(arg)->flags & (STRING|NUMBER|USER_INPUT|REGEX)) {
@@ -4126,13 +4086,7 @@ do_typeof(int nargs)
 		}
 		if (dbg) {
 			const char *s = flags2str(arg->flags);
-			NODE *sub = make_string("flags", 5);
-			NODE **lhs = assoc_lookup(dbg, sub);
-			unref(*lhs);
-			*lhs = make_string(s, strlen(s));
-			if (dbg->astore != NULL)
-				(*dbg->astore)(dbg, sub);
-			unref(sub);
+			assoc_set(dbg, make_string("flags", 5), make_string(s, strlen(s)));
 		}
 		break;
 	case Node_var_new:
diff --git a/debug.c b/debug.c
index f8aa6adf..05083a48 100644
--- a/debug.c
+++ b/debug.c
@@ -1250,6 +1250,7 @@ do_set_var(CMDARG *arg, int cmd ATTRIBUTE_UNUSED)
 	{
 		NODE *subs, *value;
 		int count = arg->a_count;
+		NODE *newval;
 
 		assert(count > 0);
 		name = arg->a_string;
@@ -1268,11 +1269,12 @@ do_set_var(CMDARG *arg, int cmd ATTRIBUTE_UNUSED)
 				else {
 					arg = arg->next;
 					val = arg->a_node;
-					lhs = assoc_lookup(r, subs);
-					unref(*lhs);
-					*lhs = dupnode(val);
+					newval = dupnode(val);
+					// subs should not be freed, so
+					// use dupnode in call to assoc_set.
+					assoc_set(r, dupnode(subs), newval);
 					fprintf(out_fp, "%s[\"%.*s\"] = ", name, (int) subs->stlen, subs->stptr);
-					valinfo(*lhs, fprintf, out_fp);
+					valinfo(newval, fprintf, out_fp);
 				}
 			} else {
 				if (value == NULL) {
@@ -1280,9 +1282,9 @@ do_set_var(CMDARG *arg, int cmd ATTRIBUTE_UNUSED)
 					array = make_array();
 					array->vname = estrdup(subs->stptr, subs->stlen);
 					array->parent_array = r;
-					lhs = assoc_lookup(r, subs);
-					unref(*lhs);
-					*lhs = array;
+					// subs should not be freed, so
+					// use dupnode in call to assoc_set.
+					assoc_set(r, dupnode(subs), array);
 					r = array;
 				} else if (value->type != Node_var_array) {
 					d_error(_("attempt to use scalar `%s[\"%.*s\"]' as array"),
diff --git a/field.c b/field.c
index ccf47df1..d8bb52b7 100644
--- a/field.c
+++ b/field.c
@@ -949,18 +949,12 @@ static void
 set_element(long num, char *s, long len, NODE *n)
 {
 	NODE *it;
-	NODE **lhs;
 	NODE *sub;
 
 	it = make_string(s, len);
 	it->flags |= USER_INPUT;
 	sub = make_number((AWKNUM) (num));
-	lhs = assoc_lookup(n, sub);
-	unref(*lhs);
-	*lhs = it;
-        if (n->astore != NULL)
-                (*n->astore)(n, sub);
-	unref(sub);
+	assoc_set(n, sub, it);
 }
 
 /* do_split --- implement split(), semantics are same as for field splitting */
@@ -1412,16 +1406,12 @@ current_field_sep_str()
 void
 update_PROCINFO_str(const char *subscript, const char *str)
 {
-	NODE **aptr;
 	NODE *tmp;
 
 	if (PROCINFO_node == NULL)
 		return;
 	tmp = make_string(subscript, strlen(subscript));
-	aptr = assoc_lookup(PROCINFO_node, tmp);
-	unref(tmp);
-	unref(*aptr);
-	*aptr = make_string(str, strlen(str));
+	assoc_set(PROCINFO_node, tmp, make_string(str, strlen(str)));
 }
 
 /* update_PROCINFO_num --- update PROCINFO[sub] with numeric value */
@@ -1429,16 +1419,12 @@ update_PROCINFO_str(const char *subscript, const char *str)
 void
 update_PROCINFO_num(const char *subscript, AWKNUM val)
 {
-	NODE **aptr;
 	NODE *tmp;
 
 	if (PROCINFO_node == NULL)
 		return;
 	tmp = make_string(subscript, strlen(subscript));
-	aptr = assoc_lookup(PROCINFO_node, tmp);
-	unref(tmp);
-	unref(*aptr);
-	*aptr = make_number(val);
+	assoc_set(PROCINFO_node, tmp, make_number(val));
 }
 
 /* set_FPAT --- handle an assignment to FPAT */
diff --git a/gawkapi.c b/gawkapi.c
index 3227d55f..c76c7b53 100644
--- a/gawkapi.c
+++ b/gawkapi.c
@@ -1046,7 +1046,6 @@ api_set_array_element(awk_ext_id_t id, awk_array_t a_cookie,
 	NODE *array = (NODE *)a_cookie;
 	NODE *tmp;
 	NODE *elem;
-	NODE **aptr;
 
 	/* don't check for index len zero, null str is ok as index */
 	if (   array == NULL
@@ -1058,17 +1057,13 @@ api_set_array_element(awk_ext_id_t id, awk_array_t a_cookie,
 		return awk_false;
 
 	tmp = awk_value_to_node(index);
-	aptr = assoc_lookup(array, tmp);
-	unref(*aptr);
-	elem = *aptr = awk_value_to_node(value);
+	elem = awk_value_to_node(value);
 	if (elem->type == Node_var_array) {
 		elem->parent_array = array;
 		elem->vname = estrdup(index->str_value.str,
 					index->str_value.len);
 	}
-	if (array->astore != NULL)
-		(*array->astore)(array, tmp);
-	unref(tmp);
+	assoc_set(array, tmp, elem);
 
 	return awk_true;
 }
diff --git a/interpret.h b/interpret.h
index 82329e85..e9896672 100644
--- a/interpret.h
+++ b/interpret.h
@@ -303,22 +303,16 @@ uninitialized_scalar:
 			if (r == NULL) {
 				r = make_array();
 				r->parent_array = t1;
-				lhs = assoc_lookup(t1, t2);
-				unref(*lhs);
-				*lhs = r;
 				t2 = force_string(t2);
 				r->vname = estrdup(t2->stptr, t2->stlen);	/* the subscript in parent array */
-
-				/* execute post-assignment routine if any */
-				if (t1->astore != NULL)
-					(*t1->astore)(t1, t2);
+				assoc_set(t1, t2, r);
 			} else if (r->type != Node_var_array) {
 				t2 = force_string(t2);
 				fatal(_("attempt to use scalar `%s[\"%.*s\"]' as an array"),
 						array_vname(t1), (int) t2->stlen, t2->stptr);
-			}
+			} else
+				DEREF(t2);
 
-			DEREF(t2);
 			PUSH(r);
 			break;
 
diff --git a/main.c b/main.c
index 1eb6d6a9..3d3af968 100644
--- a/main.c
+++ b/main.c
@@ -3,7 +3,7 @@
  */
 
 /*
- * Copyright (C) 1986, 1988, 1989, 1991-2018 the Free Software Foundation, Inc.
+ * Copyright (C) 1986, 1988, 1989, 1991-2019 the Free Software Foundation, Inc.
  *
  * This file is part of GAWK, the GNU implementation of the
  * AWK Programming Language.
@@ -742,23 +742,19 @@ static void
 init_args(int argc0, int argc, const char *argv0, char **argv)
 {
 	int i, j;
-	NODE **aptr;
-	NODE *tmp;
+	NODE *sub, *val;
 
 	ARGV_node = install_symbol(estrdup("ARGV", 4), Node_var_array);
-	tmp =  make_number(0.0);
-	aptr = assoc_lookup(ARGV_node, tmp);
-	unref(tmp);
-	unref(*aptr);
-	*aptr = make_string(argv0, strlen(argv0));
-	(*aptr)->flags |= USER_INPUT;
+	sub = make_number(0.0);
+	val = make_string(argv0, strlen(argv0));
+	val->flags |= USER_INPUT;
+	assoc_set(ARGV_node, sub, val);
+
 	for (i = argc0, j = 1; i < argc; i++, j++) {
-		tmp = make_number((AWKNUM) j);
-		aptr = assoc_lookup(ARGV_node, tmp);
-		unref(tmp);
-		unref(*aptr);
-		*aptr = make_string(argv[i], strlen(argv[i]));
-		(*aptr)->flags |= USER_INPUT;
+		sub = make_number((AWKNUM) j);
+		val = make_string(argv[i], strlen(argv[i]));
+		val->flags |= USER_INPUT;
+		assoc_set(ARGV_node, sub, val);
 	}
 
 	ARGC_node = install_symbol(estrdup("ARGC", 4), Node_var);
@@ -887,9 +883,8 @@ load_environ()
 	extern char **environ;
 #endif
 	char *var, *val;
-	NODE **aptr;
 	int i;
-	NODE *tmp;
+	NODE *sub, *newval;
 	static bool been_here = false;
 
 	if (been_here)
@@ -907,12 +902,10 @@ load_environ()
 			*val++ = '\0';
 		else
 			val = nullstr;
-		tmp = make_string(var, strlen(var));
-		aptr = assoc_lookup(ENVIRON_node, tmp);
-		unref(tmp);
-		unref(*aptr);
-		*aptr = make_string(val, strlen(val));
-		(*aptr)->flags |= USER_INPUT;
+		sub = make_string(var, strlen(var));
+		newval = make_string(val, strlen(val));
+		newval->flags |= USER_INPUT;
+		assoc_set(ENVIRON_node, sub, newval);
 
 		/* restore '=' so that system() gets a valid environment */
 		if (val != nullstr)
@@ -936,31 +929,32 @@ load_environ()
 	return ENVIRON_node;
 }
 
+/* load_procinfo_argv --- populate PROCINFO["argv"] */
+
 static void
 load_procinfo_argv()
 {
-	NODE *tmp;
-	NODE **aptr;
+	NODE *sub;
+	NODE *val;
 	NODE *argv_array;
 	int i;
 
-	tmp = make_string("argv", 4);
-	aptr = assoc_lookup(PROCINFO_node, tmp);
-	unref(tmp);
-	unref(*aptr);
+	// build the sub-array first
 	getnode(argv_array);
  	memset(argv_array, '\0', sizeof(NODE));  /* valgrind wants this */
 	null_array(argv_array);
-	*aptr = argv_array;
 	argv_array->parent_array = PROCINFO_node;
 	argv_array->vname = estrdup("argv", 4);
 	for (i = 0; d_argv[i] != NULL; i++) {
-		tmp = make_number(i);
-		aptr = assoc_lookup(argv_array, tmp);
-		unref(tmp);
-		unref(*aptr);
-		*aptr = make_string(d_argv[i], strlen(d_argv[i]));
+		sub = make_number(i);
+		val = make_string(d_argv[i], strlen(d_argv[i]));
+		assoc_set(argv_array, sub, val);
 	}
+
+	// hook it into PROCINFO
+	sub = make_string("argv", 4);
+	assoc_set(PROCINFO_node, sub, argv_array);
+
 }
 
 /* load_procinfo --- populate the PROCINFO array */
diff --git a/mpfr.c b/mpfr.c
index c3ba2a6a..e1712863 100644
--- a/mpfr.c
+++ b/mpfr.c
@@ -1262,16 +1262,10 @@ do_mpfr_intdiv(int nargs)
 	unref(denominator);
 
 	sub = make_string("quotient", 8);
-	lhs = assoc_lookup(result, sub);
-	unref(*lhs);
-	*lhs = quotient;
-	unref(sub);
+	assoc_set(result, sub, quotient);
 
 	sub = make_string("remainder", 9);
-	lhs = assoc_lookup(result, sub);
-	unref(*lhs);
-	*lhs = remainder;
-	unref(sub);
+	assoc_set(result, sub, remainder);
 
 	return make_number((AWKNUM) 0.0);
 }
diff --git a/str_array.c b/str_array.c
index 2fdd1bf2..85e554b8 100644
--- a/str_array.c
+++ b/str_array.c
@@ -121,7 +121,7 @@ str_array_init(NODE *symbol ATTRIBUTE_UNUSED, NODE *subs ATTRIBUTE_UNUSED)
 
 
 /*
- * assoc_lookup:
+ * str_lookup:
  * Find SYMBOL[SUBS] in the assoc array.  Install it with value "" if it
  * isn't there. Returns a pointer ala get_lhs to where its value is stored.
  *
diff --git a/symbol.c b/symbol.c
index fc598da5..94d66fe8 100644
--- a/symbol.c
+++ b/symbol.c
@@ -307,7 +307,6 @@ static NODE *
 install(const char *name, NODE *parm, NODETYPE type)
 {
 	NODE *r;
-	NODE **aptr;
 	NODE *table;
 	NODE *n_name;
 	NODE *prev;
@@ -353,14 +352,12 @@ install(const char *name, NODE *parm, NODETYPE type)
 			goto simple;
 		r->dup_ent = prev->dup_ent;
 		prev->dup_ent = r;
+		unref(n_name);
 	} else {
 simple:
 		/* the simple case */
-		aptr = assoc_lookup(table, n_name);
-		unref(*aptr);
-		*aptr = r;
+		assoc_set(table, n_name, r);
 	}
-	unref(n_name);
 
 	if (install_func)
 		(*install_func)(r);