Val: pass IntrusivePtr<> to TableVal::ExpandAndInit()

Clarifies ownership and fixes memory leaks.

Closes https://github.com/zeek/zeek/issues/811

src/Expr.cc

@@ -2477,19 +2477,17 @@ Val* AssignExpr::InitVal(const BroType* t, Val* aggr) const
 		TableVal* tv = aggr->AsTableVal();
 		const TableType* tt = tv->Type()->AsTableType();
 		const BroType* yt = tv->Type()->YieldType();
-		Val* index = op1->InitVal(tt->Indices(), 0);
+		IntrusivePtr<Val> index{AdoptRef{}, op1->InitVal(tt->Indices(), 0)};
-		Val* v = op2->InitVal(yt, 0);
+		IntrusivePtr<Val> v{AdoptRef{}, op2->InitVal(yt, 0)};
 		if ( ! index || ! v )
 			return 0;
 
-		if ( ! tv->ExpandAndInit(index, v) )
+		if ( ! tv->ExpandAndInit(std::move(index), std::move(v)) )
 			{
-			Unref(index);
 			Unref(tv);
 			return 0;
 			}
 
-		Unref(index);
 		return tv;
 		}
 
@@ -4899,14 +4897,12 @@ Val* ListExpr::AddSetInit(const BroType* t, Val* aggr) const
 		if ( ! element )
 			return 0;
 
-		if ( ! tv->ExpandAndInit(element, 0) )
+		if ( ! tv->ExpandAndInit({AdoptRef{}, element}, 0) )
 			{
-			Unref(element);
 			Unref(tv);
 			return 0;
 			}
 
-		Unref(element);
 		}
 
 	return tv;

src/Val.cc

@@ -1676,20 +1676,19 @@ bool TableVal::IsSubsetOf(const TableVal* tv) const
 	return true;
 	}
 
-int TableVal::ExpandAndInit(Val* index, Val* new_val)
+int TableVal::ExpandAndInit(IntrusivePtr<Val> index, IntrusivePtr<Val> new_val)
 	{
 	BroType* index_type = index->Type();
 
 	if ( index_type->IsSet() )
 		{
-		Val* new_index = index->AsTableVal()->ConvertToList();
+		index = {AdoptRef{}, index->AsTableVal()->ConvertToList()};
-		Unref(index);
+		return ExpandAndInit(std::move(index), std::move(new_val));
-		return ExpandAndInit(new_index, new_val);
 		}
 
 	if ( index_type->Tag() != TYPE_LIST )
 		// Nothing to expand.
-		return CheckAndAssign(index, new_val);
+		return CheckAndAssign(index.get(), std::move(new_val));
 
 	ListVal* iv = index->AsListVal();
 	if ( iv->BaseTag() != TYPE_ANY )
@@ -1698,10 +1697,9 @@ int TableVal::ExpandAndInit(Val* index, Val* new_val)
 			reporter->InternalError("bad singleton list index");
 
 		for ( int i = 0; i < iv->Length(); ++i )
-			if ( ! ExpandAndInit(iv->Index(i), new_val ? new_val->Ref() : 0) )
+			if ( ! ExpandAndInit({NewRef{}, iv->Index(i)}, new_val) )
 				return 0;
 
-		Unref(new_val);
 		return 1;
 		}
 
@@ -1720,13 +1718,9 @@ int TableVal::ExpandAndInit(Val* index, Val* new_val)
 
 		if ( i >= vl->length() )
 			// Nothing to expand.
-			return CheckAndAssign(index, new_val);
+			return CheckAndAssign(index.get(), std::move(new_val));
 		else
-			{
+			return ExpandCompoundAndInit(vl, i, std::move(new_val));
-			int result = ExpandCompoundAndInit(vl, i, new_val);
-			Unref(new_val);
-			return result;
-			}
 		}
 	}
 
@@ -2191,17 +2185,17 @@ void TableVal::Describe(ODesc* d) const
 		}
 	}
 
-int TableVal::ExpandCompoundAndInit(val_list* vl, int k, Val* new_val)
+int TableVal::ExpandCompoundAndInit(val_list* vl, int k, IntrusivePtr<Val> new_val)
 	{
 	Val* ind_k_v = (*vl)[k];
-	ListVal* ind_k = ind_k_v->Type()->IsSet() ?
+	auto ind_k = ind_k_v->Type()->IsSet() ?
-				ind_k_v->AsTableVal()->ConvertToList() :
+				IntrusivePtr<ListVal>{AdoptRef{}, ind_k_v->AsTableVal()->ConvertToList()} :
-				ind_k_v->Ref()->AsListVal();
+				IntrusivePtr<ListVal>{NewRef{}, ind_k_v->AsListVal()};
 
 	for ( int i = 0; i < ind_k->Length(); ++i )
 		{
 		Val* ind_k_i = ind_k->Index(i);
-		ListVal* expd = new ListVal(TYPE_ANY);
+		auto expd = make_intrusive<ListVal>(TYPE_ANY);
 		loop_over_list(*vl, j)
 			{
 			if ( j == k )
@@ -2210,21 +2204,16 @@ int TableVal::ExpandCompoundAndInit(val_list* vl, int k, Val* new_val)
 				expd->Append((*vl)[j]->Ref());
 			}
 
-		int success = ExpandAndInit(expd, new_val ? new_val->Ref() : 0);
+		int success = ExpandAndInit(std::move(expd), new_val);
-		Unref(expd);
 
 		if ( ! success )
-			{
-			Unref(ind_k);
 			return 0;
 		}
-		}
 
-	Unref(ind_k);
 	return 1;
 	}
 
-int TableVal::CheckAndAssign(Val* index, Val* new_val)
+int TableVal::CheckAndAssign(Val* index, IntrusivePtr<Val> new_val)
 	{
 	Val* v = 0;
 	if ( subnets )
@@ -2236,7 +2225,7 @@ int TableVal::CheckAndAssign(Val* index, Val* new_val)
 	if ( v )
 		index->Warn("multiple initializations for index");
 
-	return Assign(index, new_val);
+	return Assign(index, new_val.release());
 	}
 
 void TableVal::InitDefaultFunc(Frame* f)

src/Val.h

@@ -27,6 +27,7 @@ using std::string;
 #define UDP_PORT_MASK	0x20000
 #define ICMP_PORT_MASK	0x30000
 
+template <class T> class IntrusivePtr;
 template<typename T> class PDict;
 class IterCookie;
 
@@ -754,7 +755,7 @@ public:
 
 	// Expands any lists in the index into multiple initializations.
 	// Returns true if the initializations typecheck, false if not.
-	int ExpandAndInit(Val* index, Val* new_val);
+	int ExpandAndInit(IntrusivePtr<Val> index, IntrusivePtr<Val> new_val);
 
 	// Returns the element's value if it exists in the table,
 	// nil otherwise.  Note, "index" is not const because we
@@ -825,8 +826,8 @@ protected:
 	void Init(TableType* t);
 
 	void CheckExpireAttr(attr_tag at);
-	int ExpandCompoundAndInit(val_list* vl, int k, Val* new_val);
+	int ExpandCompoundAndInit(val_list* vl, int k, IntrusivePtr<Val> new_val);
-	int CheckAndAssign(Val* index, Val* new_val);
+	int CheckAndAssign(Val* index, IntrusivePtr<Val> new_val);
 
 	// Calculates default value for index.  Returns 0 if none.
 	Val* Default(Val* index);