diff --git a/.clang-tidy b/.clang-tidy index ebcd6d380b..bbbc745364 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,3 +1,4 @@ Checks: [-*, bugprone-assignment-in-if-condition, + bugprone-branch-clone, ] diff --git a/auxil/binpac b/auxil/binpac index f7c94581f1..e1bb430912 160000 --- a/auxil/binpac +++ b/auxil/binpac @@ -1 +1 @@ -Subproject commit f7c94581f1f4d1bef1537824c27503d58000933e +Subproject commit e1bb4309129abc475250d36ca6822624cd3036bb diff --git a/src/CompHash.cc b/src/CompHash.cc index bd366e276c..c9cb324cdb 100644 --- a/src/CompHash.cc +++ b/src/CompHash.cc @@ -232,16 +232,8 @@ bool CompositeHash::RecoverOneVal(const HashKey& hk, Type* t, ValPtr* pval, bool if ( ! pvt ) reporter->InternalError("bad aggregate Val in CompositeHash::RecoverOneVal()"); - else if ( t->Tag() != TYPE_FUNC && ! same_type(pvt, t) ) - // ### Maybe fix later, but may be fundamentally un-checkable --US - { - reporter->InternalError("inconsistent aggregate Val in CompositeHash::RecoverOneVal()"); - *pval = nullptr; - return false; - } - - // ### A crude approximation for now. - else if ( t->Tag() == TYPE_FUNC && pvt->Tag() != TYPE_FUNC ) { + else if ( (t->Tag() != TYPE_FUNC && ! same_type(pvt, t)) || + (t->Tag() == TYPE_FUNC && pvt->Tag() != TYPE_FUNC) ) { reporter->InternalError("inconsistent aggregate Val in CompositeHash::RecoverOneVal()"); *pval = nullptr; return false; diff --git a/src/Expr.cc b/src/Expr.cc index 305f0ab356..74f4637fef 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -1383,8 +1383,8 @@ AddExpr::AddExpr(ExprPtr arg_op1, ExprPtr arg_op2) : BinaryExpr(EXPR_ADD, std::m TypePtr base_result_type; - if ( bt2 == TYPE_INTERVAL && (bt1 == TYPE_TIME || bt1 == TYPE_INTERVAL) ) - base_result_type = base_type(bt1); + if ( (bt2 == TYPE_INTERVAL && (bt1 == TYPE_TIME || bt1 == TYPE_INTERVAL)) ) + base_result_type = base_type(bt1); // NOLINT(bugprone-branch-clone) else if ( bt2 == TYPE_TIME && bt1 == TYPE_INTERVAL ) base_result_type = base_type(bt2); else if ( BothArithmetic(bt1, bt2) ) @@ -1925,7 +1925,7 @@ CmpExpr::CmpExpr(ExprTag tag, ExprPtr _op1, ExprPtr _op2) : BinaryExpr(tag, std: void CmpExpr::Canonicalize() { if ( tag == EXPR_EQ || tag == EXPR_NE ) { if ( op2->GetType()->Tag() == TYPE_PATTERN ) - SwapOps(); + SwapOps(); //NOLINT(bugprone-branch-clone) else if ( op1->GetType()->Tag() == TYPE_PATTERN ) ; diff --git a/src/TraverseTypes.h b/src/TraverseTypes.h index 5a2cd4a613..abe157346a 100644 --- a/src/TraverseTypes.h +++ b/src/TraverseTypes.h @@ -14,20 +14,22 @@ enum TraversalCode { #define HANDLE_TC_STMT_PRE(code) \ { \ - if ( (code) == zeek::detail::TC_ABORTALL ) \ - return (code); \ - else if ( (code) == zeek::detail::TC_ABORTSTMT ) \ - return zeek::detail::TC_CONTINUE; \ + switch ( code ) { \ + case zeek::detail::TC_ABORTALL: return (code); \ + case zeek::detail::TC_ABORTSTMT: return zeek::detail::TC_CONTINUE; \ + case zeek::detail::TC_CONTINUE: \ + default: break; \ + } \ } #define HANDLE_TC_STMT_POST(code) \ { \ - if ( (code) == zeek::detail::TC_ABORTALL ) \ - return (code); \ - else if ( (code) == zeek::detail::TC_ABORTSTMT ) \ - return zeek::detail::TC_CONTINUE; \ - else \ - return (code); \ + switch ( code ) { \ + case zeek::detail::TC_ABORTSTMT: return zeek::detail::TC_CONTINUE; \ + case zeek::detail::TC_ABORTALL: \ + case zeek::detail::TC_CONTINUE: \ + default: return (code); \ + } \ } #define HANDLE_TC_EXPR_PRE(code) HANDLE_TC_STMT_PRE(code) diff --git a/src/Type.cc b/src/Type.cc index 5a5c62b627..9b318b3873 100644 --- a/src/Type.cc +++ b/src/Type.cc @@ -422,8 +422,7 @@ static bool is_supported_index_type(const TypePtr& t, const char** tname, std::u switch ( tag ) { // Allow functions, since they can be compared for Func* pointer equality. - case TYPE_FUNC: return true; - + case TYPE_FUNC: case TYPE_PATTERN: return true; case TYPE_RECORD: { @@ -1843,7 +1842,7 @@ bool same_type(const Type& arg_t1, const Type& arg_t2, bool is_init, bool match_ // First do all checks that don't require any recursion. switch ( t1->Tag() ) { - case TYPE_VOID: + case TYPE_VOID: // NOLINT(bugprone-branch-clone) case TYPE_BOOL: case TYPE_INT: case TYPE_COUNT: @@ -2143,8 +2142,7 @@ bool is_assignable(TypeTag t) { case TYPE_FUNC: case TYPE_ANY: case TYPE_ERROR: - case TYPE_LIST: return true; - + case TYPE_LIST: case TYPE_VECTOR: case TYPE_FILE: case TYPE_OPAQUE: diff --git a/src/analyzer/protocol/dns/DNS.cc b/src/analyzer/protocol/dns/DNS.cc index 87b3f7fbd4..dc0132bbfd 100644 --- a/src/analyzer/protocol/dns/DNS.cc +++ b/src/analyzer/protocol/dns/DNS.cc @@ -924,18 +924,18 @@ bool DNS_Interpreter::ParseRR_RRSIG(detail::DNS_MsgInfo* msg, const u_char*& dat switch ( dsa ) { case detail::RSA_MD5: analyzer->Weird("DNSSEC_RRSIG_NotRecommended_ZoneSignAlgo", util::fmt("%d", algo)); break; - case detail::Diffie_Hellman: break; - case detail::DSA_SHA1: break; - case detail::Elliptic_Curve: break; - case detail::RSA_SHA1: break; - case detail::DSA_NSEC3_SHA1: break; - case detail::RSA_SHA1_NSEC3_SHA1: break; - case detail::RSA_SHA256: break; - case detail::RSA_SHA512: break; - case detail::GOST_R_34_10_2001: break; - case detail::ECDSA_curveP256withSHA256: break; - case detail::ECDSA_curveP384withSHA384: break; - case detail::Ed25519: break; + case detail::Diffie_Hellman: + case detail::DSA_SHA1: + case detail::Elliptic_Curve: + case detail::RSA_SHA1: + case detail::DSA_NSEC3_SHA1: + case detail::RSA_SHA1_NSEC3_SHA1: + case detail::RSA_SHA256: + case detail::RSA_SHA512: + case detail::GOST_R_34_10_2001: + case detail::ECDSA_curveP256withSHA256: + case detail::ECDSA_curveP384withSHA384: + case detail::Ed25519: case detail::Ed448: break; case detail::Indirect: analyzer->Weird("DNSSEC_RRSIG_Indirect_ZoneSignAlgo", util::fmt("%d", algo)); break; case detail::PrivateDNS: analyzer->Weird("DNSSEC_RRSIG_PrivateDNS_ZoneSignAlgo", util::fmt("%d", algo)); break; @@ -999,18 +999,18 @@ bool DNS_Interpreter::ParseRR_DNSKEY(detail::DNS_MsgInfo* msg, const u_char*& da case detail::RSA_MD5: analyzer->Weird("DNSSEC_DNSKEY_NotRecommended_ZoneSignAlgo", util::fmt("%d", dalgorithm)); break; - case detail::Diffie_Hellman: break; - case detail::DSA_SHA1: break; - case detail::Elliptic_Curve: break; - case detail::RSA_SHA1: break; - case detail::DSA_NSEC3_SHA1: break; - case detail::RSA_SHA1_NSEC3_SHA1: break; - case detail::RSA_SHA256: break; - case detail::RSA_SHA512: break; - case detail::GOST_R_34_10_2001: break; - case detail::ECDSA_curveP256withSHA256: break; - case detail::ECDSA_curveP384withSHA384: break; - case detail::Ed25519: break; + case detail::Diffie_Hellman: + case detail::DSA_SHA1: + case detail::Elliptic_Curve: + case detail::RSA_SHA1: + case detail::DSA_NSEC3_SHA1: + case detail::RSA_SHA1_NSEC3_SHA1: + case detail::RSA_SHA256: + case detail::RSA_SHA512: + case detail::GOST_R_34_10_2001: + case detail::ECDSA_curveP256withSHA256: + case detail::ECDSA_curveP384withSHA384: + case detail::Ed25519: case detail::Ed448: break; case detail::Indirect: analyzer->Weird("DNSSEC_DNSKEY_Indirect_ZoneSignAlgo", util::fmt("%d", dalgorithm)); @@ -1216,9 +1216,9 @@ bool DNS_Interpreter::ParseRR_DS(detail::DNS_MsgInfo* msg, const u_char*& data, String* ds_digest = ExtractStream(data, len, rdlength - 4); switch ( ds_digest_type ) { - case detail::SHA1: break; - case detail::SHA256: break; - case detail::GOST_R_34_11_94: break; + case detail::SHA1: + case detail::SHA256: + case detail::GOST_R_34_11_94: case detail::SHA384: break; case detail::reserved: analyzer->Weird("DNSSEC_DS_ReservedDigestType", util::fmt("%d", ds_dtype)); break; default: analyzer->Weird("DNSSEC_DS_unknown_DigestType", util::fmt("%d", ds_dtype)); break; diff --git a/src/analyzer/protocol/ftp/FTP.cc b/src/analyzer/protocol/ftp/FTP.cc index 368162005b..53be3f0452 100644 --- a/src/analyzer/protocol/ftp/FTP.cc +++ b/src/analyzer/protocol/ftp/FTP.cc @@ -309,25 +309,21 @@ void FTP_ADAT_Analyzer::DeliverStream(int len, const u_char* data, bool orig) { break; + // Server isn't going to accept named security mechanism. + // Client has to restart back at the AUTH. case 421: case 431: case 500: case 501: case 503: case 535: - // Server isn't going to accept named security mechanism. - // Client has to restart back at the AUTH. - done = true; - break; + // If the server is sending protected replies, the security + // data exchange must have already succeeded. It does have + // encoded data in the reply, but 632 and 633 are also encrypted. case 631: case 632: - case 633: - // If the server is sending protected replies, the security - // data exchange must have already succeeded. It does have - // encoded data in the reply, but 632 and 633 are also encrypted. - done = true; - break; + case 633: done = true; break; default: break; } diff --git a/src/analyzer/protocol/http/HTTP.cc b/src/analyzer/protocol/http/HTTP.cc index 527693d4d1..4aefbea232 100644 --- a/src/analyzer/protocol/http/HTTP.cc +++ b/src/analyzer/protocol/http/HTTP.cc @@ -103,7 +103,7 @@ void HTTP_Entity::Deliver(int len, const char* data, bool trailing_CRLF) { // Entity body. if ( content_type == analyzer::mime::CONTENT_TYPE_MULTIPART || content_type == analyzer::mime::CONTENT_TYPE_MESSAGE ) - DeliverBody(len, data, trailing_CRLF); + DeliverBody(len, data, trailing_CRLF); // NOLINT(bugprone-branch-clone) else if ( chunked_transfer_state != NON_CHUNKED_TRANSFER ) { switch ( chunked_transfer_state ) { diff --git a/src/analyzer/protocol/irc/IRC.cc b/src/analyzer/protocol/irc/IRC.cc index 9981895cc1..a7f79ba5c2 100644 --- a/src/analyzer/protocol/irc/IRC.cc +++ b/src/analyzer/protocol/irc/IRC.cc @@ -280,9 +280,7 @@ void IRC_Analyzer::DeliverStream(int length, const u_char* line, bool orig) { int servers = 0; for ( size_t i = 1; i < parts.size(); ++i ) { - if ( parts[i] == "users," ) - users = atoi(parts[i - 1].c_str()); - else if ( parts[i] == "clients" ) + if ( parts[i] == "users," || parts[i] == "clients" ) users = atoi(parts[i - 1].c_str()); else if ( parts[i] == "services" ) services = atoi(parts[i - 1].c_str()); diff --git a/src/analyzer/protocol/mime/MIME.cc b/src/analyzer/protocol/mime/MIME.cc index 71a36e9d8f..5965f70242 100644 --- a/src/analyzer/protocol/mime/MIME.cc +++ b/src/analyzer/protocol/mime/MIME.cc @@ -952,10 +952,7 @@ void MIME_Entity::DecodeQuotedPrintable(int len, const char* data) { else if ( (data[i] >= 33 && data[i] <= 60) || // except controls, whitespace and '=' - (data[i] >= 62 && data[i] <= 126) ) - DataOctet(data[i]); - - else if ( data[i] == HT || data[i] == SP ) + (data[i] >= 62 && data[i] <= 126) || (data[i] == HT || data[i] == SP) ) DataOctet(data[i]); else { diff --git a/src/analyzer/protocol/mysql/mysql-protocol.pac b/src/analyzer/protocol/mysql/mysql-protocol.pac index 41fbff5808..60ea7e240c 100644 --- a/src/analyzer/protocol/mysql/mysql-protocol.pac +++ b/src/analyzer/protocol/mysql/mysql-protocol.pac @@ -861,101 +861,47 @@ refine connection MySQL_Conn += { %{ switch ( cmd ) { case COM_SLEEP: - expected_ = EXPECT_STATUS; - break; case COM_QUIT: - expected_ = EXPECT_STATUS; - break; case COM_INIT_DB: + case COM_CREATE_DB: + case COM_DROP_DB: + case COM_REFRESH: + case COM_SHUTDOWN: + case COM_CONNECT: + case COM_PROCESS_KILL: + case COM_DEBUG: + case COM_PING: + case COM_TIME: + case COM_DELAYED_INSERT: + case COM_DAEMON: + case COM_RESET_CONNECTION: expected_ = EXPECT_STATUS; break; case COM_QUERY: + case COM_PROCESS_INFO: expected_ = EXPECT_COLUMN_COUNT; break; case COM_FIELD_LIST: expected_ = EXPECT_COLUMN_DEFINITION_OR_EOF; break; - case COM_CREATE_DB: - expected_ = EXPECT_STATUS; - break; - case COM_DROP_DB: - expected_ = EXPECT_STATUS; - break; - case COM_REFRESH: - expected_ = EXPECT_STATUS; - break; - case COM_SHUTDOWN: - expected_ = EXPECT_STATUS; - break; case COM_STATISTICS: expected_ = EXPECT_REST_OF_PACKET; break; - case COM_PROCESS_INFO: - expected_ = EXPECT_COLUMN_COUNT; - break; - case COM_CONNECT: - expected_ = EXPECT_STATUS; - break; - case COM_PROCESS_KILL: - expected_ = EXPECT_STATUS; - break; - case COM_DEBUG: - expected_ = EXPECT_STATUS; - break; - case COM_PING: - expected_ = EXPECT_STATUS; - break; - case COM_TIME: - expected_ = EXPECT_STATUS; - break; - case COM_DELAYED_INSERT: - expected_ = EXPECT_STATUS; - break; case COM_CHANGE_USER: update_state(CONNECTION_PHASE); break; case COM_BINLOG_DUMP: - expected_ = NO_EXPECTATION; - break; case COM_TABLE_DUMP: - expected_ = NO_EXPECTATION; - break; case COM_CONNECT_OUT: - expected_ = NO_EXPECTATION; - break; case COM_REGISTER_SLAVE: - expected_ = NO_EXPECTATION; - break; case COM_STMT_PREPARE: - expected_ = NO_EXPECTATION; - break; case COM_STMT_EXECUTE: - expected_ = NO_EXPECTATION; - break; case COM_STMT_SEND_LONG_DATA: - expected_ = NO_EXPECTATION; - break; case COM_STMT_CLOSE: - expected_ = NO_EXPECTATION; - break; case COM_STMT_RESET: - expected_ = NO_EXPECTATION; - break; case COM_SET_OPTION: - expected_ = NO_EXPECTATION; - break; case COM_STMT_FETCH: - expected_ = NO_EXPECTATION; - break; - case COM_DAEMON: - expected_ = EXPECT_STATUS; - break; case COM_BINLOG_DUMP_GTID: - expected_ = NO_EXPECTATION; - break; - case COM_RESET_CONNECTION: - expected_ = EXPECT_STATUS; - break; default: expected_ = NO_EXPECTATION; break; diff --git a/src/analyzer/protocol/rpc/MOUNT.cc b/src/analyzer/protocol/rpc/MOUNT.cc index c4b391fbad..8d39d07632 100644 --- a/src/analyzer/protocol/rpc/MOUNT.cc +++ b/src/analyzer/protocol/rpc/MOUNT.cc @@ -22,10 +22,8 @@ bool MOUNT_Interp::RPC_BuildCall(RPC_CallInfo* c, const u_char*& buf, int& n) { switch ( proc ) { case BifEnum::MOUNT3::PROC_NULL: break; - case BifEnum::MOUNT3::PROC_MNT: callarg = mount3_dirmntargs(buf, n); break; - - case BifEnum::MOUNT3::PROC_UMNT: callarg = mount3_dirmntargs(buf, n); break; - + case BifEnum::MOUNT3::PROC_MNT: + case BifEnum::MOUNT3::PROC_UMNT: case BifEnum::MOUNT3::PROC_UMNT_ALL: callarg = mount3_dirmntargs(buf, n); break; default: @@ -88,11 +86,6 @@ bool MOUNT_Interp::RPC_BuildReply(RPC_CallInfo* c, BifEnum::rpc_status rpc_statu break; case BifEnum::MOUNT3::PROC_UMNT: - n = 0; - mount_status = BifEnum::MOUNT3::MNT3_OK; - event = mount_proc_umnt; - break; - case BifEnum::MOUNT3::PROC_UMNT_ALL: n = 0; mount_status = BifEnum::MOUNT3::MNT3_OK; diff --git a/src/analyzer/protocol/rpc/NFS.cc b/src/analyzer/protocol/rpc/NFS.cc index de9feefe5c..c16ab6b231 100644 --- a/src/analyzer/protocol/rpc/NFS.cc +++ b/src/analyzer/protocol/rpc/NFS.cc @@ -27,7 +27,9 @@ bool NFS_Interp::RPC_BuildCall(RPC_CallInfo* c, const u_char*& buf, int& n) { case BifEnum::NFS3::PROC_SETATTR: callarg = nfs3_sattrargs(buf, n); break; - case BifEnum::NFS3::PROC_LOOKUP: callarg = nfs3_diropargs(buf, n); break; + case BifEnum::NFS3::PROC_LOOKUP: + case BifEnum::NFS3::PROC_REMOVE: + case BifEnum::NFS3::PROC_RMDIR: callarg = nfs3_diropargs(buf, n); break; case BifEnum::NFS3::PROC_READ: callarg = nfs3_readargs(buf, n); break; @@ -40,23 +42,13 @@ bool NFS_Interp::RPC_BuildCall(RPC_CallInfo* c, const u_char*& buf, int& n) { case BifEnum::NFS3::PROC_WRITE: callarg = nfs3_writeargs(buf, n); break; case BifEnum::NFS3::PROC_CREATE: - callarg = nfs3_diropargs(buf, n); - // TODO: implement create attributes. For now we just skip - // over them. - n = 0; - break; - case BifEnum::NFS3::PROC_MKDIR: callarg = nfs3_diropargs(buf, n); - // TODO: implement mkdir attributes. For now we just skip + // TODO: implement create and mkdir attributes. For now we just skip // over them. n = 0; break; - case BifEnum::NFS3::PROC_REMOVE: callarg = nfs3_diropargs(buf, n); break; - - case BifEnum::NFS3::PROC_RMDIR: callarg = nfs3_diropargs(buf, n); break; - case BifEnum::NFS3::PROC_RENAME: callarg = nfs3_renameopargs(buf, n); break; case BifEnum::NFS3::PROC_READDIR: callarg = nfs3_readdirargs(false, buf, n); break; diff --git a/src/analyzer/protocol/smtp/SMTP.cc b/src/analyzer/protocol/smtp/SMTP.cc index 616ebecd8b..ac910f667f 100644 --- a/src/analyzer/protocol/smtp/SMTP.cc +++ b/src/analyzer/protocol/smtp/SMTP.cc @@ -523,7 +523,7 @@ void SMTP_Analyzer::UpdateState(int cmd_code, int reply_code, bool orig) { state = detail::SMTP_RCPT_OK; break; - case 250: + case 250: // NOLINT(bugprone-branch-clone) case 251: // ?? Shall we catch 251? (RFC 2821) break; @@ -590,8 +590,7 @@ void SMTP_Analyzer::UpdateState(int cmd_code, int reply_code, bool orig) { state = detail::SMTP_IN_BDAT; break; - case 250: break; // server accepted BDAT transfer. - + case 250: // server accepted BDAT transfer. case 421: case 500: case 501: @@ -620,8 +619,7 @@ void SMTP_Analyzer::UpdateState(int cmd_code, int reply_code, bool orig) { state = detail::SMTP_AFTER_DATA; break; - case 250: break; - + case 250: case 421: case 451: case 452: @@ -668,8 +666,7 @@ void SMTP_Analyzer::UpdateState(int cmd_code, int reply_code, bool orig) { case 334: state = detail::SMTP_IN_AUTH; break; - case 235: state = detail::SMTP_INITIATED; break; - + case 235: case 432: case 454: case 501: diff --git a/src/analyzer/protocol/ssh/ssh-protocol.pac b/src/analyzer/protocol/ssh/ssh-protocol.pac index f0f1f37d79..73022b21e4 100644 --- a/src/analyzer/protocol/ssh/ssh-protocol.pac +++ b/src/analyzer/protocol/ssh/ssh-protocol.pac @@ -406,6 +406,7 @@ refine connection SSH_Conn += { version_ = version_server_; } // SSH1 vs SSH2 -> Undefined + // NOLINTBEGIN(bugprone-branch-clone) else if ( version_client_ == SSH1 && version_server_ == SSH2 ) version_ = UNK; // SSH2 vs SSH1 -> Undefined @@ -423,7 +424,7 @@ refine connection SSH_Conn += { // SSH199 vs SSH1 -> 1 else if ( version_client_ == SSH199 && version_server_ == SSH1 ) version_ = version_server_; - + // NOLINTEND(bugprone-branch-clone) return true; %} diff --git a/src/analyzer/protocol/tcp/TCP_Reassembler.cc b/src/analyzer/protocol/tcp/TCP_Reassembler.cc index bdfd655359..103ec6d6eb 100644 --- a/src/analyzer/protocol/tcp/TCP_Reassembler.cc +++ b/src/analyzer/protocol/tcp/TCP_Reassembler.cc @@ -375,7 +375,7 @@ void TCP_Reassembler::BlockInserted(DataBlockMap::const_iterator it) { // Our endpoint's peer doesn't do reassembly and so // (presumably) isn't processing acks. So don't hold // the now-delivered data. - TrimToSeq(last_reassem_seq); + TrimToSeq(last_reassem_seq); // NOLINT(bugprone-branch-clone) else if ( e->NoDataAcked() && zeek::detail::tcp_max_initial_window && e->Size() > static_cast(zeek::detail::tcp_max_initial_window) ) diff --git a/src/broker/Data.cc b/src/broker/Data.cc index 33d67a334c..cadc86fdd2 100644 --- a/src/broker/Data.cc +++ b/src/broker/Data.cc @@ -458,7 +458,7 @@ struct type_checker { result_type operator()(const std::string& a) { switch ( type->Tag() ) { - case TYPE_STRING: return true; + case TYPE_STRING: case TYPE_FILE: return true; default: return false; } diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index 2e36509a2f..c09e53e00e 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -1937,7 +1937,7 @@ void Manager::ProcessStoreResponse(detail::StoreHandleVal* s, broker::store::res BrokerData tmp{std::move(*response.answer)}; request->second->Result(detail::query_result(std::move(tmp).ToRecordVal())); } - else if ( response.answer.error() == broker::ec::request_timeout ) { + else if ( response.answer.error() == broker::ec::request_timeout ) { // NOLINT(bugprone-branch-clone) // Fine, trigger's timeout takes care of things. } else if ( response.answer.error() == broker::ec::stale_data ) { diff --git a/src/cluster/websocket/auxil/CMakeLists.txt b/src/cluster/websocket/auxil/CMakeLists.txt index 0fc5b821f7..f13a5f8e14 100644 --- a/src/cluster/websocket/auxil/CMakeLists.txt +++ b/src/cluster/websocket/auxil/CMakeLists.txt @@ -9,3 +9,7 @@ add_subdirectory(IXWebSocket) # We are not interested in any warnings from `IXWebSocket`'s compilation # itself, suppress them. target_compile_options(ixwebsocket PRIVATE "-w") + +# Disable clang-tidy and iwyu. +set_target_properties(ixwebsocket PROPERTIES CXX_INCLUDE_WHAT_YOU_USE "") +set_target_properties(ixwebsocket PROPERTIES CXX_CLANG_TIDY "") diff --git a/src/logging/Manager.cc b/src/logging/Manager.cc index c702750bb6..56f75dc485 100644 --- a/src/logging/Manager.cc +++ b/src/logging/Manager.cc @@ -777,19 +777,8 @@ bool Manager::TraverseRecord(Stream* stream, Filter* filter, RecordType* rt, Tab continue; } - else if ( t->Tag() == TYPE_TABLE && t->AsTableType()->IsSet() ) { - // That's ok, we handle it below. - } - - else if ( t->Tag() == TYPE_VECTOR ) { - // That's ok, we handle it below. - } - - else if ( t->Tag() == TYPE_FILE ) { - // That's ok, we handle it below. - } - - else if ( t->Tag() == TYPE_FUNC ) { + else if ( (t->Tag() == TYPE_TABLE && t->AsTableType()->IsSet()) || t->Tag() == TYPE_VECTOR || + t->Tag() == TYPE_FILE || t->Tag() == TYPE_FUNC ) { // That's ok, we handle it below. } diff --git a/src/logging/writers/sqlite/SQLite.cc b/src/logging/writers/sqlite/SQLite.cc index 67de679e6d..02871c1cfd 100644 --- a/src/logging/writers/sqlite/SQLite.cc +++ b/src/logging/writers/sqlite/SQLite.cc @@ -68,12 +68,12 @@ string SQLite::GetTableType(int arg_type, int arg_subtype) { case TYPE_ENUM: case TYPE_STRING: case TYPE_FILE: - case TYPE_FUNC: type = "text"; break; - + case TYPE_FUNC: case TYPE_TABLE: case TYPE_VECTOR: - type = "text"; // dirty - but sqlite does not directly support arrays. so - we just roll - // it into a ","-separated string. + // For TYPE_TABLE and TYPE_VECTOR, this is sort of dirty, but sqlite does not + // directly support arrays. so we just roll it all into a comma-separated string. + type = "text"; break; default: diff --git a/src/packet_analysis/protocol/tcp/TCPSessionAdapter.cc b/src/packet_analysis/protocol/tcp/TCPSessionAdapter.cc index f1f5cd45f3..aa867aa93a 100644 --- a/src/packet_analysis/protocol/tcp/TCPSessionAdapter.cc +++ b/src/packet_analysis/protocol/tcp/TCPSessionAdapter.cc @@ -460,23 +460,27 @@ static int32_t update_last_seq(analyzer::tcp::TCP_Endpoint* endpoint, uint32_t l // ### perhaps trust RST seq #'s if initial and not too // outlandish, but not if they're coming after the other // side has sent a FIN - trust the FIN ack instead - ; + ; // NOLINT(bugprone-branch-clone) + else if ( flags.FIN() && endpoint->LastSeq() == endpoint->StartSeq() + 1 ) // Update last_seq based on the FIN even if delta_last < 0. // This is to accommodate > 2 GB connections for which // we've only seen the SYN and the FIN (hence the check // for last_seq == start_seq + 1). - endpoint->UpdateLastSeq(last_seq); + endpoint->UpdateLastSeq(last_seq); // NOLINT(bugprone-branch-clone) + else if ( endpoint->state == analyzer::tcp::TCP_ENDPOINT_RESET ) // don't trust any subsequent sequence numbers - ; + ; // NOLINT(bugprone-branch-clone) + else if ( delta_last > 0 ) // ### check for large jumps here. // ## endpoint->last_seq = last_seq; - endpoint->UpdateLastSeq(last_seq); + endpoint->UpdateLastSeq(last_seq); // NOLINT(bugprone-branch-clone) + else if ( delta_last <= 0 && len > 0 ) endpoint->DidRxmit(); diff --git a/src/script_opt/CPP/Consts.cc b/src/script_opt/CPP/Consts.cc index d29f1d6b5f..41428b2053 100644 --- a/src/script_opt/CPP/Consts.cc +++ b/src/script_opt/CPP/Consts.cc @@ -73,14 +73,11 @@ shared_ptr CPPCompile::RegisterConstant(const ValPtr& vp, int& con case TYPE_COUNT: gi = make_shared(to_string(vp->AsCount()) + "ULL"); break; - case TYPE_DOUBLE: gi = make_shared(to_string(vp->AsDouble())); break; - - case TYPE_TIME: gi = make_shared(to_string(vp->AsDouble())); break; - + case TYPE_DOUBLE: + case TYPE_TIME: case TYPE_INTERVAL: gi = make_shared(to_string(vp->AsDouble())); break; - case TYPE_ADDR: gi = make_shared(this, vp); break; - + case TYPE_ADDR: case TYPE_SUBNET: gi = make_shared(this, vp); break; case TYPE_ENUM: gi = make_shared(this, vp); break; diff --git a/src/script_opt/CPP/Types.cc b/src/script_opt/CPP/Types.cc index 42f4c7ec82..93093b7aaa 100644 --- a/src/script_opt/CPP/Types.cc +++ b/src/script_opt/CPP/Types.cc @@ -32,11 +32,9 @@ bool CPPCompile::IsNativeType(const TypePtr& t) const { case TYPE_SUBNET: case TYPE_TABLE: case TYPE_TYPE: - case TYPE_VECTOR: return false; - - case TYPE_LIST: - // These occur when initializing tables. - return false; + case TYPE_VECTOR: + // These occur when initializing tables. + case TYPE_LIST: return false; default: reporter->InternalError("bad type in CPPCompile::IsNativeType"); return false; } @@ -114,7 +112,7 @@ const char* CPPCompile::TypeName(const TypePtr& t) { case TYPE_BOOL: return "bool"; case TYPE_COUNT: return "zeek_uint_t"; case TYPE_DOUBLE: return "double"; - case TYPE_ENUM: return "zeek_int_t"; + case TYPE_ENUM: case TYPE_INT: return "zeek_int_t"; case TYPE_INTERVAL: return "double"; case TYPE_PORT: return "zeek_uint_t"; diff --git a/src/script_opt/GenIDDefs.cc b/src/script_opt/GenIDDefs.cc index 6a44694c6f..83e4239069 100644 --- a/src/script_opt/GenIDDefs.cc +++ b/src/script_opt/GenIDDefs.cc @@ -265,11 +265,9 @@ TraversalCode GenIDDefs::PostStmt(const Stmt* s) { break; } + // No need to do anything, the work all occurs + // with NoFlowAfter. case STMT_FALLTHROUGH: - // No need to do anything, the work all occurs - // with NoFlowAfter. - break; - default: break; } @@ -379,15 +377,13 @@ bool GenIDDefs::CheckLHS(const ExprPtr& lhs, const ExprPtr& rhs) { return true; } + // If we want to track record field initializations, + // we'd handle that here. case EXPR_FIELD: - // If we want to track record field initializations, - // we'd handle that here. - return false; - case EXPR_INDEX: - // If we wanted to track potential alterations of - // aggregates, we'd do that here. - return false; + // If we wanted to track potential alterations of + // aggregates, we'd do that here. + case EXPR_INDEX: return false; default: reporter->InternalError("bad tag in GenIDDefs::CheckLHS"); } diff --git a/src/script_opt/IDOptInfo.cc b/src/script_opt/IDOptInfo.cc index fbec619bf7..b1fa228e13 100644 --- a/src/script_opt/IDOptInfo.cc +++ b/src/script_opt/IDOptInfo.cc @@ -323,14 +323,12 @@ void IDOptInfo::ConfluenceBlockEndsAfter(const Stmt* s, bool no_orig_flow) { continue; } - else if ( ur.EndsAfter() < cs_stmt_num ) + else if ( ur.EndsAfter() < cs_stmt_num || pc.count(i) == 0 ) // Irrelevant, didn't extend into confluence region. // We test here just to avoid the set lookup in // the next test, which presumably will sometimes // be a tad expensive. - continue; - - else if ( pc.count(i) == 0 ) + // or // This region isn't active, and we're not // tracking it for confluence. continue; @@ -440,13 +438,10 @@ int IDOptInfo::FindRegionBeforeIndex(int stmt_num) { if ( ur.StartsAfter() >= stmt_num ) break; - if ( ur.EndsAfter() == NO_DEF ) - // It's active for everything beyond its start. - region_ind = i; - - else if ( ur.EndsAfter() >= stmt_num - 1 ) - // It's active at the beginning of the statement of - // interest. + // It's active for everything beyond its start. + // or + // It's active at the beginning of the statement of interest. + if ( (ur.EndsAfter() == NO_DEF) || (ur.EndsAfter() >= stmt_num - 1) ) region_ind = i; } diff --git a/src/script_opt/Reduce.cc b/src/script_opt/Reduce.cc index 344f8ec07b..bcb2044e3a 100644 --- a/src/script_opt/Reduce.cc +++ b/src/script_opt/Reduce.cc @@ -481,10 +481,8 @@ bool Reducer::ExprValid(const ID* id, const Expr* e1, const Expr* e2) const { auto aggr = e1->GetOp1(); auto aggr_t = aggr->GetType(); - if ( pfs->HasSideEffects(SideEffectsOp::READ, aggr_t) ) - has_side_effects = true; - - else if ( aggr_t->Tag() == TYPE_TABLE && pfs->IsTableWithDefaultAggr(aggr_t.get()) ) + if ( (pfs->HasSideEffects(SideEffectsOp::READ, aggr_t)) || + (aggr_t->Tag() == TYPE_TABLE && pfs->IsTableWithDefaultAggr(aggr_t.get())) ) has_side_effects = true; } diff --git a/src/script_opt/ZAM/BuiltInSupport.cc b/src/script_opt/ZAM/BuiltInSupport.cc index 927c07e615..2a66fd274a 100644 --- a/src/script_opt/ZAM/BuiltInSupport.cc +++ b/src/script_opt/ZAM/BuiltInSupport.cc @@ -12,9 +12,6 @@ FixedCatArg::FixedCatArg(TypePtr _t) : t(std::move(_t)) { case TYPE_BOOL: max_size = 1; break; case TYPE_INT: - max_size = 20; // sufficient for 64 bits - break; - case TYPE_COUNT: max_size = 20; // sufficient for 64 bits break; diff --git a/src/session/Key.cc b/src/session/Key.cc index 669ddb4c61..91eef2bdb3 100644 --- a/src/session/Key.cc +++ b/src/session/Key.cc @@ -65,9 +65,7 @@ bool Key::operator<(const Key& rhs) const { } bool Key::operator==(const Key& rhs) const { - if ( size != rhs.size ) - return false; - else if ( type != rhs.type ) + if ( size != rhs.size || type != rhs.type ) return false; return memcmp(data, rhs.data, size) == 0; diff --git a/src/spicy/runtime-support.cc b/src/spicy/runtime-support.cc index ec9d133a9d..76cf8c1f59 100644 --- a/src/spicy/runtime-support.cc +++ b/src/spicy/runtime-support.cc @@ -236,7 +236,7 @@ zeek::analyzer::ID rt::current_analyzer_id() { if ( auto x = cookie->protocol ) { return x->analyzer->GetID(); } - else if ( auto x = cookie->file ) { + else if ( auto x = cookie->file ) { // NOLINT(bugprone-branch-clone) return 0; } else if ( auto x = cookie->packet ) { diff --git a/src/util.cc b/src/util.cc index f3f04ca759..3db0b5467c 100644 --- a/src/util.cc +++ b/src/util.cc @@ -2402,14 +2402,10 @@ static bool check_ok_utf8(const unsigned char* start, const unsigned char* end) if ( result != conversionOK ) return false; - if ( (output[0] <= 0x001F) || (output[0] == 0x007F) || (output[0] >= 0x0080 && output[0] <= 0x009F) ) - // Control characters - return false; - else if ( output[0] >= 0xE000 && output[0] <= 0xF8FF ) - // Private Use Area - return false; - else if ( output[0] >= 0xFFF0 && output[0] <= 0xFFFF ) - // Specials Characters + if ( ((output[0] <= 0x001F) || (output[0] == 0x007F) || + (output[0] >= 0x0080 && output[0] <= 0x009F)) || // Control characters + (output[0] >= 0xE000 && output[0] <= 0xF8FF) || // Private Use Area + (output[0] >= 0xFFF0 && output[0] <= 0xFFFF) ) // Special characters return false; return true;