From 1465e390a2e8538fe4064f724b2f91d16b2a8517 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Mon, 19 May 2025 10:03:40 +0200 Subject: [PATCH 1/4] EventTraceMgr: Move fclose() to destructor Coverity complains about a missing fclose() in a non-existing destructor. Also sprinkle in a strerror() call for fopen() to provide a bit of a hint what might have gone wrong. --- src/EventTrace.cc | 14 +++++++++++--- src/EventTrace.h | 2 ++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/EventTrace.cc b/src/EventTrace.cc index 09eda0cb5e..b240334c30 100644 --- a/src/EventTrace.cc +++ b/src/EventTrace.cc @@ -955,7 +955,17 @@ bool ValTraceMgr::IsUnsupported(const Val* v) const { EventTraceMgr::EventTraceMgr(const std::string& trace_file) { f = fopen(trace_file.c_str(), "w"); if ( ! f ) - reporter->FatalError("can't open event trace file %s", trace_file.c_str()); + reporter->FatalError("can't open event trace file %s: %s", trace_file.c_str(), strerror(errno)); +} + +EventTraceMgr::~EventTraceMgr() { + if ( f ) { + if ( fclose(f) ) + // Not fatal, won't do anything with it anymore anyhow. + reporter->Error("failed to close event trace file: %s", strerror(errno)); + + f = nullptr; + } } void EventTraceMgr::Generate() { @@ -999,8 +1009,6 @@ void EventTraceMgr::Generate() { for ( auto& c : c_t ) fprintf(f, "#\t%s\n", c.c_str()); } - - fclose(f); } void EventTraceMgr::StartEvent(const ScriptFunc* ev, const zeek::Args* args) { diff --git a/src/EventTrace.h b/src/EventTrace.h index 710f2680b7..8474733f77 100644 --- a/src/EventTrace.h +++ b/src/EventTrace.h @@ -441,6 +441,8 @@ class EventTraceMgr { public: EventTraceMgr(const std::string& trace_file); + ~EventTraceMgr(); + // Generates the trace upon exit. void Generate(); From 5bcf6bec52a717e3f09351c6baec7b834dae60d1 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Mon, 19 May 2025 10:09:25 +0200 Subject: [PATCH 2/4] EventTraceMgr: Rename etm to event_trace_mgr Mostly to avoid having new maintainers/developers knowing about yet another abbreviation. --- src/EventTrace.cc | 2 +- src/EventTrace.h | 2 +- src/Expr.cc | 8 ++++---- src/Func.cc | 8 ++++---- src/Stmt.cc | 4 ++-- src/zeek-setup.cc | 6 +++--- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/EventTrace.cc b/src/EventTrace.cc index b240334c30..6bdd083654 100644 --- a/src/EventTrace.cc +++ b/src/EventTrace.cc @@ -13,7 +13,7 @@ namespace zeek::detail { -std::unique_ptr etm; +std::unique_ptr event_trace_mgr; // Helper function for generating a correct script-level representation // of a string constant. diff --git a/src/EventTrace.h b/src/EventTrace.h index 8474733f77..2c86efb7b2 100644 --- a/src/EventTrace.h +++ b/src/EventTrace.h @@ -467,6 +467,6 @@ private: }; // If non-nil then we're doing event tracing. -extern std::unique_ptr etm; +extern std::unique_ptr event_trace_mgr; } // namespace zeek::detail diff --git a/src/Expr.cc b/src/Expr.cc index 9e872d846f..b21c7181b5 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -3868,8 +3868,8 @@ ValPtr ScheduleExpr::Eval(Frame* f) const { if ( args ) { auto handler = event->Handler(); - if ( etm ) - etm->ScriptEventQueued(handler); + if ( event_trace_mgr ) + event_trace_mgr->ScriptEventQueued(handler); timer_mgr->Add(new ScheduleTimer(handler, std::move(*args), dt)); } @@ -4474,8 +4474,8 @@ ValPtr EventExpr::Eval(Frame* f) const { auto v = eval_list(f, args.get()); if ( handler ) { - if ( etm ) - etm->ScriptEventQueued(handler); + if ( event_trace_mgr ) + event_trace_mgr->ScriptEventQueued(handler); event_mgr.Enqueue(handler, std::move(*v)); } diff --git a/src/Func.cc b/src/Func.cc index e45a4fdcca..c9f116dfb6 100644 --- a/src/Func.cc +++ b/src/Func.cc @@ -354,8 +354,8 @@ ValPtr ScriptFunc::Invoke(zeek::Args* args, Frame* parent) const { return nullptr; } - if ( etm && Flavor() == FUNC_FLAVOR_EVENT ) - etm->StartEvent(this, args); + if ( event_trace_mgr && Flavor() == FUNC_FLAVOR_EVENT ) + event_trace_mgr->StartEvent(this, args); if ( g_trace_state.DoTrace() ) { ODesc d; @@ -432,8 +432,8 @@ ValPtr ScriptFunc::Invoke(zeek::Args* args, Frame* parent) const { result = val_mgr->True(); } - else if ( etm && Flavor() == FUNC_FLAVOR_EVENT ) - etm->EndEvent(this, args); + else if ( event_trace_mgr && Flavor() == FUNC_FLAVOR_EVENT ) + event_trace_mgr->EndEvent(this, args); // Warn if the function returns something, but we returned from // the function without an explicit return, or without a value. diff --git a/src/Stmt.cc b/src/Stmt.cc index 47e839e189..4dbcbf18ce 100644 --- a/src/Stmt.cc +++ b/src/Stmt.cc @@ -905,8 +905,8 @@ ValPtr EventStmt::Exec(Frame* f, StmtFlowType& flow) { auto h = event_expr->Handler(); if ( args && h ) { - if ( etm ) - etm->ScriptEventQueued(h); + if ( event_trace_mgr ) + event_trace_mgr->ScriptEventQueued(h); event_mgr.Enqueue(h, std::move(*args)); } diff --git a/src/zeek-setup.cc b/src/zeek-setup.cc index 0055cf1b3a..b2fd19431c 100644 --- a/src/zeek-setup.cc +++ b/src/zeek-setup.cc @@ -400,8 +400,8 @@ static void terminate_zeek() { script_coverage_mgr.WriteStats(); - if ( etm ) - etm->Generate(); + if ( event_trace_mgr ) + event_trace_mgr->Generate(); delete zeekygen_mgr; delete packet_mgr; @@ -774,7 +774,7 @@ SetupResult setup(int argc, char** argv, Options* zopts) { auto ipbb = make_intrusive(init_bifs, ipbid->Name(), false); if ( options.event_trace_file ) - etm = std::make_unique(*options.event_trace_file); + event_trace_mgr = std::make_unique(*options.event_trace_file); // Parsing involves reading input files, including any input // interactively provided by the user at the console. Temporarily From eeb08f6ba8531e06b546442ecd68b9b24d0d1e62 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Mon, 19 May 2025 10:13:39 +0200 Subject: [PATCH 3/4] zeek-setup: Free event_trace_mgr after generating trace While it'd be destructed due to being a global unique_ptr, force it to happen right after generating the trace. --- src/zeek-setup.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/zeek-setup.cc b/src/zeek-setup.cc index b2fd19431c..54acd9ffce 100644 --- a/src/zeek-setup.cc +++ b/src/zeek-setup.cc @@ -400,8 +400,10 @@ static void terminate_zeek() { script_coverage_mgr.WriteStats(); - if ( event_trace_mgr ) + if ( event_trace_mgr ) { event_trace_mgr->Generate(); + event_trace_mgr.reset(); + } delete zeekygen_mgr; delete packet_mgr; From 000cc5081316b83757ed8569caed2f5af4760d69 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Mon, 19 May 2025 18:09:46 +0200 Subject: [PATCH 4/4] btest/core: Add event-trace test --- .../btest/Baseline/core.event-trace/.stderr | 1 + testing/btest/core/event-trace.zeek | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 testing/btest/Baseline/core.event-trace/.stderr create mode 100644 testing/btest/core/event-trace.zeek diff --git a/testing/btest/Baseline/core.event-trace/.stderr b/testing/btest/Baseline/core.event-trace/.stderr new file mode 100644 index 0000000000..49d861c74c --- /dev/null +++ b/testing/btest/Baseline/core.event-trace/.stderr @@ -0,0 +1 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. diff --git a/testing/btest/core/event-trace.zeek b/testing/btest/core/event-trace.zeek new file mode 100644 index 0000000000..eeeeef61b7 --- /dev/null +++ b/testing/btest/core/event-trace.zeek @@ -0,0 +1,21 @@ +# @TEST-DOC: Verify the --event-trace feature works and produces the same logs as when reading from a pcap. +# +# Trace files produced with ZAM don't work - issue #4478 +# +# @TEST-REQUIRES: test "${ZEEK_ZAM}" != "1" +# +# @TEST-EXEC: zeek --event-trace trace.zeek -b -r $TRACES/http/get.trace %INPUT +# @TEST-EXEC: mkdir pcap-logs +# @TEST-EXEC: zeek-cut -m < http.log > pcap-logs/http.log +# @TEST-EXEC: rm -v *.log +# +# @TEST-EXEC: zeek -b --parse-only %INPUT trace.zeek +# @TEST-EXEC: zeek -b %INPUT trace.zeek +# @TEST-EXEC: mkdir trace-logs +# @TEST-EXEC: zeek-cut -m < http.log > trace-logs/http.log +# @TEST-EXEC: rm -v *.log +# +# @TEST-EXEC: diff pcap-logs/http.log trace-logs/http.log +# @TEST-EXEC: btest-diff .stderr + +@load base/protocols/http