From de47a50dde73b59f293e34e76431992dfa7dd2b4 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Wed, 25 Mar 2020 16:08:32 -0700 Subject: [PATCH] Deprecate Connection::ConnectionEvent methods And update usages to Connection::EnqueueEvent --- NEWS | 4 +++ src/Conn.cc | 66 +++++++++++++++++++++------------------- src/Conn.h | 22 +++++++++++--- src/Reporter.cc | 2 +- src/analyzer/Analyzer.cc | 18 +++++++++-- src/analyzer/Analyzer.h | 9 ++++++ 6 files changed, 80 insertions(+), 41 deletions(-) diff --git a/NEWS b/NEWS index ae171789fd..4fdc6e7a27 100644 --- a/NEWS +++ b/NEWS @@ -55,6 +55,10 @@ Deprecated Functionality - The ``EventMgr::QueueEvent()`` and EventMgr::QueueEventFast()`` methods are now deprecated, use ``EventMgr::Enqueue()`` instead. +- The ``Connection::ConnectionEvent()`` and + ``Connection::ConnectionEventFast()`` methods are now deprecated, use + ``Connection::EnqueueEvent()`` instead. + Zeek 3.1.0 ========== diff --git a/src/Conn.cc b/src/Conn.cc index d512f32166..a8987b30f5 100644 --- a/src/Conn.cc +++ b/src/Conn.cc @@ -203,7 +203,7 @@ void Connection::NextPacket(double t, int is_orig, is_successful = true; if ( ! was_successful && is_successful && connection_successful ) - ConnectionEventFast(connection_successful, nullptr, {BuildConnVal()}); + EnqueueEvent(connection_successful, nullptr, IntrusivePtr{AdoptRef{}, BuildConnVal()}); } else last_time = t; @@ -259,11 +259,11 @@ void Connection::HistoryThresholdEvent(EventHandlerPtr e, bool is_orig, // and at this stage it's not a *multiple* instance. return; - ConnectionEventFast(e, 0, { - BuildConnVal(), - val_mgr->GetBool(is_orig), - val_mgr->GetCount(threshold) - }); + EnqueueEvent(e, nullptr, + IntrusivePtr{AdoptRef{}, BuildConnVal()}, + IntrusivePtr{AdoptRef{}, val_mgr->GetBool(is_orig)}, + IntrusivePtr{AdoptRef{}, val_mgr->GetCount(threshold)} + ); } void Connection::DeleteTimer(double /* t */) @@ -323,7 +323,7 @@ void Connection::EnableStatusUpdateTimer() void Connection::StatusUpdateTimer(double t) { - ConnectionEventFast(connection_status_update, 0, { BuildConnVal() }); + EnqueueEvent(connection_status_update, nullptr, IntrusivePtr{AdoptRef{}, BuildConnVal()}); ADD_TIMER(&Connection::StatusUpdateTimer, network_time + connection_status_update_interval, 0, TIMER_CONN_STATUS_UPDATE); @@ -446,15 +446,13 @@ void Connection::Match(Rule::PatternType type, const u_char* data, int len, bool void Connection::RemovalEvent() { - auto cv = BuildConnVal(); + auto cv = IntrusivePtr{AdoptRef{}, BuildConnVal()}; if ( connection_state_remove ) - ConnectionEventFast(connection_state_remove, nullptr, {cv->Ref()}); + EnqueueEvent(connection_state_remove, nullptr, cv); if ( is_successful && successful_connection_remove ) - ConnectionEventFast(successful_connection_remove, nullptr, {cv->Ref()}); - - Unref(cv); + EnqueueEvent(successful_connection_remove, nullptr, cv); } void Connection::Event(EventHandlerPtr f, analyzer::Analyzer* analyzer, const char* name) @@ -463,10 +461,9 @@ void Connection::Event(EventHandlerPtr f, analyzer::Analyzer* analyzer, const ch return; if ( name ) - ConnectionEventFast(f, analyzer, {new StringVal(name), BuildConnVal()}); + EnqueueEvent(f, analyzer, make_intrusive(name), IntrusivePtr{AdoptRef{}, BuildConnVal()}); else - ConnectionEventFast(f, analyzer, {BuildConnVal()}); - + EnqueueEvent(f, analyzer, IntrusivePtr{AdoptRef{}, BuildConnVal()}); } void Connection::Event(EventHandlerPtr f, analyzer::Analyzer* analyzer, Val* v1, Val* v2) @@ -479,25 +476,27 @@ void Connection::Event(EventHandlerPtr f, analyzer::Analyzer* analyzer, Val* v1, } if ( v2 ) - ConnectionEventFast(f, analyzer, {BuildConnVal(), v1, v2}); + EnqueueEvent(f, analyzer, + IntrusivePtr{AdoptRef{}, BuildConnVal()}, + IntrusivePtr{AdoptRef{}, v1}, + IntrusivePtr{AdoptRef{}, v2}); else - ConnectionEventFast(f, analyzer, {BuildConnVal(), v1}); + EnqueueEvent(f, analyzer, + IntrusivePtr{AdoptRef{}, BuildConnVal()}, + IntrusivePtr{AdoptRef{}, v1}); } void Connection::ConnectionEvent(EventHandlerPtr f, analyzer::Analyzer* a, val_list vl) { + auto args = zeek::val_list_to_args(&vl); + if ( ! f ) - { // This may actually happen if there is no local handler // and a previously existing remote handler went away. - for ( const auto& v : vl) - Unref(v); - return; - } // "this" is passed as a cookie for the event - mgr.Enqueue(f, zeek::val_list_to_args(&vl), SOURCE_LOCAL, + mgr.Enqueue(f, std::move(args), SOURCE_LOCAL, a ? a->GetID() : 0, timer_mgr, this); } @@ -510,12 +509,15 @@ void Connection::ConnectionEventFast(EventHandlerPtr f, analyzer::Analyzer* a, v void Connection::ConnectionEvent(EventHandlerPtr f, analyzer::Analyzer* a, val_list* vl) { - ConnectionEvent(f, a, std::move(*vl)); + auto args = zeek::val_list_to_args(vl); delete vl; + + if ( f ) + EnqueueEvent(f, a, std::move(args)); } -void Connection::EnqueueEvent(EventHandlerPtr f, zeek::Args args, - analyzer::Analyzer* a) +void Connection::EnqueueEvent(EventHandlerPtr f, analyzer::Analyzer* a, + zeek::Args args) { // "this" is passed as a cookie for the event mgr.Enqueue(f, std::move(args), SOURCE_LOCAL, a ? a->GetID() : 0, @@ -696,12 +698,12 @@ void Connection::CheckFlowLabel(bool is_orig, uint32_t flow_label) if ( connection_flow_label_changed && (is_orig ? saw_first_orig_packet : saw_first_resp_packet) ) { - ConnectionEventFast(connection_flow_label_changed, 0, { - BuildConnVal(), - val_mgr->GetBool(is_orig), - val_mgr->GetCount(my_flow_label), - val_mgr->GetCount(flow_label), - }); + EnqueueEvent(connection_flow_label_changed, 0, + IntrusivePtr{AdoptRef{}, BuildConnVal()}, + IntrusivePtr{AdoptRef{}, val_mgr->GetBool(is_orig)}, + IntrusivePtr{AdoptRef{}, val_mgr->GetCount(my_flow_label)}, + IntrusivePtr{AdoptRef{}, val_mgr->GetCount(flow_label)} + ); } my_flow_label = flow_label; diff --git a/src/Conn.h b/src/Conn.h index 53f8ff74a7..3511275040 100644 --- a/src/Conn.h +++ b/src/Conn.h @@ -5,6 +5,8 @@ #include #include +#include +#include #include "Dict.h" #include "Timer.h" @@ -188,7 +190,7 @@ public: // If a handler exists for 'f', an event will be generated. In any case, // reference count for each element in the 'vl' list are decremented. The // arguments used for the event are whatevever is provided in 'vl'. - // TODO: deprecate + [[deprecated("Remove in v4.1. Use EnqueueEvent() instead.")]] void ConnectionEvent(EventHandlerPtr f, analyzer::Analyzer* analyzer, val_list vl); @@ -196,7 +198,7 @@ public: // pointer instead of by value. This function takes ownership of the // memory pointed to by 'vl' and also for decrementing the reference count // of each of its elements. - // TODO: deprecate + [[deprecated("Remove in v4.1. Use EnqueueEvent() instead.")]] void ConnectionEvent(EventHandlerPtr f, analyzer::Analyzer* analyzer, val_list* vl); @@ -208,15 +210,25 @@ public: // the case where there's no handlers (one usually also does that because // it would be a waste of effort to construct all the event arguments when // there's no handlers to consume them). - // TODO: deprecate + [[deprecated("Remove in v4.1. Use EnqueueEvent() instead.")]] void ConnectionEventFast(EventHandlerPtr f, analyzer::Analyzer* analyzer, val_list vl); /** * Enqueues an event associated with this connection and given analyzer. */ - void EnqueueEvent(EventHandlerPtr f, zeek::Args args, - analyzer::Analyzer* analyzer = nullptr); + void EnqueueEvent(EventHandlerPtr f, analyzer::Analyzer* analyzer, + zeek::Args args); + + /** + * A version of EnqueueEvent() taking a variable number of arguments. + */ + template + std::enable_if_t< + std::is_convertible_v< + std::tuple_element_t<0, std::tuple>, IntrusivePtr>> + EnqueueEvent(EventHandlerPtr h, analyzer::Analyzer* analyzer, Args&&... args) + { return EnqueueEvent(h, analyzer, zeek::Args{std::forward(args)...}); } void Weird(const char* name, const char* addl = ""); bool DidWeird() const { return weird != 0; } diff --git a/src/Reporter.cc b/src/Reporter.cc index 48aaa79469..14434146cc 100644 --- a/src/Reporter.cc +++ b/src/Reporter.cc @@ -499,7 +499,7 @@ void Reporter::DoLog(const char* prefix, EventHandlerPtr event, FILE* out, vl.emplace_back(AdoptRef{}, v); if ( conn ) - conn->EnqueueEvent(event, std::move(vl)); + conn->EnqueueEvent(event, nullptr, std::move(vl)); else mgr.Enqueue(event, std::move(vl)); } diff --git a/src/analyzer/Analyzer.cc b/src/analyzer/Analyzer.cc index 6a32948600..41a98273e2 100644 --- a/src/analyzer/Analyzer.cc +++ b/src/analyzer/Analyzer.cc @@ -803,17 +803,29 @@ void Analyzer::Event(EventHandlerPtr f, Val* v1, Val* v2) void Analyzer::ConnectionEvent(EventHandlerPtr f, val_list* vl) { - conn->ConnectionEvent(f, this, vl); + auto args = zeek::val_list_to_args(vl); + + if ( f ) + conn->EnqueueEvent(f, this, std::move(args)); } void Analyzer::ConnectionEvent(EventHandlerPtr f, val_list vl) { - conn->ConnectionEvent(f, this, std::move(vl)); + auto args = zeek::val_list_to_args(&vl); + + if ( f ) + conn->EnqueueEvent(f, this, std::move(args)); } void Analyzer::ConnectionEventFast(EventHandlerPtr f, val_list vl) { - conn->ConnectionEventFast(f, this, std::move(vl)); + auto args = zeek::val_list_to_args(&vl); + conn->EnqueueEvent(f, this, std::move(args)); + } + +void Analyzer::EnqueueConnEvent(EventHandlerPtr f, zeek::Args args) + { + conn->EnqueueEvent(f, this, std::move(args)); } void Analyzer::Weird(const char* name, const char* addl) diff --git a/src/analyzer/Analyzer.h b/src/analyzer/Analyzer.h index 1a94ad3cf2..96e51a2b25 100644 --- a/src/analyzer/Analyzer.h +++ b/src/analyzer/Analyzer.h @@ -567,20 +567,29 @@ public: * Convenience function that forwards directly to * Connection::ConnectionEvent(). */ + // TODO: deprecate void ConnectionEvent(EventHandlerPtr f, val_list* vl); /** * Convenience function that forwards directly to * Connection::ConnectionEvent(). */ + // TODO: deprecate void ConnectionEvent(EventHandlerPtr f, val_list vl); /** * Convenience function that forwards directly to * Connection::ConnectionEventFast(). */ + // TODO: deprecate void ConnectionEventFast(EventHandlerPtr f, val_list vl); + /** + * Convenience function that forwards directly to + * Connection::EnqueueEvent(). + */ + void EnqueueConnEvent(EventHandlerPtr f, zeek::Args args); + /** * Convenience function that forwards directly to the corresponding * Connection::Weird().