Merge remote-tracking branch 'origin/topic/timw/clang-tidy-bugprone-fixes'

* origin/topic/timw/clang-tidy-bugprone-fixes: (22 commits)
  Update .clang-tidy to have bugprone-* enabled with some exclusions
  Fix clang-tidy bugprone-unused-return-value warnings
  Fix clang-tidy bugprone-unsafe-functions warnings
  Fix clang-tidy bugprone-unused-local-non-trivial-variable warnings
  Fix clang-tidy bugprone-throw-keyword-missing warnings
  Fix clang-tidy bugprone-switch-missing-default-case warnings
  Fix clang-tidy bugprone-suspicious-realloc-usage warnings
  Fix clang-tidy bugprone-suspicious-include warnings
  Fix clang-tidy bugprone-suspicious-string-compare warnings
  Fix clang-tidy bugprone-suspicious-stringview-data-usage warnings
  Fix clang-tidy bugprone-string-literal-with-embedded-nul warnings
  Fix clang-tidy bugprone-parent-virtual-call warnings
  Fix clang-tidy bugprone-misplaced-widening-cast warnings
  Fix clang-tidy bugprone-inc-dec-in-conditions warnings
  Fix clang-tidy bugprone-multi-level-implicit-pointer-conversion warnings
  Fix clang-tidy bugprone-macro-parentheses warnings
  Fix clang-tidy bugprone-incorrect-roundings warnings
  Fix clang-tidy bugprone-incorrect-division warnings
  Fix clang-tidy bugprone-implicit-widening-of-multiplication-result warnings
  Remove noexcept from util::tokenize_string
  ...

.clang-tidy

@@ -1,5 +1,26 @@
-Checks: '-*,
+Checks: [-*,
          bugprone-*,
+
+         # Skipping these temporarily because they are very noisy
+         -bugprone-narrowing-conversions,
+         -bugprone-unchecked-optional-access,
+
+         # The following cause either lots of pointless or advisory warnings
          -bugprone-easily-swappable-parameters,
-         clang-analyzer-*,
+         -bugprone-nondeterministic-pointer-iteration-order,
-         performance-*'
+
+         # bifcl generates a lot of code with double underscores in their name.
+         # ZAM uses a few identifiers that start with underscores or have
+         # double-underscores in the name.
+         -bugprone-reserved-identifier,
+
+         # bifcl generates almost every switch statement without a default case
+         # and so this one generates a lot of warnings.
+         -bugprone-switch-missing-default-case,
+
+         # These report warnings that are rather difficult to fix.
+         -bugprone-undefined-memory-manipulation,
+         -bugprone-pointer-arithmetic-on-polymorphic-object,
+         -bugprone-empty-catch,
+         -bugprone-exception-escape
+]

CHANGES

@@ -1,3 +1,9 @@
+8.0.0-dev.252 | 2025-05-27 11:59:00 -0700
+
+  * Update .clang-tidy to have bugprone-* enabled with some exclusions (Tim Wojtulewicz, Corelight)
+
+  * Fix clang-tidy bugprone-* warnings (Tim Wojtulewicz, Corelight)
+
 8.0.0-dev.229 | 2025-05-27 17:46:35 +0100
 
   * Add explicit TLS support for FTP (Johanna Amann, Corelight)

VERSION

@@ -1 +1 @@
-8.0.0-dev.229
+8.0.0-dev.252

auxil/binpac

@@ -1 +1 @@
-Subproject commit f7c94581f1f4d1bef1537824c27503d58000933e
+Subproject commit e1bb4309129abc475250d36ca6822624cd3036bb

src/Anon.cc

@@ -54,7 +54,7 @@ static int bi_ffs(uint32_t value) {
     return add + bvals[value & 0xf];
 }
 
