Improve TableVal HashKey management

* Deprecated ComputeHash() methods and replaced with MakeHashKey()
  which returns std::unique_ptr<HashKey>

* Deprecated RecoverIndex() and replaced with RecreateIndex()
  which takes HashKey& and returns IntrusivePtr.

* Updated the new TableVal Assign()/Remove() methods to take either
  std::unique_ptr<HashKey> or HashKey& as appropriate for clarity of
  ownership expectations.

src/CompHash.cc

@@ -229,7 +229,7 @@ char* CompositeHash::SingleValHash(bool type_check, char* kp0,
 			while ( tbl->NextEntry(k, it) )
 				{
 				hashkeys[k] = idx++;
-				lv->Append(tv->RecoverIndex(k));
+				lv->Append(tv->RecreateIndex(*k));
 				}
 
 			for ( auto& kv : hashkeys )
@@ -333,7 +333,7 @@ char* CompositeHash::SingleValHash(bool type_check, char* kp0,
 	}
 
 
-HashKey* CompositeHash::ComputeHash(const Val& argv, bool type_check) const
+std::unique_ptr<HashKey> CompositeHash::MakeHashKey(const Val& argv, bool type_check) const
 	{
 	auto v = &argv;
 
@@ -349,8 +349,7 @@ HashKey* CompositeHash::ComputeHash(const Val& argv, bool type_check) const
 		// be okay; the only thing is that the ListVal unref's it.
 		Val* ncv = (Val*) v;
 		lv.Append({NewRef{}, ncv});
-		HashKey* hk = ComputeHash(lv, type_check);
+		return MakeHashKey(lv, type_check);
-		return hk;
 		}
 
 	char* k = key;
@@ -383,10 +382,10 @@ HashKey* CompositeHash::ComputeHash(const Val& argv, bool type_check) const
 			return nullptr;
 		}
 
-	return new HashKey((k == key), (void*) k, kp - k);
+	return std::make_unique<HashKey>((k == key), (void*) k, kp - k);
 	}
 
