From 95d2af45016dc03965ca1fdb88a7ef8f91cb98d6 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Mon, 3 Feb 2020 11:14:03 -0500 Subject: [PATCH 01/12] Move constructors/operators should be marked noexcept to avoid the compiler picking the copy constructor instead (performance-noexcept-move-constructor) --- src/Tag.cc | 2 +- src/Tag.h | 2 +- src/logging/Tag.cc | 2 +- src/logging/Tag.h | 2 +- src/probabilistic/CardinalityCounter.cc | 2 +- src/probabilistic/CardinalityCounter.h | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Tag.cc b/src/Tag.cc index 3b7c9a5bd9..b834bad9c9 100644 --- a/src/Tag.cc +++ b/src/Tag.cc @@ -65,7 +65,7 @@ Tag& Tag::operator=(const Tag& other) return *this; } -Tag& Tag::operator=(const Tag&& other) +Tag& Tag::operator=(const Tag&& other) noexcept { if ( this != &other ) { diff --git a/src/Tag.h b/src/Tag.h index a1c24702c0..f10c59cd4b 100644 --- a/src/Tag.h +++ b/src/Tag.h @@ -82,7 +82,7 @@ protected: /** * Move assignment operator. */ - Tag& operator=(const Tag&& other); + Tag& operator=(const Tag&& other) noexcept; /** * Compares two tags for equality. diff --git a/src/logging/Tag.cc b/src/logging/Tag.cc index d5f3f553d1..2ab2844e25 100644 --- a/src/logging/Tag.cc +++ b/src/logging/Tag.cc @@ -16,7 +16,7 @@ logging::Tag& logging::Tag::operator=(const logging::Tag& other) return *this; } -logging::Tag& logging::Tag::operator=(const logging::Tag&& other) +logging::Tag& logging::Tag::operator=(const logging::Tag&& other) noexcept { ::Tag::operator=(other); return *this; diff --git a/src/logging/Tag.h b/src/logging/Tag.h index 40a7b40b5f..29c7fd6cd4 100644 --- a/src/logging/Tag.h +++ b/src/logging/Tag.h @@ -56,7 +56,7 @@ public: /** * Move assignment operator. */ - Tag& operator=(const Tag&& other); + Tag& operator=(const Tag&& other) noexcept; /** * Compares two tags for equality. diff --git a/src/probabilistic/CardinalityCounter.cc b/src/probabilistic/CardinalityCounter.cc index bdc9797125..710bc86d21 100644 --- a/src/probabilistic/CardinalityCounter.cc +++ b/src/probabilistic/CardinalityCounter.cc @@ -76,7 +76,7 @@ CardinalityCounter::CardinalityCounter(CardinalityCounter& other) p = other.p; } -CardinalityCounter::CardinalityCounter(CardinalityCounter&& o) +CardinalityCounter::CardinalityCounter(CardinalityCounter&& o) noexcept { V = o.V; alpha_m = o.alpha_m; diff --git a/src/probabilistic/CardinalityCounter.h b/src/probabilistic/CardinalityCounter.h index 26f61591b1..a36f7aa610 100644 --- a/src/probabilistic/CardinalityCounter.h +++ b/src/probabilistic/CardinalityCounter.h @@ -43,7 +43,7 @@ public: /** * Move-Constructor */ - CardinalityCounter(CardinalityCounter&& o); + CardinalityCounter(CardinalityCounter&& o) noexcept; /** * Constructor for a known number of buckets. From c32566420a1779ee58e33a1170f659ac55c07406 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Mon, 3 Feb 2020 11:19:37 -0500 Subject: [PATCH 02/12] Use single-character version of string find() (performance-faster-string-find) --- src/analyzer/protocol/http/HTTP.cc | 4 ++-- src/analyzer/protocol/irc/IRC.cc | 2 +- src/analyzer/protocol/xmpp/xmpp-analyzer.pac | 2 +- src/input/readers/ascii/Ascii.cc | 2 +- src/input/readers/binary/Binary.cc | 2 +- src/input/readers/sqlite/SQLite.cc | 2 +- src/plugin/Manager.cc | 4 ++-- src/threading/formatters/Ascii.cc | 2 +- 8 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/analyzer/protocol/http/HTTP.cc b/src/analyzer/protocol/http/HTTP.cc index e4e02b2049..83349d100a 100644 --- a/src/analyzer/protocol/http/HTTP.cc +++ b/src/analyzer/protocol/http/HTTP.cc @@ -402,7 +402,7 @@ void HTTP_Entity::SubmitHeader(mime::MIME_Header* h) return; } - size_t p = byte_range.find("/"); + size_t p = byte_range.find('/'); if ( p == string::npos ) { http_message->Weird("HTTP_content_range_cannot_parse"); @@ -412,7 +412,7 @@ void HTTP_Entity::SubmitHeader(mime::MIME_Header* h) string byte_range_resp_spec = byte_range.substr(0, p); string instance_length_str = byte_range.substr(p + 1); - p = byte_range_resp_spec.find("-"); + p = byte_range_resp_spec.find('-'); if ( p == string::npos ) { http_message->Weird("HTTP_content_range_cannot_parse"); diff --git a/src/analyzer/protocol/irc/IRC.cc b/src/analyzer/protocol/irc/IRC.cc index 5fa1c87bc1..e02789f791 100644 --- a/src/analyzer/protocol/irc/IRC.cc +++ b/src/analyzer/protocol/irc/IRC.cc @@ -34,7 +34,7 @@ void IRC_Analyzer::Done() inline void IRC_Analyzer::SkipLeadingWhitespace(string& str) { - const auto first_char = str.find_first_not_of(" "); + const auto first_char = str.find_first_not_of(' '); if ( first_char == string::npos ) str = ""; else diff --git a/src/analyzer/protocol/xmpp/xmpp-analyzer.pac b/src/analyzer/protocol/xmpp/xmpp-analyzer.pac index 26a9c69b5b..9cfb7e0bf3 100644 --- a/src/analyzer/protocol/xmpp/xmpp-analyzer.pac +++ b/src/analyzer/protocol/xmpp/xmpp-analyzer.pac @@ -13,7 +13,7 @@ refine connection XMPP_Conn += { string token = std_str(name); // Result will either be text after ":" or original string; this discards the namespace string token_no_ns = std_str(name); - auto offset = token_no_ns.find(":"); + auto offset = token_no_ns.find(':'); if ( offset != std::string::npos && token_no_ns.length() > offset + 1 ) token_no_ns = token_no_ns.substr(offset + 1); diff --git a/src/input/readers/ascii/Ascii.cc b/src/input/readers/ascii/Ascii.cc index 4a0cfea41e..2bf2cff996 100644 --- a/src/input/readers/ascii/Ascii.cc +++ b/src/input/readers/ascii/Ascii.cc @@ -130,7 +130,7 @@ bool Ascii::OpenFile() if ( fname.front() != '/' && ! path_prefix.empty() ) { string path = path_prefix; - std::size_t last = path.find_last_not_of("/"); + std::size_t last = path.find_last_not_of('/'); if ( last == string::npos ) // Nothing but slashes -- weird but ok... path = "/"; diff --git a/src/input/readers/binary/Binary.cc b/src/input/readers/binary/Binary.cc index 9964cced1e..87c38d16aa 100644 --- a/src/input/readers/binary/Binary.cc +++ b/src/input/readers/binary/Binary.cc @@ -111,7 +111,7 @@ bool Binary::DoInit(const ReaderInfo& info, int num_fields, if ( fname.front() != '/' && ! path_prefix.empty() ) { string path = path_prefix; - std::size_t last = path.find_last_not_of("/"); + std::size_t last = path.find_last_not_of('/'); if ( last == string::npos ) // Nothing but slashes -- weird but ok... path = "/"; diff --git a/src/input/readers/sqlite/SQLite.cc b/src/input/readers/sqlite/SQLite.cc index f08167ed25..821e6cdd9a 100644 --- a/src/input/readers/sqlite/SQLite.cc +++ b/src/input/readers/sqlite/SQLite.cc @@ -210,7 +210,7 @@ Value* SQLite::EntryToVal(sqlite3_stmt *st, const threading::Field *field, int p { const char *text = (const char*) sqlite3_column_text(st, pos); string s(text, sqlite3_column_bytes(st, pos)); - int pos = s.find("/"); + int pos = s.find('/'); int width = atoi(s.substr(pos+1).c_str()); string addr = s.substr(0, pos); diff --git a/src/plugin/Manager.cc b/src/plugin/Manager.cc index 39ac0568ba..c7d52a3100 100644 --- a/src/plugin/Manager.cc +++ b/src/plugin/Manager.cc @@ -48,7 +48,7 @@ void Manager::SearchDynamicPlugins(const std::string& dir) if ( dir.empty() ) return; - if ( dir.find(":") != string::npos ) + if ( dir.find(':') != string::npos ) { // Split at ":". std::stringstream s(dir); @@ -492,7 +492,7 @@ Plugin* Manager::LookupPluginByPath(std::string_view _path) if ( i != plugins_by_path.end() ) return i->second; - auto j = path.rfind("/"); + auto j = path.rfind('/'); if ( j == std::string::npos ) break; diff --git a/src/threading/formatters/Ascii.cc b/src/threading/formatters/Ascii.cc index 17c7a9e876..ce8d5461f6 100644 --- a/src/threading/formatters/Ascii.cc +++ b/src/threading/formatters/Ascii.cc @@ -300,7 +300,7 @@ threading::Value* Ascii::ParseValue(const string& s, const string& name, TypeTag case TYPE_SUBNET: { string unescaped = strstrip(get_unescaped_string(s)); - size_t pos = unescaped.find("/"); + size_t pos = unescaped.find('/'); if ( pos == unescaped.npos ) { GetThread()->Warning(GetThread()->Fmt("Invalid value for subnet: %s", start)); From eda1b4a23e01f08cb68e007ae856d754d7ab1f54 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Mon, 3 Feb 2020 11:20:54 -0500 Subject: [PATCH 03/12] Use const references over copying variables (performance-unnecessary-copy-initialization, performance-for-range-copy) --- src/Val.cc | 3 +-- src/input/readers/config/Config.cc | 4 ++-- src/probabilistic/CardinalityCounter.cc | 2 +- src/zeekygen/Target.cc | 2 +- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/Val.cc b/src/Val.cc index 62a40a6a50..ee706c0996 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -961,8 +961,7 @@ IPAddr SubNetVal::Mask() const bool SubNetVal::Contains(const IPAddr& addr) const { - IPAddr a(addr); - return val.subnet_val->Contains(a); + return val.subnet_val->Contains(addr); } Val* SubNetVal::DoClone(CloneState* state) diff --git a/src/input/readers/config/Config.cc b/src/input/readers/config/Config.cc index c215a44733..ab11f80487 100644 --- a/src/input/readers/config/Config.cc +++ b/src/input/readers/config/Config.cc @@ -176,7 +176,7 @@ bool Config::DoUpdate() // keep a list of options to remove because they were no longer in the input file. // Start out with all element and removes while going along std::unordered_set unseen_options; - for ( auto i : option_values ) + for ( const auto& i : option_values ) { unseen_options.insert(i.first); } @@ -282,7 +282,7 @@ bool Config::DoUpdate() EndCurrentSend(); // clean up all options we did not see - for ( auto i : unseen_options ) + for ( const auto& i : unseen_options ) option_values.erase(i); return true; diff --git a/src/probabilistic/CardinalityCounter.cc b/src/probabilistic/CardinalityCounter.cc index 710bc86d21..a2d224a09e 100644 --- a/src/probabilistic/CardinalityCounter.cc +++ b/src/probabilistic/CardinalityCounter.cc @@ -173,7 +173,7 @@ bool CardinalityCounter::Merge(CardinalityCounter* c) if ( m != c->GetM() ) return false; - const vector temp = c->GetBuckets(); + const vector& temp = c->GetBuckets(); V = 0; diff --git a/src/zeekygen/Target.cc b/src/zeekygen/Target.cc index 85071b31e7..34c3df6c9f 100644 --- a/src/zeekygen/Target.cc +++ b/src/zeekygen/Target.cc @@ -23,7 +23,7 @@ using namespace zeekygen; static void write_plugin_section_heading(FILE* f, const plugin::Plugin* p) { - string name = p->Name(); + const string& name = p->Name(); fprintf(f, "%s\n", name.c_str()); for ( size_t i = 0; i < name.size(); ++i ) From 92afe6452596d36bd1929727e618993728f0997d Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Mon, 3 Feb 2020 11:35:26 -0500 Subject: [PATCH 04/12] Use string_view for a couple of Dbg methods --- src/DbgBreakpoint.cc | 4 ++-- src/DbgBreakpoint.h | 2 +- src/DebugCmds.cc | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/DbgBreakpoint.cc b/src/DbgBreakpoint.cc index 2ca288ffa6..21753f4035 100644 --- a/src/DbgBreakpoint.cc +++ b/src/DbgBreakpoint.cc @@ -119,7 +119,7 @@ void DbgBreakpoint::RemoveFromStmt() } -bool DbgBreakpoint::SetLocation(ParseLocationRec plr, string loc_str) +bool DbgBreakpoint::SetLocation(ParseLocationRec plr, string_view loc_str) { if ( plr.type == plrUnknown ) { @@ -150,7 +150,7 @@ bool DbgBreakpoint::SetLocation(ParseLocationRec plr, string loc_str) { kind = BP_FUNC; function_name = make_full_var_name(current_module.c_str(), - loc_str.c_str()); + loc_str.data()); at_stmt = plr.stmt; const Location* loc = at_stmt->GetLocationInfo(); snprintf(description, sizeof(description), "%s at %s:%d", diff --git a/src/DbgBreakpoint.h b/src/DbgBreakpoint.h index be5d27d6dd..ab1cf45489 100644 --- a/src/DbgBreakpoint.h +++ b/src/DbgBreakpoint.h @@ -21,7 +21,7 @@ public: void SetID(int newID) { BPID = newID; } // True if breakpoint could be set; false otherwise - bool SetLocation(ParseLocationRec plr, string loc_str); + bool SetLocation(ParseLocationRec plr, string_view loc_str); bool SetLocation(Stmt* stmt); bool SetLocation(double time); diff --git a/src/DebugCmds.cc b/src/DebugCmds.cc index 0632d24a9a..983ee69e84 100644 --- a/src/DebugCmds.cc +++ b/src/DebugCmds.cc @@ -27,9 +27,9 @@ // // Helper routines // -bool string_is_regex(string s) +bool string_is_regex(string_view s) { - return strpbrk(s.c_str(), "?*\\+"); + return strpbrk(s.data(), "?*\\+"); } void lookup_global_symbols_regex(const string& orig_regex, vector& matches, From 5a237d3a3f3040261e0102c4b1ec4710a40aa728 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Tue, 11 Feb 2020 14:11:18 -0800 Subject: [PATCH 05/12] Use const-references in lots of places (preformance-unnecessary-value-param) --- src/Conn.cc | 2 +- src/Conn.h | 2 +- src/Frame.cc | 2 +- src/Frame.h | 2 +- src/analyzer/Analyzer.cc | 4 ++-- src/analyzer/Analyzer.h | 4 ++-- src/analyzer/Manager.cc | 14 +++++------ src/analyzer/Manager.h | 14 +++++------ src/analyzer/protocol/irc/IRC.cc | 4 ++-- src/analyzer/protocol/irc/IRC.h | 4 ++-- .../protocol/modbus/modbus-analyzer.pac | 4 ++-- src/analyzer/protocol/pop3/POP3.cc | 2 +- src/analyzer/protocol/pop3/POP3.h | 4 ++-- src/analyzer/protocol/ssh/ssh-analyzer.pac | 4 ++-- src/file_analysis/AnalyzerSet.cc | 16 ++++++------- src/file_analysis/AnalyzerSet.h | 18 +++++++------- src/file_analysis/Manager.cc | 24 +++++++++---------- src/file_analysis/Manager.h | 24 +++++++++---------- src/file_analysis/analyzer/x509/X509Common.cc | 4 ++-- src/file_analysis/analyzer/x509/X509Common.h | 4 ++-- src/input/Manager.cc | 2 +- src/input/Manager.h | 2 +- src/logging/Manager.cc | 6 ++--- src/logging/Manager.h | 6 ++--- src/zeekygen/Manager.cc | 2 +- src/zeekygen/ReStructuredTextTable.cc | 2 +- src/zeekygen/ReStructuredTextTable.h | 2 +- 27 files changed, 89 insertions(+), 89 deletions(-) diff --git a/src/Conn.cc b/src/Conn.cc index 650105ddd3..67b7ad772c 100644 --- a/src/Conn.cc +++ b/src/Conn.cc @@ -405,7 +405,7 @@ analyzer::Analyzer* Connection::FindAnalyzer(analyzer::ID id) return root_analyzer ? root_analyzer->FindChild(id) : 0; } -analyzer::Analyzer* Connection::FindAnalyzer(analyzer::Tag tag) +analyzer::Analyzer* Connection::FindAnalyzer(const analyzer::Tag& tag) { return root_analyzer ? root_analyzer->FindChild(tag) : 0; } diff --git a/src/Conn.h b/src/Conn.h index 562511d8b9..1468d7041e 100644 --- a/src/Conn.h +++ b/src/Conn.h @@ -111,7 +111,7 @@ public: void FlipRoles(); analyzer::Analyzer* FindAnalyzer(analyzer::ID id); - analyzer::Analyzer* FindAnalyzer(analyzer::Tag tag); // find first in tree. + analyzer::Analyzer* FindAnalyzer(const analyzer::Tag& tag); // find first in tree. analyzer::Analyzer* FindAnalyzer(const char* name); // find first in tree. TransportProto ConnTransport() const { return proto; } diff --git a/src/Frame.cc b/src/Frame.cc index e6b42ddd04..d3ab29f5f5 100644 --- a/src/Frame.cc +++ b/src/Frame.cc @@ -276,7 +276,7 @@ Frame* Frame::SelectiveClone(const id_list& selection, BroFunc* func) const return other; } -broker::expected Frame::Serialize(const Frame* target, id_list selection) +broker::expected Frame::Serialize(const Frame* target, const id_list& selection) { broker::vector rval; diff --git a/src/Frame.h b/src/Frame.h index 8e9cbdc4e7..8a5c3b339a 100644 --- a/src/Frame.h +++ b/src/Frame.h @@ -180,7 +180,7 @@ public: * @return the broker representaton, or an error if the serialization * failed. */ - static broker::expected Serialize(const Frame* target, const id_list selection); + static broker::expected Serialize(const Frame* target, const id_list& selection); /** * Instantiates a Frame from a serialized one. diff --git a/src/analyzer/Analyzer.cc b/src/analyzer/Analyzer.cc index 4c5c84b221..6d340f11b5 100644 --- a/src/analyzer/Analyzer.cc +++ b/src/analyzer/Analyzer.cc @@ -403,7 +403,7 @@ bool Analyzer::AddChildAnalyzer(Analyzer* analyzer, bool init) return true; } -Analyzer* Analyzer::AddChildAnalyzer(Tag analyzer) +Analyzer* Analyzer::AddChildAnalyzer(const Tag& analyzer) { if ( HasChildAnalyzer(analyzer) ) return nullptr; @@ -605,7 +605,7 @@ void Analyzer::RemoveSupportAnalyzer(SupportAnalyzer* analyzer) return; } -bool Analyzer::HasSupportAnalyzer(Tag tag, bool orig) +bool Analyzer::HasSupportAnalyzer(const Tag& tag, bool orig) { SupportAnalyzer* s = orig ? orig_supporters : resp_supporters; for ( ; s; s = s->sibling ) diff --git a/src/analyzer/Analyzer.h b/src/analyzer/Analyzer.h index 2fcde63187..1a94ad3cf2 100644 --- a/src/analyzer/Analyzer.h +++ b/src/analyzer/Analyzer.h @@ -378,7 +378,7 @@ public: * @param tag The type of analyzer to add. * @return the new analyzer instance that was added. */ - Analyzer* AddChildAnalyzer(Tag tag); + Analyzer* AddChildAnalyzer(const Tag& tag); /** * Removes a child analyzer. It's ok for the analyzer to not to be a @@ -647,7 +647,7 @@ protected: * * @param orig True if asking about the originator side. */ - bool HasSupportAnalyzer(Tag tag, bool orig); + bool HasSupportAnalyzer(const Tag& tag, bool orig); /** * Returns the first still active support analyzer for the given diff --git a/src/analyzer/Manager.cc b/src/analyzer/Manager.cc index 5641993e2e..189a365263 100644 --- a/src/analyzer/Manager.cc +++ b/src/analyzer/Manager.cc @@ -145,7 +145,7 @@ void Manager::Done() { } -bool Manager::EnableAnalyzer(Tag tag) +bool Manager::EnableAnalyzer(const Tag& tag) { Component* p = Lookup(tag); @@ -171,7 +171,7 @@ bool Manager::EnableAnalyzer(EnumVal* val) return true; } -bool Manager::DisableAnalyzer(Tag tag) +bool Manager::DisableAnalyzer(const Tag& tag) { Component* p = Lookup(tag); @@ -211,7 +211,7 @@ analyzer::Tag Manager::GetAnalyzerTag(const char* name) return GetComponentTag(name); } -bool Manager::IsEnabled(Tag tag) +bool Manager::IsEnabled(const Tag& tag) { if ( ! tag ) return false; @@ -255,7 +255,7 @@ bool Manager::UnregisterAnalyzerForPort(EnumVal* val, PortVal* port) return UnregisterAnalyzerForPort(p->Tag(), port->PortType(), port->Port()); } -bool Manager::RegisterAnalyzerForPort(Tag tag, TransportProto proto, uint32_t port) +bool Manager::RegisterAnalyzerForPort(const Tag& tag, TransportProto proto, uint32_t port) { tag_set* l = LookupPort(proto, port, true); @@ -271,7 +271,7 @@ bool Manager::RegisterAnalyzerForPort(Tag tag, TransportProto proto, uint32_t po return true; } -bool Manager::UnregisterAnalyzerForPort(Tag tag, TransportProto proto, uint32_t port) +bool Manager::UnregisterAnalyzerForPort(const Tag& tag, TransportProto proto, uint32_t port) { tag_set* l = LookupPort(proto, port, true); @@ -287,7 +287,7 @@ bool Manager::UnregisterAnalyzerForPort(Tag tag, TransportProto proto, uint32_t return true; } -Analyzer* Manager::InstantiateAnalyzer(Tag tag, Connection* conn) +Analyzer* Manager::InstantiateAnalyzer(const Tag& tag, Connection* conn) { Component* c = Lookup(tag); @@ -542,7 +542,7 @@ void Manager::ExpireScheduledAnalyzers() void Manager::ScheduleAnalyzer(const IPAddr& orig, const IPAddr& resp, uint16_t resp_p, - TransportProto proto, Tag analyzer, + TransportProto proto, const Tag& analyzer, double timeout) { if ( ! network_time ) diff --git a/src/analyzer/Manager.h b/src/analyzer/Manager.h index 94dac8e0f1..fe32e43be9 100644 --- a/src/analyzer/Manager.h +++ b/src/analyzer/Manager.h @@ -90,7 +90,7 @@ public: * * @return True if successful. */ - bool EnableAnalyzer(Tag tag); + bool EnableAnalyzer(const Tag& tag); /** * Enables an analyzer type. Only enabled analyzers will be @@ -111,7 +111,7 @@ public: * * @return True if successful. */ - bool DisableAnalyzer(Tag tag); + bool DisableAnalyzer(const Tag& tag); /** * Disables an analyzer type. Disabled analyzers will not be @@ -142,7 +142,7 @@ public: * * @param tag The analyzer's tag. */ - bool IsEnabled(Tag tag); + bool IsEnabled(const Tag& tag); /** * Returns true if an analyzer is enabled. @@ -179,7 +179,7 @@ public: * * @return True if successful. */ - bool RegisterAnalyzerForPort(Tag tag, TransportProto proto, uint32_t port); + bool RegisterAnalyzerForPort(const Tag& tag, TransportProto proto, uint32_t port); /** * Unregisters a well-known port for an anlyzers. @@ -207,7 +207,7 @@ public: * @param tag The analyzer's tag as an enum of script type \c * Analyzer::Tag. */ - bool UnregisterAnalyzerForPort(Tag tag, TransportProto proto, uint32_t port); + bool UnregisterAnalyzerForPort(const Tag& tag, TransportProto proto, uint32_t port); /** * Instantiates a new analyzer instance for a connection. @@ -221,7 +221,7 @@ public: * null if tag is invalid, the requested analyzer is disabled, or the * analyzer can't be instantiated. */ - Analyzer* InstantiateAnalyzer(Tag tag, Connection* c); + Analyzer* InstantiateAnalyzer(const Tag& tag, Connection* c); /** * Instantiates a new analyzer instance for a connection. @@ -269,7 +269,7 @@ public: * schedule this analyzer. Must be non-zero. */ void ScheduleAnalyzer(const IPAddr& orig, const IPAddr& resp, uint16_t resp_p, - TransportProto proto, Tag analyzer, double timeout); + TransportProto proto, const Tag& analyzer, double timeout); /** * Schedules a particular analyzer for an upcoming connection. Once diff --git a/src/analyzer/protocol/irc/IRC.cc b/src/analyzer/protocol/irc/IRC.cc index e02789f791..de8a494c6c 100644 --- a/src/analyzer/protocol/irc/IRC.cc +++ b/src/analyzer/protocol/irc/IRC.cc @@ -1206,11 +1206,11 @@ void IRC_Analyzer::StartTLS() ConnectionEventFast(irc_starttls, {BuildConnVal()}); } -vector IRC_Analyzer::SplitWords(const string input, const char split) +vector IRC_Analyzer::SplitWords(const string& input, char split) { vector words; - if ( input.size() < 1 ) + if ( input.empty() ) return words; unsigned int start = 0; diff --git a/src/analyzer/protocol/irc/IRC.h b/src/analyzer/protocol/irc/IRC.h index e9ae91f272..1146a653aa 100644 --- a/src/analyzer/protocol/irc/IRC.h +++ b/src/analyzer/protocol/irc/IRC.h @@ -62,11 +62,11 @@ private: * \param split character which separates the words * \return vector containing words */ - vector SplitWords(const string input, const char split); + vector SplitWords(const string& input, char split); tcp::ContentLine_Analyzer* cl_orig; tcp::ContentLine_Analyzer* cl_resp; bool starttls; // if true, connection has been upgraded to tls }; -} } // namespace analyzer::* +} } // namespace analyzer::* diff --git a/src/analyzer/protocol/modbus/modbus-analyzer.pac b/src/analyzer/protocol/modbus/modbus-analyzer.pac index d93a40fb87..e30b5e3867 100644 --- a/src/analyzer/protocol/modbus/modbus-analyzer.pac +++ b/src/analyzer/protocol/modbus/modbus-analyzer.pac @@ -8,13 +8,13 @@ # %header{ - VectorVal* bytestring_to_coils(bytestring coils, uint quantity); + VectorVal* bytestring_to_coils(const bytestring& coils, uint quantity); RecordVal* HeaderToBro(ModbusTCP_TransportHeader *header); VectorVal* create_vector_of_count(); %} %code{ - VectorVal* bytestring_to_coils(bytestring coils, uint quantity) + VectorVal* bytestring_to_coils(const bytestring& coils, uint quantity) { VectorVal* modbus_coils = new VectorVal(BifType::Vector::ModbusCoils); for ( uint i = 0; i < quantity; i++ ) diff --git a/src/analyzer/protocol/pop3/POP3.cc b/src/analyzer/protocol/pop3/POP3.cc index d0c5d0b3bd..7da678f855 100644 --- a/src/analyzer/protocol/pop3/POP3.cc +++ b/src/analyzer/protocol/pop3/POP3.cc @@ -884,7 +884,7 @@ int POP3_Analyzer::ParseCmd(string cmd) return -1; } -vector POP3_Analyzer::TokenizeLine(const string input, const char split) +vector POP3_Analyzer::TokenizeLine(const string& input, char split) { vector tokens; diff --git a/src/analyzer/protocol/pop3/POP3.h b/src/analyzer/protocol/pop3/POP3.h index 8159f0f6f1..1af2e4226c 100644 --- a/src/analyzer/protocol/pop3/POP3.h +++ b/src/analyzer/protocol/pop3/POP3.h @@ -99,7 +99,7 @@ protected: void EndData(); void StartTLS(); - vector TokenizeLine(const string input, const char split); + vector TokenizeLine(const string& input, char split); int ParseCmd(string cmd); void AuthSuccessfull(); void POP3Event(EventHandlerPtr event, bool is_orig, @@ -114,4 +114,4 @@ private: tcp::ContentLine_Analyzer* cl_resp; }; -} } // namespace analyzer::* +} } // namespace analyzer::* diff --git a/src/analyzer/protocol/ssh/ssh-analyzer.pac b/src/analyzer/protocol/ssh/ssh-analyzer.pac index 80bc472db6..f9c41755d4 100644 --- a/src/analyzer/protocol/ssh/ssh-analyzer.pac +++ b/src/analyzer/protocol/ssh/ssh-analyzer.pac @@ -5,12 +5,12 @@ %} %header{ -VectorVal* name_list_to_vector(const bytestring nl); +VectorVal* name_list_to_vector(const bytestring& nl); %} %code{ // Copied from IRC_Analyzer::SplitWords -VectorVal* name_list_to_vector(const bytestring nl) +VectorVal* name_list_to_vector(const bytestring& nl) { VectorVal* vv = new VectorVal(internal_type("string_vec")->AsVectorType()); diff --git a/src/file_analysis/AnalyzerSet.cc b/src/file_analysis/AnalyzerSet.cc index 35968c9a02..4bc49e802d 100644 --- a/src/file_analysis/AnalyzerSet.cc +++ b/src/file_analysis/AnalyzerSet.cc @@ -38,7 +38,7 @@ AnalyzerSet::~AnalyzerSet() delete analyzer_hash; } -Analyzer* AnalyzerSet::Find(file_analysis::Tag tag, RecordVal* args) +Analyzer* AnalyzerSet::Find(const file_analysis::Tag& tag, RecordVal* args) { HashKey* key = GetKey(tag, args); Analyzer* rval = analyzer_map.Lookup(key); @@ -46,7 +46,7 @@ Analyzer* AnalyzerSet::Find(file_analysis::Tag tag, RecordVal* args) return rval; } -bool AnalyzerSet::Add(file_analysis::Tag tag, RecordVal* args) +bool AnalyzerSet::Add(const file_analysis::Tag& tag, RecordVal* args) { HashKey* key = GetKey(tag, args); @@ -73,7 +73,7 @@ bool AnalyzerSet::Add(file_analysis::Tag tag, RecordVal* args) return true; } -Analyzer* AnalyzerSet::QueueAdd(file_analysis::Tag tag, RecordVal* args) +Analyzer* AnalyzerSet::QueueAdd(const file_analysis::Tag& tag, RecordVal* args) { HashKey* key = GetKey(tag, args); file_analysis::Analyzer* a = InstantiateAnalyzer(tag, args); @@ -106,12 +106,12 @@ bool AnalyzerSet::AddMod::Perform(AnalyzerSet* set) return true; } -bool AnalyzerSet::Remove(file_analysis::Tag tag, RecordVal* args) +bool AnalyzerSet::Remove(const file_analysis::Tag& tag, RecordVal* args) { return Remove(tag, GetKey(tag, args)); } -bool AnalyzerSet::Remove(file_analysis::Tag tag, HashKey* key) +bool AnalyzerSet::Remove(const file_analysis::Tag& tag, HashKey* key) { file_analysis::Analyzer* a = (file_analysis::Analyzer*) analyzer_map.Remove(key); @@ -139,7 +139,7 @@ bool AnalyzerSet::Remove(file_analysis::Tag tag, HashKey* key) return true; } -bool AnalyzerSet::QueueRemove(file_analysis::Tag tag, RecordVal* args) +bool AnalyzerSet::QueueRemove(const file_analysis::Tag& tag, RecordVal* args) { HashKey* key = GetKey(tag, args); @@ -153,7 +153,7 @@ bool AnalyzerSet::RemoveMod::Perform(AnalyzerSet* set) return set->Remove(tag, key); } -HashKey* AnalyzerSet::GetKey(file_analysis::Tag t, RecordVal* args) const +HashKey* AnalyzerSet::GetKey(const file_analysis::Tag& t, RecordVal* args) const { ListVal* lv = new ListVal(TYPE_ANY); lv->Append(t.AsEnumVal()->Ref()); @@ -166,7 +166,7 @@ HashKey* AnalyzerSet::GetKey(file_analysis::Tag t, RecordVal* args) const return key; } -file_analysis::Analyzer* AnalyzerSet::InstantiateAnalyzer(Tag tag, +file_analysis::Analyzer* AnalyzerSet::InstantiateAnalyzer(const Tag& tag, RecordVal* args) const { file_analysis::Analyzer* a = file_mgr->InstantiateAnalyzer(tag, args, file); diff --git a/src/file_analysis/AnalyzerSet.h b/src/file_analysis/AnalyzerSet.h index cc5f9b1122..3f8848d22e 100644 --- a/src/file_analysis/AnalyzerSet.h +++ b/src/file_analysis/AnalyzerSet.h @@ -41,7 +41,7 @@ public: * @param args an \c AnalyzerArgs record. * @return pointer to an analyzer instance, or a null pointer if not found. */ - Analyzer* Find(file_analysis::Tag tag, RecordVal* args); + Analyzer* Find(const file_analysis::Tag& tag, RecordVal* args); /** * Attach an analyzer to #file immediately. @@ -49,7 +49,7 @@ public: * @param args an \c AnalyzerArgs value which specifies an analyzer. * @return true if analyzer was instantiated/attached, else false. */ - bool Add(file_analysis::Tag tag, RecordVal* args); + bool Add(const file_analysis::Tag& tag, RecordVal* args); /** * Queue the attachment of an analyzer to #file. @@ -58,7 +58,7 @@ 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(file_analysis::Tag tag, RecordVal* args); + file_analysis::Analyzer* QueueAdd(const file_analysis::Tag& tag, RecordVal* args); /** * Remove an analyzer from #file immediately. @@ -66,7 +66,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(file_analysis::Tag tag, RecordVal* args); + bool Remove(const file_analysis::Tag& tag, RecordVal* args); /** * Queue the removal of an analyzer from #file. @@ -74,7 +74,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(file_analysis::Tag tag, RecordVal* args); + bool QueueRemove(const file_analysis::Tag& tag, RecordVal* args); /** * Perform all queued modifications to the current analyzer set. @@ -107,7 +107,7 @@ protected: * @param args an \c AnalyzerArgs value which specifies an analyzer. * @return the hash key calculated from \a args */ - HashKey* GetKey(file_analysis::Tag tag, RecordVal* args) const; + HashKey* GetKey(const file_analysis::Tag& tag, RecordVal* args) const; /** * Create an instance of a file analyzer. @@ -115,7 +115,7 @@ protected: * @param args an \c AnalyzerArgs value which specifies an analyzer. * @return a new file analyzer instance. */ - file_analysis::Analyzer* InstantiateAnalyzer(file_analysis::Tag tag, + file_analysis::Analyzer* InstantiateAnalyzer(const file_analysis::Tag& tag, RecordVal* args) const; /** @@ -131,7 +131,7 @@ protected: * just used for debugging messages. * @param key the hash key which represents the analyzer's \c AnalyzerArgs. */ - bool Remove(file_analysis::Tag tag, HashKey* key); + bool Remove(const file_analysis::Tag& tag, HashKey* key); private: @@ -190,7 +190,7 @@ 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(file_analysis::Tag arg_tag, HashKey* arg_key) + RemoveMod(const file_analysis::Tag& arg_tag, HashKey* arg_key) : Modification(), tag(arg_tag), key(arg_key) {} ~RemoveMod() override {} bool Perform(AnalyzerSet* set) override; diff --git a/src/file_analysis/Manager.cc b/src/file_analysis/Manager.cc index 9c7f87d8b5..bc55c734e5 100644 --- a/src/file_analysis/Manager.cc +++ b/src/file_analysis/Manager.cc @@ -100,7 +100,7 @@ void Manager::SetHandle(const string& handle) } string Manager::DataIn(const u_char* data, uint64_t len, uint64_t offset, - analyzer::Tag tag, Connection* conn, bool is_orig, + const analyzer::Tag& tag, Connection* conn, bool is_orig, const string& precomputed_id, const string& mime_type) { string id = precomputed_id.empty() ? GetFileID(tag, conn, is_orig) : precomputed_id; @@ -129,7 +129,7 @@ string Manager::DataIn(const u_char* data, uint64_t len, uint64_t offset, return id; } -string Manager::DataIn(const u_char* data, uint64_t len, analyzer::Tag tag, +string Manager::DataIn(const u_char* data, uint64_t len, const analyzer::Tag& tag, Connection* conn, bool is_orig, const string& precomputed_id, const string& mime_type) { @@ -170,13 +170,13 @@ void Manager::DataIn(const u_char* data, uint64_t len, const string& file_id, RemoveFile(file->GetID()); } -void Manager::EndOfFile(analyzer::Tag tag, Connection* conn) +void Manager::EndOfFile(const analyzer::Tag& tag, Connection* conn) { EndOfFile(tag, conn, true); EndOfFile(tag, conn, false); } -void Manager::EndOfFile(analyzer::Tag tag, Connection* conn, bool is_orig) +void Manager::EndOfFile(const analyzer::Tag& tag, Connection* conn, bool is_orig) { // Don't need to create a file if we're just going to remove it right away. RemoveFile(GetFileID(tag, conn, is_orig)); @@ -187,7 +187,7 @@ void Manager::EndOfFile(const string& file_id) RemoveFile(file_id); } -string Manager::Gap(uint64_t offset, uint64_t len, analyzer::Tag tag, +string Manager::Gap(uint64_t offset, uint64_t len, const analyzer::Tag& tag, Connection* conn, bool is_orig, const string& precomputed_id) { string id = precomputed_id.empty() ? GetFileID(tag, conn, is_orig) : precomputed_id; @@ -200,7 +200,7 @@ string Manager::Gap(uint64_t offset, uint64_t len, analyzer::Tag tag, return id; } -string Manager::SetSize(uint64_t size, analyzer::Tag tag, Connection* conn, +string Manager::SetSize(uint64_t size, const analyzer::Tag& tag, Connection* conn, bool is_orig, const string& precomputed_id) { string id = precomputed_id.empty() ? GetFileID(tag, conn, is_orig) : precomputed_id; @@ -278,7 +278,7 @@ bool Manager::SetExtractionLimit(const string& file_id, RecordVal* args, return file->SetExtractionLimit(args, n); } -bool Manager::AddAnalyzer(const string& file_id, file_analysis::Tag tag, +bool Manager::AddAnalyzer(const string& file_id, const file_analysis::Tag& tag, RecordVal* args) const { File* file = LookupFile(file_id); @@ -289,7 +289,7 @@ bool Manager::AddAnalyzer(const string& file_id, file_analysis::Tag tag, return file->AddAnalyzer(tag, args); } -bool Manager::RemoveAnalyzer(const string& file_id, file_analysis::Tag tag, +bool Manager::RemoveAnalyzer(const string& file_id, const file_analysis::Tag& tag, RecordVal* args) const { File* file = LookupFile(file_id); @@ -301,7 +301,7 @@ bool Manager::RemoveAnalyzer(const string& file_id, file_analysis::Tag tag, } File* Manager::GetFile(const string& file_id, Connection* conn, - analyzer::Tag tag, bool is_orig, bool update_conn, + const analyzer::Tag& tag, bool is_orig, bool update_conn, const char* source_name) { if ( file_id.empty() ) @@ -417,7 +417,7 @@ bool Manager::IsIgnored(const string& file_id) return ignored.find(file_id) != ignored.end(); } -string Manager::GetFileID(analyzer::Tag tag, Connection* c, bool is_orig) +string Manager::GetFileID(const analyzer::Tag& tag, Connection* c, bool is_orig) { current_file_id.clear(); @@ -442,7 +442,7 @@ string Manager::GetFileID(analyzer::Tag tag, Connection* c, bool is_orig) return current_file_id; } -bool Manager::IsDisabled(analyzer::Tag tag) +bool Manager::IsDisabled(const analyzer::Tag& tag) { if ( ! disabled ) disabled = internal_const_val("Files::disable")->AsTableVal(); @@ -460,7 +460,7 @@ bool Manager::IsDisabled(analyzer::Tag tag) return rval; } -Analyzer* Manager::InstantiateAnalyzer(Tag tag, RecordVal* args, File* f) const +Analyzer* Manager::InstantiateAnalyzer(const Tag& tag, RecordVal* args, File* f) const { Component* c = Lookup(tag); diff --git a/src/file_analysis/Manager.h b/src/file_analysis/Manager.h index 37c48402b5..0c4d46e95b 100644 --- a/src/file_analysis/Manager.h +++ b/src/file_analysis/Manager.h @@ -110,7 +110,7 @@ public: * indicates the associate file is not going to be analyzed further. */ std::string DataIn(const u_char* data, uint64_t len, uint64_t offset, - analyzer::Tag tag, Connection* conn, bool is_orig, + const analyzer::Tag& tag, Connection* conn, bool is_orig, const std::string& precomputed_file_id = "", const std::string& mime_type = ""); @@ -136,7 +136,7 @@ public: * the \c get_file_handle script-layer event). An empty string * indicates the associated file is not going to be analyzed further. */ - std::string DataIn(const u_char* data, uint64_t len, analyzer::Tag tag, + std::string DataIn(const u_char* data, uint64_t len, const analyzer::Tag& tag, Connection* conn, bool is_orig, const std::string& precomputed_file_id = "", const std::string& mime_type = ""); @@ -159,7 +159,7 @@ public: * @param tag network protocol over which the file data is transferred. * @param conn network connection over which the file data is transferred. */ - void EndOfFile(analyzer::Tag tag, Connection* conn); + void EndOfFile(const analyzer::Tag& tag, Connection* conn); /** * Signal the end of file data being transferred over a connection in @@ -167,7 +167,7 @@ public: * @param tag network protocol over which the file data is transferred. * @param conn network connection over which the file data is transferred. */ - void EndOfFile(analyzer::Tag tag, Connection* conn, bool is_orig); + void EndOfFile(const analyzer::Tag& tag, Connection* conn, bool is_orig); /** * Signal the end of file data being transferred using the file identifier. @@ -191,7 +191,7 @@ public: * the \c get_file_handle script-layer event). An empty string * indicates the associate file is not going to be analyzed further. */ - std::string Gap(uint64_t offset, uint64_t len, analyzer::Tag tag, + std::string Gap(uint64_t offset, uint64_t len, const analyzer::Tag& tag, Connection* conn, bool is_orig, const std::string& precomputed_file_id = ""); @@ -210,7 +210,7 @@ public: * the \c get_file_handle script-layer event). An empty string * indicates the associate file is not going to be analyzed further. */ - std::string SetSize(uint64_t size, analyzer::Tag tag, Connection* conn, + std::string SetSize(uint64_t size, const analyzer::Tag& tag, Connection* conn, bool is_orig, const std::string& precomputed_file_id = ""); /** @@ -276,7 +276,7 @@ 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 string& file_id, file_analysis::Tag tag, + bool AddAnalyzer(const string& file_id, const file_analysis::Tag& tag, RecordVal* args) const; /** @@ -286,7 +286,7 @@ 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 string& file_id, file_analysis::Tag tag, + bool RemoveAnalyzer(const string& file_id, const file_analysis::Tag& tag, RecordVal* args) const; /** @@ -303,7 +303,7 @@ public: * @param f The file analzer is to be associated with. * @return The new analyzer instance or null if tag is invalid. */ - Analyzer* InstantiateAnalyzer(Tag tag, RecordVal* args, File* f) const; + Analyzer* InstantiateAnalyzer(const Tag& tag, RecordVal* args, File* f) const; /** * Returns a set of all matching MIME magic signatures for a given @@ -359,7 +359,7 @@ protected: * connection-related fields. */ File* GetFile(const string& file_id, Connection* conn = 0, - analyzer::Tag tag = analyzer::Tag::Error, + const analyzer::Tag& tag = analyzer::Tag::Error, bool is_orig = false, bool update_conn = true, const char* source_name = 0); @@ -390,7 +390,7 @@ protected: * @return #current_file_id, which is a hash of a unique file handle string * set by a \c get_file_handle event handler. */ - std::string GetFileID(analyzer::Tag tag, Connection* c, bool is_orig); + std::string GetFileID(const analyzer::Tag& tag, Connection* c, bool is_orig); /** * Check if analysis is available for files transferred over a given @@ -400,7 +400,7 @@ protected: * @return whether file analysis is disabled for the analyzer given by * \a tag. */ - static bool IsDisabled(analyzer::Tag tag); + static bool IsDisabled(const analyzer::Tag& tag); private: typedef set TagSet; diff --git a/src/file_analysis/analyzer/x509/X509Common.cc b/src/file_analysis/analyzer/x509/X509Common.cc index a14e73085a..dcad79ed7f 100644 --- a/src/file_analysis/analyzer/x509/X509Common.cc +++ b/src/file_analysis/analyzer/x509/X509Common.cc @@ -16,7 +16,7 @@ using namespace file_analysis; -X509Common::X509Common(file_analysis::Tag arg_tag, RecordVal* arg_args, File* arg_file) +X509Common::X509Common(const file_analysis::Tag& arg_tag, RecordVal* arg_args, File* arg_file) : file_analysis::Analyzer(arg_tag, arg_args, arg_file) { } @@ -230,7 +230,7 @@ void file_analysis::X509Common::ParseSignedCertificateTimestamps(X509_EXTENSION* delete conn; } -void file_analysis::X509Common::ParseExtension(X509_EXTENSION* ex, EventHandlerPtr h, bool global) +void file_analysis::X509Common::ParseExtension(X509_EXTENSION* ex, const EventHandlerPtr& h, bool global) { char name[256]; char oid[256]; diff --git a/src/file_analysis/analyzer/x509/X509Common.h b/src/file_analysis/analyzer/x509/X509Common.h index 019433a47a..72f667e6e9 100644 --- a/src/file_analysis/analyzer/x509/X509Common.h +++ b/src/file_analysis/analyzer/x509/X509Common.h @@ -35,9 +35,9 @@ public: static double GetTimeFromAsn1(const ASN1_TIME* atime, File* f, Reporter* reporter); protected: - X509Common(file_analysis::Tag arg_tag, RecordVal* arg_args, File* arg_file); + X509Common(const file_analysis::Tag& arg_tag, RecordVal* arg_args, File* arg_file); - void ParseExtension(X509_EXTENSION* ex, EventHandlerPtr h, bool global); + void ParseExtension(X509_EXTENSION* ex, const EventHandlerPtr& h, bool global); void ParseSignedCertificateTimestamps(X509_EXTENSION* ext); virtual void ParseExtensionsSpecific(X509_EXTENSION* ex, bool, ASN1_OBJECT*, const char*) = 0; }; diff --git a/src/input/Manager.cc b/src/input/Manager.cc index 53981e589f..ac2e6fb7cf 100644 --- a/src/input/Manager.cc +++ b/src/input/Manager.cc @@ -735,7 +735,7 @@ bool Manager::CreateTableStream(RecordVal* fval) return true; } -bool Manager::CheckErrorEventTypes(std::string stream_name, const Func* ev, bool table) const +bool Manager::CheckErrorEventTypes(const std::string& stream_name, const Func* ev, bool table) const { if ( ev == nullptr ) return true; diff --git a/src/input/Manager.h b/src/input/Manager.h index 69e3c2964d..1077a07cae 100644 --- a/src/input/Manager.h +++ b/src/input/Manager.h @@ -191,7 +191,7 @@ private: // Check if the types of the error_ev event are correct. If table is // true, check for tablestream type, otherwhise check for eventstream // type. - bool CheckErrorEventTypes(std::string stream_name, const Func* error_event, bool table) const; + bool CheckErrorEventTypes(const std::string& stream_name, const Func* error_event, bool table) const; // SendEntry implementation for Table stream. int SendEntryTable(Stream* i, const threading::Value* const *vals); diff --git a/src/logging/Manager.cc b/src/logging/Manager.cc index 0b92c911b9..d89d86d755 100644 --- a/src/logging/Manager.cc +++ b/src/logging/Manager.cc @@ -386,7 +386,7 @@ bool Manager::DisableStream(EnumVal* id) // Helper for recursive record field unrolling. bool Manager::TraverseRecord(Stream* stream, Filter* filter, RecordType* rt, - TableVal* include, TableVal* exclude, string path, list indices) + TableVal* include, TableVal* exclude, const string& path, const list& indices) { // Only include extensions for the outer record. int num_ext_fields = (indices.size() == 0) ? filter->num_ext_fields : 0; @@ -676,7 +676,7 @@ bool Manager::RemoveFilter(EnumVal* id, StringVal* name) return RemoveFilter(id, name->AsString()->CheckString()); } -bool Manager::RemoveFilter(EnumVal* id, string name) +bool Manager::RemoveFilter(EnumVal* id, const string& name) { Stream* stream = FindStream(id); if ( ! stream ) @@ -1259,7 +1259,7 @@ void Manager::DeleteVals(int num_fields, threading::Value** vals) delete [] vals; } -bool Manager::WriteFromRemote(EnumVal* id, EnumVal* writer, string path, int num_fields, +bool Manager::WriteFromRemote(EnumVal* id, EnumVal* writer, const string& path, int num_fields, threading::Value** vals) { Stream* stream = FindStream(id); diff --git a/src/logging/Manager.h b/src/logging/Manager.h index 2d6a9728c3..4d062c6a07 100644 --- a/src/logging/Manager.h +++ b/src/logging/Manager.h @@ -112,7 +112,7 @@ public: * This methods corresponds directly to the internal BiF defined in * logging.bif, which just forwards here. */ - bool RemoveFilter(EnumVal* id, string name); + bool RemoveFilter(EnumVal* id, const string& name); /** * Write a record to a log stream. @@ -165,7 +165,7 @@ public: * @param vals An array of log values to write, of size num_fields. * The method takes ownership of the array. */ - bool WriteFromRemote(EnumVal* stream, EnumVal* writer, string path, + bool WriteFromRemote(EnumVal* stream, EnumVal* writer, const string& path, int num_fields, threading::Value** vals); /** @@ -256,7 +256,7 @@ private: struct WriterInfo; bool TraverseRecord(Stream* stream, Filter* filter, RecordType* rt, - TableVal* include, TableVal* exclude, string path, list indices); + TableVal* include, TableVal* exclude, const string& path, const list& indices); threading::Value** RecordToFilterVals(Stream* stream, Filter* filter, RecordVal* columns); diff --git a/src/zeekygen/Manager.cc b/src/zeekygen/Manager.cc index 555f178133..8650238c74 100644 --- a/src/zeekygen/Manager.cc +++ b/src/zeekygen/Manager.cc @@ -27,7 +27,7 @@ static void DbgAndWarn(const char* msg) } static void WarnMissingScript(const char* type, const ID* id, - string script) + const string& script) { if ( script == "" ) return; diff --git a/src/zeekygen/ReStructuredTextTable.cc b/src/zeekygen/ReStructuredTextTable.cc index 55c576a2a4..ad1cdfde9e 100644 --- a/src/zeekygen/ReStructuredTextTable.cc +++ b/src/zeekygen/ReStructuredTextTable.cc @@ -24,7 +24,7 @@ void ReStructuredTextTable::AddRow(const vector& new_row) longest_row_in_column[i] = new_row[i].size(); } -string ReStructuredTextTable::MakeBorder(const vector col_sizes, +string ReStructuredTextTable::MakeBorder(const vector& col_sizes, char border) { string rval; diff --git a/src/zeekygen/ReStructuredTextTable.h b/src/zeekygen/ReStructuredTextTable.h index 39381d79d0..30bd1c34a6 100644 --- a/src/zeekygen/ReStructuredTextTable.h +++ b/src/zeekygen/ReStructuredTextTable.h @@ -31,7 +31,7 @@ public: * @return A border sized appropriated for the table with columns of sizes * denoted by \a col_sizes. */ - static std::string MakeBorder(const std::vector col_sizes, + static std::string MakeBorder(const std::vector& col_sizes, char border); /** From d23b15c08fd88ef2ac724b4fbafcb994281a09e1 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Mon, 3 Feb 2020 17:23:31 -0500 Subject: [PATCH 06/12] Use std::move in a few places instead of copying a pass-by-value argument (performance-unnecessary-value-param) --- src/Reporter.cc | 2 +- src/RuleMatcher.cc | 2 +- src/broker/Manager.cc | 2 +- src/iosource/Packet.cc | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Reporter.cc b/src/Reporter.cc index 7b7b6a6675..94ba5302f3 100644 --- a/src/Reporter.cc +++ b/src/Reporter.cc @@ -255,7 +255,7 @@ public: using IPPair = std::pair; FlowWeirdTimer(double t, IPPair p, double timeout) - : Timer(t + timeout, TIMER_FLOW_WEIRD_EXPIRE), endpoints(p) + : Timer(t + timeout, TIMER_FLOW_WEIRD_EXPIRE), endpoints(std::move(p)) {} void Dispatch(double t, int is_expire) override diff --git a/src/RuleMatcher.cc b/src/RuleMatcher.cc index d6cdae8bd2..441e41a0b4 100644 --- a/src/RuleMatcher.cc +++ b/src/RuleMatcher.cc @@ -62,7 +62,7 @@ RuleHdrTest::RuleHdrTest(Prot arg_prot, Comp arg_comp, vector arg_v) size = 0; comp = arg_comp; vals = new maskedvalue_list; - prefix_vals = arg_v; + prefix_vals = std::move(arg_v); sibling = 0; child = 0; pattern_rules = 0; diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index 29564a386f..eae0ce723f 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -394,7 +394,7 @@ bool Manager::PublishEvent(string topic, RecordVal* args) xs.emplace_back(data_val->data); } - return PublishEvent(topic, event_name, std::move(xs)); + return PublishEvent(std::move(topic), event_name, std::move(xs)); } bool Manager::PublishIdentifier(std::string topic, std::string id) diff --git a/src/iosource/Packet.cc b/src/iosource/Packet.cc index 49d0cf35b0..0195f18be4 100644 --- a/src/iosource/Packet.cc +++ b/src/iosource/Packet.cc @@ -29,7 +29,7 @@ void Packet::Init(int arg_link_type, pkt_timeval *arg_ts, uint32_t arg_caplen, ts = *arg_ts; cap_len = arg_caplen; len = arg_len; - tag = arg_tag; + tag = std::move(arg_tag); copy = arg_copy; From 3572e38ec29cea0d28d2032291bfe611a0dce1dc Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Mon, 3 Feb 2020 17:57:22 -0500 Subject: [PATCH 07/12] Mark a few clang-tidy findings as false-positive --- src/CompHash.cc | 4 ++++ src/RE.cc | 2 +- src/broker/Data.cc | 2 ++ src/zeekygen/Manager.cc | 2 ++ 4 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/CompHash.cc b/src/CompHash.cc index 107eeefde8..ba640d50f8 100644 --- a/src/CompHash.cc +++ b/src/CompHash.cc @@ -918,6 +918,10 @@ const char* CompositeHash::RecoverOneVal(const HashKey* k, const char* kp0, kp = RecoverOneVal(k, kp, k_end, rt->FieldType(i), v, optional); + + // An earlier call to reporter->InternalError would have called abort() and broken the + // call tree that clang-tidy is relying on to get the error described. + // NOLINTNEXTLINE(clang-analyzer-core.uninitialized.Branch) if ( ! (v || optional) ) { reporter->InternalError("didn't recover expected number of fields from HashKey"); diff --git a/src/RE.cc b/src/RE.cc index 7ed075a0be..083356b5c4 100644 --- a/src/RE.cc +++ b/src/RE.cc @@ -436,7 +436,7 @@ unsigned int Specific_RE_Matcher::MemoryAllocation() const + equiv_class.Size() - padded_sizeof(EquivClass) + (dfa ? dfa->MemoryAllocation() : 0) // this is ref counted; consider the bytes here? + padded_sizeof(*any_ccl) - + padded_sizeof(*accepted) + + padded_sizeof(*accepted) // NOLINT(bugprone-sizeof-container) + accepted->size() * padded_sizeof(AcceptingSet::key_type); } diff --git a/src/broker/Data.cc b/src/broker/Data.cc index 0447ec882e..a659c1c938 100644 --- a/src/broker/Data.cc +++ b/src/broker/Data.cc @@ -1148,6 +1148,8 @@ broker::data& bro_broker::opaque_field_to_data(RecordVal* v, Frame* f) reporter->RuntimeError(f->GetCall()->GetLocationInfo(), "Broker::Data's opaque field is not set"); + // RuntimeError throws an exception which causes this line to never exceute. + // NOLINTNEXTLINE(clang-analyzer-core.uninitialized.UndefReturn) return static_cast(d)->data; } diff --git a/src/zeekygen/Manager.cc b/src/zeekygen/Manager.cc index 8650238c74..3029d1640f 100644 --- a/src/zeekygen/Manager.cc +++ b/src/zeekygen/Manager.cc @@ -90,6 +90,8 @@ Manager::Manager(const string& arg_config, const string& bro_command) reporter->InternalError("Zeekygen can't get mtime of zeek binary %s (try again by specifying the absolute or relative path to Zeek): %s", path_to_bro.c_str(), strerror(errno)); + // Internal error will abort above in the case that stat isn't initialized + // NOLINTNEXTLINE(clang-analyzer-core.uninitialized.Assign) bro_mtime = s.st_mtime; } From c5748e4494b257df4fd2b0ceb49d05ced4c8159e Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Mon, 3 Feb 2020 18:15:38 -0500 Subject: [PATCH 08/12] Give real variable names to SegmentProfiler objects when defining them (bugprone-unused-raii) The reason behind this one is that without a real variable name, the profile objects are immediately desctructed and the profiling only happens for the small window when they were valid. If the intention is to profile the method where they were defined, this doesn't actually happen. --- src/Event.cc | 2 +- src/Func.cc | 4 ++-- src/Net.cc | 2 +- src/Sessions.cc | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Event.cc b/src/Event.cc index 292d70b05f..ef69e7a42d 100644 --- a/src/Event.cc +++ b/src/Event.cc @@ -152,7 +152,7 @@ void EventMgr::Drain() if ( event_queue_flush_point ) QueueEventFast(event_queue_flush_point, val_list{}); - SegmentProfiler(segment_logger, "draining-events"); + SegmentProfiler prof(segment_logger, "draining-events"); PLUGIN_HOOK_VOID(HOOK_DRAIN_EVENTS, HookDrainEvents()); diff --git a/src/Func.cc b/src/Func.cc index 759b132a9a..cd5062af9b 100644 --- a/src/Func.cc +++ b/src/Func.cc @@ -309,7 +309,7 @@ Val* BroFunc::Call(val_list* args, Frame* parent) const #ifdef PROFILE_BRO_FUNCTIONS DEBUG_MSG("Function: %s\n", Name()); #endif - SegmentProfiler(segment_logger, location); + SegmentProfiler prof(segment_logger, location); if ( sample_logger ) sample_logger->FunctionSeen(this); @@ -629,7 +629,7 @@ Val* BuiltinFunc::Call(val_list* args, Frame* parent) const #ifdef PROFILE_BRO_FUNCTIONS DEBUG_MSG("Function: %s\n", Name()); #endif - SegmentProfiler(segment_logger, Name()); + SegmentProfiler prof(segment_logger, Name()); if ( sample_logger ) sample_logger->FunctionSeen(this); diff --git a/src/Net.cc b/src/Net.cc index 1b35907a93..fa9bbd56a2 100644 --- a/src/Net.cc +++ b/src/Net.cc @@ -212,7 +212,7 @@ void net_init(const std::optional& interface, void expire_timers(iosource::PktSrc* src_ps) { - SegmentProfiler(segment_logger, "expiring-timers"); + SegmentProfiler prof(segment_logger, "expiring-timers"); current_dispatched += timer_mgr->Advance(network_time, diff --git a/src/Sessions.cc b/src/Sessions.cc index 8f66ae8633..c86395267b 100644 --- a/src/Sessions.cc +++ b/src/Sessions.cc @@ -120,7 +120,7 @@ void NetSessions::Done() void NetSessions::NextPacket(double t, const Packet* pkt) { - SegmentProfiler(segment_logger, "dispatching-packet"); + SegmentProfiler prof(segment_logger, "dispatching-packet"); if ( raw_packet ) mgr.QueueEventFast(raw_packet, {pkt->BuildPktHdrVal()}); From 66c4a9338310c67be28e06c77d41bb61d93f0d15 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Thu, 6 Feb 2020 15:40:51 -0500 Subject: [PATCH 09/12] Remove unnecessary const from return value (readability-const-return-type) --- src/DNS_Mgr.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/DNS_Mgr.cc b/src/DNS_Mgr.cc index 90b42db748..bb44bfb319 100644 --- a/src/DNS_Mgr.cc +++ b/src/DNS_Mgr.cc @@ -64,7 +64,7 @@ public: // Returns nil if this was an address request. const char* ReqHost() const { return host; } const IPAddr& ReqAddr() const { return addr; } - const bool ReqIsTxt() const { return qtype == 16; } + bool ReqIsTxt() const { return qtype == 16; } int MakeRequest(nb_dns_info* nb_dns); int RequestPending() const { return request_pending; } From 9bfe162cadacf2616f03fae4897648a173362703 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Thu, 6 Feb 2020 16:29:51 -0500 Subject: [PATCH 10/12] Fix missing namespace that was causing a build error --- src/DbgBreakpoint.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/DbgBreakpoint.h b/src/DbgBreakpoint.h index ab1cf45489..e27fabe2c1 100644 --- a/src/DbgBreakpoint.h +++ b/src/DbgBreakpoint.h @@ -21,7 +21,7 @@ public: void SetID(int newID) { BPID = newID; } // True if breakpoint could be set; false otherwise - bool SetLocation(ParseLocationRec plr, string_view loc_str); + bool SetLocation(ParseLocationRec plr, std::string_view loc_str); bool SetLocation(Stmt* stmt); bool SetLocation(double time); From da7749fc43b480c296245e4dc2678051010829ae Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Thu, 6 Feb 2020 16:30:20 -0500 Subject: [PATCH 11/12] Add a couple of missing #includes that clang-tidy complains about (clang-diagnostic-error) --- src/iosource/pcap/pcap.bif | 4 ++++ src/probabilistic/cardinality-counter.bif | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/iosource/pcap/pcap.bif b/src/iosource/pcap/pcap.bif index e7b97cf4dd..5aa7f2bcf2 100644 --- a/src/iosource/pcap/pcap.bif +++ b/src/iosource/pcap/pcap.bif @@ -4,6 +4,10 @@ module Pcap; const snaplen: count; const bufsize: count; +%%{ +#include "iosource/Manager.h" +%%} + ## Precompiles a PCAP filter and binds it to a given identifier. ## ## id: The PCAP identifier to reference the filter *s* later on. diff --git a/src/probabilistic/cardinality-counter.bif b/src/probabilistic/cardinality-counter.bif index 1e12765b57..dea6581df2 100644 --- a/src/probabilistic/cardinality-counter.bif +++ b/src/probabilistic/cardinality-counter.bif @@ -2,6 +2,7 @@ %%{ #include "probabilistic/CardinalityCounter.h" +#include "OpaqueVal.h" using namespace probabilistic; %%} @@ -132,4 +133,3 @@ function hll_cardinality_copy%(handle: opaque of cardinality%): opaque of cardin return out; %} - From d69d0da62e17607bacca0e2d1c623ddbb4593350 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Tue, 11 Feb 2020 11:14:55 -0800 Subject: [PATCH 12/12] fixup! Use string_view for a couple of Dbg methods --- src/DbgBreakpoint.cc | 3 ++- src/DebugCmds.cc | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/DbgBreakpoint.cc b/src/DbgBreakpoint.cc index 21753f4035..a1ef53d6ec 100644 --- a/src/DbgBreakpoint.cc +++ b/src/DbgBreakpoint.cc @@ -148,9 +148,10 @@ bool DbgBreakpoint::SetLocation(ParseLocationRec plr, string_view loc_str) else if ( plr.type == plrFunction ) { + std::string loc_s(loc_str); kind = BP_FUNC; function_name = make_full_var_name(current_module.c_str(), - loc_str.data()); + loc_s.c_str()); at_stmt = plr.stmt; const Location* loc = at_stmt->GetLocationInfo(); snprintf(description, sizeof(description), "%s at %s:%d", diff --git a/src/DebugCmds.cc b/src/DebugCmds.cc index 983ee69e84..255d36f060 100644 --- a/src/DebugCmds.cc +++ b/src/DebugCmds.cc @@ -27,7 +27,7 @@ // // Helper routines // -bool string_is_regex(string_view s) +bool string_is_regex(const string& s) { return strpbrk(s.data(), "?*\\+"); }