diff --git a/NEWS b/NEWS index d3777ae5f7..f4a0707596 100644 --- a/NEWS +++ b/NEWS @@ -12,6 +12,11 @@ New Functionality Changed Functionality --------------------- +- Several C++ functions have been changed to pass smart pointers + (`class IntrusivePtr<>`) instead of raw pointers. This makes the + code more robust. External plugins may need to be updated to this + API change. + Removed Functionality --------------------- diff --git a/src/OpaqueVal.cc b/src/OpaqueVal.cc index 96f67f55a8..56f12451ee 100644 --- a/src/OpaqueVal.cc +++ b/src/OpaqueVal.cc @@ -56,7 +56,7 @@ const std::string& OpaqueMgr::TypeID(const OpaqueVal* v) const return x->first; } -OpaqueVal* OpaqueMgr::Instantiate(const std::string& id) const +IntrusivePtr OpaqueMgr::Instantiate(const std::string& id) const { auto x = _types.find(id); return x != _types.end() ? (*x->second)() : nullptr; @@ -73,7 +73,7 @@ broker::expected OpaqueVal::Serialize() const return {broker::vector{std::move(type), std::move(*d)}}; } -OpaqueVal* OpaqueVal::Unserialize(const broker::data& data) +IntrusivePtr OpaqueVal::Unserialize(const broker::data& data) { auto v = caf::get_if(&data); @@ -90,7 +90,6 @@ OpaqueVal* OpaqueVal::Unserialize(const broker::data& data) if ( ! val->DoUnserialize((*v)[1]) ) { - Unref(val); return nullptr; } @@ -154,7 +153,7 @@ Val* OpaqueVal::DoClone(CloneState* state) return nullptr; auto rval = OpaqueVal::Unserialize(std::move(*d)); - return state->NewClone(this, rval); + return state->NewClone(this, rval.release()); } bool HashVal::IsValid() const @@ -171,12 +170,13 @@ bool HashVal::Init() return valid; } -StringVal* HashVal::Get() +IntrusivePtr HashVal::Get() { if ( ! valid ) - return val_mgr->GetEmptyString(); + return IntrusivePtr(AdoptRef{}, + val_mgr->GetEmptyString()); - StringVal* result = DoGet(); + auto result = DoGet(); valid = false; return result; } @@ -202,10 +202,10 @@ bool HashVal::DoFeed(const void*, size_t) return false; } -StringVal* HashVal::DoGet() +IntrusivePtr HashVal::DoGet() { assert(! "missing implementation of DoGet()"); - return val_mgr->GetEmptyString(); + return IntrusivePtr(AdoptRef{}, val_mgr->GetEmptyString()); } HashVal::HashVal(OpaqueType* t) : OpaqueVal(t) @@ -285,14 +285,14 @@ bool MD5Val::DoFeed(const void* data, size_t size) return true; } -StringVal* MD5Val::DoGet() +IntrusivePtr MD5Val::DoGet() { if ( ! IsValid() ) - return val_mgr->GetEmptyString(); + return IntrusivePtr(AdoptRef{}, val_mgr->GetEmptyString()); u_char digest[MD5_DIGEST_LENGTH]; hash_final(ctx, digest); - return new StringVal(md5_digest_print(digest)); + return make_intrusive(md5_digest_print(digest)); } IMPLEMENT_OPAQUE_VALUE(MD5Val) @@ -425,14 +425,15 @@ bool SHA1Val::DoFeed(const void* data, size_t size) return true; } -StringVal* SHA1Val::DoGet() +IntrusivePtr SHA1Val::DoGet() { if ( ! IsValid() ) - return val_mgr->GetEmptyString(); + return IntrusivePtr(AdoptRef{}, + val_mgr->GetEmptyString()); u_char digest[SHA_DIGEST_LENGTH]; hash_final(ctx, digest); - return new StringVal(sha1_digest_print(digest)); + return make_intrusive(sha1_digest_print(digest)); } IMPLEMENT_OPAQUE_VALUE(SHA1Val) @@ -568,14 +569,15 @@ bool SHA256Val::DoFeed(const void* data, size_t size) return true; } -StringVal* SHA256Val::DoGet() +IntrusivePtr SHA256Val::DoGet() { if ( ! IsValid() ) - return val_mgr->GetEmptyString(); + return IntrusivePtr(AdoptRef{}, + val_mgr->GetEmptyString()); u_char digest[SHA256_DIGEST_LENGTH]; hash_final(ctx, digest); - return new StringVal(sha256_digest_print(digest)); + return make_intrusive(sha256_digest_print(digest)); } IMPLEMENT_OPAQUE_VALUE(SHA256Val) @@ -833,8 +835,8 @@ string BloomFilterVal::InternalState() const return bloom_filter->InternalState(); } -BloomFilterVal* BloomFilterVal::Merge(const BloomFilterVal* x, - const BloomFilterVal* y) +IntrusivePtr BloomFilterVal::Merge(const BloomFilterVal* x, + const BloomFilterVal* y) { if ( x->Type() && // any one 0 is ok here y->Type() && @@ -859,11 +861,10 @@ BloomFilterVal* BloomFilterVal::Merge(const BloomFilterVal* x, return 0; } - BloomFilterVal* merged = new BloomFilterVal(copy); + auto merged = make_intrusive(copy); if ( x->Type() && ! merged->Typify(x->Type()) ) { - Unref(merged); reporter->Error("failed to set type on merged Bloom filter"); return 0; } @@ -1035,9 +1036,9 @@ ParaglobVal::ParaglobVal(std::unique_ptr p) this->internal_paraglob = std::move(p); } -VectorVal* ParaglobVal::Get(StringVal* &pattern) +IntrusivePtr ParaglobVal::Get(StringVal* &pattern) { - VectorVal* rval = new VectorVal(internal_type("string_vec")->AsVectorType()); + auto rval = make_intrusive(internal_type("string_vec")->AsVectorType()); std::string string_pattern (reinterpret_cast(pattern->Bytes()), pattern->Len()); std::vector matches = this->internal_paraglob->get(string_pattern); diff --git a/src/OpaqueVal.h b/src/OpaqueVal.h index 835bc0bb3f..4b1bae4f61 100644 --- a/src/OpaqueVal.h +++ b/src/OpaqueVal.h @@ -2,6 +2,7 @@ #pragma once +#include "IntrusivePtr.h" #include "RandTest.h" #include "Val.h" #include "digest.h" @@ -20,7 +21,7 @@ class OpaqueVal; */ class OpaqueMgr { public: - using Factory = OpaqueVal* (); + using Factory = IntrusivePtr (); /** * Return's a unique ID for the type of an opaque value. @@ -44,7 +45,7 @@ public: * is unknown, this will return null. * */ - OpaqueVal* Instantiate(const std::string& id) const; + IntrusivePtr Instantiate(const std::string& id) const; /** Returns the global manager singleton object. */ static OpaqueMgr* mgr(); @@ -67,10 +68,11 @@ private: /** Macro to insert into an OpaqueVal-derived class's declaration. */ #define DECLARE_OPAQUE_VALUE(T) \ friend class OpaqueMgr::Register; \ + friend IntrusivePtr make_intrusive(); \ broker::expected DoSerialize() const override; \ bool DoUnserialize(const broker::data& data) override; \ const char* OpaqueName() const override { return #T; } \ - static OpaqueVal* OpaqueInstantiate() { return new T(); } + static IntrusivePtr OpaqueInstantiate() { return make_intrusive(); } #define __OPAQUE_MERGE(a, b) a ## b #define __OPAQUE_ID(x) __OPAQUE_MERGE(_opaque, x) @@ -102,7 +104,7 @@ public: * @param data Broker representation as returned by *Serialize()*. * @return unserialized instances with reference count at +1 */ - static OpaqueVal* Unserialize(const broker::data& data); + static IntrusivePtr Unserialize(const broker::data& data); protected: friend class Val; @@ -163,7 +165,7 @@ public: bool IsValid() const; bool Init(); bool Feed(const void* data, size_t size); - StringVal* Get(); + IntrusivePtr Get(); protected: HashVal() { valid = false; } @@ -171,7 +173,7 @@ protected: virtual bool DoInit(); virtual bool DoFeed(const void* data, size_t size); - virtual StringVal* DoGet(); + virtual IntrusivePtr DoGet(); private: // This flag exists because Get() can only be called once. @@ -196,7 +198,7 @@ protected: bool DoInit() override; bool DoFeed(const void* data, size_t size) override; - StringVal* DoGet() override; + IntrusivePtr DoGet() override; DECLARE_OPAQUE_VALUE(MD5Val) private: @@ -217,7 +219,7 @@ protected: bool DoInit() override; bool DoFeed(const void* data, size_t size) override; - StringVal* DoGet() override; + IntrusivePtr DoGet() override; DECLARE_OPAQUE_VALUE(SHA1Val) private: @@ -238,7 +240,7 @@ protected: bool DoInit() override; bool DoFeed(const void* data, size_t size) override; - StringVal* DoGet() override; + IntrusivePtr DoGet() override; DECLARE_OPAQUE_VALUE(SHA256Val) private: @@ -277,8 +279,8 @@ public: bool Empty() const; string InternalState() const; - static BloomFilterVal* Merge(const BloomFilterVal* x, - const BloomFilterVal* y); + static IntrusivePtr Merge(const BloomFilterVal* x, + const BloomFilterVal* y); protected: friend class Val; @@ -324,7 +326,7 @@ private: class ParaglobVal : public OpaqueVal { public: explicit ParaglobVal(std::unique_ptr p); - VectorVal* Get(StringVal* &pattern); + IntrusivePtr Get(StringVal* &pattern); Val* DoClone(CloneState* state) override; bool operator==(const ParaglobVal& other) const; diff --git a/src/broker/Data.cc b/src/broker/Data.cc index 513d13bfd6..7a92998fd2 100644 --- a/src/broker/Data.cc +++ b/src/broker/Data.cc @@ -442,7 +442,7 @@ struct val_converter { return rval; } else if ( type->Tag() == TYPE_OPAQUE ) - return OpaqueVal::Unserialize(a); + return OpaqueVal::Unserialize(a).release(); return nullptr; } @@ -772,9 +772,7 @@ struct type_checker { // TODO: Could avoid doing the full unserialization here // and just check if the type is a correct match. auto ov = OpaqueVal::Unserialize(a); - auto rval = ov != nullptr; - Unref(ov); - return rval; + return ov != nullptr; } return false; diff --git a/src/broker/data.bif b/src/broker/data.bif index f0862c0f66..15fa215756 100644 --- a/src/broker/data.bif +++ b/src/broker/data.bif @@ -52,7 +52,7 @@ function Broker::__opaque_clone_through_serialization%(d: any%): any return val_mgr->GetFalse(); } - return OpaqueVal::Unserialize(std::move(*x)); + return OpaqueVal::Unserialize(std::move(*x)).release(); %} function Broker::__set_create%(%): Broker::Data diff --git a/src/file_analysis/analyzer/hash/Hash.cc b/src/file_analysis/analyzer/hash/Hash.cc index 0d5787e30c..03d5ba801d 100644 --- a/src/file_analysis/analyzer/hash/Hash.cc +++ b/src/file_analysis/analyzer/hash/Hash.cc @@ -54,6 +54,6 @@ void Hash::Finalize() mgr.QueueEventFast(file_hash, { GetFile()->GetVal()->Ref(), new StringVal(kind), - hash->Get(), + hash->Get().release(), }); } diff --git a/src/probabilistic/bloom-filter.bif b/src/probabilistic/bloom-filter.bif index 5f88f36799..f2f627a6d2 100644 --- a/src/probabilistic/bloom-filter.bif +++ b/src/probabilistic/bloom-filter.bif @@ -233,7 +233,7 @@ function bloomfilter_merge%(bf1: opaque of bloomfilter, return 0; } - return BloomFilterVal::Merge(bfv1, bfv2); + return BloomFilterVal::Merge(bfv1, bfv2).release(); %} ## Returns a string with a representation of a Bloom filter's internal diff --git a/src/zeek.bif b/src/zeek.bif index ffa5acee02..272e1aa117 100644 --- a/src/zeek.bif +++ b/src/zeek.bif @@ -762,7 +762,7 @@ function sha256_hash_update%(handle: opaque of sha256, data: string%): bool ## sha256_hash sha256_hash_init sha256_hash_update sha256_hash_finish function md5_hash_finish%(handle: opaque of md5%): string %{ - return static_cast(handle)->Get(); + return static_cast(handle)->Get().release(); %} ## Returns the final SHA1 digest of an incremental hash computation. @@ -776,7 +776,7 @@ function md5_hash_finish%(handle: opaque of md5%): string ## sha256_hash sha256_hash_init sha256_hash_update sha256_hash_finish function sha1_hash_finish%(handle: opaque of sha1%): string %{ - return static_cast(handle)->Get(); + return static_cast(handle)->Get().release(); %} ## Returns the final SHA256 digest of an incremental hash computation. @@ -790,7 +790,7 @@ function sha1_hash_finish%(handle: opaque of sha1%): string ## sha256_hash sha256_hash_init sha256_hash_update function sha256_hash_finish%(handle: opaque of sha256%): string %{ - return static_cast(handle)->Get(); + return static_cast(handle)->Get().release(); %} ## Initializes and returns a new paraglob. @@ -842,7 +842,7 @@ function paraglob_init%(v: any%) : opaque of paraglob ## ## .. zeek:see::paraglob_add paraglob_equals paraglob_init function paraglob_match%(handle: opaque of paraglob, match: string%): string_vec %{ - return static_cast(handle)->Get(match); + return static_cast(handle)->Get(match).release(); %} ## Compares two paraglobs for equality.