-HashKey* CompositeHash::ComputeSingletonHash(const Val* v, bool type_check) const
+std::unique_ptr<HashKey> CompositeHash::ComputeSingletonHash(const Val* v, bool type_check) const
 	{
 	if ( v->GetType()->Tag() == TYPE_LIST )
 		{
@@ -404,21 +403,21 @@ HashKey* CompositeHash::ComputeSingletonHash(const Val* v, bool type_check) cons
 	switch ( singleton_tag ) {
 	case TYPE_INTERNAL_INT:
 	case TYPE_INTERNAL_UNSIGNED:
-		return new HashKey(v->ForceAsInt());
+		return std::make_unique<HashKey>(v->ForceAsInt());
 
 	case TYPE_INTERNAL_ADDR:
-		return v->AsAddr().GetHashKey();
+		return v->AsAddr().MakeHashKey();
 
 	case TYPE_INTERNAL_SUBNET:
-		return v->AsSubNet().GetHashKey();
+		return v->AsSubNet().MakeHashKey();
 
 	case TYPE_INTERNAL_DOUBLE:
-		return new HashKey(v->InternalDouble());
+		return std::make_unique<HashKey>(v->InternalDouble());
 
 	case TYPE_INTERNAL_VOID:
 	case TYPE_INTERNAL_OTHER:
 		if ( v->GetType()->Tag() == TYPE_FUNC )
-			return new HashKey(v->AsFunc()->GetUniqueFuncID());
+			return std::make_unique<HashKey>(v->AsFunc()->GetUniqueFuncID());
 
 		if ( v->GetType()->Tag() == TYPE_PATTERN )
 			{
@@ -430,14 +429,14 @@ HashKey* CompositeHash::ComputeSingletonHash(const Val* v, bool type_check) cons
 			char* key = new char[n];
 			std::memcpy(key, texts[0], strlen(texts[0]) + 1);
 			std::memcpy(key + strlen(texts[0]) + 1, texts[1], strlen(texts[1]) + 1);
-			return new HashKey(false, key, n);
+			return std::make_unique<HashKey>(false, key, n);
 			}
 
 		reporter->InternalError("bad index type in CompositeHash::ComputeSingletonHash");
 		return nullptr;
 
 	case TYPE_INTERNAL_STRING:
-		return new HashKey(v->AsString());
+		return std::make_unique<HashKey>(v->AsString());
 
 	case TYPE_INTERNAL_ERROR:
 		return nullptr;
@@ -707,12 +706,12 @@ int CompositeHash::SizeAlign(int offset, unsigned int size) const
 	return offset;
 	}
 
-IntrusivePtr<ListVal> CompositeHash::RecoverVals(const HashKey* k) const
+IntrusivePtr<ListVal> CompositeHash::RecoverVals(const HashKey& k) const
 	{
 	auto l = make_intrusive<ListVal>(TYPE_ANY);
 	const auto& tl = type->Types();
-	const char* kp = (const char*) k->Key();
+	const char* kp = (const char*) k.Key();
-	const char* const k_end = kp + k->Size();
+	const char* const k_end = kp + k.Size();
 
 	for ( const auto& type : tl )
 		{
@@ -728,12 +727,12 @@ IntrusivePtr<ListVal> CompositeHash::RecoverVals(const HashKey* k) const
 	return l;
 	}
 
-const char* CompositeHash::RecoverOneVal(const HashKey* k, const char* kp0,
+const char* CompositeHash::RecoverOneVal(const HashKey& k, const char* kp0,
 					 const char* const k_end, BroType* t,
 					 IntrusivePtr<Val>* pval, bool optional) const
 	{
 	// k->Size() == 0 for a single empty string.
-	if ( kp0 >= k_end && k->Size() > 0 )
+	if ( kp0 >= k_end && k.Size() > 0 )
 		reporter->InternalError("over-ran key in CompositeHash::RecoverVals");
 
 	TypeTag tag = t->Tag();
@@ -874,7 +873,7 @@ const char* CompositeHash::RecoverOneVal(const HashKey* k, const char* kp0,
 				kp1 = kp0;
 				int divider = strlen(kp0) + 1;
 				re = new RE_Matcher(kp1, kp1 + divider);
-				kp1 += k->Size();
+				kp1 += k.Size();
 				}
 			else
 				{
@@ -1033,7 +1032,7 @@ const char* CompositeHash::RecoverOneVal(const HashKey* k, const char* kp0,
 		if ( is_singleton )
 			{
 			kp1 = kp0;
-			n = k->Size();
+			n = k.Size();
 			}
 		else
 			{

src/CompHash.h

@@ -2,6 +2,8 @@
 
 #pragma once
 
+#include <memory>
+
 #include "Type.h"
 #include "IntrusivePtr.h"
 
@@ -15,19 +17,23 @@ public:
 
 	// Compute the hash corresponding to the given index val,
 	// or nullptr if it fails to typecheck.
-	HashKey* ComputeHash(const Val& v, bool type_check) const;
+	std::unique_ptr<HashKey> MakeHashKey(const Val& v, bool type_check) const;
 
-	[[deprecated("Remove in v4.1.  Pass a Val& instead.")]]
+	[[deprecated("Remove in v4.1.  Use MakeHashKey().")]]
 	HashKey* ComputeHash(const Val* v, bool type_check) const
-		{ return ComputeHash(*v, type_check); }
+		{ return MakeHashKey(*v, type_check).release(); }
 
 	// Given a hash key, recover the values used to create it.
-	IntrusivePtr<ListVal> RecoverVals(const HashKey* k) const;
+	IntrusivePtr<ListVal> RecoverVals(const HashKey& k) const;
+
+	[[deprecated("Remove in v4.1.  Pass in HashKey& instead.")]]
+	IntrusivePtr<ListVal> RecoverVals(const HashKey* k) const
+		{ return RecoverVals(*k); }
 
 	unsigned int MemoryAllocation() const { return padded_sizeof(*this) + pad_size(size); }
 
 protected:
-	HashKey* ComputeSingletonHash(const Val* v, bool type_check) const;
+	std::unique_ptr<HashKey> ComputeSingletonHash(const Val* v, bool type_check) const;
 
 	// Computes the piece of the hash for Val*, returning the new kp.
 	// Used as a helper for ComputeHash in the non-singleton case.
@@ -38,7 +44,7 @@ protected:
 	// Upon return, pval will point to the recovered Val of type t.
 	// Returns and updated kp for the next Val.  Calls reporter->InternalError()
 	// upon errors, so there is no return value for invalid input.
-	const char* RecoverOneVal(const HashKey* k,
+	const char* RecoverOneVal(const HashKey& k,
 				  const char* kp, const char* const k_end,
 				  BroType* t, IntrusivePtr<Val>* pval, bool optional) const;
 

src/Dict.h

@@ -218,4 +218,6 @@ public:
 		{ return (T*) Dictionary::NextEntry(h, cookie, 1); }
 	T* RemoveEntry(const HashKey* key)
 		{ return (T*) Remove(key->Key(), key->Size(), key->Hash()); }
+	T* RemoveEntry(const HashKey& key)
+		{ return (T*) Remove(key.Key(), key.Size(), key.Hash()); }
 };

src/IPAddr.cc

@@ -53,8 +53,11 @@ IPAddr::IPAddr(const BroString& s)
 	}
 
 HashKey* IPAddr::GetHashKey() const
+	{ return MakeHashKey().release(); }
+
+std::unique_ptr<HashKey> IPAddr::MakeHashKey() const
 	{
-	return new HashKey((void*)in6.s6_addr, sizeof(in6.s6_addr));
+	return std::make_unique<HashKey>((void*)in6.s6_addr, sizeof(in6.s6_addr));
 	}
 
 static inline uint32_t bit_mask32(int bottom_bits)
@@ -303,6 +306,9 @@ std::string IPPrefix::AsString() const
 	}
 
 HashKey* IPPrefix::GetHashKey() const
+	{ return MakeHashKey().release(); }
+
+std::unique_ptr<HashKey> IPPrefix::MakeHashKey() const
 	{
 	struct {
 		in6_addr ip;
@@ -312,7 +318,7 @@ HashKey* IPPrefix::GetHashKey() const
 	key.ip = prefix.in6;
 	key.len = Length();
 
-	return new HashKey(&key, sizeof(key));
+	return std::make_unique<HashKey>(&key, sizeof(key));
 	}
 
 bool IPPrefix::ConvertString(const char* text, IPPrefix* result)

src/IPAddr.h

@@ -6,6 +6,7 @@
 #include <arpa/inet.h>
 #include <string.h>
 #include <string>
+#include <memory>
 
 #include "threading/SerialTypes.h"
 
@@ -247,9 +248,11 @@ public:
 		}
 
 	/**
-	 * Returns a key that can be used to lookup the IP Address in a hash
+	 * Returns a key that can be used to lookup the IP Address in a hash table.
-	 * table. Passes ownership to caller.
 	 */
+	std::unique_ptr<HashKey> MakeHashKey() const;
+
+	[[deprecated("Remove in v4.1.  Use MakeHashKey().")]]
 	HashKey* GetHashKey() const;
 
 	/**
@@ -629,9 +632,11 @@ public:
 	operator std::string() const	{ return AsString(); }
 
 	/**
-	 * Returns a key that can be used to lookup the IP Prefix in a hash
+	 * Returns a key that can be used to lookup the IP Prefix in a hash table.
-	 * table. Passes ownership to caller.
 	 */
+	std::unique_ptr<HashKey> MakeHashKey() const;
+
+	[[deprecated("Remove in v4.1.  Use MakeHashKey().")]]
 	HashKey* GetHashKey() const;
 
 	/** Converts the prefix into the type used internally by the

src/OpaqueVal.cc

@@ -739,16 +739,14 @@ bool BloomFilterVal::Typify(IntrusivePtr<BroType> arg_type)
 
 void BloomFilterVal::Add(const Val* val)
 	{
-	HashKey* key = hash->ComputeHash(*val, true);
+	auto key = hash->MakeHashKey(*val, true);
-	bloom_filter->Add(key);
+	bloom_filter->Add(key.get());
-	delete key;
 	}
 
 size_t BloomFilterVal::Count(const Val* val) const
 	{
-	HashKey* key = hash->ComputeHash(*val, true);
+	auto key = hash->MakeHashKey(*val, true);
-	size_t cnt = bloom_filter->Count(key);
+	size_t cnt = bloom_filter->Count(key.get());
-	delete key;
 	return cnt;
 	}
 
@@ -900,9 +898,8 @@ bool CardinalityVal::Typify(IntrusivePtr<BroType> arg_type)
 
 void CardinalityVal::Add(const Val* val)
 	{
-	HashKey* key = hash->ComputeHash(*val, true);
+	auto key = hash->MakeHashKey(*val, true);
 	c->AddElement(key->Hash());
-	delete key;
 	}
 
 IMPLEMENT_OPAQUE_VALUE(CardinalityVal)

src/Reporter.cc

@@ -78,7 +78,7 @@ void Reporter::InitOptions()
 
 	while ( (v = wl_table->NextEntry(k, c)) )
 		{
-		auto index = wl_val->RecoverIndex(k);
+		auto index = wl_val->RecreateIndex(*k);
 		std::string key = index->Idx(0)->AsString()->CheckString();
 		weird_sampling_whitelist.emplace(move(key));
 		delete k;

src/Stmt.cc

@@ -719,7 +719,7 @@ SwitchStmt::~SwitchStmt()
 
 bool SwitchStmt::AddCaseLabelValueMapping(const Val* v, int idx)
 	{
-	HashKey* hk = comp_hash->ComputeHash(*v, true);
+	auto hk = comp_hash->MakeHashKey(*v, true);
 
 	if ( ! hk )
 		{
@@ -728,16 +728,12 @@ bool SwitchStmt::AddCaseLabelValueMapping(const Val* v, int idx)
 		    type_name(v->GetType()->Tag()), type_name(e->GetType()->Tag()));
 		}
 
-	int* label_idx = case_label_value_map.Lookup(hk);
+	int* label_idx = case_label_value_map.Lookup(hk.get());
 
 	if ( label_idx )
-		{
-		delete hk;
 		return false;
-		}
 
-	case_label_value_map.Insert(hk, new int(idx));
+	case_label_value_map.Insert(hk.get(), new int(idx));
-	delete hk;
 	return true;
 	}
 
@@ -763,7 +759,7 @@ std::pair<int, ID*> SwitchStmt::FindCaseLabelMatch(const Val* v) const
 	// Find matching expression cases.
 	if ( case_label_value_map.Length() )
 		{
-		HashKey* hk = comp_hash->ComputeHash(*v, true);
+		auto hk = comp_hash->MakeHashKey(*v, true);
 
 		if ( ! hk )
 			{
@@ -773,10 +769,8 @@ std::pair<int, ID*> SwitchStmt::FindCaseLabelMatch(const Val* v) const
 			return std::make_pair(-1, nullptr);
 			}
 
-		if ( auto i = case_label_value_map.Lookup(hk) )
+		if ( auto i = case_label_value_map.Lookup(hk.get()) )
 			label_idx = *i;
-
-		delete hk;
 		}
 
 	// Find matching type cases.
@@ -1193,7 +1187,7 @@ IntrusivePtr<Val> ForStmt::DoExec(Frame* f, Val* v, stmt_flow_type& flow) const
 		IterCookie* c = loop_vals->InitForIteration();
 		while ( (current_tev = loop_vals->NextEntry(k, c)) )
 			{
-			auto ind_lv = tv->RecoverIndex(k);
+			auto ind_lv = tv->RecreateIndex(*k);
 			delete k;
 
 			if ( value_var )

src/Val.cc

@@ -540,7 +540,7 @@ static void BuildJSON(threading::formatter::JSON::NullDoubleWriter& writer, Val*
 			auto c = table->InitForIteration();
 			while ( (entry = table->NextEntry(k, c)) )
 				{
-				auto lv = tval->RecoverIndex(k);
+				auto lv = tval->RecreateIndex(*k);
 				delete k;
 				Val* entry_key = lv->Length() == 1 ? lv->Idx(0).get() : lv.get();
 
@@ -1508,14 +1508,15 @@ void TableVal::CheckExpireAttr(attr_tag at)
 
 bool TableVal::Assign(IntrusivePtr<Val> index, IntrusivePtr<Val> new_val)
 	{
-	HashKey* k = ComputeHash(*index);
+	auto k = MakeHashKey(*index);
+
 	if ( ! k )
 		{
 		index->Error("index type doesn't match table", table_type->Indices());
 		return false;
 		}
 
-	return Assign(std::move(index), k, std::move(new_val));
+	return Assign(std::move(index), std::move(k), std::move(new_val));
 	}
 
 bool TableVal::Assign(Val* index, Val* new_val)
@@ -1523,7 +1524,8 @@ bool TableVal::Assign(Val* index, Val* new_val)
 	return Assign({NewRef{}, index}, {AdoptRef{}, new_val});
 	}
 
-bool TableVal::Assign(IntrusivePtr<Val> index, HashKey* k, IntrusivePtr<Val> new_val)
+bool TableVal::Assign(IntrusivePtr<Val> index, std::unique_ptr<HashKey> k,
+                      IntrusivePtr<Val> new_val)
 	{
 	bool is_set = table_type->IsSet();
 
@@ -1532,19 +1534,18 @@ bool TableVal::Assign(IntrusivePtr<Val> index, HashKey* k, IntrusivePtr<Val> new
 
 	TableEntryVal* new_entry_val = new TableEntryVal(new_val);
 	HashKey k_copy(k->Key(), k->Size(), k->Hash());
-	TableEntryVal* old_entry_val = AsNonConstTable()->Insert(k, new_entry_val);
+	TableEntryVal* old_entry_val = AsNonConstTable()->Insert(k.get(), new_entry_val);
 
 	// If the dictionary index already existed, the insert may free up the
 	// memory allocated to the key bytes, so have to assume k is invalid
 	// from here on out.
-	delete k;
 	k = nullptr;
 
 	if ( subnets )
 		{
 		if ( ! index )
 			{
-			auto v = RecoverIndex(&k_copy);
+			auto v = RecreateIndex(k_copy);
 			subnets->Insert(v.get(), new_entry_val);
 			}
 		else
@@ -1559,7 +1560,7 @@ bool TableVal::Assign(IntrusivePtr<Val> index, HashKey* k, IntrusivePtr<Val> new
 
 	if ( change_func )
 		{
-		auto change_index = index ? std::move(index) : RecoverIndex(&k_copy);
+		auto change_index = index ? std::move(index) : RecreateIndex(k_copy);
 		auto v = old_entry_val ? old_entry_val->GetVal() : new_val;
 		CallChangeFunc(change_index.get(), v.get(), old_entry_val ? ELEMENT_CHANGED : ELEMENT_NEW);
 		}
@@ -1571,7 +1572,7 @@ bool TableVal::Assign(IntrusivePtr<Val> index, HashKey* k, IntrusivePtr<Val> new
 
 bool TableVal::Assign(Val* index, HashKey* k, Val* new_val)
 	{
-	return Assign({NewRef{}, index}, k, {AdoptRef{}, new_val});
+	return Assign({NewRef{}, index}, std::unique_ptr<HashKey>{k}, {AdoptRef{}, new_val});
 	}
 
 IntrusivePtr<Val> TableVal::SizeVal() const
@@ -1607,9 +1608,11 @@ bool TableVal::AddTo(Val* val, bool is_first_init, bool propagate_ops) const
 	TableEntryVal* v;
 	while ( (v = tbl->NextEntry(k, c)) )
 		{
+		std::unique_ptr<HashKey> hk{k};
+
 		if ( is_first_init && t->AsTable()->Lookup(k) )
 			{
-			auto key = table_hash->RecoverVals(k);
+			auto key = table_hash->RecoverVals(*k);
 			// ### Shouldn't complain if their values are equal.
 			key->Warn("multiple initializations for index");
 			continue;
@@ -1617,12 +1620,12 @@ bool TableVal::AddTo(Val* val, bool is_first_init, bool propagate_ops) const
 
 		if ( type->IsSet() )
 			{
-			if ( ! t->Assign(v->GetVal(), k, nullptr) )
+			if ( ! t->Assign(v->GetVal(), std::move(hk), nullptr) )
 				 return false;
 			}
 		else
 			{
-			if ( ! t->Assign(nullptr, k, v->GetVal()) )
+			if ( ! t->Assign(nullptr, std::move(hk), v->GetVal()) )
 				 return false;
 			}
 		}
@@ -1657,7 +1660,7 @@ bool TableVal::RemoveFrom(Val* val) const
 		// OTOH, they are both the same type, so as long as
 		// we don't have hash keys that are keyed per dictionary,
 		// it should work ...
-		t->Remove(k);
+		t->Remove(*k);
 		delete k;
 		}
 
@@ -1910,11 +1913,11 @@ const IntrusivePtr<Val>& TableVal::Find(const IntrusivePtr<Val>& index)
 
 	if ( tbl->Length() > 0 )
 		{
-		HashKey* k = ComputeHash(*index);
+		auto k = MakeHashKey(*index);
+
 		if ( k )
 			{
-			TableEntryVal* v = AsTable()->Lookup(k);
+			TableEntryVal* v = AsTable()->Lookup(k.get());
-			delete k;
 
 			if ( v )
 				{
@@ -2006,13 +2009,12 @@ bool TableVal::UpdateTimestamp(Val* index)
 		v = (TableEntryVal*) subnets->Lookup(index);
 	else
 		{
-		HashKey* k = ComputeHash(*index);
+		auto k = MakeHashKey(*index);
+
 		if ( ! k )
 			return false;
 
-		v = AsTable()->Lookup(k);
+		v = AsTable()->Lookup(k.get());
-
-		delete k;
 		}
 
 	if ( ! v )
@@ -2023,7 +2025,7 @@ bool TableVal::UpdateTimestamp(Val* index)
 	return true;
 	}
 
-IntrusivePtr<ListVal> TableVal::RecoverIndex(const HashKey* k) const
+IntrusivePtr<ListVal> TableVal::RecreateIndex(const HashKey& k) const
 	{
 	return table_hash->RecoverVals(k);
 	}
@@ -2091,8 +2093,8 @@ void TableVal::CallChangeFunc(const Val* index, Val* old_value, OnChangeType tpe
 
 IntrusivePtr<Val> TableVal::Remove(const Val& index)
 	{
-	HashKey* k = ComputeHash(index);
+	auto k = MakeHashKey(index);
-	TableEntryVal* v = k ? AsNonConstTable()->RemoveEntry(k) : nullptr;
+	TableEntryVal* v = k ? AsNonConstTable()->RemoveEntry(k.get()) : nullptr;
 	IntrusivePtr<Val> va;
 
 	if ( v )
@@ -2101,7 +2103,6 @@ IntrusivePtr<Val> TableVal::Remove(const Val& index)
 	if ( subnets && ! subnets->Remove(&index) )
 		reporter->InternalWarning("index not in prefix table");
 
-	delete k;
 	delete v;
 
 	Modified();
@@ -2112,7 +2113,7 @@ IntrusivePtr<Val> TableVal::Remove(const Val& index)
 	return va;
 	}
 
-IntrusivePtr<Val> TableVal::Remove(const HashKey* k)
+IntrusivePtr<Val> TableVal::Remove(const HashKey& k)
 	{
 	TableEntryVal* v = AsNonConstTable()->RemoveEntry(k);
 	IntrusivePtr<Val> va;
@@ -2151,7 +2152,7 @@ IntrusivePtr<ListVal> TableVal::ToListVal(TypeTag t) const
 	HashKey* k;
 	while ( tbl->NextEntry(k, c) )
 		{
-		auto index = table_hash->RecoverVals(k);
+		auto index = table_hash->RecoverVals(*k);
 
 		if ( t == TYPE_ANY )
 			l->Append(std::move(index));
@@ -2226,7 +2227,7 @@ void TableVal::Describe(ODesc* d) const
 		if ( ! v )
 			reporter->InternalError("hash table underflow in TableVal::Describe");
 
-		auto vl = table_hash->RecoverVals(k);
+		auto vl = table_hash->RecoverVals(*k);
 		int dim = vl->Length();
 
 		if ( i > 0 )
@@ -2398,7 +2399,7 @@ void TableVal::DoExpire(double t)
 
 			if ( expire_func )
 				{
-				idx = RecoverIndex(k);
+				idx = RecreateIndex(*k);
 				double secs = CallExpireFunc(idx);
 
 				// It's possible that the user-provided
@@ -2428,7 +2429,7 @@ void TableVal::DoExpire(double t)
 			if ( subnets )
 				{
 				if ( ! idx )
-					idx = RecoverIndex(k);
+					idx = RecreateIndex(*k);
 				if ( ! subnets->Remove(idx.get()) )
 					reporter->InternalWarning("index not in prefix table");
 				}
@@ -2437,7 +2438,7 @@ void TableVal::DoExpire(double t)
 			if ( change_func )
 				{
 				if ( ! idx )
-					idx = RecoverIndex(k);
+					idx = RecreateIndex(*k);
 				CallChangeFunc(idx.get(), v->GetVal().get(), ELEMENT_EXPIRED);
 				}
 
@@ -2568,7 +2569,7 @@ IntrusivePtr<Val> TableVal::DoClone(CloneState* state)
 
 		if ( subnets )
 			{
-			auto idx = RecoverIndex(key);
+			auto idx = RecreateIndex(*key);
 			tv->subnets->Insert(idx.get(), nval);
 			}
 
@@ -2615,9 +2616,12 @@ unsigned int TableVal::MemoryAllocation() const
 		+ table_hash->MemoryAllocation();
 	}
 
-HashKey* TableVal::ComputeHash(const Val& index) const
+HashKey* TableVal::ComputeHash(const Val* index) const
+	{ return MakeHashKey(*index).release(); }
+
+std::unique_ptr<HashKey> TableVal::MakeHashKey(const Val& index) const
 	{
-	return table_hash->ComputeHash(index, true);
+	return table_hash->MakeHashKey(index, true);
 	}
 
 void TableVal::SaveParseTimeTableState(RecordType* rt)
@@ -2658,7 +2662,7 @@ TableVal::ParseTimeTableState TableVal::DumpTableState()
 
 	while ( (val = tbl->NextEntry(key, cookie)) )
 		{
-		rval.emplace_back(RecoverIndex(key), val->GetVal());
+		rval.emplace_back(RecreateIndex(*key), val->GetVal());
 		delete key;
 		}
 

src/Val.h

@@ -761,13 +761,13 @@ public:
 	 * case of a set, just adds the index).
 	 * @param index  The key to assign.  For tables, this is allowed to be null
 	 * (if needed, the index val can be recovered from the hash key).
-	 * @param k  A precomputed hash key to use (this method takes ownership
+	 * @param k  A precomputed hash key to use.
-	 * of deleting it).
 	 * @param new_val  The value to assign at the index.  For a set, this
 	 * must be nullptr.
 	 * @return  True if the assignment type-checked.
 	 */
-	bool Assign(IntrusivePtr<Val> index, HashKey* k, IntrusivePtr<Val> new_val);
+	bool Assign(IntrusivePtr<Val> index, std::unique_ptr<HashKey> k,
+	            IntrusivePtr<Val> new_val);
 
 	// Returns true if the assignment typechecked, false if not. The
 	// methods take ownership of new_val, but not of the index.  If we're
@@ -877,8 +877,14 @@ public:
 	// Returns false if index does not exist.
 	bool UpdateTimestamp(Val* index);
 
-	// Returns the index corresponding to the given HashKey.
+	/**
-	IntrusivePtr<ListVal> RecoverIndex(const HashKey* k) const;
+	 * @return  The index corresponding to the given HashKey.
+	 */
+	IntrusivePtr<ListVal> RecreateIndex(const HashKey& k) const;
+
+	[[deprecated("Remove in v4.1.  Use RecreateIndex().")]]
+	ListVal* RecoverIndex(const HashKey* k) const
+		{ return RecreateIndex(*k).release(); }
 
 	/**
 	 * Remove an element from the table and return it.
@@ -892,11 +898,10 @@ public:
 
 	/**
 	 * Same as Remove(const Val&), but uses a precomputed hash key.
-	 * @param k  The hash key to lookup.  This method takes ownership of
+	 * @param k  The hash key to lookup.
-	 * deleting it.
 	 * @return  Same as Remove(const Val&).
 	 */
-	IntrusivePtr<Val> Remove(const HashKey* k);
+	IntrusivePtr<Val> Remove(const HashKey& k);
 
 	[[deprecated("Remove in v4.1.  Use Remove().")]]
 	Val* Delete(const Val* index)
@@ -904,7 +909,7 @@ public:
 
 	[[deprecated("Remove in v4.1.  Use Remove().")]]
 	Val* Delete(const HashKey* k)
-		{ return Remove(k).release(); }
+		{ return Remove(*k).release(); }
 
 	// Returns a ListVal representation of the table (which must be a set).
 	IntrusivePtr<ListVal> ToListVal(TypeTag t = TYPE_ANY) const;
@@ -954,11 +959,15 @@ public:
 			timer = nullptr;
 		}
 
-	HashKey* ComputeHash(const Val& index) const;
+	/**
+	 * @param  The index value to hash.
+	 * @return  The hash of the index value or nullptr if
+	 * type-checking failed.
+	 */
+	std::unique_ptr<HashKey> MakeHashKey(const Val& index) const;
 
-	[[deprecated("Remove in v4.1.  Pass a Val& instead.")]]
+	[[deprecated("Remove in v4.1.  Use MakeHashKey().")]]
-	HashKey* ComputeHash(const Val* index) const
+	HashKey* ComputeHash(const Val* index) const;
-		{ return ComputeHash(*index); }
 
 	notifier::Modifiable* Modifiable() override	{ return this; }
 

src/broker/Data.cc

@@ -901,7 +901,7 @@ broker::expected<broker::data> bro_broker::val_to_data(const Val* v)
 
 		while ( (entry = table->NextEntry(hk, c)) )
 			{
-			auto vl = table_val->RecoverIndex(hk);
+			auto vl = table_val->RecreateIndex(*hk);
 			delete hk;
 
 			broker::vector composite_key;

src/broker/messaging.bif

@@ -38,7 +38,7 @@ std::set<std::string> val_to_topic_set(Val* val)
 
 		while ( tbl->NextEntry(k, c) )
 			{
-			auto index = val->AsTableVal()->RecoverIndex(k);
+			auto index = val->AsTableVal()->RecreateIndex(*k);
 			rval.emplace(index->Idx(0)->AsString()->CheckString());
 			delete k;
 			}

src/file_analysis/AnalyzerSet.cc

@@ -166,12 +166,12 @@ HashKey* AnalyzerSet::GetKey(const file_analysis::Tag& t, RecordVal* args) const
 	auto lv = make_intrusive<ListVal>(TYPE_ANY);
 	lv->Append(t.AsVal());
 	lv->Append({NewRef{}, args});
-	HashKey* key = analyzer_hash->ComputeHash(*lv, true);
+	auto key = analyzer_hash->MakeHashKey(*lv, true);
 
 	if ( ! key )
 		reporter->InternalError("AnalyzerArgs type mismatch");
 
-	return key;
+	return key.release();
 	}
 
 file_analysis::Analyzer* AnalyzerSet::InstantiateAnalyzer(const Tag& tag,

src/input/Manager.cc

@@ -285,7 +285,7 @@ bool Manager::CreateStream(Stream* info, RecordVal* description)
 		TableEntryVal* v;
 		while ( (v = info->config->AsTable()->NextEntry(k, c)) )
 			{
-			auto index = info->config->RecoverIndex(k);
+			auto index = info->config->RecreateIndex(*k);
 			string key = index->Idx(0)->AsString()->CheckString();
 			string value = v->GetVal()->AsString()->CheckString();
 			rinfo.config.insert(std::make_pair(copy_string(key.c_str()), copy_string(value.c_str())));
@@ -1248,7 +1248,8 @@ int Manager::SendEntryTable(Stream* i, const Value* const *vals)
 		oldval = stream->tab->Find({NewRef{}, idxval});
 		}
 
-	HashKey* k = stream->tab->ComputeHash(*idxval);
+	auto k = stream->tab->MakeHashKey(*idxval);
+
 	if ( ! k )
 		reporter->InternalError("could not hash");
 
@@ -1256,7 +1257,7 @@ int Manager::SendEntryTable(Stream* i, const Value* const *vals)
 	ih->idxkey = new HashKey(k->Key(), k->Size(), k->Hash());
 	ih->valhash = valhash;
 
-	stream->tab->Assign({AdoptRef{}, idxval}, k, {AdoptRef{}, valval});
+	stream->tab->Assign({AdoptRef{}, idxval}, std::move(k), {AdoptRef{}, valval});
 
 	if ( predidx != nullptr )
 		Unref(predidx);
@@ -1342,7 +1343,7 @@ void Manager::EndCurrentSend(ReaderFrontend* reader)
 
 		if ( stream->pred || stream->event )
 			{
-			auto idx = stream->tab->RecoverIndex(ih->idxkey);
+			auto idx = stream->tab->RecreateIndex(*ih->idxkey);
 			assert(idx != nullptr);
 			val = stream->tab->FindOrDefault(idx);
 			assert(val != nullptr);
@@ -1371,7 +1372,7 @@ void Manager::EndCurrentSend(ReaderFrontend* reader)
 			SendEvent(stream->event, 4, stream->description->Ref(), ev->Ref(),
 			          predidx->Ref(), val->Ref());
 
-		stream->tab->Remove(ih->idxkey);
+		stream->tab->Remove(*ih->idxkey);
 		stream->lastDict->Remove(lastDictIdxKey); // delete in next line
 		delete lastDictIdxKey;
 		delete(ih);

src/logging/Manager.cc

@@ -865,7 +865,7 @@ bool Manager::Write(EnumVal* id, RecordVal* columns_arg)
 			TableEntryVal* v;
 			while ( (v = filter->config->AsTable()->NextEntry(k, c)) )
 				{
-				auto index = filter->config->RecoverIndex(k);
+				auto index = filter->config->RecreateIndex(*k);
 				string key = index->Idx(0)->AsString()->CheckString();
 				string value = v->GetVal()->AsString()->CheckString();
 				info->config.insert(std::make_pair(copy_string(key.c_str()), copy_string(value.c_str())));

src/probabilistic/Topk.cc

@@ -29,9 +29,9 @@ void TopkVal::Typify(IntrusivePtr<BroType> t)
 
 HashKey* TopkVal::GetHash(Val* v) const
 	{
-	HashKey* key = hash->ComputeHash(*v, true);
+	auto key = hash->MakeHashKey(*v, true);
 	assert(key);
-	return key;
+	return key.release();
 	}
 
 TopkVal::TopkVal(uint64_t arg_size) : OpaqueVal(topk_type)

src/reporter.bif

@@ -179,7 +179,7 @@ function Reporter::set_weird_sampling_whitelist%(weird_sampling_whitelist: strin
 
 	while ( (v = wl_table->NextEntry(k, c)) )
 		{
-		auto index = wl_val->RecoverIndex(k);
+		auto index = wl_val->RecreateIndex(*k);
 		string key = index->Idx(0)->AsString()->CheckString();
 		whitelist_set.emplace(move(key));
 		delete k;

src/supervisor/Supervisor.cc

@@ -1024,7 +1024,7 @@ Supervisor::NodeConfig Supervisor::NodeConfig::FromRecord(const RecordVal* node)
 
 	while ( (v = cluster_table->NextEntry(k, c)) )
 		{
-		auto key = cluster_table_val->RecoverIndex(k);
+		auto key = cluster_table_val->RecreateIndex(*k);
 		delete k;
 		auto name = key->Idx(0)->AsStringVal()->ToStdString();
 		auto rv = v->GetVal()->AsRecordVal();