-#define first_n_bit_mask(n) (~(0xFFFFFFFFU >> n))
+static inline uint64_t first_n_bit_mask(int n) { return ~(0xFFFFFFFFU >> n); }
 
 ipaddr32_t AnonymizeIPAddr::Anonymize(ipaddr32_t addr) {
     std::map<ipaddr32_t, ipaddr32_t>::iterator p = mapping.find(addr);

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) )
+                    else if ( (t->Tag() != TYPE_FUNC && ! same_type(pvt, t)) ||
-                    // ### Maybe fix later, but may be fundamentally un-checkable --US
+                              (t->Tag() == TYPE_FUNC && pvt->Tag() != TYPE_FUNC) ) {
-                    {
-                        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 ) {
                         reporter->InternalError("inconsistent aggregate Val in CompositeHash::RecoverOneVal()");
                         *pval = nullptr;
                         return false;

src/DNS_Mapping.cc

@@ -300,17 +300,17 @@ TEST_CASE("dns_mapping save reload") {
     // Try loading from the file at EOF. This should cause a mapping failure.
     DNS_Mapping mapping(tmpfile);
     CHECK(mapping.NoMapping());
-    rewind(tmpfile);
+    fseek(tmpfile, 0, SEEK_SET);
 
     // Try reading from the empty file. This should cause an init failure.
     DNS_Mapping mapping2(tmpfile);
     CHECK(mapping2.InitFailed());
-    rewind(tmpfile);
+    fseek(tmpfile, 0, SEEK_SET);
 
     // Save a valid mapping into the file and rewind to the start.
     DNS_Mapping mapping3(addr, &he, 123);
     mapping3.Save(tmpfile);
-    rewind(tmpfile);
+    fseek(tmpfile, 0, SEEK_SET);
 
     // Test loading the mapping back out of the file
     DNS_Mapping mapping4(tmpfile);

src/DebugCmds.cc

@@ -12,7 +12,7 @@
 
 #include "zeek/DbgBreakpoint.h"
 #include "zeek/Debug.h"
-#include "zeek/DebugCmdInfoConstants.cc"
+#include "zeek/DebugCmdInfoConstants.cc" // NOLINT(bugprone-suspicious-include)
 #include "zeek/Desc.h"
 #include "zeek/Frame.h"
 #include "zeek/Func.h"
@@ -147,7 +147,7 @@ int find_all_matching_cmds(const string& prefix, const char* array_of_matches[])
 
         for ( int j = 0; j < g_DebugCmdInfos[i]->NumNames(); ++j ) {
             const char* curr_name = g_DebugCmdInfos[i]->Names()[j];
-            if ( strncmp(curr_name, prefix.c_str(), arglen) )
+            if ( strncmp(curr_name, prefix.c_str(), arglen) != 0 )
                 continue;
 
             // If exact match, then only return that one.

src/Expr.cc

@@ -892,11 +892,13 @@ ValPtr BinaryExpr::StringFold(Val* v1, Val* v2) const {
 
     switch ( tag ) {
 #undef DO_FOLD
+// NOLINTBEGIN(bugprone-macro-parentheses)
 #define DO_FOLD(sense)                                                                                                 \
     {                                                                                                                  \
         result = Bstr_cmp(s1, s2) sense 0;                                                                             \
         break;                                                                                                         \
     }
+            // NOLINTEND(bugprone-macro-parentheses)
 
         case EXPR_LT: DO_FOLD(<)
         case EXPR_LE: DO_FOLD(<=)
@@ -1383,8 +1385,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) )
+    if ( (bt2 == TYPE_INTERVAL && (bt1 == TYPE_TIME || bt1 == TYPE_INTERVAL)) )
-        base_result_type = base_type(bt1);
+        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 +1927,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 )
             ;
@@ -4082,13 +4084,16 @@ CallExpr::CallExpr(ExprPtr arg_func, ListExprPtr arg_args, bool in_hook, bool _i
              util::streq(((NameExpr*)func.get())->Id()->Name(), "fmt") &&
              // The following is needed because fmt might not yet
              // be bound as a name.
-             did_builtin_init && (func_val = func->Eval(nullptr)) ) {
+             did_builtin_init ) {
+            func_val = func->Eval(nullptr);
+            if ( func_val ) {
                 zeek::Func* f = func_val->AsFunc();
                 if ( f->GetKind() == Func::BUILTIN_FUNC && ! check_built_in_call((BuiltinFunc*)f, this) )
                     SetError();
             }
         }
     }
+}
 
 bool CallExpr::IsPure() const {
     if ( IsError() )

src/Frag.cc

@@ -158,7 +158,7 @@ void FragReassembler::Weird(const char* name) const {
 }
 
 void FragReassembler::Overlap(const u_char* b1, const u_char* b2, uint64_t n) {
-    if ( memcmp((const void*)b1, (const void*)b2, n) )
+    if ( memcmp((const void*)b1, (const void*)b2, n) != 0 )
         Weird("fragment_inconsistency");
     else
         Weird("fragment_overlap");

src/Func.cc

@@ -915,7 +915,7 @@ zeek::RecordValPtr make_backtrace_element(std::string_view name, const VectorVal
     static auto line_location_idx = elem_type->FieldOffset("line_location");
 
     auto elem = make_intrusive<RecordVal>(elem_type);
-    elem->Assign(function_name_idx, name.data());
+    elem->Assign(function_name_idx, name);
     elem->Assign(function_args_idx, args);
 
     if ( loc ) {

src/MMDB.cc

@@ -118,7 +118,8 @@ bool MMDB::EnsureLoaded() {
 
     if ( ! res && ! reported_error ) {
         reported_error = true;
-        zeek::emit_builtin_error(zeek::util::fmt("Failed to open %s", Description().data()));
+        zeek::emit_builtin_error(
+            zeek::util::fmt("Failed to open %.*s", static_cast<int>(Description().size()), Description().data()));
     }
 
     return res;

src/PrefixTable.cc

@@ -69,7 +69,12 @@ std::list<std::tuple<IPPrefix, void*>> PrefixTable::FindAll(const IPAddr& addr,
         out.emplace_back(PrefixToIPPrefix(list[i]->prefix), list[i]->data);
 
     Deref_Prefix(prefix);
-    free(list);
+
+    // clang-tidy reports a bugprone-multi-level-implicit-pointer-conversion warning here
+    // because the double-pointer is implicitly converted to a void* for the call to
+    // free(). The double-pointer was calloc'd in patricia_search_all as an array of
+    // pointers, so it's safe to free. Explicitly cast it to void* to silence the warning.
+    free(static_cast<void*>(list));
     return out;
 }
 

src/RandTest.cc

@@ -60,7 +60,7 @@ void RandTest::add(const void* buf, int bufl) {
             montey = 0;
             for ( int mj = 0; mj < RT_MONTEN / 2; mj++ ) {
                 montex = (montex * 256.0) + monte[mj];
-                montey = (montey * 256.0) + monte[(RT_MONTEN / 2) + mj];
+                montey = (montey * 256.0) + monte[static_cast<size_t>((RT_MONTEN / 2) + mj)];
             }
             if ( montex * montex + montey * montey <= RT_INCIRC ) {
                 inmont++;

src/RandTest.h

@@ -4,11 +4,9 @@
 
 #include <cstdint>
 
-#define RT_MONTEN                                                                                                      \
+// Bytes used as Monte Carlo co-ordinates. This should be no more bits than the mantissa
-    6 /* Bytes used as Monte Carlo                                                                                     \
+// of your "double" floating point type.
-         co-ordinates. This should be no more                                                                          \
+constexpr int RT_MONTEN = 6;
-         bits than the mantissa of your "double"                                                                       \
-         floating point type. */
 
 namespace zeek {
 

src/Reporter.cc

@@ -467,7 +467,7 @@ void Reporter::Deprecation(std::string_view msg, const detail::Location* loc1, c
     if ( loc1 || loc2 )
         PushLocation(loc1, loc2);
 
-    Warning("%s", msg.data());
+    Warning("%.*s", static_cast<int>(msg.size()), msg.data());
 
     if ( loc1 || loc2 )
         PopLocation();
@@ -475,7 +475,8 @@ void Reporter::Deprecation(std::string_view msg, const detail::Location* loc1, c
 
 void Reporter::DoLog(const char* prefix, EventHandlerPtr event, FILE* out, Connection* conn, ValPList* addl,
                      bool location, bool time, const char* postfix, const char* fmt, va_list ap) {
-    static char tmp[512];
+    constexpr size_t DEFAULT_BUFFER_SIZE = 512;
+    static char tmp[DEFAULT_BUFFER_SIZE];
 
     int size = sizeof(tmp);
     char* buffer = tmp;
@@ -484,7 +485,6 @@ void Reporter::DoLog(const char* prefix, EventHandlerPtr event, FILE* out, Conne
     std::string loc_str;
 
     if ( location ) {
-        std::string loc_file = "";
         int loc_line = 0;
 
         if ( locations.size() ) {
@@ -522,27 +522,38 @@ void Reporter::DoLog(const char* prefix, EventHandlerPtr event, FILE* out, Conne
         }
     }
 
-    while ( true ) {
+    // Figure out how big of a buffer is needed here.
-        va_list aq;
+    va_list ap_copy;
-        va_copy(aq, ap);
+    va_copy(ap_copy, ap);
-        int n = vsnprintf(buffer, size, fmt, aq);
+    size_t req_buffer_size = vsnprintf(nullptr, 0, fmt, ap_copy);
-        va_end(aq);
 
-        if ( postfix )
+    if ( req_buffer_size < 0 ) {
-            n += strlen(postfix) + 10; // Add a bit of slack.
+        va_end(ap_copy);
-
-        if ( n > -1 && n < size )
-            // We had enough space;
-            break;
-
-        // Enlarge buffer;
-        size *= 2;
-        buffer = allocated = (char*)realloc(allocated, size);
-
-        if ( ! buffer )
         FatalError("out of memory in Reporter");
     }
 
+    if ( postfix && *postfix )
+        req_buffer_size += strlen(postfix) + 10;
+
+    // Add one byte for a null terminator.
+    req_buffer_size++;
+
+    if ( req_buffer_size > DEFAULT_BUFFER_SIZE ) {
+        buffer = (char*)malloc(req_buffer_size);
+        if ( ! buffer ) {
+            va_end(ap_copy);
+            FatalError("out of memory in Reporter");
+        }
+
+        allocated = buffer;
+    }
+
+    va_end(ap_copy);
+
+    req_buffer_size = vsnprintf(buffer, req_buffer_size, fmt, ap);
+    if ( req_buffer_size < 0 )
+        FatalError("out of memory in Reporter");
+
     if ( postfix && *postfix )
         // Note, if you change this fmt string, adjust the additional
         // buffer size above.
@@ -641,6 +652,7 @@ void Reporter::DoLog(const char* prefix, EventHandlerPtr event, FILE* out, Conne
 #endif
     }
 
+    // If the buffer got reallocated because it was too small, free the reallocated one.
     if ( allocated )
         free(allocated);
 }

src/SmithWaterman.cc

@@ -187,9 +187,9 @@ public:
 
     SWNode* operator()(int row, int col) {
         // Make sure access is in allowed range.
-        if ( row < 0 || row >= _rows )
+        if ( row < 0 || static_cast<size_t>(row) >= _rows )
             return nullptr;
-        if ( col < 0 || col >= _cols )
+        if ( col < 0 || static_cast<size_t>(col) >= _cols )
             return nullptr;
 
         return &(_nodes[row * _cols + col]);
@@ -215,7 +215,7 @@ private:
     const String* _s1;
     const String* _s2;
 
-    int _rows, _cols;
+    size_t _rows, _cols;
     SWNode* _nodes;
 };
 

src/Stmt.cc

@@ -1226,7 +1226,7 @@ ValPtr ForStmt::DoExec(Frame* f, Val* v, StmtFlowType& flow) {
 bool ForStmt::IsPure() const { return e->IsPure() && body->IsPure(); }
 
 void ForStmt::StmtDescribe(ODesc* d) const {
-    Stmt::StmtDescribe(d);
+    Stmt::StmtDescribe(d); // NOLINT(bugprone-parent-virtual-call)
 
     if ( d->IsReadable() )
         d->Add("(");
@@ -1398,7 +1398,7 @@ ValPtr ReturnStmt::Exec(Frame* f, StmtFlowType& flow) {
 }
 
 void ReturnStmt::StmtDescribe(ODesc* d) const {
-    Stmt::StmtDescribe(d);
+    Stmt::StmtDescribe(d); // NOLINT(bugprone-parent-virtual-call)
     if ( ! d->IsReadable() )
         d->Add(e != nullptr);
 
@@ -1607,7 +1607,7 @@ ValPtr AssertStmt::Exec(Frame* f, StmtFlowType& flow) {
 }
 
 void AssertStmt::StmtDescribe(ODesc* d) const {
-    Stmt::StmtDescribe(d);
+    Stmt::StmtDescribe(d); // NOLINT(bugprone-parent-virtual-call)
 
     // Quoting strings looks better when describing assert
     // statements. So turn it on explicitly.

src/TraverseTypes.h

@@ -14,20 +14,22 @@ enum TraversalCode {
 
 #define HANDLE_TC_STMT_PRE(code)                                                                                       \
     {                                                                                                                  \
-        if ( (code) == zeek::detail::TC_ABORTALL )                                                                     \
+        switch ( code ) {                                                                                              \
-            return (code);                                                                                             \
+            case zeek::detail::TC_ABORTALL: return (code);                                                             \
-        else if ( (code) == zeek::detail::TC_ABORTSTMT )                                                               \
+            case zeek::detail::TC_ABORTSTMT: return zeek::detail::TC_CONTINUE;                                         \
-            return zeek::detail::TC_CONTINUE;                                                                          \
+            case zeek::detail::TC_CONTINUE:                                                                            \
+            default: break;                                                                                            \
+        }                                                                                                              \
     }
 
 #define HANDLE_TC_STMT_POST(code)                                                                                      \
     {                                                                                                                  \
-        if ( (code) == zeek::detail::TC_ABORTALL )                                                                     \
+        switch ( code ) {                                                                                              \
-            return (code);                                                                                             \
+            case zeek::detail::TC_ABORTSTMT: return zeek::detail::TC_CONTINUE;                                         \
-        else if ( (code) == zeek::detail::TC_ABORTSTMT )                                                               \
+            case zeek::detail::TC_ABORTALL:                                                                            \
-            return zeek::detail::TC_CONTINUE;                                                                          \
+            case zeek::detail::TC_CONTINUE:                                                                            \
-        else                                                                                                           \
+            default: return (code);                                                                                    \
-            return (code);                                                                                             \
+        }                                                                                                              \
     }
 
 #define HANDLE_TC_EXPR_PRE(code) HANDLE_TC_STMT_PRE(code)

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:
@@ -2157,10 +2155,6 @@ bool is_assignable(TypeTag t) {
     return false;
 }
 
-#define CHECK_TYPE(t)                                                                                                  \
-    if ( t1 == t || t2 == t )                                                                                          \
-        return t;
-
 TypeTag max_type(TypeTag t1, TypeTag t2) {
     if ( t1 == TYPE_INTERVAL || t1 == TYPE_TIME )
         t1 = TYPE_DOUBLE;
@@ -2168,9 +2162,12 @@ TypeTag max_type(TypeTag t1, TypeTag t2) {
         t2 = TYPE_DOUBLE;
 
     if ( BothArithmetic(t1, t2) ) {
-        CHECK_TYPE(TYPE_DOUBLE);
+        if ( t1 == TYPE_DOUBLE || t2 == TYPE_DOUBLE )
-        CHECK_TYPE(TYPE_INT);
+            return TYPE_DOUBLE;
-        CHECK_TYPE(TYPE_COUNT);
+        else if ( t1 == TYPE_INT || t2 == TYPE_INT )
+            return TYPE_INT;
+        else if ( t1 == TYPE_COUNT || t2 == TYPE_COUNT )
+            return TYPE_COUNT;
 
         return TYPE_COUNT;
     }

src/UID.h

@@ -9,7 +9,7 @@
 
 namespace zeek {
 
-constexpr int UID_LEN = 2;
+constexpr size_t UID_LEN = 2;
 
 /**
  * A class for creating/managing UIDs of arbitrary bit-length and converting

src/Val.cc

@@ -981,8 +981,8 @@ static zeek::expected<ValPtr, std::string> BuildVal(const rapidjson::Value& j, c
                     else if ( unit == "usec" || unit == "usecs" )
                         interval_secs += (value * Microseconds);
                     else
-                        return zeek::unexpected<std::string>(
+                        return zeek::unexpected<std::string>(util::fmt("wrong interval format, invalid unit type %.*s",
-                            util::fmt("wrong interval format, invalid unit type %s", unit.data()));
+                                                                       static_cast<int>(unit.size()), unit.data()));
                 }
 
                 return make_intrusive<IntervalVal>(interval_secs, Seconds);

src/Val.h

@@ -1186,6 +1186,7 @@ public:
     }
     void Assign(int field, const char* new_val) { Assign(field, new StringVal(new_val)); }
     void Assign(int field, const std::string& new_val) { Assign(field, new StringVal(new_val)); }
+    void Assign(int field, std::string_view new_val) { Assign(field, new StringVal(new_val)); }
     void Assign(int field, String* new_val) { Assign(field, new StringVal(new_val)); }
 
     /**

src/Var.cc

@@ -203,6 +203,7 @@ static void make_var(const IDPtr& id, TypePtr t, InitClass c, ExprPtr init, std:
     }
 
     if ( id->GetType() && id->GetType()->Tag() != TYPE_ERROR && ! id->IsBlank() ) {
+        // NOLINTNEXTLINE(bugprone-assignment-in-if-condition)
         if ( dt != VAR_REDEF && (! init || ! do_init || (! t && ! (t = init_type(init)))) ) {
             id->Error("already defined", init.get());
             return;
@@ -749,6 +750,7 @@ void end_func(StmtPtr body, const char* module_name, bool free_of_conditionals)
     // Note: ideally, something would take ownership of this memory until the
     // end of script execution, but that's essentially the same as the
     // lifetime of the process at the moment, so ok to "leak" it.
+    // NOLINTNEXTLINE(bugprone-unused-return-value)
     ingredients.release();
 }
 

src/analyzer/protocol/bittorrent/bittorrent-analyzer.pac

@@ -38,7 +38,7 @@ flow BitTorrent_Flow(is_orig: bool) {
 	function validate_handshake(pstrlen: uint8, pstr: const_bytestring): bool
 		%{
 		if ( pstrlen != 19 ||
-		     memcmp("BitTorrent protocol", pstr.begin(), 19) )
+		     memcmp("BitTorrent protocol", pstr.begin(), 19) != 0 )
 			{
 			throw Exception("invalid handshake");
 			}

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::Diffie_Hellman:
-        case detail::DSA_SHA1: break;
+        case detail::DSA_SHA1:
-        case detail::Elliptic_Curve: break;
+        case detail::Elliptic_Curve:
-        case detail::RSA_SHA1: break;
+        case detail::RSA_SHA1:
-        case detail::DSA_NSEC3_SHA1: break;
+        case detail::DSA_NSEC3_SHA1:
-        case detail::RSA_SHA1_NSEC3_SHA1: break;
+        case detail::RSA_SHA1_NSEC3_SHA1:
-        case detail::RSA_SHA256: break;
+        case detail::RSA_SHA256:
-        case detail::RSA_SHA512: break;
+        case detail::RSA_SHA512:
-        case detail::GOST_R_34_10_2001: break;
+        case detail::GOST_R_34_10_2001:
-        case detail::ECDSA_curveP256withSHA256: break;
+        case detail::ECDSA_curveP256withSHA256:
-        case detail::ECDSA_curveP384withSHA384: break;
+        case detail::ECDSA_curveP384withSHA384:
-        case detail::Ed25519: break;
+        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::Diffie_Hellman:
-        case detail::DSA_SHA1: break;
+        case detail::DSA_SHA1:
-        case detail::Elliptic_Curve: break;
+        case detail::Elliptic_Curve:
-        case detail::RSA_SHA1: break;
+        case detail::RSA_SHA1:
-        case detail::DSA_NSEC3_SHA1: break;
+        case detail::DSA_NSEC3_SHA1:
-        case detail::RSA_SHA1_NSEC3_SHA1: break;
+        case detail::RSA_SHA1_NSEC3_SHA1:
-        case detail::RSA_SHA256: break;
+        case detail::RSA_SHA256:
-        case detail::RSA_SHA512: break;
+        case detail::RSA_SHA512:
-        case detail::GOST_R_34_10_2001: break;
+        case detail::GOST_R_34_10_2001:
-        case detail::ECDSA_curveP256withSHA256: break;
+        case detail::ECDSA_curveP256withSHA256:
-        case detail::ECDSA_curveP384withSHA384: break;
+        case detail::ECDSA_curveP384withSHA384:
-        case detail::Ed25519: break;
+        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::SHA1:
-        case detail::SHA256: break;
+        case detail::SHA256:
-        case detail::GOST_R_34_11_94: break;
+        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;

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;
 
-            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;
+            case 631:
-                break;
+            case 632:
+            case 633: done = true; break;
 
             default: break;
         }

src/analyzer/protocol/ftp/functions.bif

@@ -4,7 +4,7 @@ type ftp_port: record;
 %%{
 #include "zeek/Reporter.h"
 
-static zeek::RecordValPtr parse_port(std::string_view line)
+static zeek::RecordValPtr parse_port(const std::string& line)
 	{
 	auto r = zeek::make_intrusive<zeek::RecordVal>(zeek::BifType::Record::ftp_port);
 
@@ -13,7 +13,7 @@ static zeek::RecordValPtr parse_port(std::string_view line)
 	uint32_t addr = 0;
 
 	int32_t bytes[6];
-	if ( line.size() >= 11 && sscanf(line.data(),
+	if ( line.size() >= 11 && sscanf(line.c_str(),
                                      "%" SCNd32 ",%" SCNd32 ",%" SCNd32 ",%" SCNd32 ",%" SCNd32 ",%" SCNd32,
 	                                 &bytes[0], &bytes[1], &bytes[2],
 	                                 &bytes[3], &bytes[4], &bytes[5]) == 6 )
@@ -125,7 +125,7 @@ static zeek::RecordValPtr parse_eftp(const char* line)
 ## .. zeek:see:: parse_eftp_port parse_ftp_pasv parse_ftp_epsv fmt_ftp_port
 function parse_ftp_port%(s: string%): ftp_port
 	%{
-	return parse_port(s->ToStdStringView());
+	return parse_port(s->ToStdString());
 	%}
 
 ## Converts a string representation of the FTP EPRT command (see :rfc:`2428`)
@@ -160,20 +160,28 @@ function parse_ftp_pasv%(str: string%): ftp_port
 	const char* line = strchr(s, '(');
 	if ( line )
 		++line;	// move past '('
-	else if ( (line = strstr(s, "PORT")) )
+	else if ( line = strstr(s, "PORT"); line != nullptr )
 		line += 5;	// Skip over
-	else if ( (line = strchr(s, ',')) )
+	else if ( line = strchr(s, ','); line != nullptr )
 		{ // Look for comma-separated list.
-		while ( --line >= s && isdigit(*line) )
+		do
-			;	// Back up over preceding digits.
+			{
-		++line;	// now points to first digit, or beginning of s
+			// Back up over preceding digits. We'll end on the
+			// first digit or the beginning of s.
+			--line;
+			}
+		while ( line >= s && isdigit(*line) );
+
+		// Scoot forward one to point at the first digit or at the
+		// beginning of s.
+		++line;
 		}
 
 	// Make sure we didn't step past the end of the string.
 	if ( ! line || ( line - s ) > str->Len() )
 		return parse_port("");
 	else
-		return parse_port(std::string_view{line});
+		return parse_port(std::string{line});
 	%}
 
 ## Converts the result of the FTP EPSV command (see :rfc:`2428`) to an

src/analyzer/protocol/gnutella/Gnutella.cc

@@ -117,11 +117,11 @@ bool Gnutella_Analyzer::IsHTTP(std::string header) {
 }
 
 bool Gnutella_Analyzer::GnutellaOK(std::string header) {
-    if ( strncmp("GNUTELLA", header.data(), 8) )
+    if ( strncmp("GNUTELLA", header.data(), 8) != 0 )
         return false;
 
     int codepos = header.find(' ') + 1;
-    if ( ! strncmp("200", header.data() + codepos, 3) )
+    if ( strncmp("200", header.data() + codepos, 3) == 0 )
         return true;
 
     return false;

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 ) {
@@ -1111,7 +1111,7 @@ const char* HTTP_Analyzer::PrefixMatch(const char* line, const char* end_of_line
 
 const char* HTTP_Analyzer::PrefixWordMatch(const char* line, const char* end_of_line, const char* prefix,
                                            bool ignore_case) {
-    if ( (line = PrefixMatch(line, end_of_line, prefix, ignore_case)) == nullptr )
+    if ( line = PrefixMatch(line, end_of_line, prefix, ignore_case); line == nullptr )
         return nullptr;
 
     const char* orig_line = line;

src/analyzer/protocol/ident/Ident.cc

@@ -142,7 +142,7 @@ void Ident_Analyzer::DeliverStream(int length, const u_char* data, bool is_orig)
 
             const char* sys_end = (comma && comma < colon) ? comma : colon;
 
-            while ( --sys_end > sys_type && isspace(*sys_end) )
+            for ( ; sys_end > sys_type && isspace(*sys_end); --sys_end )
                 ;
 
             String* sys_type_s = new String((const u_char*)sys_type, sys_end - sys_type + 1, true);

src/analyzer/protocol/imap/imap-protocol.pac

@@ -53,7 +53,6 @@ refine connection IMAP_Conn += {
 		%{
 		string cmdstr = std_str(command);
 		std::transform(cmdstr.begin(), cmdstr.end(), cmdstr.begin(), ::tolower);
-		string tagstr = std_str(tag);
 
 		if ( !is_orig && cmdstr == "capability" && tag == "*" ) {
 			return CMD_CAPABILITY;

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," )
+                    if ( parts[i] == "users," || parts[i] == "clients" )
-                        users = atoi(parts[i - 1].c_str());
-                    else if ( parts[i] == "clients" )
                         users = atoi(parts[i - 1].c_str());
                     else if ( parts[i] == "services" )
                         services = atoi(parts[i - 1].c_str());

src/analyzer/protocol/login/NVT.cc

@@ -9,7 +9,7 @@
 #include "zeek/analyzer/protocol/login/events.bif.h"
 #include "zeek/analyzer/protocol/tcp/TCP.h"
 
-#define IS_3_BYTE_OPTION(c) (c >= 251 && c <= 254)
+#define IS_3_BYTE_OPTION(c) ((c) >= 251 && (c) <= 254)
 
 #define TELNET_OPT_SB 250
 #define TELNET_OPT_SE 240

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) )
+                  (data[i] >= 62 && data[i] <= 126) || (data[i] == HT || data[i] == SP) )
-            DataOctet(data[i]);
-
-        else if ( data[i] == HT || data[i] == SP )
             DataOctet(data[i]);
 
         else {

src/analyzer/protocol/modbus/modbus-analyzer.pac

@@ -114,7 +114,7 @@ refine flow ModbusTCP_Flow += {
 		%}
 
 	# EXCEPTION
-	function deliver_Exception(header: ModbusTCP_TransportHeader, message: Exception): bool
+	function deliver_Exception(header: ModbusTCP_TransportHeader, message: ModbusTCP_ExceptResponse): bool
 		%{
 		if ( ::modbus_exception )
 			{

src/analyzer/protocol/modbus/modbus-protocol.pac

@@ -112,7 +112,7 @@ type ModbusTCP_Request(header: ModbusTCP_TransportHeader) = case header.fc of {
 #
 type ModbusTCP_Response(header: ModbusTCP_TransportHeader) = case header.fc & 0x80 of {
     0       -> normal_response      : ModbusTCP_NormalResponse(header);
-    default -> exception_response   : Exception(header);
+    default -> exception_response   : ModbusTCP_ExceptResponse(header);
 };
 
 type ModbusTCP_NormalResponse(header: ModbusTCP_TransportHeader) = case header.fc of {
@@ -140,7 +140,7 @@ type ModbusTCP_NormalResponse(header: ModbusTCP_TransportHeader) = case header.f
 	default                                 -> unknown:                         bytestring &restofdata;
 };
 
-type Exception(header: ModbusTCP_TransportHeader) = record {
+type ModbusTCP_ExceptResponse(header: ModbusTCP_TransportHeader) = record {
 	code: uint8;
 } &let {
 	deliver: bool = $context.flow.deliver_Exception(header, this);

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;

src/analyzer/protocol/netbios/NetbiosSSN.cc

@@ -13,11 +13,11 @@
 constexpr double netbios_ssn_session_timeout = 15.0;
 
 #define MAKE_INT16(dest, src)                                                                                          \
-    dest = *src;                                                                                                       \
+    (dest) = *(src);                                                                                                   \
-    dest <<= 8;                                                                                                        \
+    (dest) <<= 8;                                                                                                      \
-    src++;                                                                                                             \
+    (src)++;                                                                                                           \
-    dest |= *src;                                                                                                      \
+    (dest) |= *(src);                                                                                                  \
-    src++;
+    (src)++;
 
 namespace zeek::analyzer::netbios_ssn {
 namespace detail {

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_MNT:
-
+        case BifEnum::MOUNT3::PROC_UMNT:
-        case BifEnum::MOUNT3::PROC_UMNT: callarg = mount3_dirmntargs(buf, n); break;
-
         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;

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;

src/analyzer/protocol/smtp/SMTP.cc

@@ -20,7 +20,7 @@ static const char* smtp_cmd_word[] = {
 
 static const char* unknown_cmd = "(UNKNOWN)";
 
-#define SMTP_CMD_WORD(code) ((code >= 0) ? smtp_cmd_word[code] : unknown_cmd)
+#define SMTP_CMD_WORD(code) (((code) >= 0) ? smtp_cmd_word[code] : unknown_cmd)
 
 namespace zeek::analyzer::smtp {
 
@@ -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:

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;
 		%}
 

src/analyzer/protocol/ssl/SSL.cc

@@ -341,7 +341,7 @@ bool SSL_Analyzer::TryDecryptApplicationData(int len, const u_char* data, bool i
         decrypted.resize(decrypted_len);
 
         int res = 0;
-        if ( ! (res = EVP_DecryptFinal(ctx, NULL, &res)) ) {
+        if ( res = EVP_DecryptFinal(ctx, NULL, &res); res == 0 ) {
             DBG_LOG(DBG_ANALYZER, "Decryption failed with return code: %d. Invalid key?\n", res);
             EVP_CIPHER_CTX_free(ctx);
             return false;

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<uint64_t>(zeek::detail::tcp_max_initial_window) )
@@ -396,7 +396,7 @@ void TCP_Reassembler::Overlap(const u_char* b1, const u_char* b2, uint64_t n) {
     if ( DEBUG_tcp_contents )
         DEBUG_MSG("%.6f TCP contents overlap: %" PRIu64 " IsOrig()=%d\n", run_state::network_time, n, IsOrig());
 
-    if ( rexmit_inconsistency && memcmp((const void*)b1, (const void*)b2, n) &&
+    if ( rexmit_inconsistency && (memcmp((const void*)b1, (const void*)b2, n) != 0) &&
          // The following weeds out keep-alives for which that's all
          // we've ever seen for the connection.
          (n > 1 || endp->peer->HasDoneSomething()) ) {

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;
         }

src/broker/Manager.cc

@@ -1590,7 +1590,7 @@ void Manager::ProcessMessage(std::string_view topic, broker::zeek::Event& ev) {
         if ( p.size() > topic.size() )
             continue;
 
-        if ( strncmp(p.data(), topic.data(), p.size()) != 0 )
+        if ( strncmp(p.data(), topic.data(), p.size()) != 0 ) // NOLINT(bugprone-suspicious-stringview-data-usage)
             continue;
 
         DBG_LOG(DBG_BROKER, "Skip processing of forwarded event: %s %s", std::string{name}.c_str(),
@@ -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 ) {

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 "")

src/file_analysis/analyzer/x509/X509.cc

@@ -110,7 +110,10 @@ RecordValPtr X509::ParseCertificate(X509Val* cert_val, file_analysis::File* f) {
     auto pX509Cert = make_intrusive<RecordVal>(BifType::Record::X509::Certificate);
     BIO* bio = BIO_new(BIO_s_mem());
 
+    // The cast here is intentional to force it into a specific version of Assign()
+    // NOLINTNEXTLINE(bugprone-misplaced-widening-cast)
     pX509Cert->Assign(0, static_cast<uint64_t>(X509_get_version(ssl_cert) + 1));
+
     i2a_ASN1_INTEGER(bio, X509_get_serialNumber(ssl_cert));
     int len = BIO_read(bio, buf, sizeof(buf));
     pX509Cert->Assign(1, make_intrusive<StringVal>(len, buf));
@@ -374,6 +377,8 @@ void X509::ParseSAN(X509_EXTENSION* ext) {
 
                     emails->Assign(emails->Size(), std::move(bs));
                     break;
+
+                default: break;
             }
         }
 

src/file_analysis/analyzer/x509/X509Common.cc

@@ -128,8 +128,8 @@ double X509Common::GetTimeFromAsn1(const ASN1_TIME* atime, file_analysis::File*
             return 0;
         }
 
-        lSecondsFromUTC = ((pString[1] - '0') * 10 + (pString[2] - '0')) * 60;
+        lSecondsFromUTC = static_cast<time_t>((pString[1] - '0') * 10) + static_cast<time_t>((pString[2] - '0') * 60);
-        lSecondsFromUTC += (pString[3] - '0') * 10 + (pString[4] - '0');
+        lSecondsFromUTC += static_cast<time_t>((pString[3] - '0') * 10) + (pString[4] - '0');
 
         if ( *pString == '-' )
             lSecondsFromUTC = -lSecondsFromUTC;

src/input/readers/raw/Raw.cc

@@ -607,7 +607,6 @@ bool Raw::DoUpdate() {
         }
     }
 
-    std::string line;
     assert((NumFields() == 1 && ! use_stderr) || (NumFields() == 2 && use_stderr));
     for ( ;; ) {
         if ( stdin_towrite > 0 )

src/iosource/Manager.cc

@@ -38,7 +38,7 @@ void Manager::WakeupHandler::Ping(std::string_view where) {
     // Calling DBG_LOG calls fprintf, which isn't safe to call in a signal
     // handler.
     if ( signal_val != 0 )
-        DBG_LOG(DBG_MAINLOOP, "Pinging WakeupHandler from %s", where.data());
+        DBG_LOG(DBG_MAINLOOP, "Pinging WakeupHandler from %.*s", static_cast<int>(where.size()), where.data());
 
     flare.Fire(true);
 }

src/logging/Manager.cc

@@ -340,7 +340,9 @@ Manager::Filter::~Filter() {
     for ( int i = 0; i < num_fields; ++i )
         delete fields[i];
 
-    free(fields);
+    // Static cast this to void* to avoid a clang-tidy warning about converting from the
+    // double-pointer to void*
+    free(static_cast<void*>(fields));
 
     Unref(path_val);
     Unref(config);
@@ -777,19 +779,8 @@ bool Manager::TraverseRecord(Stream* stream, Filter* filter, RecordType* rt, Tab
                 continue;
             }
 
-            else if ( t->Tag() == TYPE_TABLE && t->AsTableType()->IsSet() ) {
+            else if ( (t->Tag() == TYPE_TABLE && t->AsTableType()->IsSet()) || t->Tag() == TYPE_VECTOR ||
-                // That's ok, we handle it below.
+                      t->Tag() == TYPE_FILE || t->Tag() == TYPE_FUNC ) {
-            }
-
-            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 ) {
                 // That's ok, we handle it below.
             }
 
@@ -822,7 +813,9 @@ bool Manager::TraverseRecord(Stream* stream, Filter* filter, RecordType* rt, Tab
         // Alright, we want this field.
         filter->indices.push_back(new_indices);
 
-        void* tmp = realloc(filter->fields, sizeof(threading::Field*) * (filter->num_fields + 1));
+        // Static cast this to void* to avoid a clang-tidy warning about converting from the
+        // double-pointer to void*
+        void* tmp = realloc(static_cast<void*>(filter->fields), sizeof(threading::Field*) * (filter->num_fields + 1));
 
         if ( ! tmp ) {
             reporter->Error("out of memory in add_filter");

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
+            // For TYPE_TABLE and TYPE_VECTOR, this is sort of dirty, but sqlite does not
-                           // it into a ","-separated string.
+            // directly support arrays. so we just roll it all into a comma-separated string.
+            type = "text";
             break;
 
         default:

src/module_util.cc

@@ -66,8 +66,8 @@ TEST_CASE("module_util normalized_module_name") {
 }
 
 string normalized_module_name(const char* module_name) {
-    int mod_len;
+    int mod_len = strlen(module_name);
-    if ( (mod_len = strlen(module_name)) >= 2 && streq(module_name + mod_len - 2, "::") )
+    if ( (mod_len >= 2) && streq(module_name + mod_len - 2, "::") != 0 )
         mod_len -= 2;
 
     return {module_name, static_cast<size_t>(mod_len)};

src/net_util.cc

@@ -167,6 +167,7 @@ TEST_CASE("fmt_mac") {
     CHECK(my_fmt_mac("", 0) == "");
     CHECK(my_fmt_mac("\x01\x02\x03\x04\x05\x06", 4) == "");
     CHECK(my_fmt_mac("\x01\x02\x03\x04\x05\x06", 6) == "01:02:03:04:05:06");
+    // NOLINTNEXTLINE(bugprone-string-literal-with-embedded-nul)
     CHECK(my_fmt_mac("\x01\x02\x03\x04\x05\x06\x00\x00", 8) == "01:02:03:04:05:06");
     CHECK(my_fmt_mac("\x01\x02\x03\x04\x05\x06\x07\x08", 8) == "01:02:03:04:05:06:07:08");
     CHECK(my_fmt_mac("\x08\x07\x06\x05\x04\x03\x02\x01", 8) == "08:07:06:05:04:03:02:01");
@@ -191,6 +192,7 @@ TEST_CASE("fmt_mac_bytes") {
     CHECK(len2 == 2 * 6 + 5);
     CHECK(memcmp(buf2.get(), "01:02:03:04:05:06", len2 + 1) == 0);
 
+    // NOLINTNEXTLINE(bugprone-string-literal-with-embedded-nul)
     auto [buf3, len3] = my_fmt_mac_bytes("\x01\x02\x03\x04\x05\x06\x00\x00", 8);
     CHECK(len3 == 2 * 6 + 5);
     CHECK(memcmp(buf3.get(), "01:02:03:04:05:06", len3 + 1) == 0);

src/packet_analysis/protocol/arp/ARP.cc

@@ -60,7 +60,7 @@ ARPAnalyzer::ARPAnalyzer() : zeek::packet_analysis::Analyzer("ARP") {}
 #endif
 
 #ifndef ar_tpa
-#define ar_tpa(ap) ((caddr_t((ap) + 1)) + 2 * (ap)->ar_hln + (ap)->ar_pln)
+#define ar_tpa(ap) ((caddr_t((ap) + 1)) + 2 * static_cast<size_t>((ap)->ar_hln) + (ap)->ar_pln)
 #endif
 
 #ifndef ARPOP_REVREQUEST

src/packet_analysis/protocol/tcp/TCPSessionAdapter.cc

@@ -57,7 +57,7 @@ void TCPSessionAdapter::Init() {
 }
 
 void TCPSessionAdapter::Done() {
-    Analyzer::Done();
+    IP::SessionAdapter::Done();
 
     if ( run_state::terminating && connection_pending && is_active && ! BothClosed() )
         Event(connection_pending);
@@ -299,7 +299,7 @@ static zeek::RecordValPtr build_syn_packet_val(bool is_orig, const zeek::IP_Hdr*
 
     // Parse TCP options.
     u_char* options = (u_char*)tcp + sizeof(struct tcphdr);
-    u_char* opt_end = (u_char*)tcp + tcp->th_off * 4;
+    u_char* opt_end = (u_char*)tcp + static_cast<ptrdiff_t>(tcp->th_off * 4);
 
     while ( options < opt_end ) {
         unsigned int opt = options[0];
@@ -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();
@@ -1458,7 +1462,7 @@ void TCPSessionAdapter::SynWeirds(analyzer::tcp::TCP_Flags flags, analyzer::tcp:
 int TCPSessionAdapter::ParseTCPOptions(const struct tcphdr* tcp, bool is_orig) {
     // Parse TCP options.
     const u_char* options = (const u_char*)tcp + sizeof(struct tcphdr);
-    const u_char* opt_end = (const u_char*)tcp + tcp->th_off * 4;
+    const u_char* opt_end = (const u_char*)tcp + static_cast<ptrdiff_t>(tcp->th_off * 4);
     std::vector<const u_char*> opts;
 
     while ( options < opt_end ) {

src/probabilistic/Hasher.cc

@@ -71,6 +71,8 @@ std::unique_ptr<Hasher> Hasher::Unserialize(BrokerDataView data) {
         case Default: hasher.reset(new DefaultHasher(k, {h1, h2})); break;
 
         case Double: hasher.reset(new DoubleHasher(k, {h1, h2})); break;
+
+        default: break;
     }
 
     // Note that the derived classed don't hold any further state of

src/script_opt/CPP/Consts.cc

@@ -63,7 +63,6 @@ shared_ptr<CPP_InitInfo> CPPCompile::RegisterConstant(const ValPtr& vp, int& con
     }
 
     auto tag = t->Tag();
-    auto const_name = const_info[tag]->NextName();
     shared_ptr<CPP_InitInfo> gi;
 
     switch ( tag ) {
@@ -73,14 +72,11 @@ shared_ptr<CPP_InitInfo> CPPCompile::RegisterConstant(const ValPtr& vp, int& con
 
         case TYPE_COUNT: gi = make_shared<BasicConstInfo>(to_string(vp->AsCount()) + "ULL"); break;
 
-        case TYPE_DOUBLE: gi = make_shared<BasicConstInfo>(to_string(vp->AsDouble())); break;
+        case TYPE_DOUBLE:
-
+        case TYPE_TIME:
-        case TYPE_TIME: gi = make_shared<BasicConstInfo>(to_string(vp->AsDouble())); break;
-
         case TYPE_INTERVAL: gi = make_shared<BasicConstInfo>(to_string(vp->AsDouble())); break;
 
-        case TYPE_ADDR: gi = make_shared<DescConstInfo>(this, vp); break;
+        case TYPE_ADDR:
-
         case TYPE_SUBNET: gi = make_shared<DescConstInfo>(this, vp); break;
 
         case TYPE_ENUM: gi = make_shared<EnumConstInfo>(this, vp); break;

src/script_opt/CPP/Exprs.cc

@@ -1241,7 +1241,6 @@ string CPPCompile::GenField(const ExprPtr& rec, int field) {
         auto pt = processed_types.find(rt);
         ASSERT(pt != processed_types.end());
         auto rt_offset = pt->second->Offset();
-        string field_name = rt->FieldName(field);
         field_decls.emplace_back(rt_offset, rt->FieldDecl(field));
 
         if ( rfm != record_field_mappings.end() )

src/script_opt/CPP/InitsInfo.cc

@@ -491,7 +491,6 @@ ListTypeInfo::ListTypeInfo(CPPCompile* _c, TypePtr _t)
 }
 
 void ListTypeInfo::AddInitializerVals(std::vector<std::string>& ivs) const {
-    string type_list;
     for ( auto& t : types ) {
         auto iv = Fmt(c->TypeOffset(t));
         ivs.emplace_back(iv);

src/script_opt/CPP/RuntimeInitSupport.cc

@@ -211,9 +211,11 @@ IDPtr find_global__CPP(const char* g) {
 RecordTypePtr get_record_type__CPP(const char* record_type_name) {
     IDPtr existing_type;
 
-    if ( record_type_name && (existing_type = global_scope()->Find(record_type_name)) &&
+    if ( record_type_name ) {
-         existing_type->GetType()->Tag() == TYPE_RECORD )
+        IDPtr existing_type = global_scope()->Find(record_type_name);
+        if ( existing_type && existing_type->GetType()->Tag() == TYPE_RECORD )
             return cast_intrusive<RecordType>(existing_type->GetType());
+    }
 
     return make_intrusive<RecordType>(new type_decl_list());
 }

src/script_opt/CPP/RuntimeVec.cc

@@ -56,6 +56,7 @@ static VectorTypePtr base_vector_type__CPP(const VectorTypePtr& vt, bool is_bool
 // is an optional kernel to use for vectors whose underlying type
 // is "double".  It needs to be optional because C++ will (rightfully)
 // complain about applying certain C++ unary operations to doubles.
+// NOLINTBEGIN(bugprone-macro-parentheses)
 #define VEC_OP1(name, op, double_kernel)                                                                               \
     VectorValPtr vec_op_##name##__CPP(const VectorValPtr& v, const TypePtr& t) {                                       \
         auto vt = base_vector_type__CPP(cast_intrusive<VectorType>(t));                                                \
@@ -79,6 +80,7 @@ static VectorTypePtr base_vector_type__CPP(const VectorTypePtr& vt, bool is_bool
                                                                                                                        \
         return v_result;                                                                                               \
     }
+// NOLINTEND(bugprone-macro-parentheses)
 
 // Instantiates a double_kernel for a given operation.
 #define VEC_OP1_WITH_DOUBLE(name, op)                                                                                  \
@@ -96,6 +98,7 @@ VEC_OP1(comp, ~, )
 
 // A kernel for applying a binary operation element-by-element to two
 // vectors of a given low-level type.
+// NOLINTBEGIN(bugprone-macro-parentheses)
 #define VEC_OP2_KERNEL(accessor, type, op, zero_check)                                                                 \
     for ( unsigned int i = 0; i < v1->Size(); ++i ) {                                                                  \
         auto v1_i = v1->ValAt(i);                                                                                      \
@@ -107,6 +110,7 @@ VEC_OP1(comp, ~, )
                 v_result->Assign(i, make_intrusive<type>(v1_i->accessor() op v2_i->accessor()));                       \
         }                                                                                                              \
     }
+// NOLINTEND(bugprone-macro-parentheses)
 
 // Analogous to VEC_OP1, instantiates a function for a given binary operation,
 // with customizable kernels for "int" and "double" operations.

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_VECTOR:
-
-        case TYPE_LIST:
         // These occur when initializing tables.
-            return false;
+        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";

src/script_opt/CSE.cc

@@ -255,7 +255,6 @@ bool CSE_ValidityChecker::CheckTableRef(const TypePtr& t) { return CheckSideEffe
 
 bool CSE_ValidityChecker::CheckCall(const CallExpr* c) {
     auto func = c->Func();
-    std::string desc;
     if ( func->Tag() != EXPR_NAME )
         // Can't analyze indirect calls.
         return Invalid();

src/script_opt/GenIDDefs.cc

@@ -265,11 +265,9 @@ TraversalCode GenIDDefs::PostStmt(const Stmt* s) {
             break;
         }
 
-        case STMT_FALLTHROUGH:
         // No need to do anything, the work all occurs
         // with NoFlowAfter.
-            break;
+        case STMT_FALLTHROUGH:
-
         default: break;
     }
 
@@ -379,15 +377,13 @@ bool GenIDDefs::CheckLHS(const ExprPtr& lhs, const ExprPtr& rhs) {
             return true;
         }
 
-        case EXPR_FIELD:
         // If we want to track record field initializations,
         // we'd handle that here.
-            return false;
+        case EXPR_FIELD:
 
-        case EXPR_INDEX:
         // If we wanted to track potential alterations of
         // aggregates, we'd do that here.
-            return false;
+        case EXPR_INDEX: return false;
 
         default: reporter->InternalError("bad tag in GenIDDefs::CheckLHS");
     }

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;
+            // or
-
-        else if ( pc.count(i) == 0 )
             // 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;
+        // or
-
+        // It's active at the beginning of the statement of interest.
-        else if ( ur.EndsAfter() >= stmt_num - 1 )
+        if ( (ur.EndsAfter() == NO_DEF) || (ur.EndsAfter() >= stmt_num - 1) )
-            // It's active at the beginning of the statement of
-            // interest.
             region_ind = i;
     }
 

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) )
+            if ( (pfs->HasSideEffects(SideEffectsOp::READ, aggr_t)) ||
-                has_side_effects = true;
+                 (aggr_t->Tag() == TYPE_TABLE && pfs->IsTableWithDefaultAggr(aggr_t.get())) )
-
-            else if ( aggr_t->Tag() == TYPE_TABLE && pfs->IsTableWithDefaultAggr(aggr_t.get()) )
                 has_side_effects = true;
         }
 

src/script_opt/Stmt.cc

@@ -1281,7 +1281,7 @@ StmtPtr CheckAnyLenStmt::DoReduce(Reducer* c) {
 }
 
 void CheckAnyLenStmt::StmtDescribe(ODesc* d) const {
-    Stmt::StmtDescribe(d);
+    Stmt::StmtDescribe(d); // NOLINT(bugprone-parent-virtual-call)
 
     e->Describe(d);
     if ( ! d->IsBinary() )

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;

src/script_opt/ZAM/ZBody.cc

@@ -202,7 +202,7 @@ static void vec_exec(ZOp op, TypePtr t, VectorVal*& v1, const VectorVal* v2, con
                 auto vi = (*v[i]).rhs_accessor;                                                                        \
                 if ( ov_check(vi) ) {                                                                                  \
                     std::string err = "overflow promoting from ";                                                      \
-                    err += ov_err;                                                                                     \
+                    err += (ov_err);                                                                                   \
                     err += " arithmetic value";                                                                        \
                     /* The run-time error will throw an exception, so recover intermediary memory. */                  \
                     delete res_zv;                                                                                     \

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 )
+    if ( size != rhs.size || type != rhs.type )
-        return false;
-    else if ( type != rhs.type )
         return false;
 
     return memcmp(data, rhs.data, size) == 0;

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 ) {

src/storage/backend/redis/Redis.cc

@@ -246,7 +246,7 @@ OperationResult Redis::DoOpen(OpenResultCallback* cb, RecordValPtr options) {
     StringValPtr host = backend_options->GetField<StringVal>("server_host");
     if ( host ) {
         PortValPtr port = backend_options->GetField<PortVal>("server_port");
-        server_addr = util::fmt("%s:%d", host->ToStdStringView().data(), port->Port());
+        server_addr = util::fmt("%s:%d", host->ToStdString().c_str(), port->Port());
         REDIS_OPTIONS_SET_TCP(&opt, host->ToStdStringView().data(), port->Port());
     }
     else {
@@ -695,10 +695,12 @@ OperationResult Redis::ParseReplyError(std::string_view op_str, std::string_view
     if ( async_ctx->err == REDIS_ERR_TIMEOUT )
         return {ReturnCode::TIMEOUT};
     else if ( async_ctx->err == REDIS_ERR_IO )
-        return {ReturnCode::OPERATION_FAILED, util::fmt("%s operation IO error: %s", op_str.data(), strerror(errno))};
+        return {ReturnCode::OPERATION_FAILED, util::fmt("%.*s operation IO error: %s", static_cast<int>(op_str.size()),
+                                                        op_str.data(), strerror(errno))};
     else
         return {ReturnCode::OPERATION_FAILED,
-                util::fmt("%s operation failed: %s", op_str.data(), reply_err_str.data())};
+                util::fmt("%.*s operation failed: %.*s", static_cast<int>(op_str.size()), op_str.data(),
+                          static_cast<int>(reply_err_str.size()), reply_err_str.data())};
 }
 
 void Redis::DoPoll() {

src/storage/serializer/json/JSON.cc

@@ -35,8 +35,9 @@ zeek::expected<ValPtr, std::string> JSON::Unserialize(byte_buffer_span buf, Type
 
     std::string_view version = std::string_view(text).substr(0, semicolon);
     if ( version != versioned_name )
-        return zeek::unexpected<std::string>(
+        return zeek::unexpected<std::string>(util::fmt("Version doesn't match: %.*s vs %s",
-            util::fmt("Version doesn't match: %s vs %s", version.data(), versioned_name.c_str()));
+                                                       static_cast<int>(version.size()), version.data(),
+                                                       versioned_name.c_str()));
 
     return zeek::detail::ValFromJSON(text.substr(semicolon + 1), type, Func::nil);
 }

src/telemetry/Manager.cc

@@ -301,7 +301,8 @@ ValPtr Manager::CollectMetrics(std::string_view prefix_pattern, std::string_view
 
     // Due to the name containing the full information about a metric including a potential unit add an
     // asterisk to the end of the full pattern so matches work correctly.
-    std::string full_pattern = util::fmt("%s_%s", prefix_pattern.data(), name_pattern.data());
+    std::string full_pattern = util::fmt("%.*s_%.*s", static_cast<int>(prefix_pattern.size()), prefix_pattern.data(),
+                                         static_cast<int>(name_pattern.size()), name_pattern.data());
     if ( full_pattern[full_pattern.size() - 1] != '*' )
         full_pattern.append("*");
 
@@ -380,7 +381,8 @@ ValPtr Manager::CollectHistogramMetrics(std::string_view prefix_pattern, std::st
 
     // Due to the name containing the full information about a metric including a potential unit add an
     // asterisk to the end of the full pattern so matches work correctly.
-    std::string full_pattern = util::fmt("%s_%s", prefix_pattern.data(), name_pattern.data());
+    std::string full_pattern = util::fmt("%.*s_%.*s", static_cast<int>(prefix_pattern.size()), prefix_pattern.data(),
+                                         static_cast<int>(name_pattern.size()), name_pattern.data());
     if ( full_pattern[full_pattern.size() - 1] != '*' )
         full_pattern.append("*");
 

src/telemetry/Utils.cc

@@ -16,7 +16,8 @@ std::string BuildFullPrometheusName(std::string_view prefix, std::string_view na
     if ( prefix.empty() || name.empty() )
         reporter->FatalError("Telemetry metric families must have a non-zero-length prefix and name");
 
-    std::string fn = util::fmt("%s_%s", prefix.data(), name.data());
+    std::string fn = util::fmt("%.*s_%.*s", static_cast<int>(prefix.size()), prefix.data(),
+                               static_cast<int>(name.size()), name.data());
     std::for_each(fn.begin(), fn.end(), [](char& c) {
         if ( ! std::isalnum(c) )
             c = '_';

src/threading/SerialTypes.cc

@@ -245,8 +245,8 @@ bool Value::Read(detail::SerializationFormat* fmt) {
 
             switch ( family ) {
                 case 4: val.addr_val.family = IPv4; return fmt->Read(&val.addr_val.in.in4, "addr-in4");
-
                 case 6: val.addr_val.family = IPv6; return fmt->Read(&val.addr_val.in.in6, "addr-in6");
+                default: reporter->Warning("Unknown family type %d when reading addr\n", family); break;
             }
 
             // Can't be reached.
@@ -270,6 +270,7 @@ bool Value::Read(detail::SerializationFormat* fmt) {
                     val.subnet_val.length = (uint8_t)length;
                     val.subnet_val.prefix.family = IPv6;
                     return fmt->Read(&val.subnet_val.prefix.in.in6, "subnet-in6");
+                default: reporter->Warning("Unknown family type %d when reading subnet\n", family); break;
             }
 
             // Can't be reached.

src/util.cc

@@ -93,8 +93,7 @@ std::string extract_ip(const std::string& i) {
     if ( s.size() > 1 && s.substr(0, 2) == "0x" )
         s.erase(0, 2);
 
-    size_t pos = 0;
+    if ( size_t pos = s.find(']'); pos != std::string::npos )
-    if ( (pos = s.find(']')) != std::string::npos )
         s = s.substr(0, pos);
 
     return s;
@@ -298,9 +297,9 @@ void hmac_md5(size_t size, const unsigned char* bytes, unsigned char digest[16])
 
 static bool read_random_seeds(const char* read_file, uint32_t* seed,
                               std::array<uint32_t, zeek::detail::KeyedHash::SEED_INIT_SIZE>& buf) {
-    FILE* f = nullptr;
+    FILE* f = fopen(read_file, "r");
 
-    if ( ! (f = fopen(read_file, "r")) ) {
+    if ( ! f ) {
         reporter->Warning("Could not open seed file '%s': %s", read_file, strerror(errno));
         return false;
     }
@@ -328,9 +327,9 @@ static bool read_random_seeds(const char* read_file, uint32_t* seed,
 
 static bool write_random_seeds(const char* write_file, uint32_t seed,
                                std::array<uint32_t, zeek::detail::KeyedHash::SEED_INIT_SIZE>& buf) {
-    FILE* f = nullptr;
+    FILE* f = fopen(write_file, "w+");
 
-    if ( ! (f = fopen(write_file, "w+")) ) {
+    if ( ! f ) {
         reporter->Warning("Could not create seed file '%s': %s", write_file, strerror(errno));
         return false;
     }
@@ -1764,7 +1763,7 @@ vector<string>* tokenize_string(std::string_view input, std::string_view delim,
     return rval;
 }
 
-vector<std::string_view> tokenize_string(std::string_view input, const char delim) noexcept {
+vector<std::string_view> tokenize_string(std::string_view input, const char delim) {
     vector<std::string_view> rval;
 
     size_t pos = 0;
@@ -1876,9 +1875,9 @@ double current_time(bool real) {
 struct timeval double_to_timeval(double t) {
     struct timeval tv;
 
-    double t1 = floor(t);
+    double t1 = std::floor(t);
-    tv.tv_sec = int(t1);
+    tv.tv_sec = static_cast<time_t>(t1);
-    tv.tv_usec = int((t - t1) * 1e6 + 0.5);
+    tv.tv_usec = std::lround((t - t1) * 1e6);
 
     return tv;
 }
@@ -2337,6 +2336,7 @@ TEST_CASE("util json_escape_utf8") {
 
     // Valid ASCII and valid ASCII control characters
     CHECK(json_escape_utf8("a") == "a");
+    // NOLINTNEXTLINE(bugprone-string-literal-with-embedded-nul)
     CHECK(json_escape_utf8("\b\f\n\r\t\x00\x15") == "\b\f\n\r\t\x00\x15");
 
     // Table 3-7 in https://www.unicode.org/versions/Unicode12.0.0/ch03.pdf describes what is
@@ -2403,14 +2403,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) )
+    if ( ((output[0] <= 0x001F) || (output[0] == 0x007F) ||
-        // Control characters
+          (output[0] >= 0x0080 && output[0] <= 0x009F)) || // Control characters
-        return false;
+         (output[0] >= 0xE000 && output[0] <= 0xF8FF) ||   // Private Use Area
-    else if ( output[0] >= 0xE000 && output[0] <= 0xF8FF )
+         (output[0] >= 0xFFF0 && output[0] <= 0xFFFF) )    // Special characters
-        // Private Use Area
-        return false;
-    else if ( output[0] >= 0xFFF0 && output[0] <= 0xFFFF )
-        // Specials Characters
         return false;
 
     return true;

src/util.h

@@ -338,7 +338,7 @@ inline std::string get_escaped_string(const std::string& str, bool escape_all) {
 std::vector<std::string>* tokenize_string(std::string_view input, std::string_view delim,
                                           std::vector<std::string>* rval = nullptr, int limit = 0);
 
-std::vector<std::string_view> tokenize_string(std::string_view input, const char delim) noexcept;
+std::vector<std::string_view> tokenize_string(std::string_view input, const char delim);
 
 extern char* copy_string(const char* str, size_t len);
 extern char* copy_string(const char* s);

src/zeek-setup.cc

@@ -1018,7 +1018,7 @@ SetupResult setup(int argc, char** argv, Options* zopts) {
     }
 
     // Cooperate with nohup(1).
-    if ( (oldhandler = setsignal(SIGHUP, sig_handler)) != SIG_DFL )
+    if ( oldhandler = setsignal(SIGHUP, sig_handler); oldhandler != SIG_DFL )
         (void)setsignal(SIGHUP, oldhandler);
 
     // If we were priming the DNS cache (i.e. -P was passed as an argument), flush anything

src/zeekygen/Target.cc

@@ -346,7 +346,7 @@ void PackageTarget::DoFindDependencies(const vector<Info*>& infos) {
             continue;
 
         for ( size_t j = 0; j < pkg_deps.size(); ++j ) {
-            if ( strncmp(script->Name().c_str(), pkg_deps[j]->Name().c_str(), pkg_deps[j]->Name().size()) )
+            if ( strncmp(script->Name().c_str(), pkg_deps[j]->Name().c_str(), pkg_deps[j]->Name().size()) != 0 )
                 continue;
 
             DBG_LOG(DBG_ZEEKYGEN, "Script %s associated with package %s", script->Name().c_str(),