From 8f95a2a0bb97c141e65812ab7a3b01bcd8a99c88 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Tue, 12 May 2020 22:20:42 -0700 Subject: [PATCH] Deprecate Scope::Lookup(), replace with Scope::Find() --- NEWS | 2 ++ src/Anon.cc | 6 +++--- src/Expr.cc | 2 +- src/ID.cc | 15 +++++---------- src/ID.h | 2 +- src/Net.cc | 2 +- src/OpaqueVal.cc | 2 +- src/RuleCondition.cc | 2 +- src/Scope.cc | 11 +++++------ src/Scope.h | 14 +++++++++++--- src/Type.cc | 2 +- src/Val.cc | 2 +- src/Var.cc | 2 +- src/analyzer/Manager.cc | 2 +- src/analyzer/protocol/mqtt/MQTT.cc | 10 ---------- src/analyzer/protocol/mqtt/MQTT.h | 2 -- src/analyzer/protocol/mqtt/commands/publish.pac | 3 ++- src/broker/Data.cc | 4 ++-- src/broker/Manager.cc | 6 +++--- src/broker/messaging.bif | 4 ++-- src/input/Manager.cc | 2 +- src/logging/Manager.cc | 4 ++-- src/option.bif | 6 +++--- src/supervisor/Supervisor.cc | 8 ++++---- src/zeek-setup.cc | 10 +++++----- src/zeek.bif | 4 ++-- src/zeekygen/ScriptInfo.cc | 8 ++++---- 27 files changed, 65 insertions(+), 72 deletions(-) diff --git a/NEWS b/NEWS index 1178de57a9..9ccc59bc80 100644 --- a/NEWS +++ b/NEWS @@ -174,6 +174,8 @@ Deprecated Functionality - Most global type/value pointers in NetVar.h are deprecated, but one can still always perform the lookup themselves. +- ``Scope::Lookup()`` is deprecated, use ``Scope::Find()``. + Zeek 3.1.0 ========== diff --git a/src/Anon.cc b/src/Anon.cc index 96b8d5473c..d129d0663a 100644 --- a/src/Anon.cc +++ b/src/Anon.cc @@ -368,17 +368,17 @@ void init_ip_addr_anonymizers() ip_anonymizer[PREFIX_PRESERVING_A50] = new AnonymizeIPAddr_A50(); ip_anonymizer[PREFIX_PRESERVING_MD5] = new AnonymizeIPAddr_PrefixMD5(); - auto id = global_scope()->Lookup("preserve_orig_addr"); + auto id = global_scope()->Find("preserve_orig_addr"); if ( id ) anon_preserve_orig_addr = cast_intrusive(id->GetVal()); - id = global_scope()->Lookup("preserve_resp_addr"); + id = global_scope()->Find("preserve_resp_addr"); if ( id ) anon_preserve_resp_addr = cast_intrusive(id->GetVal()); - id = global_scope()->Lookup("preserve_other_addr"); + id = global_scope()->Find("preserve_other_addr"); if ( id ) anon_preserve_other_addr = cast_intrusive(id->GetVal()); diff --git a/src/Expr.cc b/src/Expr.cc index 9a8bec21e1..8e70088fc6 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -4227,7 +4227,7 @@ LambdaExpr::LambdaExpr(std::unique_ptr arg_ing, my_name = "lambda_<" + std::to_string(h[0]) + ">"; auto fullname = make_full_var_name(current_module.data(), my_name.data()); - auto id = global_scope()->Lookup(fullname); + const auto& id = global_scope()->Find(fullname); if ( id ) // Just try again to make a unique lambda name. If two peer diff --git a/src/ID.cc b/src/ID.cc index 3184aa2aef..bf8972f324 100644 --- a/src/ID.cc +++ b/src/ID.cc @@ -31,19 +31,14 @@ IntrusivePtr zeek::id::count_set; IntrusivePtr zeek::id::string_vec; IntrusivePtr zeek::id::index_vec; -IntrusivePtr zeek::id::lookup(const char* name) +const IntrusivePtr& zeek::id::lookup(const char* name) { - auto id = global_scope()->Lookup(name); - - if ( ! id ) - return nullptr; - - return {NewRef{}, id}; + return global_scope()->Find(name); } const IntrusivePtr& zeek::id::lookup_type(const char* name) { - auto id = zeek::id::lookup(name); + auto id = global_scope()->Find(name); if ( ! id ) reporter->InternalError("Failed to find type named: %s", name); @@ -53,7 +48,7 @@ const IntrusivePtr& zeek::id::lookup_type(const char* name) const IntrusivePtr& zeek::id::lookup_val(const char* name) { - auto id = zeek::id::lookup(name); + auto id = global_scope()->Find(name); if ( ! id ) reporter->InternalError("Failed to find variable named: %s", name); @@ -63,7 +58,7 @@ const IntrusivePtr& zeek::id::lookup_val(const char* name) const IntrusivePtr& zeek::id::lookup_const(const char* name) { - auto id = zeek::id::lookup(name); + auto id = global_scope()->Find(name); if ( ! id ) reporter->InternalError("Failed to find variable named: %s", name); diff --git a/src/ID.h b/src/ID.h index 7965fc3c6d..d80be7f7c9 100644 --- a/src/ID.h +++ b/src/ID.h @@ -165,7 +165,7 @@ namespace zeek { namespace id { * @return The identifier, which may reference a nil object if no such * name exists. */ -IntrusivePtr lookup(const char* name); +const IntrusivePtr& lookup(const char* name); /** * Lookup an ID by its name and return its type. A fatal occurs if the ID diff --git a/src/Net.cc b/src/Net.cc index faac9e569a..9be8191000 100644 --- a/src/Net.cc +++ b/src/Net.cc @@ -193,7 +193,7 @@ void net_init(const std::optional& interface, reporter->FatalError("problem opening dump file %s (%s)", writefile, pkt_dumper->ErrorMsg()); - if ( ID* id = global_scope()->Lookup("trace_output_file") ) + if ( const auto& id = global_scope()->Find("trace_output_file") ) id->SetVal(make_intrusive(writefile)); else reporter->Error("trace_output_file not defined in bro.init"); diff --git a/src/OpaqueVal.cc b/src/OpaqueVal.cc index dd6f75620c..5f561ae9f5 100644 --- a/src/OpaqueVal.cc +++ b/src/OpaqueVal.cc @@ -126,7 +126,7 @@ BroType* OpaqueVal::UnserializeType(const broker::data& data) if ( ! name ) return nullptr; - ID* id = global_scope()->Lookup(*name); + const auto& id = global_scope()->Find(*name); if ( ! id ) return nullptr; diff --git a/src/RuleCondition.cc b/src/RuleCondition.cc index b7c43ccab8..9cb335e8e0 100644 --- a/src/RuleCondition.cc +++ b/src/RuleCondition.cc @@ -129,7 +129,7 @@ bool RuleConditionPayloadSize::DoMatch(Rule* rule, RuleEndpointState* state, RuleConditionEval::RuleConditionEval(const char* func) { - id = global_scope()->Lookup(func); + id = global_scope()->Find(func).get(); if ( ! id ) { rules_error("unknown identifier", func); diff --git a/src/Scope.cc b/src/Scope.cc index 79b7f46ced..5138361aed 100644 --- a/src/Scope.cc +++ b/src/Scope.cc @@ -129,24 +129,23 @@ IntrusivePtr lookup_ID(const char* name, const char* curr_module, for ( int i = scopes.length() - 1; i >= 0; --i ) { - ID* id = scopes[i]->Lookup(fullname); + const auto& id = scopes[i]->Find(fullname); + if ( id ) { if ( need_export && ! id->IsExport() && ! in_debug ) reporter->Error("identifier is not exported: %s", fullname.c_str()); - return {NewRef{}, id}; + return id; } } if ( ! no_global && (strcmp(GLOBAL_MODULE_NAME, curr_module) == 0 || - ! same_module_only) ) + ! same_module_only) ) { std::string globalname = make_full_var_name(GLOBAL_MODULE_NAME, name); - ID* id = global_scope()->Lookup(globalname); - if ( id ) - return {NewRef{}, id}; + return global_scope()->Find(globalname); } return nullptr; diff --git a/src/Scope.h b/src/Scope.h index 9fec692e8c..6687be3bfd 100644 --- a/src/Scope.h +++ b/src/Scope.h @@ -22,14 +22,22 @@ public: ~Scope() override; template - ID* Lookup(N&& name) const + const IntrusivePtr& Find(N&& name) const { + static IntrusivePtr nil; const auto& entry = local.find(std::forward(name)); if ( entry != local.end() ) - return entry->second.get(); + return entry->second; - return nullptr; + return nil; + } + + template + [[deprecated("Remove in v4.1. Use Find().")]] + ID* Lookup(N&& name) const + { + return Find(name).get(); } template diff --git a/src/Type.cc b/src/Type.cc index 9dfe0ded36..c7fd3cdbaa 100644 --- a/src/Type.cc +++ b/src/Type.cc @@ -1766,7 +1766,7 @@ IntrusivePtr merge_types(const BroType* t1, const BroType* t2) // Doing a lookup here as a roundabout way of ref-ing t1, without // changing the function params which has t1 as const and also // (potentially) avoiding a pitfall mentioned earlier about clones. - auto id = global_scope()->Lookup(t1->GetName()); + const auto& id = global_scope()->Find(t1->GetName()); if ( id && id->IsType() && id->GetType()->Tag() == TYPE_ENUM ) // It should make most sense to return the real type here rather diff --git a/src/Val.cc b/src/Val.cc index fd729116b8..7b5962f403 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -371,7 +371,7 @@ void Val::ValDescribeReST(ODesc* d) const #ifdef DEBUG ID* Val::GetID() const { - return bound_id ? global_scope()->Lookup(bound_id) : nullptr; + return bound_id ? global_scope()->Find(bound_id).get() : nullptr; } void Val::SetID(ID* id) diff --git a/src/Var.cc b/src/Var.cc index 01b5329edb..13cc3c7ee7 100644 --- a/src/Var.cc +++ b/src/Var.cc @@ -609,7 +609,7 @@ TraversalCode OuterIDBindingFinder::PreExpr(const Expr* expr) return TC_CONTINUE; for ( const auto& scope : scopes ) - if ( scope->Lookup(e->Id()->Name()) ) + if ( scope->Find(e->Id()->Name()) ) // Shadowing is not allowed, so if it's found at inner scope, it's // not something we have to worry about also being at outer scope. return TC_CONTINUE; diff --git a/src/analyzer/Manager.cc b/src/analyzer/Manager.cc index 3ef7b211f0..9e339280da 100644 --- a/src/analyzer/Manager.cc +++ b/src/analyzer/Manager.cc @@ -90,7 +90,7 @@ void Manager::InitPreScript() void Manager::InitPostScript() { - auto id = global_scope()->Lookup("Tunnel::vxlan_ports"); + const auto& id = global_scope()->Find("Tunnel::vxlan_ports"); if ( ! (id && id->GetVal()) ) reporter->FatalError("Tunnel::vxlan_ports not defined"); diff --git a/src/analyzer/protocol/mqtt/MQTT.cc b/src/analyzer/protocol/mqtt/MQTT.cc index 7addafb3f9..4514f4d907 100644 --- a/src/analyzer/protocol/mqtt/MQTT.cc +++ b/src/analyzer/protocol/mqtt/MQTT.cc @@ -9,20 +9,10 @@ using namespace analyzer::MQTT; -const ::ID* MQTT_Analyzer::max_payload_size = nullptr; - MQTT_Analyzer::MQTT_Analyzer(Connection* c) : tcp::TCP_ApplicationAnalyzer("MQTT", c) { interp = new binpac::MQTT::MQTT_Conn(this); - - if ( ! max_payload_size ) - { - max_payload_size = global_scope()->Lookup("MQTT::max_payload_size"); - - if ( ! max_payload_size ) - reporter->FatalError("option not defined: 'MQTT::max_payload_size'"); - } } MQTT_Analyzer::~MQTT_Analyzer() diff --git a/src/analyzer/protocol/mqtt/MQTT.h b/src/analyzer/protocol/mqtt/MQTT.h index 5997d3430f..a1c4e36a2d 100644 --- a/src/analyzer/protocol/mqtt/MQTT.h +++ b/src/analyzer/protocol/mqtt/MQTT.h @@ -23,8 +23,6 @@ public: static analyzer::Analyzer* InstantiateAnalyzer(Connection* conn) { return new MQTT_Analyzer(conn); } - static const ::ID* max_payload_size; - protected: binpac::MQTT::MQTT_Conn* interp; diff --git a/src/analyzer/protocol/mqtt/commands/publish.pac b/src/analyzer/protocol/mqtt/commands/publish.pac index 029f52b899..72bce23c65 100644 --- a/src/analyzer/protocol/mqtt/commands/publish.pac +++ b/src/analyzer/protocol/mqtt/commands/publish.pac @@ -31,7 +31,8 @@ refine flow MQTT_Flow += { reinterpret_cast(${msg.topic.str}.begin()))); auto len = ${msg.payload}.length(); - auto max = analyzer::MQTT::MQTT_Analyzer::max_payload_size->GetVal()->AsCount(); + static auto max_payload_size = zeek::id::lookup("MQTT::max_payload_size"); + auto max = max_payload_size->GetVal()->AsCount(); if ( len > static_cast(max) ) len = max; diff --git a/src/broker/Data.cc b/src/broker/Data.cc index 39a84027dd..24f6d55822 100644 --- a/src/broker/Data.cc +++ b/src/broker/Data.cc @@ -353,7 +353,7 @@ struct val_converter { if ( ! name ) return nullptr; - auto id = global_scope()->Lookup(*name); + const auto& id = global_scope()->Find(*name); if ( ! id ) return nullptr; @@ -697,7 +697,7 @@ struct type_checker { if ( ! name ) return false; - auto id = global_scope()->Lookup(*name); + const auto& id = global_scope()->Find(*name); if ( ! id ) return false; diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index f52e10259c..9d7cc5f324 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -29,7 +29,7 @@ namespace bro_broker { static inline Val* get_option(const char* option) { - auto id = global_scope()->Lookup(option); + const auto& id = global_scope()->Find(option); if ( ! (id && id->GetVal()) ) reporter->FatalError("Unknown Broker option %s", option); @@ -412,7 +412,7 @@ bool Manager::PublishIdentifier(std::string topic, std::string id) if ( peer_count == 0 ) return true; - ID* i = global_scope()->Lookup(id); + const auto& i = global_scope()->Find(id); if ( ! i ) return false; @@ -1186,7 +1186,7 @@ bool Manager::ProcessIdentifierUpdate(broker::zeek::IdentifierUpdate iu) ++statistics.num_ids_incoming; auto id_name = std::move(iu.id_name()); auto id_value = std::move(iu.id_value()); - auto id = global_scope()->Lookup(id_name); + const auto& id = global_scope()->Find(id_name); if ( ! id ) { diff --git a/src/broker/messaging.bif b/src/broker/messaging.bif index fc5cfd3f31..0189c07c82 100644 --- a/src/broker/messaging.bif +++ b/src/broker/messaging.bif @@ -185,7 +185,7 @@ function Cluster::publish_rr%(pool: Pool, key: string, ...%): bool static Func* topic_func = 0; if ( ! topic_func ) - topic_func = global_scope()->Lookup("Cluster::rr_topic")->GetVal()->AsFunc(); + topic_func = global_scope()->Find("Cluster::rr_topic")->GetVal()->AsFunc(); zeek::Args vl{{NewRef{}, pool}, {NewRef{}, key}}; auto topic = topic_func->Call(vl); @@ -222,7 +222,7 @@ function Cluster::publish_hrw%(pool: Pool, key: any, ...%): bool static Func* topic_func = 0; if ( ! topic_func ) - topic_func = global_scope()->Lookup("Cluster::hrw_topic")->GetVal()->AsFunc(); + topic_func = global_scope()->Find("Cluster::hrw_topic")->GetVal()->AsFunc(); zeek::Args vl{{NewRef{}, pool}, {NewRef{}, key}}; auto topic = topic_func->Call(vl); diff --git a/src/input/Manager.cc b/src/input/Manager.cc index 23931b6f56..7034225a48 100644 --- a/src/input/Manager.cc +++ b/src/input/Manager.cc @@ -2498,7 +2498,7 @@ Val* Manager::ValueToVal(const Stream* i, const Value* val, bool& have_error) co // Enums are not a base-type, so need to look it up. const auto& sv = val->val.set_val.vals[0]->val.string_val; std::string enum_name(sv.data, sv.length); - auto enum_id = global_scope()->Lookup(enum_name); + const auto& enum_id = global_scope()->Find(enum_name); if ( ! enum_id ) { diff --git a/src/logging/Manager.cc b/src/logging/Manager.cc index eda63641bc..1c4ff43653 100644 --- a/src/logging/Manager.cc +++ b/src/logging/Manager.cc @@ -1182,7 +1182,7 @@ WriterFrontend* Manager::CreateWriter(EnumVal* id, EnumVal* writer, WriterBacken if ( ! found_filter_match ) { - ID* id = global_scope()->Lookup("Log::default_rotation_interval"); + const auto& id = global_scope()->Find("Log::default_rotation_interval"); assert(id); winfo->interval = id->GetVal()->AsInterval(); } @@ -1525,7 +1525,7 @@ bool Manager::FinishedRotation(WriterFrontend* writer, const char* new_name, con Func* func = winfo->postprocessor; if ( ! func ) { - ID* id = global_scope()->Lookup("Log::__default_rotation_postprocessor"); + const auto& id = global_scope()->Find("Log::__default_rotation_postprocessor"); assert(id); func = id->GetVal()->AsFunc(); } diff --git a/src/option.bif b/src/option.bif index 353dc414c1..352ffb9631 100644 --- a/src/option.bif +++ b/src/option.bif @@ -7,7 +7,7 @@ module Option; #include "NetVar.h" #include "broker/Data.h" -static bool call_option_handlers_and_set_value(StringVal* name, ID* i, +static bool call_option_handlers_and_set_value(StringVal* name, const IntrusivePtr& i, IntrusivePtr val, StringVal* location) { @@ -59,7 +59,7 @@ static bool call_option_handlers_and_set_value(StringVal* name, ID* i, ## lower-level function. function Option::set%(ID: string, val: any, location: string &default=""%): bool %{ - auto i = global_scope()->Lookup(ID->CheckString()); + const auto& i = global_scope()->Find(ID->CheckString()); if ( ! i ) { builtin_error(fmt("Could not find ID named '%s'", ID->CheckString())); @@ -144,7 +144,7 @@ function Option::set%(ID: string, val: any, location: string &default=""%): bool ## .. zeek:see:: Option::set function Option::set_change_handler%(ID: string, on_change: any, priority: int &default=0%): bool %{ - auto i = global_scope()->Lookup(ID->CheckString()); + const auto& i = global_scope()->Find(ID->CheckString()); if ( ! i ) { builtin_error(fmt("Could not find ID named '%s'", ID->CheckString())); diff --git a/src/supervisor/Supervisor.cc b/src/supervisor/Supervisor.cc index 0a7e588112..bd52f241b0 100644 --- a/src/supervisor/Supervisor.cc +++ b/src/supervisor/Supervisor.cc @@ -1172,7 +1172,7 @@ IntrusivePtr Supervisor::Node::ToRecord() const static IntrusivePtr supervisor_role_to_cluster_node_type(BifEnum::Supervisor::ClusterRole role) { - static auto node_type = global_scope()->Lookup("Cluster::NodeType")->GetType()->AsEnumType(); + static auto node_type = global_scope()->Find("Cluster::NodeType")->GetType()->AsEnumType(); switch ( role ) { case BifEnum::Supervisor::LOGGER: @@ -1193,9 +1193,9 @@ bool Supervisor::SupervisedNode::InitCluster() const if ( config.cluster.empty() ) return false; - auto cluster_node_type = global_scope()->Lookup("Cluster::Node")->GetType()->AsRecordType(); - auto cluster_nodes_id = global_scope()->Lookup("Cluster::nodes"); - auto cluster_manager_is_logger_id = global_scope()->Lookup("Cluster::manager_is_logger"); + const auto& cluster_node_type = global_scope()->Find("Cluster::Node")->GetType()->AsRecordType(); + const auto& cluster_nodes_id = global_scope()->Find("Cluster::nodes"); + const auto& cluster_manager_is_logger_id = global_scope()->Find("Cluster::manager_is_logger"); auto cluster_nodes = cluster_nodes_id->GetVal()->AsTableVal(); auto has_logger = false; std::optional manager_name; diff --git a/src/zeek-setup.cc b/src/zeek-setup.cc index e6e7ebc3d8..30d0486a6f 100644 --- a/src/zeek-setup.cc +++ b/src/zeek-setup.cc @@ -630,11 +630,11 @@ zeek::detail::SetupResult zeek::detail::setup(int argc, char** argv, // Must come after plugin activation (and also after hash // initialization). binpac::FlowBuffer::Policy flowbuffer_policy; - flowbuffer_policy.max_capacity = global_scope()->Lookup( + flowbuffer_policy.max_capacity = global_scope()->Find( "BinPAC::flowbuffer_capacity_max")->GetVal()->AsCount(); - flowbuffer_policy.min_capacity = global_scope()->Lookup( + flowbuffer_policy.min_capacity = global_scope()->Find( "BinPAC::flowbuffer_capacity_min")->GetVal()->AsCount(); - flowbuffer_policy.contract_threshold = global_scope()->Lookup( + flowbuffer_policy.contract_threshold = global_scope()->Find( "BinPAC::flowbuffer_contract_threshold")->GetVal()->AsCount(); binpac::init(&flowbuffer_policy); @@ -685,7 +685,7 @@ zeek::detail::SetupResult zeek::detail::setup(int argc, char** argv, if ( options.pcap_filter ) { - ID* id = global_scope()->Lookup("cmd_line_bpf_filter"); + const auto& id = global_scope()->Find("cmd_line_bpf_filter"); if ( ! id ) reporter->InternalError("global cmd_line_bpf_filter not defined"); @@ -769,7 +769,7 @@ zeek::detail::SetupResult zeek::detail::setup(int argc, char** argv, // Print the ID. if ( options.identifier_to_print ) { - ID* id = global_scope()->Lookup(*options.identifier_to_print); + const auto& id = global_scope()->Find(*options.identifier_to_print); if ( ! id ) reporter->FatalError("No such ID: %s\n", options.identifier_to_print->data()); diff --git a/src/zeek.bif b/src/zeek.bif index ecfd522bc8..cd202df1d7 100644 --- a/src/zeek.bif +++ b/src/zeek.bif @@ -1975,7 +1975,7 @@ function global_ids%(%): id_table ## the string ``""`` or ``""`` is returned. function lookup_ID%(id: string%) : any %{ - ID* i = global_scope()->Lookup(id->CheckString()); + const auto& i = global_scope()->Find(id->CheckString()); if ( ! i ) return make_intrusive(""); @@ -1998,7 +1998,7 @@ function record_fields%(rec: any%): record_field_table if ( rec->GetType()->Tag() == TYPE_STRING ) { - auto id = global_scope()->Lookup(rec->AsStringVal()->ToStdString()); + const auto& id = global_scope()->Find(rec->AsStringVal()->ToStdString()); if ( ! id || ! id->IsType() || id->GetType()->Tag() != TYPE_RECORD ) { diff --git a/src/zeekygen/ScriptInfo.cc b/src/zeekygen/ScriptInfo.cc index 0861116902..7249f141d3 100644 --- a/src/zeekygen/ScriptInfo.cc +++ b/src/zeekygen/ScriptInfo.cc @@ -257,13 +257,13 @@ void ScriptInfo::DoInitPostScript() // so just manually associating them with scripts for now. if ( name == "base/frameworks/input/main.zeek" ) { - auto id = global_scope()->Lookup("Input::Reader"); - types.push_back(new IdentifierInfo({NewRef{}, id}, this)); + const auto& id = global_scope()->Find("Input::Reader"); + types.push_back(new IdentifierInfo(id, this)); } else if ( name == "base/frameworks/logging/main.zeek" ) { - auto id = global_scope()->Lookup("Log::Writer"); - types.push_back(new IdentifierInfo({NewRef{}, id}, this)); + const auto& id = global_scope()->Find("Log::Writer"); + types.push_back(new IdentifierInfo(id, this)); } }