Deprecate TableVal::Lookup(), replace with Find()/FindOrDefault()

NEWS

@@ -208,6 +208,9 @@ Deprecated Functionality
 - ``TableVal::Assign`` methods taking raw ``Val*`` are deprecated, use the
   overloads taking ``IntrusivePtr``.
 
+- ``TableVal::Lookup()`` is deprecated, use ``TableVal::Find()`` or
+  ``TableVal::FindOrDefault()``.
+
 Zeek 3.1.0
 ==========
 

src/Anon.cc

@@ -387,7 +387,7 @@ void init_ip_addr_anonymizers()
 ipaddr32_t anonymize_ip(ipaddr32_t ip, enum ip_addr_anonymization_class_t cl)
 	{
 	TableVal* preserve_addr = nullptr;
-	AddrVal addr(ip);
+	auto addr = make_intrusive<AddrVal>(ip);
 
 	int method = -1;
 
@@ -410,7 +410,7 @@ ipaddr32_t anonymize_ip(ipaddr32_t ip, enum ip_addr_anonymization_class_t cl)
 
 	ipaddr32_t new_ip = 0;
 
-	if ( preserve_addr && preserve_addr->Lookup(&addr) )
+	if ( preserve_addr && preserve_addr->FindOrDefault(addr) )
 		new_ip = ip;
 
 	else if ( method >= 0 && method < NUM_ADDR_ANONYMIZATION_METHODS )

src/CompHash.cc

@@ -238,15 +238,15 @@ char* CompositeHash::SingleValHash(bool type_check, char* kp0,
 			for ( auto& kv : hashkeys )
 				{
 				auto idx = kv.second;
-				Val* key = lv->Idx(idx).get();
+				const auto& key = lv->Idx(idx);
 
-				if ( ! (kp1 = SingleValHash(type_check, kp1, key->GetType().get(), key,
+				if ( ! (kp1 = SingleValHash(type_check, kp1, key->GetType().get(),
-				                            false)) )
+				                            key.get(), false)) )
 					return nullptr;
 
 				if ( ! v->GetType()->IsSet() )
 					{
-					auto val = tv->Lookup(key);
+					auto val = tv->FindOrDefault(key);
 
 					if ( ! (kp1 = SingleValHash(type_check, kp1, val->GetType().get(),
 								    val.get(), false)) )
@@ -537,15 +537,15 @@ int CompositeHash::SingleTypeKeySize(BroType* bt, const Val* v,
 			auto lv = tv->ToListVal();
 			for ( int i = 0; i < tv->Size(); ++i )
 				{
-				Val* key = lv->Idx(i).get();
+				const auto& key = lv->Idx(i);
-				sz = SingleTypeKeySize(key->GetType().get(), key, type_check, sz, false,
+				sz = SingleTypeKeySize(key->GetType().get(), key.get(), type_check, sz, false,
 				                       calc_static_size);
 				if ( ! sz )
 					return 0;
 
 				if ( ! bt->IsSet() )
 					{
-					auto val = tv->Lookup(key);
+					auto val = tv->FindOrDefault(key);
 					sz = SingleTypeKeySize(val->GetType().get(), val.get(), type_check, sz,
 					                       false, calc_static_size);
 					if ( ! sz )

src/Expr.cc

@@ -2669,7 +2669,7 @@ IntrusivePtr<Val> IndexExpr::Fold(Val* v1, Val* v2) const
 		break;
 
 	case TYPE_TABLE:
-		v = v1->AsTableVal()->Lookup(v2); // Then, we jump into the TableVal here.
+		v = v1->AsTableVal()->FindOrDefault({NewRef{}, v2}); // Then, we jump into the TableVal here.
 		break;
 
 	case TYPE_STRING:
@@ -3995,7 +3995,7 @@ IntrusivePtr<Val> InExpr::Fold(Val* v1, Val* v2) const
 	if ( is_vector(v2) )
 		res = (bool)v2->AsVectorVal()->Lookup(v1);
 	else
-		res = (bool)v2->AsTableVal()->Lookup(v1, false);
+		res = (bool)v2->AsTableVal()->Find({NewRef{}, v1});
 
 	return val_mgr->Bool(res);
 	}

src/Val.cc

@@ -1801,7 +1801,7 @@ bool TableVal::ExpandAndInit(IntrusivePtr<Val> index, IntrusivePtr<Val> new_val)
 	}
 
 
-IntrusivePtr<Val> TableVal::Default(Val* index)
+IntrusivePtr<Val> TableVal::Default(const IntrusivePtr<Val>& index)
 	{
 	Attr* def_attr = FindAttr(ATTR_DEFAULT);
 
@@ -1863,7 +1863,7 @@ IntrusivePtr<Val> TableVal::Default(Val* index)
 			vl.emplace_back(v);
 		}
 	else
-		vl.emplace_back(NewRef{}, index);
+		vl.emplace_back(index);
 
 	IntrusivePtr<Val> result;
 
@@ -1884,26 +1884,26 @@ IntrusivePtr<Val> TableVal::Default(Val* index)
 	return result;
 	}
 
-IntrusivePtr<Val> TableVal::Lookup(Val* index, bool use_default_val)
+const IntrusivePtr<Val>& TableVal::Find(const IntrusivePtr<Val>& index)
 	{
+	static IntrusivePtr<Val> nil;
+	static IntrusivePtr<Val> exists = val_mgr->True();
+
 	if ( subnets )
 		{
-		TableEntryVal* v = (TableEntryVal*) subnets->Lookup(index);
+		TableEntryVal* v = (TableEntryVal*) subnets->Lookup(index.get());
 		if ( v )
 			{
 			if ( attrs && attrs->FindAttr(ATTR_EXPIRE_READ) )
-					v->SetExpireAccess(network_time);
+				v->SetExpireAccess(network_time);
 
 			if ( v->GetVal() )
 				return v->GetVal();
 
-			return {NewRef{}, this};
+			return exists;
 			}
 
-		if ( ! use_default_val )
+		return nil;
-			return nullptr;
-
-		return Default(index);
 		}
 
 	const PDict<TableEntryVal>* tbl = AsTable();
@@ -1924,15 +1924,36 @@ IntrusivePtr<Val> TableVal::Lookup(Val* index, bool use_default_val)
 				if ( v->GetVal() )
 					return v->GetVal();
 
-				return {NewRef{}, this};
+				return exists;
 				}
 			}
 		}
 
+	return nil;
+	}
+
+IntrusivePtr<Val> TableVal::FindOrDefault(const IntrusivePtr<Val>& index)
+	{
+	if ( auto rval = Find(index) )
+		return rval;
+
+	return Default(index);
+	}
+
+Val* TableVal::Lookup(Val* index, bool use_default_val)
+	{
+	static IntrusivePtr<Val> last_default;
+	last_default = nullptr;
+	IntrusivePtr<Val> idx{NewRef{}, index};
+
+	if ( const auto& rval = Find(idx) )
+		return rval.get();
+
 	if ( ! use_default_val )
 		return nullptr;
 
-	return Default(index);
+	last_default = Default(idx);
+	return last_default.get();
 	}
 
 IntrusivePtr<VectorVal> TableVal::LookupSubnets(const SubNetVal* search)
@@ -2298,7 +2319,7 @@ bool TableVal::CheckAndAssign(IntrusivePtr<Val> index, IntrusivePtr<Val> new_val
 		// We need an exact match here.
 		v = (Val*) subnets->Lookup(index.get(), true);
 	else
-		v = Lookup(index.get(), false).get();
+		v = Find(index).get();
 
 	if ( v )
 		index->Warn("multiple initializations for index");

src/Val.h

@@ -831,10 +831,32 @@ public:
 	// Returns true if the initializations typecheck, false if not.
 	bool ExpandAndInit(IntrusivePtr<Val> index, IntrusivePtr<Val> new_val);
 
+	/**
+	 * Finds an index in the table and returns its associated value.
+	 * @param index  The index to lookup in the table.
+	 * @return  The value associated with the index.  If the index doesn't
+	 * exist, this is a nullptr.  For sets that don't really contain associated
+	 * values, a placeholder value is returned to differentiate it from
+	 * non-existent index (nullptr), but otherwise has no meaning in relation
+	 * to the set's contents.
+	 */
+	const IntrusivePtr<Val>& Find(const IntrusivePtr<Val>& index);
+
+	/**
+	 * Finds an index in the table and returns its associated value or else
+	 * the &default value.
+	 * @param index  The index to lookup in the table.
+	 * @return  The value associated with the index.  If the index doesn't
+	 * exist, instead returns the &default value.  If there's no &default
+	 * attribute, then nullptr is still returned for non-existent index.
+	 */
+	IntrusivePtr<Val> FindOrDefault(const IntrusivePtr<Val>& index);
+
 	// Returns the element's value if it exists in the table,
 	// nil otherwise.  Note, "index" is not const because we
 	// need to Ref/Unref it when calling the default function.
-	IntrusivePtr<Val> Lookup(Val* index, bool use_default_val = true);
+	[[deprecated("Remove in v4.1.  Use Find() or FindOrDefault().")]]
+	Val* Lookup(Val* index, bool use_default_val = true);
 
 	// For a table[subnet]/set[subnet], return all subnets that cover
 	// the given subnet.
@@ -936,8 +958,8 @@ protected:
 	bool ExpandCompoundAndInit(ListVal* lv, int k, IntrusivePtr<Val> new_val);
 	bool CheckAndAssign(IntrusivePtr<Val> index, IntrusivePtr<Val> new_val);
 
-	// Calculates default value for index.  Returns 0 if none.
+	// Calculates default value for index.  Returns nullptr if none.
-	IntrusivePtr<Val> Default(Val* index);
+	IntrusivePtr<Val> Default(const IntrusivePtr<Val>& index);
 
 	// Returns true if item expiration is enabled.
 	bool ExpirationEnabled()	{ return expire_time != nullptr; }

src/analyzer/Manager.cc

@@ -443,10 +443,10 @@ bool Manager::BuildInitialAnalyzerTree(Connection* conn)
 			const auto& dport = val_mgr->Port(ntohs(conn->RespPort()), TRANSPORT_TCP);
 
 			if ( ! reass )
-				reass = (bool)tcp_content_delivery_ports_orig->Lookup(dport.get());
+				reass = (bool)tcp_content_delivery_ports_orig->FindOrDefault(dport);
 
 			if ( ! reass )
-				reass = (bool)tcp_content_delivery_ports_resp->Lookup(dport.get());
+				reass = (bool)tcp_content_delivery_ports_resp->FindOrDefault(dport);
 			}
 
 		if ( reass )
@@ -463,9 +463,9 @@ bool Manager::BuildInitialAnalyzerTree(Connection* conn)
 			if ( resp_port == 22 || resp_port == 23 || resp_port == 513 )
 				{
 				static auto stp_skip_src = zeek::id::find_val<TableVal>("stp_skip_src");
+				auto src = make_intrusive<AddrVal>(conn->OrigAddr());
 
-				AddrVal src(conn->OrigAddr());
+				if ( ! stp_skip_src->FindOrDefault(src) )
-				if ( ! stp_skip_src->Lookup(&src) )
 					tcp->AddChildAnalyzer(new stepping_stone::SteppingStone_Analyzer(conn), false);
 				}
 			}

src/analyzer/protocol/dns/DNS.cc

@@ -85,19 +85,18 @@ void DNS_Interpreter::ParseMessage(const u_char* data, int len, int is_query)
 
 	analyzer->ProtocolConfirmation();
 
-	AddrVal server(analyzer->Conn()->RespAddr());
-
 	int skip_auth = dns_skip_all_auth;
 	int skip_addl = dns_skip_all_addl;
 	if ( msg.ancount > 0 )
 		{ // We did an answer, so can potentially skip auth/addl.
 		static auto dns_skip_auth = zeek::id::find_val<TableVal>("dns_skip_auth");
 		static auto dns_skip_addl = zeek::id::find_val<TableVal>("dns_skip_addl");
+		auto server = make_intrusive<AddrVal>(analyzer->Conn()->RespAddr());
 
 		skip_auth = skip_auth || msg.nscount == 0 ||
-				dns_skip_auth->Lookup(&server);
+				dns_skip_auth->FindOrDefault(server);
 		skip_addl = skip_addl || msg.arcount == 0 ||
-				dns_skip_addl->Lookup(&server);
+				dns_skip_addl->FindOrDefault(server);
 		}
 
 	if ( skip_auth && skip_addl )

src/analyzer/protocol/radius/radius-analyzer.pac

@@ -21,7 +21,7 @@ refine flow RADIUS_Flow += {
 				auto index = val_mgr->Count(${msg.attributes[i].code});
 
 				// Do we already have a vector of attributes for this type?
-				auto current = attributes->Lookup(index.get());
+				auto current = attributes->FindOrDefault(index);
 				IntrusivePtr<Val> val = to_stringval(${msg.attributes[i].value});
 
 				if ( current )

src/analyzer/protocol/tcp/TCP_Reassembler.cc

@@ -48,7 +48,7 @@ TCP_Reassembler::TCP_Reassembler(analyzer::Analyzer* arg_dst_analyzer,
 		const auto& ports = IsOrig() ?
 			tcp_content_delivery_ports_orig :
 			tcp_content_delivery_ports_resp;
-		auto result = ports->Lookup(dst_port_val.get());
+		auto result = ports->FindOrDefault(dst_port_val);
 
 		if ( (IsOrig() && tcp_content_deliver_all_orig) ||
 		     (! IsOrig() && tcp_content_deliver_all_resp) ||

src/analyzer/protocol/udp/UDP.cc

@@ -141,8 +141,8 @@ void UDP_Analyzer::DeliverPacket(int len, const u_char* data, bool is_orig,
 		const auto& sport_val = val_mgr->Port(ntohs(up->uh_sport), TRANSPORT_UDP);
 		const auto& dport_val = val_mgr->Port(ntohs(up->uh_dport), TRANSPORT_UDP);
 
-		if ( udp_content_ports->Lookup(dport_val.get()) ||
+		if ( udp_content_ports->FindOrDefault(dport_val) ||
-		     udp_content_ports->Lookup(sport_val.get()) )
+		     udp_content_ports->FindOrDefault(sport_val) )
 			do_udp_contents = true;
 		else
 			{
@@ -152,14 +152,14 @@ void UDP_Analyzer::DeliverPacket(int len, const u_char* data, bool is_orig,
 
 			if ( is_orig )
 				{
-				auto result = udp_content_delivery_ports_orig->Lookup(port_val.get());
+				auto result = udp_content_delivery_ports_orig->FindOrDefault(port_val);
 
 				if ( udp_content_deliver_all_orig || (result && result->AsBool()) )
 					do_udp_contents = true;
 				}
 			else
 				{
-				auto result = udp_content_delivery_ports_resp->Lookup(port_val.get());
+				auto result = udp_content_delivery_ports_resp->FindOrDefault(port_val);
 
 				if ( udp_content_deliver_all_resp || (result && result->AsBool()) )
 					do_udp_contents = true;

src/file_analysis/File.cc

@@ -140,7 +140,7 @@ bool File::UpdateConnectionFields(Connection* conn, bool is_orig)
 
 	auto idx = get_conn_id_val(conn);
 
-	if ( conns->AsTableVal()->Lookup(idx.get()) )
+	if ( conns->AsTableVal()->FindOrDefault(idx) )
 		return false;
 
 	conns->AsTableVal()->Assign(std::move(idx), conn->ConnVal());

src/file_analysis/Manager.cc

@@ -434,7 +434,7 @@ bool Manager::IsDisabled(const analyzer::Tag& tag)
 		disabled = zeek::id::find_const("Files::disable")->AsTableVal();
 
 	auto index = val_mgr->Count(bool(tag));
-	auto yield = disabled->Lookup(index.get());
+	auto yield = disabled->FindOrDefault(index);
 
 	if ( ! yield )
 		return false;

src/file_analysis/analyzer/x509/X509.cc

@@ -51,7 +51,8 @@ bool file_analysis::X509::EndOfFile()
 		hash_final(ctx, buf);
 		std::string cert_sha256 = sha256_digest_print(buf);
 		auto index = make_intrusive<StringVal>(cert_sha256);
-		auto entry = certificate_cache->Lookup(index.get(), false);
+		const auto& entry = certificate_cache->Find(index);
+
 		if ( entry )
 			// in this case, the certificate is in the cache and we do not
 			// do any further processing here. However, if there is a callback, we execute it.
@@ -61,8 +62,7 @@ bool file_analysis::X509::EndOfFile()
 			// yup, let's call the callback.
 
 			cache_hit_callback->Call(IntrusivePtr{NewRef{}, GetFile()->GetVal()},
-			                         std::move(entry),
+			                         entry, make_intrusive<StringVal>(cert_sha256));
-			                         make_intrusive<StringVal>(cert_sha256));
 			return false;
 			}
 		}
@@ -250,7 +250,8 @@ X509_STORE* file_analysis::X509::GetRootStore(TableVal* root_certs)
 	for ( int i = 0; i < idxs->Length(); ++i )
 		{
 		const auto& key = idxs->Idx(i);
-		StringVal *sv = root_certs->Lookup(key.get())->AsStringVal();
+		auto val = root_certs->FindOrDefault(key);
+		StringVal* sv = val->AsStringVal();
 		assert(sv);
 		const uint8_t* data = sv->Bytes();
 		::X509* x = d2i_X509(NULL, &data, sv->Len());

src/input/Manager.cc

@@ -1245,7 +1245,7 @@ int Manager::SendEntryTable(Stream* i, const Value* const *vals)
 		{
 		assert(stream->num_val_fields > 0);
 		// in that case, we need the old value to send the event (if we send an event).
-		oldval = stream->tab->Lookup(idxval, false);
+		oldval = stream->tab->Find({NewRef{}, idxval});
 		}
 
 	HashKey* k = stream->tab->ComputeHash(*idxval);
@@ -1344,7 +1344,7 @@ void Manager::EndCurrentSend(ReaderFrontend* reader)
 			{
 			auto idx = stream->tab->RecoverIndex(ih->idxkey);
 			assert(idx != nullptr);
-			val = stream->tab->Lookup(idx.get());
+			val = stream->tab->FindOrDefault(idx);
 			assert(val != nullptr);
 			predidx = {AdoptRef{}, ListValToRecordVal(idx.get(), stream->itype, &startpos)};
 			ev = zeek::BifType::Enum::Input::Event->GetVal(BifEnum::Input::EVENT_REMOVED);
@@ -1555,7 +1555,7 @@ int Manager::PutTable(Stream* i, const Value* const *vals)
 		if ( stream->num_val_fields > 0 )
 			{
 			// in that case, we need the old value to send the event (if we send an event).
-			oldval = stream->tab->Lookup(idxval, false);
+			oldval = stream->tab->Find({NewRef{}, idxval});
 			}
 
 		if ( oldval != nullptr )
@@ -1699,7 +1699,7 @@ bool Manager::Delete(ReaderFrontend* reader, Value* *vals)
 
 		if ( stream->pred || stream->event )
 			{
-			auto val = stream->tab->Lookup(idxval);
+			auto val = stream->tab->FindOrDefault({NewRef{}, idxval});
 
 			if ( stream->pred )
 				{

src/logging/Manager.cc

@@ -477,10 +477,8 @@ bool Manager::TraverseRecord(Stream* stream, Filter* filter, RecordType* rt,
 		// If include fields are specified, only include if explicitly listed.
 		if ( include )
 			{
-			StringVal* new_path_val = new StringVal(new_path.c_str());
+			auto new_path_val = make_intrusive<StringVal>(new_path.c_str());
-			bool result = (bool)include->Lookup(new_path_val);
+			bool result = (bool)include->FindOrDefault(new_path_val);
-
-			Unref(new_path_val);
 
 			if ( ! result )
 				continue;
@@ -489,10 +487,8 @@ bool Manager::TraverseRecord(Stream* stream, Filter* filter, RecordType* rt,
 		// If exclude fields are specified, do not only include if listed.
 		if ( exclude )
 			{
-			StringVal* new_path_val = new StringVal(new_path.c_str());
+			auto new_path_val = make_intrusive<StringVal>(new_path.c_str());
-			bool result = (bool)exclude->Lookup(new_path_val);
+			bool result = (bool)exclude->FindOrDefault(new_path_val);
-
-			Unref(new_path_val);
 
 			if ( result )
 				continue;
@@ -848,13 +844,13 @@ bool Manager::Write(EnumVal* id, RecordVal* columns_arg)
 				if ( filter->field_name_map )
 					{
 					const char* name = filter->fields[j]->name;
-					StringVal *fn = new StringVal(name);
+					auto fn = make_intrusive<StringVal>(name);
-					if ( auto val = filter->field_name_map->Lookup(fn, false) )
+
+					if ( const auto& val = filter->field_name_map->Find(fn) )
 						{
 						delete [] filter->fields[j]->name;
 						filter->fields[j]->name = copy_string(val->AsStringVal()->CheckString());
 						}
-					delete fn;
 					}
 				arg_fields[j] = new threading::Field(*filter->fields[j]);
 				}

src/zeek.bif

@@ -406,7 +406,7 @@ static bool prepare_environment(TableVal* tbl, bool set)
 	for ( int i = 0; i < idxs->Length(); ++i )
 		{
 		const auto& key = idxs->Idx(i);
-		auto val = tbl->Lookup(key.get(), false);
+		const auto& val = tbl->Find(key);
 
 		if ( key->GetType()->Tag() != TYPE_STRING ||
 		     val->GetType()->Tag() != TYPE_STRING )