diff --git a/src/file_analysis/AnalyzerSet.cc b/src/file_analysis/AnalyzerSet.cc index a5abef9437..8d1397e6d6 100644 --- a/src/file_analysis/AnalyzerSet.cc +++ b/src/file_analysis/AnalyzerSet.cc @@ -40,60 +40,54 @@ AnalyzerSet::~AnalyzerSet() delete analyzer_hash; } -Analyzer* AnalyzerSet::Find(const file_analysis::Tag& tag, RecordVal* args) +Analyzer* AnalyzerSet::Find(const file_analysis::Tag& tag, + IntrusivePtr args) { - HashKey* key = GetKey(tag, args); - Analyzer* rval = analyzer_map.Lookup(key); - delete key; + auto key = GetKey(tag, std::move(args)); + Analyzer* rval = analyzer_map.Lookup(key.get()); return rval; } -bool AnalyzerSet::Add(const file_analysis::Tag& tag, RecordVal* args) +bool AnalyzerSet::Add(const file_analysis::Tag& tag, IntrusivePtr args) { - HashKey* key = GetKey(tag, args); + auto key = GetKey(tag, args); - if ( analyzer_map.Lookup(key) ) + if ( analyzer_map.Lookup(key.get()) ) { DBG_LOG(DBG_FILE_ANALYSIS, "[%s] Instantiate analyzer %s skipped: already exists", file->GetID().c_str(), file_mgr->GetComponentName(tag).c_str()); - delete key; return true; } - file_analysis::Analyzer* a = InstantiateAnalyzer(tag, args); + file_analysis::Analyzer* a = InstantiateAnalyzer(tag, std::move(args)); if ( ! a ) - { - delete key; return false; - } - Insert(a, key); + Insert(a, std::move(key)); return true; } -Analyzer* AnalyzerSet::QueueAdd(const file_analysis::Tag& tag, RecordVal* args) +Analyzer* AnalyzerSet::QueueAdd(const file_analysis::Tag& tag, + IntrusivePtr args) { - HashKey* key = GetKey(tag, args); - file_analysis::Analyzer* a = InstantiateAnalyzer(tag, args); + auto key = GetKey(tag, args); + file_analysis::Analyzer* a = InstantiateAnalyzer(tag, std::move(args)); if ( ! a ) - { - delete key; return nullptr; - } - mod_queue.push(new AddMod(a, key)); + mod_queue.push(new AddMod(a, std::move(key))); return a; } bool AnalyzerSet::AddMod::Perform(AnalyzerSet* set) { - if ( set->analyzer_map.Lookup(key) ) + if ( set->analyzer_map.Lookup(key.get()) ) { DBG_LOG(DBG_FILE_ANALYSIS, "[%s] Add analyzer %s skipped: already exists", a->GetFile()->GetID().c_str(), @@ -103,7 +97,7 @@ bool AnalyzerSet::AddMod::Perform(AnalyzerSet* set) return true; } - set->Insert(a, key); + set->Insert(a, std::move(key)); return true; } @@ -111,20 +105,18 @@ bool AnalyzerSet::AddMod::Perform(AnalyzerSet* set) void AnalyzerSet::AddMod::Abort() { delete a; - delete key; } -bool AnalyzerSet::Remove(const file_analysis::Tag& tag, RecordVal* args) +bool AnalyzerSet::Remove(const file_analysis::Tag& tag, + IntrusivePtr args) { - return Remove(tag, GetKey(tag, args)); + return Remove(tag, GetKey(tag, std::move(args))); } -bool AnalyzerSet::Remove(const file_analysis::Tag& tag, HashKey* key) +bool AnalyzerSet::Remove(const file_analysis::Tag& tag, + std::unique_ptr key) { - file_analysis::Analyzer* a = - (file_analysis::Analyzer*) analyzer_map.Remove(key); - - delete key; + auto a = (file_analysis::Analyzer*) analyzer_map.Remove(key.get()); if ( ! a ) { @@ -147,37 +139,38 @@ bool AnalyzerSet::Remove(const file_analysis::Tag& tag, HashKey* key) return true; } -bool AnalyzerSet::QueueRemove(const file_analysis::Tag& tag, RecordVal* args) +bool AnalyzerSet::QueueRemove(const file_analysis::Tag& tag, + IntrusivePtr args) { - HashKey* key = GetKey(tag, args); - - mod_queue.push(new RemoveMod(tag, key)); - - return analyzer_map.Lookup(key); + auto key = GetKey(tag, std::move(args)); + auto rval = analyzer_map.Lookup(key.get()); + mod_queue.push(new RemoveMod(tag, std::move(key))); + return rval; } bool AnalyzerSet::RemoveMod::Perform(AnalyzerSet* set) { - return set->Remove(tag, key); + return set->Remove(tag, std::move(key)); } -HashKey* AnalyzerSet::GetKey(const file_analysis::Tag& t, RecordVal* args) const +std::unique_ptr AnalyzerSet::GetKey(const file_analysis::Tag& t, + IntrusivePtr args) const { auto lv = make_intrusive(TYPE_ANY); lv->Append(t.AsVal()); - lv->Append({NewRef{}, args}); + lv->Append(std::move(args)); auto key = analyzer_hash->MakeHashKey(*lv, true); if ( ! key ) reporter->InternalError("AnalyzerArgs type mismatch"); - return key.release(); + return key; } file_analysis::Analyzer* AnalyzerSet::InstantiateAnalyzer(const Tag& tag, - RecordVal* args) const + IntrusivePtr args) const { - auto a = file_mgr->InstantiateAnalyzer(tag, {NewRef{}, args}, file); + auto a = file_mgr->InstantiateAnalyzer(tag, std::move(args), file); if ( ! a ) { @@ -190,12 +183,12 @@ file_analysis::Analyzer* AnalyzerSet::InstantiateAnalyzer(const Tag& tag, return a; } -void AnalyzerSet::Insert(file_analysis::Analyzer* a, HashKey* key) +void AnalyzerSet::Insert(file_analysis::Analyzer* a, + std::unique_ptr key) { DBG_LOG(DBG_FILE_ANALYSIS, "[%s] Add analyzer %s", file->GetID().c_str(), file_mgr->GetComponentName(a->Tag()).c_str()); - analyzer_map.Insert(key, a); - delete key; + analyzer_map.Insert(key.get(), a); a->Init(); } diff --git a/src/file_analysis/AnalyzerSet.h b/src/file_analysis/AnalyzerSet.h index 4605371eef..c0ef1d6fc0 100644 --- a/src/file_analysis/AnalyzerSet.h +++ b/src/file_analysis/AnalyzerSet.h @@ -3,6 +3,7 @@ #pragma once #include +#include #include "Dict.h" #include "Tag.h" @@ -42,7 +43,7 @@ public: * @param args an \c AnalyzerArgs record. * @return pointer to an analyzer instance, or a null pointer if not found. */ - Analyzer* Find(const file_analysis::Tag& tag, RecordVal* args); + Analyzer* Find(const file_analysis::Tag& tag, IntrusivePtr args); /** * Attach an analyzer to #file immediately. @@ -50,7 +51,7 @@ public: * @param args an \c AnalyzerArgs value which specifies an analyzer. * @return true if analyzer was instantiated/attached, else false. */ - bool Add(const file_analysis::Tag& tag, RecordVal* args); + bool Add(const file_analysis::Tag& tag, IntrusivePtr args); /** * Queue the attachment of an analyzer to #file. @@ -59,7 +60,8 @@ public: * @return if successful, a pointer to a newly instantiated analyzer else * a null pointer. The caller does *not* take ownership of the memory. */ - file_analysis::Analyzer* QueueAdd(const file_analysis::Tag& tag, RecordVal* args); + file_analysis::Analyzer* QueueAdd(const file_analysis::Tag& tag, + IntrusivePtr args); /** * Remove an analyzer from #file immediately. @@ -67,7 +69,7 @@ public: * @param args an \c AnalyzerArgs value which specifies an analyzer. * @return false if analyzer didn't exist and so wasn't removed, else true. */ - bool Remove(const file_analysis::Tag& tag, RecordVal* args); + bool Remove(const file_analysis::Tag& tag, IntrusivePtr args); /** * Queue the removal of an analyzer from #file. @@ -75,7 +77,7 @@ public: * @param args an \c AnalyzerArgs value which specifies an analyzer. * @return true if analyzer exists at time of call, else false; */ - bool QueueRemove(const file_analysis::Tag& tag, RecordVal* args); + bool QueueRemove(const file_analysis::Tag& tag, IntrusivePtr args); /** * Perform all queued modifications to the current analyzer set. @@ -108,7 +110,8 @@ protected: * @param args an \c AnalyzerArgs value which specifies an analyzer. * @return the hash key calculated from \a args */ - HashKey* GetKey(const file_analysis::Tag& tag, RecordVal* args) const; + std::unique_ptr GetKey(const file_analysis::Tag& tag, + IntrusivePtr args) const; /** * Create an instance of a file analyzer. @@ -117,14 +120,14 @@ protected: * @return a new file analyzer instance. */ file_analysis::Analyzer* InstantiateAnalyzer(const file_analysis::Tag& tag, - RecordVal* args) const; + IntrusivePtr args) const; /** * Insert an analyzer instance in to the set. * @param a an analyzer instance. * @param key the hash key which represents the analyzer's \c AnalyzerArgs. */ - void Insert(file_analysis::Analyzer* a, HashKey* key); + void Insert(file_analysis::Analyzer* a, std::unique_ptr key); /** * Remove an analyzer instance from the set. @@ -132,7 +135,7 @@ protected: * just used for debugging messages. * @param key the hash key which represents the analyzer's \c AnalyzerArgs. */ - bool Remove(const file_analysis::Tag& tag, HashKey* key); + bool Remove(const file_analysis::Tag& tag, std::unique_ptr key); private: @@ -170,15 +173,15 @@ private: * @param arg_a an analyzer instance to add to an analyzer set. * @param arg_key hash key representing the analyzer's \c AnalyzerArgs. */ - AddMod(file_analysis::Analyzer* arg_a, HashKey* arg_key) - : Modification(), a(arg_a), key(arg_key) {} + AddMod(file_analysis::Analyzer* arg_a, std::unique_ptr arg_key) + : Modification(), a(arg_a), key(std::move(arg_key)) {} ~AddMod() override {} bool Perform(AnalyzerSet* set) override; void Abort() override; protected: file_analysis::Analyzer* a; - HashKey* key; + std::unique_ptr key; }; /** @@ -191,15 +194,15 @@ private: * @param arg_a an analyzer instance to add to an analyzer set. * @param arg_key hash key representing the analyzer's \c AnalyzerArgs. */ - RemoveMod(const file_analysis::Tag& arg_tag, HashKey* arg_key) - : Modification(), tag(arg_tag), key(arg_key) {} + RemoveMod(const file_analysis::Tag& arg_tag, std::unique_ptr arg_key) + : Modification(), tag(arg_tag), key(std::move(arg_key)) {} ~RemoveMod() override {} bool Perform(AnalyzerSet* set) override; - void Abort() override { delete key; } + void Abort() override {} protected: file_analysis::Tag tag; - HashKey* key; + std::unique_ptr key; }; using ModQueue = std::queue; diff --git a/src/file_analysis/File.cc b/src/file_analysis/File.cc index fabe76c98d..4146bf9dd3 100644 --- a/src/file_analysis/File.cc +++ b/src/file_analysis/File.cc @@ -204,8 +204,12 @@ void File::SetTimeoutInterval(double interval) } bool File::SetExtractionLimit(RecordVal* args, uint64_t bytes) + { return SetExtractionLimit({NewRef{}, args}, bytes); } + +bool File::SetExtractionLimit(IntrusivePtr args, uint64_t bytes) { - Analyzer* a = analyzers.Find(file_mgr->GetComponentTag("EXTRACT"), args); + Analyzer* a = analyzers.Find(file_mgr->GetComponentTag("EXTRACT"), + std::move(args)); if ( ! a ) return false; @@ -250,6 +254,9 @@ void File::ScheduleInactivityTimer() const } bool File::AddAnalyzer(file_analysis::Tag tag, RecordVal* args) + { return AddAnalyzer(tag, {NewRef{}, args}); } + +bool File::AddAnalyzer(file_analysis::Tag tag, IntrusivePtr args) { DBG_LOG(DBG_FILE_ANALYSIS, "[%s] Queuing addition of %s analyzer", id.c_str(), file_mgr->GetComponentName(tag).c_str()); @@ -257,15 +264,18 @@ bool File::AddAnalyzer(file_analysis::Tag tag, RecordVal* args) if ( done ) return false; - return analyzers.QueueAdd(tag, args) != nullptr; + return analyzers.QueueAdd(tag, std::move(args)) != nullptr; } bool File::RemoveAnalyzer(file_analysis::Tag tag, RecordVal* args) + { return RemoveAnalyzer(tag, {NewRef{}, args}); } + +bool File::RemoveAnalyzer(file_analysis::Tag tag, IntrusivePtr args) { DBG_LOG(DBG_FILE_ANALYSIS, "[%s] Queuing remove of %s analyzer", id.c_str(), file_mgr->GetComponentName(tag).c_str()); - return done ? false : analyzers.QueueRemove(tag, args); + return done ? false : analyzers.QueueRemove(tag, std::move(args)); } void File::EnableReassembly() @@ -410,7 +420,7 @@ void File::DeliverStream(const u_char* data, uint64_t len) bof_buffer.chunks[i]->Len()) ) { a->SetSkip(true); - analyzers.QueueRemove(a->Tag(), a->GetArgs().get()); + analyzers.QueueRemove(a->Tag(), a->GetArgs()); } } @@ -427,7 +437,7 @@ void File::DeliverStream(const u_char* data, uint64_t len) if ( ! a->DeliverStream(data, len) ) { a->SetSkip(true); - analyzers.QueueRemove(a->Tag(), a->GetArgs().get()); + analyzers.QueueRemove(a->Tag(), a->GetArgs()); } } } @@ -498,7 +508,7 @@ void File::DeliverChunk(const u_char* data, uint64_t len, uint64_t offset) if ( ! a->DeliverChunk(data, len, offset) ) { a->SetSkip(true); - analyzers.QueueRemove(a->Tag(), a->GetArgs().get()); + analyzers.QueueRemove(a->Tag(), a->GetArgs()); } } } @@ -557,7 +567,7 @@ void File::EndOfFile() while ( (a = analyzers.NextEntry(c)) ) { if ( ! a->EndOfFile() ) - analyzers.QueueRemove(a->Tag(), a->GetArgs().get()); + analyzers.QueueRemove(a->Tag(), a->GetArgs()); } FileEvent(file_state_remove); @@ -590,7 +600,7 @@ void File::Gap(uint64_t offset, uint64_t len) while ( (a = analyzers.NextEntry(c)) ) { if ( ! a->Undelivered(offset, len) ) - analyzers.QueueRemove(a->Tag(), a->GetArgs().get()); + analyzers.QueueRemove(a->Tag(), a->GetArgs()); } if ( FileEventAvailable(file_gap) ) diff --git a/src/file_analysis/File.h b/src/file_analysis/File.h index d201c416af..f7afe45b77 100644 --- a/src/file_analysis/File.h +++ b/src/file_analysis/File.h @@ -74,6 +74,9 @@ public: * @param bytes new limit. * @return false if no extraction analyzer is active, else true. */ + bool SetExtractionLimit(IntrusivePtr args, uint64_t bytes); + + [[deprecated("Remove in v4.1. Pass an IntrusivePtr instead.")]] bool SetExtractionLimit(RecordVal* args, uint64_t bytes); /** @@ -119,6 +122,9 @@ public: * @param args an \c AnalyzerArgs value representing a file analyzer. * @return false if analyzer can't be instantiated, else true. */ + bool AddAnalyzer(file_analysis::Tag tag, IntrusivePtr args); + + [[deprecated("Remove in v4.1. Pass an IntrusivePtr instead.")]] bool AddAnalyzer(file_analysis::Tag tag, RecordVal* args); /** @@ -127,6 +133,9 @@ public: * @param args an \c AnalyzerArgs value representing a file analyzer. * @return true if analyzer was active at time of call, else false. */ + bool RemoveAnalyzer(file_analysis::Tag tag, IntrusivePtr args); + + [[deprecated("Remove in v4.1. Pass an IntrusivePtr instead.")]] bool RemoveAnalyzer(file_analysis::Tag tag, RecordVal* args); /** diff --git a/src/file_analysis/Manager.cc b/src/file_analysis/Manager.cc index b8c643809e..1c2465c415 100644 --- a/src/file_analysis/Manager.cc +++ b/src/file_analysis/Manager.cc @@ -260,35 +260,47 @@ bool Manager::SetReassemblyBuffer(const string& file_id, uint64_t max) bool Manager::SetExtractionLimit(const string& file_id, RecordVal* args, uint64_t n) const + { return SetExtractionLimit(file_id, {NewRef{}, args}, n); } + +bool Manager::SetExtractionLimit(const string& file_id, + IntrusivePtr args, uint64_t n) const { File* file = LookupFile(file_id); if ( ! file ) return false; - return file->SetExtractionLimit(args, n); + return file->SetExtractionLimit(std::move(args), n); } bool Manager::AddAnalyzer(const string& file_id, const file_analysis::Tag& tag, RecordVal* args) const + { return AddAnalyzer(file_id, tag, {NewRef{}, args}); } + +bool Manager::AddAnalyzer(const string& file_id, const file_analysis::Tag& tag, + IntrusivePtr args) const { File* file = LookupFile(file_id); if ( ! file ) return false; - return file->AddAnalyzer(tag, args); + return file->AddAnalyzer(tag, std::move(args)); } bool Manager::RemoveAnalyzer(const string& file_id, const file_analysis::Tag& tag, RecordVal* args) const + { return RemoveAnalyzer(file_id, tag, {NewRef{}, args}); } + +bool Manager::RemoveAnalyzer(const string& file_id, const file_analysis::Tag& tag, + IntrusivePtr args) const { File* file = LookupFile(file_id); if ( ! file ) return false; - return file->RemoveAnalyzer(tag, args); + return file->RemoveAnalyzer(tag, std::move(args)); } File* Manager::GetFile(const string& file_id, Connection* conn, diff --git a/src/file_analysis/Manager.h b/src/file_analysis/Manager.h index 5086ab6e39..4f8874a0a1 100644 --- a/src/file_analysis/Manager.h +++ b/src/file_analysis/Manager.h @@ -253,6 +253,10 @@ public: * @return false if file identifier and analyzer did not map to anything, * else true. */ + bool SetExtractionLimit(const std::string& file_id, + IntrusivePtr args, uint64_t n) const; + + [[deprecated("Remove in v4.1. Pass IntrusivePtr args param instead.")]] bool SetExtractionLimit(const std::string& file_id, RecordVal* args, uint64_t n) const; @@ -273,6 +277,10 @@ public: * @param args a \c AnalyzerArgs value which describes a file analyzer. * @return false if the analyzer failed to be instantiated, else true. */ + bool AddAnalyzer(const std::string& file_id, const file_analysis::Tag& tag, + IntrusivePtr args) const; + + [[deprecated("Remove in v4.1. Pass IntrusivePtr args param instead.")]] bool AddAnalyzer(const std::string& file_id, const file_analysis::Tag& tag, RecordVal* args) const; @@ -283,6 +291,10 @@ public: * @param args a \c AnalyzerArgs value which describes a file analyzer. * @return true if the analyzer is active at the time of call, else false. */ + bool RemoveAnalyzer(const std::string& file_id, const file_analysis::Tag& tag, + IntrusivePtr args) const; + + [[deprecated("Remove in v4.1. Pass IntrusivePtr args param instead.")]] bool RemoveAnalyzer(const std::string& file_id, const file_analysis::Tag& tag, RecordVal* args) const; diff --git a/src/file_analysis/analyzer/extract/functions.bif b/src/file_analysis/analyzer/extract/functions.bif index 13cc904c4f..9d76849f23 100644 --- a/src/file_analysis/analyzer/extract/functions.bif +++ b/src/file_analysis/analyzer/extract/functions.bif @@ -12,7 +12,8 @@ function FileExtract::__set_limit%(file_id: string, args: any, n: count%): bool %{ using zeek::BifType::Record::Files::AnalyzerArgs; auto rv = args->AsRecordVal()->CoerceTo(AnalyzerArgs); - bool result = file_mgr->SetExtractionLimit(file_id->CheckString(), rv.get(), n); + bool result = file_mgr->SetExtractionLimit(file_id->CheckString(), + std::move(rv), n); return val_mgr->Bool(result); %} diff --git a/src/file_analysis/file_analysis.bif b/src/file_analysis/file_analysis.bif index 909e9ed20f..7f60bc2845 100644 --- a/src/file_analysis/file_analysis.bif +++ b/src/file_analysis/file_analysis.bif @@ -44,7 +44,8 @@ function Files::__add_analyzer%(file_id: string, tag: Files::Tag, args: any%): b using zeek::BifType::Record::Files::AnalyzerArgs; auto rv = args->AsRecordVal()->CoerceTo(AnalyzerArgs); bool result = file_mgr->AddAnalyzer(file_id->CheckString(), - file_mgr->GetComponentTag(tag), rv.get()); + file_mgr->GetComponentTag(tag), + std::move(rv)); return val_mgr->Bool(result); %} @@ -54,7 +55,8 @@ function Files::__remove_analyzer%(file_id: string, tag: Files::Tag, args: any%) using zeek::BifType::Record::Files::AnalyzerArgs; auto rv = args->AsRecordVal()->CoerceTo(AnalyzerArgs); bool result = file_mgr->RemoveAnalyzer(file_id->CheckString(), - file_mgr->GetComponentTag(tag) , rv.get()); + file_mgr->GetComponentTag(tag), + std::move(rv)); return val_mgr->Bool(result); %}