diff --git a/CHANGES b/CHANGES index b4be1072f5..ba5a971168 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,10 @@ +3.1.0-dev.48 | 2019-08-13 20:15:17 -0700 + + * Cleanups related to PDict -> std::map replacements (Jon Siwek, Corelight) + + * Replace various uses of PDict with std::map (Tim Wojtulewicz, Corelight) + 3.1.0-dev.40 | 2019-08-13 23:44:45 +0000 * Change over to whitelisting clang-tidy options instead of diff --git a/VERSION b/VERSION index 2469a155eb..b97f44d9ff 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.1.0-dev.40 +3.1.0-dev.48 diff --git a/src/DFA.cc b/src/DFA.cc index 5acb6f3225..23aecbad60 100644 --- a/src/DFA.cc +++ b/src/DFA.cc @@ -17,7 +17,6 @@ DFA_State::DFA_State(int arg_state_num, const EquivClass* ec, nfa_states = arg_nfa_states; accept = arg_accept; mark = 0; - centry = 0; SymPartition(ec); @@ -285,8 +284,7 @@ unsigned int DFA_State::Size() + pad_size(sizeof(DFA_State*) * num_sym) + (accept ? pad_size(sizeof(int) * accept->size()) : 0) + (nfa_states ? pad_size(sizeof(NFA_State*) * nfa_states->length()) : 0) - + (meta_ec ? meta_ec->Size() : 0) - + (centry ? padded_sizeof(CacheEntry) : 0); + + (meta_ec ? meta_ec->Size() : 0); } DFA_State_Cache::DFA_State_Cache() @@ -296,19 +294,16 @@ DFA_State_Cache::DFA_State_Cache() DFA_State_Cache::~DFA_State_Cache() { - IterCookie* i = states.InitForIteration(); - CacheEntry* e; - while ( (e = (CacheEntry*) states.NextEntry(i)) ) + for ( auto& entry : states ) { - assert(e->state); - delete e->hash; - Unref(e->state); - delete e; + assert(entry.second); + Unref(entry.second); } + + states.clear(); } -DFA_State* DFA_State_Cache::Lookup(const NFA_state_list& nfas, - HashKey** hash) +DFA_State* DFA_State_Cache::Lookup(const NFA_state_list& nfas, DigestStr* digest) { // We assume that state ID's don't exceed 10 digits, plus // we allow one more character for the delimiter. @@ -335,37 +330,27 @@ DFA_State* DFA_State_Cache::Lookup(const NFA_state_list& nfas, // We use the short MD5 instead of the full string for the // HashKey because the data is copied into the key. - u_char digest[16]; - internal_md5(id_tag, p - id_tag, digest); + u_char digest_bytes[16]; + internal_md5(id_tag, p - id_tag, digest_bytes); + *digest = DigestStr(digest_bytes, 16); - *hash = new HashKey(&digest, sizeof(digest)); - CacheEntry* e = states.Lookup(*hash); - if ( ! e ) + auto entry = states.find(*digest); + if ( entry == states.end() ) { ++misses; - return 0; + return nullptr; } ++hits; - delete *hash; - *hash = 0; + digest->clear(); - return e->state; + return entry->second; } -DFA_State* DFA_State_Cache::Insert(DFA_State* state, HashKey* hash) +DFA_State* DFA_State_Cache::Insert(DFA_State* state, DigestStr digest) { - CacheEntry* e; - - e = new CacheEntry; - - e->state = state; - e->state->centry = e; - e->hash = hash; - - states.Insert(hash, e); - - return e->state; + states.emplace(std::move(digest), state); + return state; } void DFA_State_Cache::GetStats(Stats* s) @@ -378,15 +363,13 @@ void DFA_State_Cache::GetStats(Stats* s) s->hits = hits; s->misses = misses; - CacheEntry* e; - - IterCookie* i = states.InitForIteration(); - while ( (e = (CacheEntry*) states.NextEntry(i)) ) + for ( const auto& state : states ) { + DFA_State* e = state.second; ++s->dfa_states; - s->nfa_states += e->state->NFAStateNum(); - e->state->Stats(&s->computed, &s->uncomputed); - s->mem += pad_size(e->state->Size()) + padded_sizeof(*e->state); + s->nfa_states += e->NFAStateNum(); + e->Stats(&s->computed, &s->uncomputed); + s->mem += pad_size(e->Size()) + padded_sizeof(*e); } } @@ -407,7 +390,7 @@ DFA_Machine::DFA_Machine(NFA_Machine* n, EquivClass* arg_ec) if ( ns->length() > 0 ) { NFA_state_list* state_set = epsilon_closure(ns); - (void) StateSetToDFA_State(state_set, start_state, ec); + StateSetToDFA_State(state_set, start_state, ec); } else { @@ -445,14 +428,14 @@ unsigned int DFA_Machine::MemoryAllocation() const + nfa->MemoryAllocation(); } -int DFA_Machine::StateSetToDFA_State(NFA_state_list* state_set, +bool DFA_Machine::StateSetToDFA_State(NFA_state_list* state_set, DFA_State*& d, const EquivClass* ec) { - HashKey* hash; - d = dfa_state_cache->Lookup(*state_set, &hash); + DigestStr digest; + d = dfa_state_cache->Lookup(*state_set, &digest); if ( d ) - return 0; + return false; AcceptingSet* accept = new AcceptingSet; @@ -471,9 +454,9 @@ int DFA_Machine::StateSetToDFA_State(NFA_state_list* state_set, } DFA_State* ds = new DFA_State(state_count++, ec, state_set, accept); - d = dfa_state_cache->Insert(ds, hash); + d = dfa_state_cache->Insert(ds, std::move(digest)); - return 1; + return true; } int DFA_Machine::Rep(int sym) diff --git a/src/DFA.h b/src/DFA.h index 7e17f9a1a8..04d6e5a280 100644 --- a/src/DFA.h +++ b/src/DFA.h @@ -17,7 +17,6 @@ class DFA_State; class DFA_Machine; class DFA_State; -struct CacheEntry; class DFA_State : public BroObj { public: @@ -64,15 +63,11 @@ protected: NFA_state_list* nfa_states; EquivClass* meta_ec; // which ec's make same transition DFA_State* mark; - CacheEntry* centry; static unsigned int transition_counter; // see Xtion() }; -struct CacheEntry { - DFA_State* state; - HashKey* hash; -}; +using DigestStr = basic_string; class DFA_State_Cache { public: @@ -80,13 +75,12 @@ public: ~DFA_State_Cache(); // If the caller stores the handle, it has to call Ref() on it. - DFA_State* Lookup(const NFA_state_list& nfa_states, - HashKey** hash); + DFA_State* Lookup(const NFA_state_list& nfa_states, DigestStr* digest); - // Takes ownership of both; hash is the one returned by Lookup(). - DFA_State* Insert(DFA_State* state, HashKey* hash); + // Takes ownership of state; digest is the one returned by Lookup(). + DFA_State* Insert(DFA_State* state, DigestStr digest); - int NumEntries() const { return states.Length(); } + int NumEntries() const { return states.size(); } struct Stats { // Sum of all NFA states @@ -106,7 +100,7 @@ private: int misses; // Hash indexed by NFA states (MD5s of them, actually). - PDict states; + std::map states; }; class DFA_Machine : public BroObj { @@ -134,7 +128,7 @@ protected: int state_count; // The state list has to be sorted according to IDs. - int StateSetToDFA_State(NFA_state_list* state_set, DFA_State*& d, + bool StateSetToDFA_State(NFA_state_list* state_set, DFA_State*& d, const EquivClass* ec); const EquivClass* EC() const { return ec; } diff --git a/src/DNS_Mgr.h b/src/DNS_Mgr.h index 96e73779e8..b0b2ff3638 100644 --- a/src/DNS_Mgr.h +++ b/src/DNS_Mgr.h @@ -141,8 +141,6 @@ protected: DNS_MgrMode mode; - PDict services; - HostMap host_mappings; AddrMap addr_mappings; TextMap text_mappings; diff --git a/src/Debug.cc b/src/Debug.cc index de84471691..5f6c3bd7fb 100644 --- a/src/Debug.cc +++ b/src/Debug.cc @@ -27,7 +27,7 @@ using namespace std; bool g_policy_debug = false; DebuggerState g_debugger_state; TraceState g_trace_state; -PDict g_dbgfilemaps; +std::map g_dbgfilemaps; // These variables are used only to decide whether or not to print the // current context; you don't want to do it after a step or next @@ -364,8 +364,8 @@ vector parse_location_string(const string& s) if ( plr.type == plrFileAndLine ) { - Filemap* map = g_dbgfilemaps.Lookup(loc_filename); - if ( ! map ) + auto iter = g_dbgfilemaps.find(loc_filename); + if ( iter == g_dbgfilemaps.end() ) reporter->InternalError("Policy file %s should have been loaded\n", loc_filename); @@ -378,7 +378,7 @@ vector parse_location_string(const string& s) } StmtLocMapping* hit = 0; - for ( const auto entry : *map ) + for ( const auto entry : *(iter->second) ) { plr.filename = entry->Loc().filename; diff --git a/src/Debug.h b/src/Debug.h index 7e414262c9..16d6761bce 100644 --- a/src/Debug.h +++ b/src/Debug.h @@ -176,7 +176,7 @@ string get_context_description(const Stmt* stmt, const Frame* frame); extern Frame* g_dbg_locals; // variables created within debugger context -extern PDict g_dbgfilemaps; // filename => filemap +extern std::map g_dbgfilemaps; // filename => filemap // Perhaps add a code/priority argument to do selective output. int debug_msg(const char* fmt, ...) __attribute__ ((format (printf, 1, 2))); diff --git a/src/DebugCmds.cc b/src/DebugCmds.cc index dcba46d195..6ae6a56f40 100644 --- a/src/DebugCmds.cc +++ b/src/DebugCmds.cc @@ -51,14 +51,16 @@ void lookup_global_symbols_regex(const string& orig_regex, vector& matches, } Scope* global = global_scope(); - PDict* syms = global->Vars(); + const auto& syms = global->Vars(); ID* nextid; - IterCookie* cookie = syms->InitForIteration(); - while ( (nextid = syms->NextEntry( cookie )) ) + for ( const auto& sym : syms ) + { + ID* nextid = sym.second; if ( ! func_only || nextid->Type()->Tag() == TYPE_FUNC ) if ( ! regexec (&re, nextid->Name(), 0, 0, 0) ) matches.push_back(nextid); + } } void choose_global_symbols_regex(const string& regex, vector& choices, diff --git a/src/EventRegistry.cc b/src/EventRegistry.cc index 48a1761030..a7ad0636f4 100644 --- a/src/EventRegistry.cc +++ b/src/EventRegistry.cc @@ -4,85 +4,67 @@ void EventRegistry::Register(EventHandlerPtr handler) { - HashKey key(handler->Name()); - handlers.Insert(&key, handler.Ptr()); + handlers[string(handler->Name())] = handler.Ptr(); } -EventHandler* EventRegistry::Lookup(const char* name) +EventHandler* EventRegistry::Lookup(const string& name) { - HashKey key(name); - return handlers.Lookup(&key); + auto it = handlers.find(name); + if ( it != handlers.end() ) + return it->second; + + return nullptr; } -EventRegistry::string_list* EventRegistry::Match(RE_Matcher* pattern) +EventRegistry::string_list EventRegistry::Match(RE_Matcher* pattern) { - string_list* names = new string_list; + string_list names; - IterCookie* c = handlers.InitForIteration(); - - HashKey* k; - EventHandler* v; - while ( (v = handlers.NextEntry(k, c)) ) + for ( const auto& entry : handlers ) { + EventHandler* v = entry.second; if ( v->LocalHandler() && pattern->MatchExactly(v->Name()) ) - names->push_back(v->Name()); - - delete k; + names.push_back(entry.first); } return names; } -EventRegistry::string_list* EventRegistry::UnusedHandlers() +EventRegistry::string_list EventRegistry::UnusedHandlers() { - string_list* names = new string_list; + string_list names; - IterCookie* c = handlers.InitForIteration(); - - HashKey* k; - EventHandler* v; - while ( (v = handlers.NextEntry(k, c)) ) + for ( const auto& entry : handlers ) { + EventHandler* v = entry.second; if ( v->LocalHandler() && ! v->Used() ) - names->push_back(v->Name()); - - delete k; + names.push_back(entry.first); } return names; } -EventRegistry::string_list* EventRegistry::UsedHandlers() +EventRegistry::string_list EventRegistry::UsedHandlers() { - string_list* names = new string_list; + string_list names; - IterCookie* c = handlers.InitForIteration(); - - HashKey* k; - EventHandler* v; - while ( (v = handlers.NextEntry(k, c)) ) + for ( const auto& entry : handlers ) { + EventHandler* v = entry.second; if ( v->LocalHandler() && v->Used() ) - names->push_back(v->Name()); - - delete k; + names.push_back(entry.first); } return names; } -EventRegistry::string_list* EventRegistry::AllHandlers() +EventRegistry::string_list EventRegistry::AllHandlers() { - string_list* names = new string_list(handlers.Length()); + string_list names; - IterCookie* c = handlers.InitForIteration(); - - HashKey* k; - EventHandler* v; - while ( (v = handlers.NextEntry(k, c)) ) + for ( const auto& entry : handlers ) { - names->push_back(v->Name()); - delete k; + names.push_back(entry.first); } return names; @@ -90,13 +72,9 @@ EventRegistry::string_list* EventRegistry::AllHandlers() void EventRegistry::PrintDebug() { - IterCookie* c = handlers.InitForIteration(); - - HashKey* k; - EventHandler* v; - while ( (v = handlers.NextEntry(k, c)) ) + for ( const auto& entry : handlers ) { - delete k; + EventHandler* v = entry.second; fprintf(stderr, "Registered event %s (%s handler / %s)\n", v->Name(), v->LocalHandler()? "local" : "no", *v ? "active" : "not active" @@ -104,7 +82,7 @@ void EventRegistry::PrintDebug() } } -void EventRegistry::SetErrorHandler(const char* name) +void EventRegistry::SetErrorHandler(const string& name) { EventHandler* eh = Lookup(name); @@ -114,7 +92,7 @@ void EventRegistry::SetErrorHandler(const char* name) return; } - reporter->InternalWarning( - "unknown event handler '%s' in SetErrorHandler()", name); + reporter->InternalWarning("unknown event handler '%s' in SetErrorHandler()", + name.c_str()); } diff --git a/src/EventRegistry.h b/src/EventRegistry.h index 2a570dd8bc..fb4c1d8ca8 100644 --- a/src/EventRegistry.h +++ b/src/EventRegistry.h @@ -3,6 +3,9 @@ #ifndef EVENT_REGISTRY #define EVENT_REGISTRY +#include +#include + #include "Func.h" #include "List.h" #include "Dict.h" @@ -17,28 +20,26 @@ public: void Register(EventHandlerPtr handler); // Return nil if unknown. - EventHandler* Lookup(const char* name); + EventHandler* Lookup(const string& name); // Returns a list of all local handlers that match the given pattern. // Passes ownership of list. - typedef const char constchar; // PList doesn't like "const char" - typedef PList string_list; - string_list* Match(RE_Matcher* pattern); + typedef vector string_list; + string_list Match(RE_Matcher* pattern); // Marks a handler as handling errors. Error handler will not be called // recursively to avoid infinite loops in case they trigger an error // themselves. - void SetErrorHandler(const char* name); + void SetErrorHandler(const string& name); - string_list* UnusedHandlers(); - string_list* UsedHandlers(); - string_list* AllHandlers(); + string_list UnusedHandlers(); + string_list UsedHandlers(); + string_list AllHandlers(); void PrintDebug(); private: - typedef PDict handler_map; - handler_map handlers; + std::map handlers; }; extern EventRegistry* event_registry; diff --git a/src/Expr.cc b/src/Expr.cc index 2df47ef13f..1e3c0fc1f4 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -4347,7 +4347,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.data()); + auto id = global_scope()->Lookup(fullname); if ( id ) // Just try again to make a unique lambda name. If two peer @@ -4413,7 +4413,7 @@ EventExpr::EventExpr(const char* arg_name, ListExpr* arg_args) name = arg_name; args = arg_args; - EventHandler* h = event_registry->Lookup(name.c_str()); + EventHandler* h = event_registry->Lookup(name); if ( ! h ) { h = new EventHandler(name.c_str()); diff --git a/src/OpaqueVal.cc b/src/OpaqueVal.cc index 595102e8a2..372e6b4d6b 100644 --- a/src/OpaqueVal.cc +++ b/src/OpaqueVal.cc @@ -122,7 +122,7 @@ BroType* OpaqueVal::UnserializeType(const broker::data& data) if ( ! name ) return nullptr; - ID* id = global_scope()->Lookup(name->c_str()); + ID* id = global_scope()->Lookup(*name); if ( ! id ) return nullptr; diff --git a/src/RE.cc b/src/RE.cc index 95d5e1786b..5b758dfb41 100644 --- a/src/RE.cc +++ b/src/RE.cc @@ -194,9 +194,13 @@ int Specific_RE_Matcher::CompileSet(const string_list& set, const int_list& idx) return 1; } -const char* Specific_RE_Matcher::LookupDef(const char* def) +string Specific_RE_Matcher::LookupDef(const string& def) { - return defs.Lookup(def); + const auto& iter = defs.find(def); + if ( iter != defs.end() ) + return iter->second; + + return string(); } int Specific_RE_Matcher::MatchAll(const char* s) @@ -412,10 +416,21 @@ unsigned int Specific_RE_Matcher::MemoryAllocation() const for ( int i = 0; i < ccl_list.length(); ++i ) size += ccl_list[i]->MemoryAllocation(); + size += pad_size(sizeof(CCL*) * ccl_dict.size()); + for ( const auto& entry : ccl_dict ) + { + size += padded_sizeof(std::string) + pad_size(sizeof(std::string::value_type) * entry.first.size()); + size += entry.second->MemoryAllocation(); + } + + for ( const auto& entry : defs ) + { + size += padded_sizeof(std::string) + pad_size(sizeof(std::string::value_type) * entry.first.size()); + size += padded_sizeof(std::string) + pad_size(sizeof(std::string::value_type) * entry.second.size()); + } + return size + padded_sizeof(*this) + (pattern_text ? pad_size(strlen(pattern_text) + 1) : 0) - + defs.MemoryAllocation() - padded_sizeof(defs) // FIXME: count content - + ccl_dict.MemoryAllocation() - padded_sizeof(ccl_dict) // FIXME: count content + ccl_list.MemoryAllocation() - padded_sizeof(ccl_list) + equiv_class.Size() - padded_sizeof(EquivClass) + (dfa ? dfa->MemoryAllocation() : 0) // this is ref counted; consider the bytes here? diff --git a/src/RE.h b/src/RE.h index 23804cb6aa..3815527bf8 100644 --- a/src/RE.h +++ b/src/RE.h @@ -59,15 +59,22 @@ public: // The following is vestigial from flex's use of "{name}" definitions. // It's here because at some point we may want to support such // functionality. - const char* LookupDef(const char* def); + std::string LookupDef(const std::string& def); - void InsertCCL(const char* txt, CCL* ccl) { ccl_dict.Insert(txt, ccl); } + void InsertCCL(const char* txt, CCL* ccl) { ccl_dict[string(txt)] = ccl; } int InsertCCL(CCL* ccl) { ccl_list.push_back(ccl); return ccl_list.length() - 1; } - CCL* LookupCCL(const char* txt) { return ccl_dict.Lookup(txt); } + CCL* LookupCCL(const char* txt) + { + const auto& iter = ccl_dict.find(string(txt)); + if ( iter != ccl_dict.end() ) + return iter->second; + + return nullptr; + } CCL* LookupCCL(int index) { return ccl_list[index]; } CCL* AnyCCL(); @@ -119,8 +126,8 @@ protected: int multiline; char* pattern_text; - PDict defs; - PDict ccl_dict; + std::map defs; + std::map ccl_dict; PList ccl_list; EquivClass equiv_class; int* ecs; diff --git a/src/Rule.h b/src/Rule.h index cd08d4d93e..8bf53c783c 100644 --- a/src/Rule.h +++ b/src/Rule.h @@ -2,6 +2,7 @@ #define rule_h #include +#include #include "Obj.h" #include "List.h" @@ -15,7 +16,7 @@ class RuleHdrTest; class Rule; typedef PList rule_list; -typedef PDict rule_dict; +typedef std::map rule_dict; class Rule { public: diff --git a/src/RuleMatcher.cc b/src/RuleMatcher.cc index 90fb13495c..29da1eec60 100644 --- a/src/RuleMatcher.cc +++ b/src/RuleMatcher.cc @@ -267,14 +267,14 @@ bool RuleMatcher::ReadFiles(const name_list& files) void RuleMatcher::AddRule(Rule* rule) { - if ( rules_by_id.Lookup(rule->ID()) ) + if ( rules_by_id.find(rule->ID()) != rules_by_id.end() ) { rules_error("rule defined twice"); return; } rules.push_back(rule); - rules_by_id.Insert(rule->ID(), rule); + rules_by_id[rule->ID()] = rule; } void RuleMatcher::BuildRulesTree() @@ -295,15 +295,15 @@ void RuleMatcher::InsertRuleIntoTree(Rule* r, int testnr, // Initiliaze the preconditions for ( const auto& pc : r->preconds ) { - Rule* pc_rule = rules_by_id.Lookup(pc->id); - if ( ! pc_rule ) + auto entry = rules_by_id.find(pc->id); + if ( entry == rules_by_id.end() ) { rules_error(r, "unknown rule referenced"); return; } - pc->rule = pc_rule; - pc_rule->dependents.push_back(r); + pc->rule = entry->second; + entry->second->dependents.push_back(r); } // All tests in tree already? diff --git a/src/Scope.cc b/src/Scope.cc index 20492a3f15..b445ce0e53 100644 --- a/src/Scope.cc +++ b/src/Scope.cc @@ -19,7 +19,6 @@ Scope::Scope(ID* id, attr_list* al) attrs = al; return_type = 0; - local = new PDict(ORDERED); inits = new id_list; if ( id ) @@ -42,8 +41,8 @@ Scope::Scope(ID* id, attr_list* al) Scope::~Scope() { - for ( int i = 0; i < local->Length(); ++i ) - Unref(local->NthEntry(i)); + for ( const auto& entry : local ) + Unref(entry.second); if ( attrs ) { @@ -55,7 +54,6 @@ Scope::~Scope() Unref(scope_id); Unref(return_type); - delete local; delete inits; } @@ -82,7 +80,7 @@ void Scope::Describe(ODesc* d) const d->SP(); d->Add(return_type != 0); d->SP(); - d->Add(local->Length()); + d->Add(static_cast(local.size())); d->SP(); } @@ -98,9 +96,9 @@ void Scope::Describe(ODesc* d) const d->NL(); } - for ( int i = 0; i < local->Length(); ++i ) + for ( const auto& entry : local ) { - ID* id = local->NthEntry(i); + ID* id = entry.second; id->Describe(d); d->NL(); } @@ -108,13 +106,9 @@ void Scope::Describe(ODesc* d) const TraversalCode Scope::Traverse(TraversalCallback* cb) const { - PDict* ids = GetIDs(); - IterCookie* iter = ids->InitForIteration(); - - HashKey* key; - ID* id; - while ( (id = ids->NextEntry(key, iter)) ) + for ( const auto& entry : local ) { + ID* id = entry.second; TraversalCode tc = id->Traverse(cb); HANDLE_TC_STMT_PRE(tc); } @@ -134,7 +128,7 @@ ID* lookup_ID(const char* name, const char* curr_module, bool no_global, for ( int i = scopes.length() - 1; i >= 0; --i ) { - ID* id = scopes[i]->Lookup(fullname.c_str()); + ID* id = scopes[i]->Lookup(fullname); if ( id ) { if ( need_export && ! id->IsExport() && ! in_debug ) @@ -150,7 +144,7 @@ ID* lookup_ID(const char* name, const char* curr_module, bool no_global, ! same_module_only) ) { string globalname = make_full_var_name(GLOBAL_MODULE_NAME, name); - ID* id = global_scope()->Lookup(globalname.c_str()); + ID* id = global_scope()->Lookup(globalname); if ( id ) { Ref(id); diff --git a/src/Scope.h b/src/Scope.h index 954e0a57cc..4eca1d46f5 100644 --- a/src/Scope.h +++ b/src/Scope.h @@ -4,6 +4,7 @@ #define scope_h #include +#include #include "Dict.h" #include "Obj.h" @@ -20,25 +21,37 @@ public: explicit Scope(ID* id, attr_list* al); ~Scope() override; - ID* Lookup(const char* name) const { return local->Lookup(name); } - void Insert(const char* name, ID* id) { local->Insert(name, id); } - ID* Remove(const char* name) + ID* Lookup(const std::string& name) const { - HashKey key(name); - return (ID*) local->Remove(&key); + const auto& entry = local.find(name); + if ( entry != local.end() ) + return entry->second; + + return nullptr; + } + void Insert(const std::string& name, ID* id) { local[name] = id; } + ID* Remove(const std::string& name) + { + const auto& entry = local.find(name); + if ( entry != local.end() ) + { + ID* id = entry->second; + local.erase(entry); + return id; + } + + return nullptr; } ID* ScopeID() const { return scope_id; } attr_list* Attrs() const { return attrs; } BroType* ReturnType() const { return return_type; } - int Length() const { return local->Length(); } - PDict* Vars() const { return local; } + size_t Length() const { return local.size(); } + std::map& Vars() { return local; } ID* GenerateTemporary(const char* name); - PDict* GetIDs() const { return local; } - // Returns the list of variables needing initialization, and // removes it from this Scope. id_list* GetInits(); @@ -54,7 +67,7 @@ protected: ID* scope_id; attr_list* attrs; BroType* return_type; - PDict* local; + std::map local; id_list* inits; }; diff --git a/src/Stats.cc b/src/Stats.cc index 674c2fb50c..339dd14371 100644 --- a/src/Stats.cc +++ b/src/Stats.cc @@ -239,7 +239,7 @@ void ProfileLogger::Log() // Script-level state. unsigned int size, mem = 0; - PDict* globals = global_scope()->Vars(); + const auto& globals = global_scope()->Vars(); if ( expensive ) { @@ -249,10 +249,10 @@ void ProfileLogger::Log() file->Write(fmt("%.06f Global_sizes > 100k: %dK\n", network_time, mem / 1024)); - ID* id; - IterCookie* c = globals->InitForIteration(); + for ( const auto& global : globals ) + { + ID* id = global.second; - while ( (id = globals->NextEntry(c)) ) // We don't show/count internal globals as they are always // contained in some other global user-visible container. if ( id->HasVal() ) @@ -298,6 +298,7 @@ void ProfileLogger::Log() file->Write("\n"); } } + } file->Write(fmt("%.06f Global_sizes total: %dK\n", network_time, mem / 1024)); diff --git a/src/Stmt.cc b/src/Stmt.cc index 2576fc544f..fca38a357a 100644 --- a/src/Stmt.cc +++ b/src/Stmt.cc @@ -50,11 +50,11 @@ bool Stmt::SetLocationInfo(const Location* start, const Location* end) // Update the Filemap of line number -> statement mapping for // breakpoints (Debug.h). - Filemap* map_ptr = (Filemap*) g_dbgfilemaps.Lookup(location->filename); - if ( ! map_ptr ) + auto map_iter = g_dbgfilemaps.find(location->filename); + if ( map_iter == g_dbgfilemaps.end() ) return false; - Filemap& map = *map_ptr; + Filemap& map = *(map_iter->second); StmtLocMapping* new_mapping = new StmtLocMapping(GetLocationInfo(), this); diff --git a/src/Type.cc b/src/Type.cc index 8de62e7266..ba21120bba 100644 --- a/src/Type.cc +++ b/src/Type.cc @@ -1799,7 +1799,7 @@ BroType* 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().data()); + auto id = global_scope()->Lookup(t1->GetName()); if ( id && id->AsType() && id->AsType()->Tag() == TYPE_ENUM ) // It should make most sense to return the real type here rather diff --git a/src/Type.h b/src/Type.h index 17a74476ab..a4a4bcc703 100644 --- a/src/Type.h +++ b/src/Type.h @@ -264,7 +264,7 @@ public: virtual unsigned MemoryAllocation() const; void SetName(const string& arg_name) { name = arg_name; } - string GetName() const { return name; } + const string& GetName() const { return name; } typedef std::map > TypeAliasMap; diff --git a/src/Var.cc b/src/Var.cc index 73baab9683..119ded1a50 100644 --- a/src/Var.cc +++ b/src/Var.cc @@ -429,7 +429,7 @@ TraversalCode OuterIDBindingFinder::PreExpr(const Expr* expr) if ( e->Id()->IsGlobal() ) return TC_CONTINUE; - if ( scope->GetIDs()->Lookup(e->Id()->Name()) ) + if ( scope->Lookup(e->Id()->Name()) ) return TC_CONTINUE; outer_id_references.push_back(e); diff --git a/src/analyzer/protocol/rpc/RPC.cc b/src/analyzer/protocol/rpc/RPC.cc index 57de09b4d6..d0de54ec5d 100644 --- a/src/analyzer/protocol/rpc/RPC.cc +++ b/src/analyzer/protocol/rpc/RPC.cc @@ -98,19 +98,15 @@ int RPC_CallInfo::CompareRexmit(const u_char* buf, int n) const } -void rpc_callinfo_delete_func(void* v) - { - delete (RPC_CallInfo*) v; - } - RPC_Interpreter::RPC_Interpreter(analyzer::Analyzer* arg_analyzer) { analyzer = arg_analyzer; - calls.SetDeleteFunc(rpc_callinfo_delete_func); } RPC_Interpreter::~RPC_Interpreter() { + for ( const auto& call : calls ) + delete call.second; } int RPC_Interpreter::DeliverRPC(const u_char* buf, int n, int rpclen, @@ -123,8 +119,10 @@ int RPC_Interpreter::DeliverRPC(const u_char* buf, int n, int rpclen, if ( ! buf ) return 0; - HashKey h(&xid, 1); - RPC_CallInfo* call = calls.Lookup(&h); + RPC_CallInfo* call = nullptr; + auto iter = calls.find(xid); + if ( iter != calls.end() ) + call = iter->second; if ( msg_type == RPC_CALL ) { @@ -164,7 +162,7 @@ int RPC_Interpreter::DeliverRPC(const u_char* buf, int n, int rpclen, return 0; } - calls.Insert(&h, call); + calls[xid] = call; } // We now have a valid RPC_CallInfo (either the previous one @@ -274,7 +272,8 @@ int RPC_Interpreter::DeliverRPC(const u_char* buf, int n, int rpclen, Event_RPC_Dialogue(call, status, n); - delete calls.RemoveEntry(&h); + calls.erase(xid); + delete call; } else { @@ -308,11 +307,9 @@ int RPC_Interpreter::DeliverRPC(const u_char* buf, int n, int rpclen, void RPC_Interpreter::Timeout() { - IterCookie* cookie = calls.InitForIteration(); - RPC_CallInfo* c; - - while ( (c = calls.NextEntry(cookie)) ) + for ( const auto& entry : calls ) { + RPC_CallInfo* c = entry.second; Event_RPC_Dialogue(c, BifEnum::RPC_TIMEOUT, 0); if ( c->IsValidCall() ) diff --git a/src/analyzer/protocol/rpc/RPC.h b/src/analyzer/protocol/rpc/RPC.h index 225c2c8cd4..734ed82c7f 100644 --- a/src/analyzer/protocol/rpc/RPC.h +++ b/src/analyzer/protocol/rpc/RPC.h @@ -123,7 +123,7 @@ protected: void Weird(const char* name, const char* addl = ""); - PDict calls; + std::map calls; analyzer::Analyzer* analyzer; }; diff --git a/src/analyzer/protocol/stepping-stone/SteppingStone.cc b/src/analyzer/protocol/stepping-stone/SteppingStone.cc index d47a0b6745..f762350565 100644 --- a/src/analyzer/protocol/stepping-stone/SteppingStone.cc +++ b/src/analyzer/protocol/stepping-stone/SteppingStone.cc @@ -22,7 +22,6 @@ SteppingStoneEndpoint::SteppingStoneEndpoint(tcp::TCP_Endpoint* e, SteppingStone stp_last_time = stp_resume_time = 0.0; stp_manager = m; stp_id = stp_manager->NextID(); - stp_key = new HashKey(bro_int_t(stp_id)); CreateEndpEvent(e->IsOrig()); @@ -32,7 +31,6 @@ SteppingStoneEndpoint::SteppingStoneEndpoint(tcp::TCP_Endpoint* e, SteppingStone SteppingStoneEndpoint::~SteppingStoneEndpoint() { - delete stp_key; Unref(endp->TCP()->Conn()); } @@ -42,20 +40,19 @@ void SteppingStoneEndpoint::Done() return; SteppingStoneEndpoint* ep; - IterCookie* cookie; - cookie = stp_inbound_endps.InitForIteration(); - while ( (ep = stp_inbound_endps.NextEntry(cookie)) ) + for ( const auto& entry : stp_inbound_endps ) { - ep->stp_outbound_endps.Remove(stp_key); + ep = entry.second; + ep->stp_outbound_endps.erase(stp_id); Event(stp_remove_pair, ep->stp_id, stp_id); Unref(ep); } - cookie = stp_outbound_endps.InitForIteration(); - while ( (ep = stp_outbound_endps.NextEntry(cookie)) ) + for ( const auto& entry : stp_outbound_endps ) { - ep->stp_inbound_endps.Remove(stp_key); + ep = entry.second; + ep->stp_inbound_endps.erase(stp_id); Event(stp_remove_pair, stp_id, ep->stp_id); Unref(ep); } @@ -115,8 +112,8 @@ int SteppingStoneEndpoint::DataSent(double t, uint64 seq, int len, int caplen, Ref(ep); Ref(this); - stp_inbound_endps.Insert(ep->stp_key, ep); - ep->stp_outbound_endps.Insert(stp_key, this); + stp_inbound_endps[ep->stp_id] = ep; + ep->stp_outbound_endps[stp_id] = this; Event(stp_correlate_pair, ep->stp_id, stp_id); } diff --git a/src/analyzer/protocol/stepping-stone/SteppingStone.h b/src/analyzer/protocol/stepping-stone/SteppingStone.h index 502e3caa7e..d670f3c4b8 100644 --- a/src/analyzer/protocol/stepping-stone/SteppingStone.h +++ b/src/analyzer/protocol/stepping-stone/SteppingStone.h @@ -37,9 +37,8 @@ protected: // removing correlated endpoint pairs in Bro, since there is // no LOOP in Bro language. int stp_id; - HashKey* stp_key; - PDict stp_inbound_endps; - PDict stp_outbound_endps; + std::map stp_inbound_endps; + std::map stp_outbound_endps; }; class SteppingStone_Analyzer : public tcp::TCP_ApplicationAnalyzer { diff --git a/src/broker/Data.cc b/src/broker/Data.cc index c837e4ecb1..2efbe7f277 100644 --- a/src/broker/Data.cc +++ b/src/broker/Data.cc @@ -351,7 +351,7 @@ struct val_converter { if ( ! name ) return nullptr; - auto id = global_scope()->Lookup(name->c_str()); + auto id = global_scope()->Lookup(*name); if ( ! id ) return nullptr; @@ -703,7 +703,7 @@ struct type_checker { if ( ! name ) return false; - auto id = global_scope()->Lookup(name->c_str()); + auto id = global_scope()->Lookup(*name); if ( ! id ) return false; diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index e54ffa08a7..1a82571223 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -407,7 +407,7 @@ bool Manager::PublishIdentifier(std::string topic, std::string id) if ( peer_count == 0 ) return true; - ID* i = global_scope()->Lookup(id.c_str()); + ID* i = global_scope()->Lookup(id); if ( ! i ) return false; @@ -998,7 +998,7 @@ void Manager::ProcessEvent(const broker::topic& topic, broker::zeek::Event ev) DBG_LOG(DBG_BROKER, "Process event: %s %s", name.data(), RenderMessage(args).data()); ++statistics.num_events_incoming; - auto handler = event_registry->Lookup(name.data()); + auto handler = event_registry->Lookup(name); if ( ! handler ) return; @@ -1240,7 +1240,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.c_str()); + auto id = global_scope()->Lookup(id_name); if ( ! id ) { diff --git a/src/file_analysis/Manager.cc b/src/file_analysis/Manager.cc index da6099b1fe..ca6b9722b7 100644 --- a/src/file_analysis/Manager.cc +++ b/src/file_analysis/Manager.cc @@ -24,7 +24,7 @@ string Manager::salt; Manager::Manager() : plugin::ComponentManager("Files", "Tag"), - id_map(), ignored(), current_file_id(), magic_state() + current_file_id(), magic_state(), cumulative_files(0), max_files(0) { } @@ -33,21 +33,10 @@ Manager::~Manager() for ( MIMEMap::iterator i = mime_types.begin(); i != mime_types.end(); i++ ) delete i->second; - // Have to assume that too much of Bro has been shutdown by this point + // Have to assume that too much of Zeek has been shutdown by this point // to do anything more than reclaim memory. - - File* f; - bool* b; - - IterCookie* it = id_map.InitForIteration(); - - while ( (f = id_map.NextEntry(it)) ) - delete f; - - it = ignored.InitForIteration(); - - while( (b = ignored.NextEntry(it)) ) - delete b; + for ( const auto& entry : id_map ) + delete entry.second; delete magic_state; } @@ -69,19 +58,13 @@ void Manager::InitMagic() void Manager::Terminate() { vector keys; + keys.reserve(id_map.size()); - IterCookie* it = id_map.InitForIteration(); - HashKey* key; + for ( const auto& entry : id_map ) + keys.push_back(entry.first); - while ( id_map.NextEntry(key, it) ) - { - keys.push_back(string(static_cast(key->Key()), - key->Size())); - delete key; - } - - for ( size_t i = 0; i < keys.size(); ++i ) - Timeout(keys[i], true); + for ( const string& key : keys ) + Timeout(key, true); mgr.Drain(); } @@ -329,7 +312,7 @@ File* Manager::GetFile(const string& file_id, Connection* conn, if ( IsIgnored(file_id) ) return 0; - File* rval = id_map.Lookup(file_id.c_str()); + File* rval = LookupFile(file_id); if ( ! rval ) { @@ -337,7 +320,12 @@ File* Manager::GetFile(const string& file_id, Connection* conn, source_name ? source_name : analyzer_mgr->GetComponentName(tag), conn, tag, is_orig); - id_map.Insert(file_id.c_str(), rval); + id_map[file_id] = rval; + + ++cumulative_files; + if ( id_map.size() > max_files ) + max_files = id_map.size(); + rval->ScheduleInactivityTimer(); // Generate file_new after inserting it into manager's mapping @@ -362,7 +350,11 @@ File* Manager::GetFile(const string& file_id, Connection* conn, File* Manager::LookupFile(const string& file_id) const { - return id_map.Lookup(file_id.c_str()); + const auto& entry = id_map.find(file_id); + if ( entry == id_map.end() ) + return nullptr; + + return entry->second; } void Manager::Timeout(const string& file_id, bool is_terminating) @@ -393,22 +385,21 @@ void Manager::Timeout(const string& file_id, bool is_terminating) bool Manager::IgnoreFile(const string& file_id) { - if ( ! id_map.Lookup(file_id.c_str()) ) + if ( ! LookupFile(file_id) ) return false; DBG_LOG(DBG_FILE_ANALYSIS, "Ignore FileID %s", file_id.c_str()); - delete ignored.Insert(file_id.c_str(), new bool); + ignored.insert(file_id); return true; } bool Manager::RemoveFile(const string& file_id) { - HashKey key(file_id.c_str()); // Can't remove from the dictionary/map right away as invoking EndOfFile // may cause some events to be executed which actually depend on the file // still being in the dictionary/map. - File* f = static_cast(id_map.Lookup(&key)); + File* f = LookupFile(file_id); if ( ! f ) return false; @@ -417,14 +408,15 @@ bool Manager::RemoveFile(const string& file_id) f->EndOfFile(); delete f; - id_map.Remove(&key); - delete static_cast(ignored.Remove(&key)); + + id_map.erase(file_id); + ignored.erase(file_id); return true; } bool Manager::IsIgnored(const string& file_id) { - return ignored.Lookup(file_id.c_str()) != 0; + return ignored.find(file_id) != ignored.end(); } string Manager::GetFileID(analyzer::Tag tag, Connection* c, bool is_orig) diff --git a/src/file_analysis/Manager.h b/src/file_analysis/Manager.h index 67f84134ec..31eaf94ca0 100644 --- a/src/file_analysis/Manager.h +++ b/src/file_analysis/Manager.h @@ -5,6 +5,7 @@ #include #include +#include #include "Dict.h" #include "Net.h" @@ -325,20 +326,17 @@ public: std::string DetectMIME(const u_char* data, uint64 len) const; uint64 CurrentFiles() - { return id_map.Length(); } + { return id_map.size(); } uint64 MaxFiles() - { return id_map.MaxLength(); } + { return max_files; } uint64 CumulativeFiles() - { return id_map.NumCumulativeInserts(); } + { return cumulative_files; } protected: friend class FileTimer; - typedef PDict IDSet; - typedef PDict IDMap; - /** * Create a new file to be analyzed or retrieve an existing one. * @param file_id the file identifier/hash. @@ -407,8 +405,8 @@ private: TagSet* LookupMIMEType(const string& mtype, bool add_if_not_found); - PDict id_map; /**< Map file ID to file_analysis::File records. */ - PDict ignored; /**< Ignored files. Will be finally removed on EOF. */ + std::map id_map; /**< Map file ID to file_analysis::File records. */ + std::unordered_set ignored; /**< Ignored files. Will be finally removed on EOF. */ string current_file_id; /**< Hash of what get_file_handle event sets. */ RuleFileMagicState* magic_state; /**< File magic signature match state. */ MIMEMap mime_types;/**< Mapping of MIME types to analyzers. */ @@ -416,6 +414,9 @@ private: static TableVal* disabled; /**< Table of disabled analyzers. */ static TableType* tag_set_type; /**< Type for set[tag]. */ static string salt; /**< A salt added to file handles before hashing. */ + + size_t cumulative_files; + size_t max_files; }; /** diff --git a/src/input/Manager.cc b/src/input/Manager.cc index 46a1da1aa1..e57a498645 100644 --- a/src/input/Manager.cc +++ b/src/input/Manager.cc @@ -153,31 +153,31 @@ Manager::EventStream::EventStream() Manager::EventStream::~EventStream() { - if ( fields ) - Unref(fields); + if ( fields ) + Unref(fields); } Manager::TableStream::~TableStream() { - if ( tab ) - Unref(tab); + if ( tab ) + Unref(tab); - if ( itype ) - Unref(itype); + if ( itype ) + Unref(itype); if ( rtype ) // can be 0 for sets Unref(rtype); - if ( currDict != 0 ) + if ( currDict != 0 ) { currDict->Clear(); - delete currDict; + delete currDict; } - if ( lastDict != 0 ) + if ( lastDict != 0 ) { lastDict->Clear();; - delete lastDict; + delete lastDict; } } @@ -1852,7 +1852,7 @@ bool Manager::SendEvent(ReaderFrontend* reader, const string& name, const int nu return false; } - EventHandler* handler = event_registry->Lookup(name.c_str()); + EventHandler* handler = event_registry->Lookup(name); if ( handler == 0 ) { Warning(i, "Event %s not found", name.c_str()); diff --git a/src/input/readers/config/Config.cc b/src/input/readers/config/Config.cc index 8f0447cf66..d90020e1bf 100644 --- a/src/input/readers/config/Config.cc +++ b/src/input/readers/config/Config.cc @@ -28,11 +28,11 @@ Config::Config(ReaderFrontend *frontend) : ReaderBackend(frontend) fail_on_file_problem = false; // find all option names and their types. - auto globals = global_scope()->Vars(); - auto c = globals->InitForIteration(); + const auto& globals = global_scope()->Vars(); - while ( auto id = globals->NextEntry(c) ) + for ( const auto& entry : globals ) { + ID* id = entry.second; if ( ! id->IsOption() ) continue; diff --git a/src/main.cc b/src/main.cc index d4d612779c..530da45b97 100644 --- a/src/main.cc +++ b/src/main.cc @@ -982,17 +982,15 @@ int main(int argc, char** argv) if ( zeek_init ) //### this should be a function mgr.QueueEventFast(zeek_init, val_list{}); - EventRegistry::string_list* dead_handlers = + EventRegistry::string_list dead_handlers = event_registry->UnusedHandlers(); - if ( dead_handlers->length() > 0 && check_for_unused_event_handlers ) + if ( ! dead_handlers.empty() && check_for_unused_event_handlers ) { - for ( int i = 0; i < dead_handlers->length(); ++i ) - reporter->Warning("event handler never invoked: %s", (*dead_handlers)[i]); + for ( const string& handler : dead_handlers ) + reporter->Warning("event handler never invoked: %s", handler.c_str()); } - delete dead_handlers; - if ( stmts ) { stmt_flow_type flow; diff --git a/src/re-scan.l b/src/re-scan.l index 292f7a2e02..99dde0ca6c 100644 --- a/src/re-scan.l +++ b/src/re-scan.l @@ -85,14 +85,14 @@ CCL_EXPR ("[:"[[:alpha:]]+":]") char* nmstr = copy_string(yytext+1); nmstr[yyleng - 2] = '\0'; // chop trailing brace - const char* namedef = rem->LookupDef(nmstr); + std::string namedef = rem->LookupDef(nmstr); delete nmstr; - if ( ! namedef ) + if ( namedef.empty() ) synerr("undefined definition"); else { // push back name surrounded by ()'s - int len = strlen(namedef); + int len = namedef.size(); if ( namedef[0] == '^' || (len > 0 && namedef[len - 1] == '$') ) diff --git a/src/scan.l b/src/scan.l index ce389bb9fd..1cb6119f81 100644 --- a/src/scan.l +++ b/src/scan.l @@ -671,7 +671,7 @@ static int load_files(const char* orig_file) // Add the filename to the file mapping table (Debug.h). Filemap* map = new Filemap; HashKey* key = new HashKey(file_path.c_str()); - g_dbgfilemaps.Insert(key, map); + g_dbgfilemaps.emplace(file_path, map); LoadPolicyFileText(file_path.c_str()); } diff --git a/src/zeek.bif b/src/zeek.bif index 71c49f1c9e..18b5c67a10 100644 --- a/src/zeek.bif +++ b/src/zeek.bif @@ -1895,11 +1895,11 @@ function reading_traces%(%): bool function global_sizes%(%): var_sizes %{ TableVal* sizes = new TableVal(var_sizes); - PDict* globals = global_scope()->Vars(); - IterCookie* c = globals->InitForIteration(); + const auto& globals = global_scope()->Vars(); - ID* id; - while ( (id = globals->NextEntry(c)) ) + for ( const auto& global : globals ) + { + ID* id = global.second; if ( id->HasVal() ) { Val* id_name = new StringVal(id->Name()); @@ -1907,6 +1907,7 @@ function global_sizes%(%): var_sizes sizes->Assign(id_name, id_size); Unref(id_name); } + } return sizes; %} @@ -1922,12 +1923,11 @@ function global_sizes%(%): var_sizes function global_ids%(%): id_table %{ TableVal* ids = new TableVal(id_table); - PDict* globals = global_scope()->Vars(); - IterCookie* c = globals->InitForIteration(); + const auto& globals = global_scope()->Vars(); - ID* id; - while ( (id = globals->NextEntry(c)) ) + for ( const auto& global : globals ) { + ID* id = global.second; RecordVal* rec = new RecordVal(script_id); rec->Assign(0, new StringVal(type_name(id->Type()->Tag()))); rec->Assign(1, val_mgr->GetBool(id->IsExport()));