diff --git a/CHANGES b/CHANGES index e60486f145..10e5f073df 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,31 @@ +8.0.0-dev.730 | 2025-07-23 15:26:08 -0700 + + * Fix a few other minor issues reported by Coverity (Tim Wojtulewicz, Corelight) + + * Add a few extra null checks, plus a missing initialization that led to a bad null check (Tim Wojtulewicz, Corelight) + + * Fix some integer overflow issues reported by Coverity (Tim Wojtulewicz, Corelight) + + * Ignore a couple of known-unused results reported by Coverity (Tim Wojtulewicz, Corelight) + + * Fix some bit-shifting overflow/UB issues reported by Coverity (Tim Wojtulewicz, Corelight) + + * Reset the value of a status variable in SQLite backend before using it in a loop (Tim Wojtulewicz, Corelight) + + * Fix a potential memory leak reported by Coverity (Tim Wojtulewicz, Corelight) + + * Avoid some string copies in IRC analyzer (Tim Wojtulewicz, Corelight) + + * Add some additional std::moves reported by Coverity (Tim Wojtulewicz, Corelight) + + * Fix an unsigned integer comparison reported by Coverity (Tim Wojtulewicz, Corelight) + + * Fix uninitialized class member Coverity findings (Tim Wojtulewicz, Corelight) + + * Handle uncaught exception during setup (Tim Wojtulewicz, Corelight) + + * Update gen-zam submodule for Coverity findings (Tim Wojtulewicz, Corelight) + 8.0.0-dev.716 | 2025-07-23 14:03:47 -0700 * Fix swapped storage metrics names (Tim Wojtulewicz, Corelight) diff --git a/VERSION b/VERSION index b28a5b6c0e..19b4155f15 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -8.0.0-dev.716 +8.0.0-dev.730 diff --git a/auxil/gen-zam b/auxil/gen-zam index b129f9f497..56a6db00b8 160000 --- a/auxil/gen-zam +++ b/auxil/gen-zam @@ -1 +1 @@ -Subproject commit b129f9f4977af4405724f869cdff0cd9b3dbddd9 +Subproject commit 56a6db00b887c79d26f303676677cb490d1c296d diff --git a/src/Anon.cc b/src/Anon.cc index facc5dbbb2..fbcbc00a1a 100644 --- a/src/Anon.cc +++ b/src/Anon.cc @@ -259,6 +259,10 @@ AnonymizeIPAddr_A50::Node* AnonymizeIPAddr_A50::make_peer(ipaddr32_t a, Node* n) // swivel is first bit 'a' and 'old->input' differ. int swivel = bi_ffs(a ^ n->input); + // Shifting by more than 31 bits below results in undefined behavior. + // This shouldn't be possible, but check anyways. + ASSERT(swivel > 0); + // bitvalue is the value of that bit of 'a'. int bitvalue = (a >> (32 - swivel)) & 1; @@ -305,6 +309,10 @@ AnonymizeIPAddr_A50::Node* AnonymizeIPAddr_A50::find_node(ipaddr32_t a) { // differ. int swivel = bi_ffs(n->child[0]->input ^ n->child[1]->input); + // Shifting by more than 31 bits below results in undefined behavior. + // This shouldn't be possible, but check anyways. + ASSERT(swivel > 0); + if ( bi_ffs(a ^ n->input) < swivel ) // Input differs earlier. n = make_peer(a, n); diff --git a/src/DNS_Mgr.h b/src/DNS_Mgr.h index 6b7049c5f3..b6d78311ce 100644 --- a/src/DNS_Mgr.h +++ b/src/DNS_Mgr.h @@ -207,17 +207,17 @@ public: bool Save(); struct CachedStats { - unsigned long hosts; - unsigned long addresses; - unsigned long texts; - unsigned long total; + unsigned long hosts = 0; + unsigned long addresses = 0; + unsigned long texts = 0; + unsigned long total = 0; }; struct Stats { - unsigned long requests; // These count only async requests. - unsigned long successful; - unsigned long failed; - unsigned long pending; + unsigned long requests = 0; // These count only async requests. + unsigned long successful = 0; + unsigned long failed = 0; + unsigned long pending = 0; CachedStats cached; }; diff --git a/src/Debug.cc b/src/Debug.cc index ee0057f8f9..088913a941 100644 --- a/src/Debug.cc +++ b/src/Debug.cc @@ -340,7 +340,7 @@ vector parse_location_string(const string& s) { return result; } - loc_filename = path; + loc_filename = std::move(path); plr.type = PLR_FILE_AND_LINE; } } diff --git a/src/EventTrace.cc b/src/EventTrace.cc index 33301e9fb8..3298447e89 100644 --- a/src/EventTrace.cc +++ b/src/EventTrace.cc @@ -708,7 +708,7 @@ void EventTrace::Generate(FILE* f, ValTraceMgr& vtm, const EventTrace* predecess } } - Generate(f, vtm, deltas, successor); + Generate(f, vtm, deltas, std::move(successor)); } void ValTraceMgr::TraceEventValues(std::shared_ptr et, const zeek::Args* args) { @@ -726,7 +726,7 @@ void ValTraceMgr::TraceEventValues(std::shared_ptr et, const zeek::A ev_args += ValName(a); } - curr_ev->SetArgs(ev_args); + curr_ev->SetArgs(std::move(ev_args)); // Now look for any values newly-processed with this event and // remember them so we can catch uses of them in future events. @@ -780,7 +780,7 @@ void ValTraceMgr::AddVal(ValPtr v) { else { auto vt = std::make_shared(v); AssessChange(vt.get(), mapping->second.get()); - val_map[v.get()] = vt; + val_map[v.get()] = std::move(vt); } } @@ -790,7 +790,7 @@ void ValTraceMgr::NewVal(ValPtr v) { auto vt = std::make_shared(v); AssessChange(vt.get(), nullptr); - val_map[v.get()] = vt; + val_map[v.get()] = std::move(vt); } void ValTraceMgr::ValUsed(const ValPtr& v) { @@ -834,7 +834,7 @@ void ValTraceMgr::AssessChange(const ValTrace* vt, const ValTrace* prev_vt) { previous_deltas.insert(std::move(full_delta)); ValUsed(vp); - curr_ev->AddDelta(vp, rhs, needs_lhs, is_first_def); + curr_ev->AddDelta(vp, std::move(rhs), needs_lhs, is_first_def); } auto& v = vt->GetVal(); @@ -1025,7 +1025,7 @@ void EventTraceMgr::StartEvent(const ScriptFunc* ev, const zeek::Args* args) { auto et = std::make_shared(ev, nt, events.size()); events.emplace_back(et); - vtm.TraceEventValues(et, args); + vtm.TraceEventValues(std::move(et), args); } void EventTraceMgr::EndEvent(const ScriptFunc* ev, const zeek::Args* args) { diff --git a/src/Expr.cc b/src/Expr.cc index 3bcf9d96a5..96c6a0d46d 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -4245,7 +4245,7 @@ LambdaExpr::LambdaExpr(FunctionIngredientsPtr arg_ing, IDPList arg_outer_ids, st if ( name.empty() ) BuildName(); else - my_name = name; + my_name = std::move(name); // Install that in the current scope. lambda_id = install_ID(my_name.c_str(), current_module.c_str(), true, false); diff --git a/src/ID.cc b/src/ID.cc index 7ebb744d6b..bd49d135bf 100644 --- a/src/ID.cc +++ b/src/ID.cc @@ -490,6 +490,9 @@ void ID::DescribeReST(ODesc* d, bool roles_only) const { ModuleName() != "Version" ) { d->Add(":Default:"); auto ii = zeekygen_mgr->GetIdentifierInfo(Name()); + if ( ! ii ) + return; + auto redefs = ii->GetRedefs(); const auto& iv = ! redefs.empty() && ii->InitialVal() ? ii->InitialVal() : val; diff --git a/src/IPAddr.cc b/src/IPAddr.cc index bd7f2a09b3..83265c3161 100644 --- a/src/IPAddr.cc +++ b/src/IPAddr.cc @@ -162,7 +162,7 @@ void IPAddr::Mask(int top_bits_to_keep) { if ( res.quot < 4 ) mask_bits[res.quot] = htonl(mask_bits[res.quot] & ~bit_mask32(32 - res.rem)); - for ( unsigned int i = res.quot + 1; i < 4; ++i ) + for ( size_t i = res.quot + 1; i < 4; ++i ) mask_bits[i] = 0; uint32_t* p = reinterpret_cast(in6.s6_addr); @@ -183,7 +183,7 @@ void IPAddr::ReverseMask(int top_bits_to_chop) { if ( res.quot < 4 ) mask_bits[res.quot] = htonl(bit_mask32(32 - res.rem)); - for ( unsigned int i = res.quot + 1; i < 4; ++i ) + for ( size_t i = res.quot + 1; i < 4; ++i ) mask_bits[i] = 0xffffffff; uint32_t* p = reinterpret_cast(in6.s6_addr); diff --git a/src/OpaqueVal.cc b/src/OpaqueVal.cc index 0b300cb02b..553c52d654 100644 --- a/src/OpaqueVal.cc +++ b/src/OpaqueVal.cc @@ -759,7 +759,9 @@ ValPtr BloomFilterVal::DoClone(CloneState* state) { if ( bloom_filter ) { auto bf = make_intrusive(bloom_filter->Clone()); assert(type); - bf->Typify(type); + if ( ! bf->Typify(type) ) + reporter->InternalError("Failed to typify new bloom_filter clone, clone already had valid type"); + return state->NewClone(this, std::move(bf)); } diff --git a/src/Options.cc b/src/Options.cc index 59322131f7..5f4494977c 100644 --- a/src/Options.cc +++ b/src/Options.cc @@ -616,7 +616,7 @@ Options parse_cmdline(int argc, char** argv) { exit(1); } - *path = res; + *path = std::move(res); if ( (*path)[0] == '/' || (*path)[0] == '~' ) // Now an absolute path diff --git a/src/PrefixTable.cc b/src/PrefixTable.cc index 95fff9e09c..e83dd94326 100644 --- a/src/PrefixTable.cc +++ b/src/PrefixTable.cc @@ -138,7 +138,7 @@ void* PrefixTable::Remove(const Val* value) { } PrefixTable::iterator PrefixTable::InitIterator() { - iterator i; + iterator i = {}; i.Xsp = i.Xstack; i.Xrn = tree->head; i.Xnode = nullptr; diff --git a/src/PrefixTable.h b/src/PrefixTable.h index 343ff8539f..fb2e2dd9c4 100644 --- a/src/PrefixTable.h +++ b/src/PrefixTable.h @@ -21,10 +21,10 @@ namespace detail { class PrefixTable { private: struct iterator { - patricia_node_t* Xstack[PATRICIA_MAXBITS + 1]; - patricia_node_t** Xsp; - patricia_node_t* Xrn; - patricia_node_t* Xnode; + patricia_node_t* Xstack[PATRICIA_MAXBITS + 1] = {}; + patricia_node_t** Xsp = nullptr; + patricia_node_t* Xrn = nullptr; + patricia_node_t* Xnode = nullptr; }; public: diff --git a/src/RandTest.cc b/src/RandTest.cc index c97c586d02..a8f2f4bfc1 100644 --- a/src/RandTest.cc +++ b/src/RandTest.cc @@ -68,7 +68,6 @@ void RandTest::add(const void* buf, int bufl) { scct2 = scct2 + oc; scct3 = scct3 + (oc * oc); scclast = oc; - oc <<= 1; } } diff --git a/src/Reporter.cc b/src/Reporter.cc index 88a1502972..1bc8f625d1 100644 --- a/src/Reporter.cc +++ b/src/Reporter.cc @@ -525,21 +525,23 @@ void Reporter::DoLog(const char* prefix, EventHandlerPtr event, FILE* out, Conne // Figure out how big of a buffer is needed here. va_list ap_copy; va_copy(ap_copy, ap); - size_t req_buffer_size = vsnprintf(nullptr, 0, fmt, ap_copy); + int req_buffer_size = vsnprintf(nullptr, 0, fmt, ap_copy); if ( req_buffer_size < 0 ) { va_end(ap_copy); FatalError("out of memory in Reporter"); } + size_t new_buffer_size = req_buffer_size; + if ( postfix && *postfix ) - req_buffer_size += strlen(postfix) + 10; + new_buffer_size += strlen(postfix) + 10; // Add one byte for a null terminator. - req_buffer_size++; + new_buffer_size++; - if ( req_buffer_size > DEFAULT_BUFFER_SIZE ) { - buffer = (char*)malloc(req_buffer_size); + if ( new_buffer_size > DEFAULT_BUFFER_SIZE ) { + buffer = (char*)malloc(new_buffer_size); if ( ! buffer ) { va_end(ap_copy); FatalError("out of memory in Reporter"); @@ -550,7 +552,7 @@ void Reporter::DoLog(const char* prefix, EventHandlerPtr event, FILE* out, Conne va_end(ap_copy); - req_buffer_size = vsnprintf(buffer, req_buffer_size, fmt, ap); + req_buffer_size = vsnprintf(buffer, new_buffer_size, fmt, ap); if ( req_buffer_size < 0 ) FatalError("out of memory in Reporter"); diff --git a/src/RuleAction.h b/src/RuleAction.h index 5dbe688155..1d5c42d550 100644 --- a/src/RuleAction.h +++ b/src/RuleAction.h @@ -45,7 +45,7 @@ public: private: StringValPtr msg; EventHandlerPtr handler; - bool want_end_of_match; // Whether handler accepts end_of_match parameter. + bool want_end_of_match = false; // Whether handler accepts end_of_match parameter. }; class RuleActionMIME : public RuleAction { @@ -62,7 +62,7 @@ public: private: std::string mime; - int strength; + int strength = 0; }; // Base class for enable/disable actions. diff --git a/src/ScriptCoverageManager.cc b/src/ScriptCoverageManager.cc index 4042996aaf..2a0747bd45 100644 --- a/src/ScriptCoverageManager.cc +++ b/src/ScriptCoverageManager.cc @@ -136,7 +136,7 @@ bool ScriptCoverageManager::WriteStats() { auto ft = func->GetType(); auto desc = ft->FlavorString() + " " + func->Name() + " BODY"; - TrackUsage(body, desc, body->GetAccessCount()); + TrackUsage(body, std::move(desc), body->GetAccessCount()); } for ( const auto& [cond_loc, text, was_true] : cond_instances ) diff --git a/src/ScriptProfile.h b/src/ScriptProfile.h index cea8b002fc..7cd2cccfb4 100644 --- a/src/ScriptProfile.h +++ b/src/ScriptProfile.h @@ -35,7 +35,7 @@ public: ScriptProfileStats& operator=(ScriptProfileStats&&) = default; ScriptProfileStats& operator=(const ScriptProfileStats&) = default; - const auto Name() const { return name; } + const auto& Name() const { return name; } // Number of instances included in an aggregate (like for "all BiFs"). // This is 1 for non-aggregates. diff --git a/src/Stmt.cc b/src/Stmt.cc index 071c025d89..78789a6e24 100644 --- a/src/Stmt.cc +++ b/src/Stmt.cc @@ -1734,10 +1734,10 @@ WhenInfo::WhenInfo(ExprPtr arg_cond, FuncType::CaptureList* arg_cl, bool arg_is_ } WhenInfo::WhenInfo(const WhenInfo* orig) { - if ( orig->cl ) { - cl = new FuncType::CaptureList; + cl = new FuncType::CaptureList; + + if ( orig->cl ) *cl = *orig->cl; - } cond = orig->OrigCond()->Duplicate(); diff --git a/src/Val.cc b/src/Val.cc index 35b088870c..13ff13c67e 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -751,7 +751,7 @@ IPAddr SubNetVal::Mask() const { for ( w = subnet_val->Length(); w >= 32; w -= 32 ) *(mp++) = 0xffffffff; - *mp = ~((1 << (32 - w)) - 1); + *mp = ~((static_cast(1) << (32 - w)) - 1); while ( ++mp < m + 4 ) *mp = 0; @@ -2398,7 +2398,7 @@ ValPtr TableVal::Remove(const Val& index, bool broker_forward, bool* iterators_i if ( broker_forward && ! broker_store.empty() ) SendToStore(&index, nullptr, ELEMENT_REMOVED); - if ( change_func ) { + if ( change_func && k ) { // this is totally cheating around the fact that we need a Intrusive pointer. ValPtr changefunc_val = RecreateIndex(*(k.get())); CallChangeFunc(changefunc_val, va, ELEMENT_REMOVED); diff --git a/src/analyzer/protocol/conn-size/Plugin.cc b/src/analyzer/protocol/conn-size/Plugin.cc index 51ce78b527..ef14ac2532 100644 --- a/src/analyzer/protocol/conn-size/Plugin.cc +++ b/src/analyzer/protocol/conn-size/Plugin.cc @@ -33,7 +33,7 @@ public: thresholds.emplace_back(lv->Idx(i)->AsCount()); std::sort(thresholds.begin(), thresholds.end()); - zeek::analyzer::conn_size::ConnSize_Analyzer::SetGenericPacketThresholds(thresholds); + zeek::analyzer::conn_size::ConnSize_Analyzer::SetGenericPacketThresholds(std::move(thresholds)); } } plugin; diff --git a/src/analyzer/protocol/irc/IRC.cc b/src/analyzer/protocol/irc/IRC.cc index d636319a09..bc16397239 100644 --- a/src/analyzer/protocol/irc/IRC.cc +++ b/src/analyzer/protocol/irc/IRC.cc @@ -807,8 +807,8 @@ void IRC_Analyzer::DeliverStream(int length, const u_char* line, bool orig) { } else if ( irc_quit_message && command == "QUIT" ) { - string message = params; - if ( message[0] == ':' ) + string_view message{params}; + if ( message.starts_with(":") ) message = message.substr(1); string nickname = ""; @@ -819,16 +819,16 @@ void IRC_Analyzer::DeliverStream(int length, const u_char* line, bool orig) { } EnqueueConnEvent(irc_quit_message, ConnVal(), val_mgr->Bool(orig), make_intrusive(nickname.c_str()), - make_intrusive(message.c_str())); + make_intrusive(message)); } else if ( irc_nick_message && command == "NICK" ) { - string nick = params; + string_view nick{params}; if ( nick[0] == ':' ) nick = nick.substr(1); EnqueueConnEvent(irc_nick_message, ConnVal(), val_mgr->Bool(orig), make_intrusive(prefix.c_str()), - make_intrusive(nick.c_str())); + make_intrusive(nick)); } else if ( irc_who_message && command == "WHO" ) { @@ -909,8 +909,8 @@ void IRC_Analyzer::DeliverStream(int length, const u_char* line, bool orig) { } else if ( irc_squit_message && command == "SQUIT" ) { - string server = params; - string message = ""; + string server; + string message; unsigned int pos = params.find(' '); if ( pos < params.size() ) { @@ -946,8 +946,6 @@ void IRC_Analyzer::DeliverStream(int length, const u_char* line, bool orig) { AddSupportAnalyzer(new analyzer::zip::ZIP_Analyzer(Conn(), true)); AddSupportAnalyzer(new analyzer::zip::ZIP_Analyzer(Conn(), false)); } - - return; } void IRC_Analyzer::StartTLS() { diff --git a/src/analyzer/protocol/mysql/mysql-protocol.pac b/src/analyzer/protocol/mysql/mysql-protocol.pac index 60ea7e240c..f01a40f315 100644 --- a/src/analyzer/protocol/mysql/mysql-protocol.pac +++ b/src/analyzer/protocol/mysql/mysql-protocol.pac @@ -751,6 +751,7 @@ refine connection MySQL_Conn += { deprecate_eof_ = false; server_query_attrs_ = false; client_query_attrs_ = false; + client_capabilities_ = 0; query_attr_idx_ = 0; %} diff --git a/src/analyzer/protocol/rpc/RPC.cc b/src/analyzer/protocol/rpc/RPC.cc index 7ecec676e5..e0473db3fd 100644 --- a/src/analyzer/protocol/rpc/RPC.cc +++ b/src/analyzer/protocol/rpc/RPC.cc @@ -554,10 +554,9 @@ void Contents_RPC::DeliverStream(int len, const u_char* data, bool orig) { // know yet how much we expect, so we set expected to // 0. msg_buf.Init(MAX_RPC_LEN, 0); - last_frag = false; state = WAIT_FOR_MARKER; start_time = run_state::network_time; - // no break. fall through + [[fallthrough]]; case WAIT_FOR_MARKER: { bool got_marker = marker_buf.ConsumeChunk(data, len); diff --git a/src/cluster/Backend.cc b/src/cluster/Backend.cc index ffa4872e60..3c51259e42 100644 --- a/src/cluster/Backend.cc +++ b/src/cluster/Backend.cc @@ -310,7 +310,7 @@ TEST_SUITE_BEGIN("cluster event"); TEST_CASE("add metadata") { auto* handler = zeek::event_registry->Lookup("Supervisor::node_status"); zeek::Args args{zeek::make_intrusive("TEST"), zeek::val_mgr->Count(42)}; - zeek::cluster::detail::Event event{handler, args, nullptr}; + zeek::cluster::detail::Event event{handler, std::move(args), nullptr}; auto nts = zeek::id::find_val("EventMetadata::NETWORK_TIMESTAMP"); REQUIRE(nts); diff --git a/src/input/Manager.cc b/src/input/Manager.cc index c8cc34ee1f..bfae85650d 100644 --- a/src/input/Manager.cc +++ b/src/input/Manager.cc @@ -103,9 +103,9 @@ public: EventHandlerPtr event; RecordType* fields = nullptr; - unsigned int num_fields; + unsigned int num_fields = 0; - bool want_record; + bool want_record = false; EventStream(); ~EventStream() override; }; @@ -1431,7 +1431,7 @@ int Manager::SendEventStreamEvent(Stream* i, EnumVal* type, const Value* const* Unref(val); } else - SendEvent(stream->event, out_vals); + SendEvent(stream->event, std::move(out_vals)); return stream->num_fields; } diff --git a/src/input/readers/ascii/Ascii.cc b/src/input/readers/ascii/Ascii.cc index 81fca8ef63..df9f155cbb 100644 --- a/src/input/readers/ascii/Ascii.cc +++ b/src/input/readers/ascii/Ascii.cc @@ -175,7 +175,7 @@ bool Ascii::ReadHeader(bool useCached) { line = headerline; // construct list of field names. - auto ifields = util::split(line, separator[0]); + auto ifields = util::split(std::move(line), separator[0]); // printf("Updating fields from description %s\n", line.c_str()); columnMap.clear(); diff --git a/src/main.cc b/src/main.cc index 5c0da86cff..2ce6b235ee 100644 --- a/src/main.cc +++ b/src/main.cc @@ -52,7 +52,14 @@ int main(int argc, char** argv) { #endif auto time_start = zeek::util::current_time(true); - auto setup_result = zeek::detail::setup(argc, argv); + zeek::detail::SetupResult setup_result; + + try { + setup_result = zeek::detail::setup(argc, argv); + } catch ( const zeek::InterpreterException& e ) { + fprintf(stderr, "Exception caught during initial setup\n"); + abort(); + } if ( setup_result.code ) return setup_result.code; diff --git a/src/packet_analysis/Manager.cc b/src/packet_analysis/Manager.cc index 956d9b42d1..99d751ce5b 100644 --- a/src/packet_analysis/Manager.cc +++ b/src/packet_analysis/Manager.cc @@ -216,7 +216,8 @@ bool Manager::PermitUnknownProtocol(const std::string& analyzer, uint32_t protoc ++count; if ( count == 1 ) - zeek::detail::timer_mgr->Add(new UnknownProtocolTimer(run_state::network_time, p, unknown_sampling_duration)); + zeek::detail::timer_mgr->Add( + new UnknownProtocolTimer(run_state::network_time, std::move(p), unknown_sampling_duration)); if ( count < unknown_sampling_threshold ) return true; diff --git a/src/packet_analysis/packet_analysis.bif b/src/packet_analysis/packet_analysis.bif index 77766d1185..33346a8364 100644 --- a/src/packet_analysis/packet_analysis.bif +++ b/src/packet_analysis/packet_analysis.bif @@ -24,7 +24,7 @@ function register_packet_analyzer%(parent: PacketAnalyzer::Tag, identifier: coun if ( ! child_analyzer ) return zeek::val_mgr->False(); - parent_analyzer->RegisterProtocol(identifier, child_analyzer); + parent_analyzer->RegisterProtocol(identifier, std::move(child_analyzer)); return zeek::val_mgr->True(); %} @@ -45,7 +45,7 @@ function try_register_packet_analyzer_by_name%(parent: string, identifier: count if ( ! child_analyzer ) return zeek::val_mgr->False(); - parent_analyzer->RegisterProtocol(identifier, child_analyzer); + parent_analyzer->RegisterProtocol(identifier, std::move(child_analyzer)); return zeek::val_mgr->True(); %} @@ -74,7 +74,7 @@ function register_protocol_detection%(parent: PacketAnalyzer::Tag, child: Packet if ( ! child_analyzer ) return zeek::val_mgr->False(); - parent_analyzer->RegisterProtocolDetection(child_analyzer); + parent_analyzer->RegisterProtocolDetection(std::move(child_analyzer)); return zeek::val_mgr->True(); %} diff --git a/src/packet_analysis/protocol/ip/IP.cc b/src/packet_analysis/protocol/ip/IP.cc index fd84d388d2..09008ca285 100644 --- a/src/packet_analysis/protocol/ip/IP.cc +++ b/src/packet_analysis/protocol/ip/IP.cc @@ -111,7 +111,7 @@ bool IPAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) } // If we got here, the IP_Hdr is most likely valid and safe to use. - packet->ip_hdr = ip_hdr; + packet->ip_hdr = std::move(ip_hdr); // If there's an encapsulation stack in this packet, meaning this packet is part of a chain // of tunnels, make sure to store the IP header in the last flow in the stack so it can be diff --git a/src/script_opt/CPP/Driver.cc b/src/script_opt/CPP/Driver.cc index 6e09098e30..9e273a83a3 100644 --- a/src/script_opt/CPP/Driver.cc +++ b/src/script_opt/CPP/Driver.cc @@ -204,7 +204,7 @@ bool CPPCompile::AnalyzeFuncBody(FuncInfo& fi, unordered_set& filenames_ else if ( filenames_reported_as_skipped.count(fn) == 0 ) { reporter->Warning("skipping compilation of files in %s due to presence of conditional code", fn.c_str()); - filenames_reported_as_skipped.insert(fn); + filenames_reported_as_skipped.emplace(fn); } fi.SetSkip(true); diff --git a/src/script_opt/ScriptOpt.cc b/src/script_opt/ScriptOpt.cc index b81344dd38..8e6db2f1c5 100644 --- a/src/script_opt/ScriptOpt.cc +++ b/src/script_opt/ScriptOpt.cc @@ -659,7 +659,7 @@ void analyze_scripts(bool no_unused_warnings) { } auto pfs = std::make_shared(funcs, nullptr, true, true); - analyze_scripts_for_ZAM(pfs); + analyze_scripts_for_ZAM(std::move(pfs)); if ( reporter->Errors() > 0 ) reporter->FatalError("Optimized script execution aborted due to errors"); diff --git a/src/script_opt/ZAM/IterInfo.h b/src/script_opt/ZAM/IterInfo.h index 36b63c8c61..b42cc7e5a1 100644 --- a/src/script_opt/ZAM/IterInfo.h +++ b/src/script_opt/ZAM/IterInfo.h @@ -112,8 +112,8 @@ private: TableValPtr tv = nullptr; std::vector loop_vars; - const std::vector* loop_var_types; - const std::vector* lvt_is_managed; + const std::vector* loop_var_types = nullptr; + const std::vector* lvt_is_managed = nullptr; TypePtr value_var_type; std::optional> tbl_iter; diff --git a/src/storage/backend/sqlite/SQLite.cc b/src/storage/backend/sqlite/SQLite.cc index 4641f027e8..331381e5c7 100644 --- a/src/storage/backend/sqlite/SQLite.cc +++ b/src/storage/backend/sqlite/SQLite.cc @@ -46,7 +46,7 @@ OperationResult SQLite::RunPragma(std::string_view name, std::optionalWarning("SQLite backend failed to rollback transaction during expiration: %s", errMsg); + sqlite3_free(errMsg); }); @@ -517,6 +521,7 @@ void SQLite::DoExpire(double current_network_time) { // Check if the expiration control key is less than the interval. Exit if not. stmt = unique_stmt_ptr(get_expiry_last_run_stmt.get(), sqlite3_reset); + status = SQLITE_OK; while ( status != SQLITE_ROW ) { status = sqlite3_step(stmt.get()); if ( status == SQLITE_ROW ) { @@ -573,13 +578,21 @@ void SQLite::DoExpire(double current_network_time) { Error(err.c_str()); } - // Get the number of changes from the delete statement. This should be identical to the num_to_expire - // value earlier because we're under a transaction, but this should be the exact number that changed. + // Get the number of changes from the delete statement. This should be identical to + // the num_to_expire value earlier because we're under a transaction, but this should + // be the exact number that changed. int changes = sqlite3_changes(db); IncExpiredEntriesMetric(changes); - sqlite3_exec(expire_db, "commit transaction", nullptr, nullptr, &errMsg); + status = sqlite3_exec(expire_db, "commit transaction", nullptr, nullptr, &errMsg); + if ( status != SQLITE_OK ) + reporter->Warning("SQLite backend failed to commit transaction during expiration: %s", errMsg); + sqlite3_free(errMsg); + + // Don't try to rollback the transaction we just committed, since sqlite will just + // report an error. + deferred_rollback.Cancel(); } // returns true in case of error diff --git a/src/storage/backend/sqlite/SQLite.h b/src/storage/backend/sqlite/SQLite.h index c10be5c905..927c501bac 100644 --- a/src/storage/backend/sqlite/SQLite.h +++ b/src/storage/backend/sqlite/SQLite.h @@ -74,8 +74,8 @@ private: std::string full_path; std::string table_name; - std::chrono::milliseconds pragma_timeout; - std::chrono::milliseconds pragma_wait_on_busy; + std::chrono::milliseconds pragma_timeout = {}; + std::chrono::milliseconds pragma_wait_on_busy = {}; telemetry::GaugePtr page_count_metric; telemetry::GaugePtr file_size_metric; diff --git a/src/util-types.h b/src/util-types.h index 11f61b07b8..8e2e40bea4 100644 --- a/src/util-types.h +++ b/src/util-types.h @@ -68,7 +68,12 @@ private: class Deferred { public: Deferred(std::function deferred) : deferred(std::move(deferred)) {} - ~Deferred() { deferred(); } + ~Deferred() { + if ( deferred ) { + deferred(); + } + } + void Cancel() { deferred = nullptr; } private: std::function deferred; diff --git a/src/util.cc b/src/util.cc index 4c0f46c2ea..3d38542f0b 100644 --- a/src/util.cc +++ b/src/util.cc @@ -832,7 +832,7 @@ void set_processing_status(const char* status, const char* reason) { auto write_str = [](int fd, const char* s) { int len = strlen(s); while ( len ) { - int n = write(fd, s, len); + ssize_t n = write(fd, s, len); if ( n < 0 && errno != EINTR && errno != EAGAIN ) // Ignore errors, as they're too difficult to @@ -1531,7 +1531,7 @@ TEST_CASE("util strstrip") { CHECK(strstrip(s) == "abcd"); s = " abcd "; - CHECK(strstrip(s) == "abcd"); + CHECK(strstrip(std::move(s)) == "abcd"); } std::string strstrip(std::string s) { @@ -1882,7 +1882,7 @@ uint64_t calculate_unique_id(size_t pool) { bool safe_write(int fd, const char* data, int len) { while ( len > 0 ) { - int n = write(fd, data, len); + ssize_t n = write(fd, data, len); if ( n < 0 ) { if ( errno == EINTR ) diff --git a/src/util.h b/src/util.h index 4fe069d7ae..91d20d2891 100644 --- a/src/util.h +++ b/src/util.h @@ -592,7 +592,7 @@ std::vector split(T s, const T& delim) { */ template std::vector split(T s, U delim) { - return split(s, T{delim}); + return split(std::move(s), std::move(T{delim})); } /** diff --git a/src/zeek-setup.cc b/src/zeek-setup.cc index fe9ae50be8..985bd753b0 100644 --- a/src/zeek-setup.cc +++ b/src/zeek-setup.cc @@ -605,7 +605,7 @@ SetupResult setup(int argc, char** argv, Options* zopts) { if ( options.supervisor_mode ) { Supervisor::Config cfg = {}; - cfg.zeek_exe_path = zeek_exe_path; + cfg.zeek_exe_path = std::move(zeek_exe_path); options.filter_supervisor_options(); supervisor_mgr = new Supervisor(std::move(cfg), std::move(*stem)); } diff --git a/src/zeekygen/Target.cc b/src/zeekygen/Target.cc index a9135a4e5b..f0a8e35f92 100644 --- a/src/zeekygen/Target.cc +++ b/src/zeekygen/Target.cc @@ -483,7 +483,7 @@ void ScriptTarget::DoGenerate() const { if ( zeek::detail::zeekygen_mgr->IsUpToDate(target_filename, dep) ) continue; - TargetFile file(target_filename); + TargetFile file(std::move(target_filename)); fprintf(file.f, "%s\n", d->ReStructuredText().c_str()); }