Fix clang-tidy cppcoreguidelines-macro-usage findings (macro functions)

.clang-tidy

@@ -2,6 +2,8 @@ Checks: [-*,
          bugprone-*,
          performance-*,
 
+         cppcoreguidelines-macro-usage,
+         cppcoreguidelines-misleading-capture-default-by-value,
          cppcoreguidelines-virtual-class-destructor,
 
          # Skipping these temporarily because they are very noisy

src/Expr.cc

@@ -742,6 +742,8 @@ ValPtr BinaryExpr::Fold(Val* v1, Val* v2) const {
         RuntimeErrorWithCallStack("bad type in BinaryExpr::Fold");
 
     switch ( tag ) {
+        // Once we have C++20, these macros can become templated lambdas.
+        // NOLINTBEGIN(cppcoreguidelines-macro-usage)
 #define DO_INT_FOLD(op)                                                                                                \
     if ( is_integral )                                                                                                 \
         i3 = i1 op i2;                                                                                                 \
@@ -771,6 +773,7 @@ ValPtr BinaryExpr::Fold(Val* v1, Val* v2) const {
         i3 = u1 op u2;                                                                                                 \
     else                                                                                                               \
         i3 = d1 op d2;
+            // NOLINTEND(cppcoreguidelines-macro-usage)
 
         case EXPR_ADD:
         case EXPR_ADD_TO: DO_FOLD(+); break;
@@ -892,13 +895,13 @@ ValPtr BinaryExpr::StringFold(Val* v1, Val* v2) const {
 
     switch ( tag ) {
 #undef DO_FOLD
-// NOLINTBEGIN(bugprone-macro-parentheses)
+// NOLINTBEGIN(bugprone-macro-parentheses, cppcoreguidelines-macro-usage)
 #define DO_FOLD(sense)                                                                                                 \
     {                                                                                                                  \
         result = Bstr_cmp(s1, s2) sense 0;                                                                             \
         break;                                                                                                         \
     }
-            // NOLINTEND(bugprone-macro-parentheses)
+            // NOLINTEND(bugprone-macro-parentheses, cppcoreguidelines-macro-usage)
 
         case EXPR_LT: DO_FOLD(<)
         case EXPR_LE: DO_FOLD(<=)

src/Type.cc

@@ -69,6 +69,7 @@ Type::Type(TypeTag t, bool arg_base_type)
       is_network_order(zeek::is_network_order(t)),
       base_type(arg_base_type) {}
 
+// NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
 #define CHECK_TYPE_TAG(tag_type, func_name) CHECK_TAG(tag, tag_type, func_name, type_name)
 
 const TypeList* Type::AsTypeList() const {

src/Val.cc

@@ -53,6 +53,8 @@ Val::~Val() {
 #endif
 }
 
+// NOLINTBEGIN(cppcoreguidelines-macro-usage)
+
 #define CONVERTER(tag, ctype, name)                                                                                    \
     ctype name() {                                                                                                     \
         CHECK_TAG(type->Tag(), tag, "Val::CONVERTER", type_name)                                                       \
@@ -69,6 +71,8 @@ Val::~Val() {
     CONVERTER(tag, ctype, name)                                                                                        \
     CONST_CONVERTER(tag, ctype, name)
 
+// NOLINTEND(cppcoreguidelines-macro-usage)
+
 CONVERTERS(TYPE_FUNC, FuncVal*, Val::AsFuncVal)
 CONVERTERS(TYPE_FILE, FileVal*, Val::AsFileVal)
 CONVERTERS(TYPE_PATTERN, PatternVal*, Val::AsPatternVal)

src/analyzer/protocol/bittorrent/BitTorrentTracker.cc

@@ -456,7 +456,7 @@ void BitTorrentTracker_Analyzer::ResponseBody(void) {
     }
 }
 
-int BitTorrentTracker_Analyzer::ResponseParseBenc(void) {
+// NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
 #define VIOLATION_IF(expr, msg)                                                                                        \
     {                                                                                                                  \
         if ( expr ) {                                                                                                  \
@@ -466,12 +466,12 @@ int BitTorrentTracker_Analyzer::ResponseParseBenc(void) {
         }                                                                                                              \
     }
 
-#define INC_COUNT                                                                                                      \
+int BitTorrentTracker_Analyzer::ResponseParseBenc(void) {
-    {                                                                                                                  \
+    auto INC_COUNT = [this]() {
-        unsigned int count = benc_count.back();                                                                        \
+        unsigned int count = benc_count.back();
-        benc_count.pop_back();                                                                                         \
+        benc_count.pop_back();
-        benc_count.push_back(count + 1);                                                                               \
+        benc_count.push_back(count + 1);
-    }
+    };
 
     for ( unsigned int len = res_buf_len - (res_buf_pos - res_buf); len; --len, ++res_buf_pos ) {
         switch ( benc_state ) {
@@ -551,7 +551,7 @@ int BitTorrentTracker_Analyzer::ResponseParseBenc(void) {
                         benc_count.pop_back();
 
                         if ( benc_stack.size() )
-                            INC_COUNT
+                            INC_COUNT();
                         else { // benc parsing successful
                             ++res_buf_pos;
                             return 0;
@@ -612,7 +612,7 @@ int BitTorrentTracker_Analyzer::ResponseParseBenc(void) {
                     else
                         VIOLATION_IF(1, "BitTorrentTracker: no valid bencoding")
 
-                    INC_COUNT
+                    INC_COUNT();
                     benc_state = detail::BENC_STATE_EMPTY;
                 }
 
@@ -686,7 +686,7 @@ int BitTorrentTracker_Analyzer::ResponseParseBenc(void) {
                         ++len;
                     }
 
-                    INC_COUNT
+                    INC_COUNT();
                     benc_state = detail::BENC_STATE_EMPTY;
                 }
                 break;

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)
+constexpr bool IS_3_BYTE_OPTION(unsigned int code) { return code >= 251 && code <= 254; }
 
 static constexpr uint8_t TELNET_OPT_SB = 250;
 static constexpr uint8_t TELNET_OPT_SE = 240;
@@ -411,17 +411,13 @@ void NVT_Analyzer::DeliverChunk(int& len, const u_char*& data) {
         if ( binary_mode && c != TELNET_IAC )
             c &= 0x7f;
 
-#define EMIT_LINE                                                                                                      \
-    {                                                                                                                  \
-        buf[offset] = '\0';                                                                                            \
-        ForwardStream(offset, buf, IsOrig());                                                                          \
-        offset = 0;                                                                                                    \
-    }
-
         switch ( c ) {
             case '\r':
-                if ( CRLFAsEOL() & CR_as_EOL )
+                if ( CRLFAsEOL() & CR_as_EOL ) {
-                    EMIT_LINE
+                    buf[offset] = '\0';
+                    ForwardStream(offset, buf, IsOrig());
+                    offset = 0;
+                }
                 else
                     buf[offset++] = c;
                 break;
@@ -433,12 +429,17 @@ void NVT_Analyzer::DeliverChunk(int& len, const u_char*& data) {
                         ;
                     else {
                         --offset; // remove '\r'
-                        EMIT_LINE
+                        buf[offset] = '\0';
+                        ForwardStream(offset, buf, IsOrig());
+                        offset = 0;
                     }
                 }
 
-                else if ( CRLFAsEOL() & LF_as_EOL )
+                else if ( CRLFAsEOL() & LF_as_EOL ) {
-                    EMIT_LINE
+                    buf[offset] = '\0';
+                    ForwardStream(offset, buf, IsOrig());
+                    offset = 0;
+                }
 
                 else {
                     if ( Conn()->FlagEvent(SINGULAR_LF) )

src/analyzer/protocol/ncp/NCP.cc

@@ -9,11 +9,6 @@
 
 using namespace std;
 
-#define xbyte(b, n) (((const u_char*)(b))[n])
-#define extract_uint16(little_endian, bytes)                                                                           \
-    ((little_endian) ? uint16(xbyte(bytes, 0)) | ((uint16(xbyte(bytes, 1))) << 8) :                                    \
-                       uint16(xbyte(bytes, 1)) | ((uint16(xbyte(bytes, 0))) << 8))
-
 namespace zeek::analyzer::ncp {
 namespace detail {
 

src/analyzer/protocol/netbios/NetbiosSSN.cc

@@ -10,14 +10,15 @@
 #include "zeek/analyzer/protocol/netbios/events.bif.h"
 #include "zeek/session/Manager.h"
 
-constexpr double netbios_ssn_session_timeout = 15.0;
+static constexpr double netbios_ssn_session_timeout = 15.0;
 
-#define MAKE_INT16(dest, src)                                                                                          \
+static constexpr void MAKE_INT16(uint16_t& dest, const u_char*& 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/pop3/POP3.cc

@@ -24,8 +24,6 @@ static const char* pop3_cmd_word[] = {
 #include "POP3_cmd.def"
 };
 
-#define POP3_CMD_WORD(code) (((code) >= 0) ? pop3_cmd_word[code] : "(UNKNOWN)")
-
 POP3_Analyzer::POP3_Analyzer(Connection* conn) : analyzer::tcp::TCP_ApplicationAnalyzer("POP3", conn) {
     masterState = detail::POP3_START;
     subState = detail::POP3_WOK;

src/analyzer/protocol/smtp/SMTP.cc

@@ -14,13 +14,15 @@
 #undef SMTP_CMD_DEF
 #define SMTP_CMD_DEF(cmd) #cmd,
 
+// This could be constexpr too but it would require changing the macro above. It doesn't
+// matter that much though.
 static const char* smtp_cmd_word[] = {
 #include "SMTP_cmd.def"
 };
 
-static const char* unknown_cmd = "(UNKNOWN)";
+static constexpr char unknown_cmd[] = "(UNKNOWN)";
 
-#define SMTP_CMD_WORD(code) (((code) >= 0) ? smtp_cmd_word[code] : unknown_cmd)
+static constexpr const char* SMTP_CMD_WORD(int code) { return code >= 0 ? smtp_cmd_word[code] : unknown_cmd; }
 
 namespace zeek::analyzer::smtp {
 

src/broker/Manager.cc

@@ -400,8 +400,6 @@ struct opt_mapping {
     }
 };
 
-#define WITH_OPT_MAPPING(broker_name, zeek_name) if ( auto opt = opt_mapping{&config, broker_name, zeek_name}; true )
-
 } // namespace
 
 class BrokerState {

src/broker/WebSocketShim.cc

@@ -12,6 +12,7 @@
 #include "zeek/iosource/IOSource.h"
 #include "zeek/iosource/Manager.h"
 
+// NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
 #define BROKER_WS_DEBUG(...)                                                                                           \
     do {                                                                                                               \
         DBG_LOG(DBG_BROKER, __VA_ARGS__);                                                                              \

src/cluster/backend/zeromq/ZeroMQ.cc

@@ -52,6 +52,8 @@ enum class InprocTag : uint8_t {
 
 constexpr DebugFlag operator&(uint8_t x, DebugFlag y) { return static_cast<DebugFlag>(x & static_cast<uint8_t>(y)); }
 
+// NOLINTBEGIN(cppcoreguidelines-macro-usage)
+
 #define ZEROMQ_DEBUG(...) PLUGIN_DBG_LOG(zeek::plugin::Zeek_Cluster_Backend_ZeroMQ::plugin, __VA_ARGS__)
 
 #define ZEROMQ_THREAD_PRINTF(...)                                                                                      \
@@ -66,6 +68,8 @@ constexpr DebugFlag operator&(uint8_t x, DebugFlag y) { return static_cast<Debug
         }                                                                                                              \
     } while ( 0 )
 
+// NOLINTEND(cppcoreguidelines-macro-usage)
+
 ZeroMQBackend::ZeroMQBackend(std::unique_ptr<EventSerializer> es, std::unique_ptr<LogSerializer> ls,
                              std::unique_ptr<detail::EventHandlingStrategy> ehs)
     : ThreadedBackend("ZeroMQ", std::move(es), std::move(ls), std::move(ehs)) {

src/cluster/serializer/binary-serialization-format/Serializer.cc

@@ -22,6 +22,7 @@ extern Plugin plugin;
 
 }
 
+// NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
 #define SERIALIZER_DEBUG(...) PLUGIN_DBG_LOG(zeek::plugin::Zeek_Binary_Serializer::plugin, __VA_ARGS__)
 
 bool detail::BinarySerializationFormatLogSerializer::SerializeLogWrite(byte_buffer& buf,

src/cluster/websocket/WebSocket.cc

@@ -27,7 +27,7 @@
 #include "rapidjson/document.h"
 #include "rapidjson/rapidjson.h"
 
-
+// NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
 #define WS_DEBUG(...) PLUGIN_DBG_LOG(zeek::plugin::Cluster_WebSocket::plugin, __VA_ARGS__)
 
 namespace zeek {

src/net_util.cc

@@ -129,8 +129,8 @@ constexpr uint32_t CLASS_C = 0xc0000000;
 constexpr uint32_t CLASS_D = 0xe0000000;
 constexpr uint32_t CLASS_E = 0xf0000000;
 
-#define CHECK_CLASS(addr, class) (((addr) & (class)) == (class))
 char addr_to_class(uint32_t addr) {
+    auto CHECK_CLASS = [](uint32_t addr, uint32_t cls) { return (addr & cls) == cls; };
     if ( CHECK_CLASS(addr, CLASS_E) )
         return 'E';
     else if ( CHECK_CLASS(addr, CLASS_D) )

src/packet_analysis/protocol/tcp/Stats.cc

@@ -41,6 +41,8 @@ void TCPStateStats::PrintStats(File* file, const char* prefix) {
         file->Write(prefix);
 
         switch ( i ) {
+// This macro really doesn't save us much typing, if that was the intention
+// NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
 #define STATE_STRING(state, str)                                                                                       \
     case state: file->Write(str); break;
 

src/script_opt/CPP/RuntimeVec.cc

@@ -42,6 +42,8 @@ static VectorTypePtr base_vector_type__CPP(const VectorTypePtr& vt, bool is_bool
     }
 }
 
+// NOLINTBEGIN(cppcoreguidelines-macro-usage)
+
 // The kernel used for unary vector operations.
 #define VEC_OP1_KERNEL(accessor, type, op)                                                                             \
     for ( unsigned int i = 0; i < v->Size(); ++i ) {                                                                   \
@@ -90,6 +92,8 @@ static VectorTypePtr base_vector_type__CPP(const VectorTypePtr& vt, bool is_bool
             break;                                                                                                     \
         })
 
+// NOLINTEND(cppcoreguidelines-macro-usage)
+
 // The unary operations supported for vectors.
 VEC_OP1_WITH_DOUBLE(pos, +)
 VEC_OP1_WITH_DOUBLE(neg, -)
@@ -98,6 +102,7 @@ VEC_OP1(comp, ~, )
 
 // A kernel for applying a binary operation element-by-element to two
 // vectors of a given low-level type.
+// NOLINTBEGIN(cppcoreguidelines-macro-usage)
 // NOLINTBEGIN(bugprone-macro-parentheses)
 #define VEC_OP2_KERNEL(accessor, type, op, zero_check)                                                                 \
     for ( unsigned int i = 0; i < v1->Size(); ++i ) {                                                                  \
@@ -167,6 +172,7 @@ VEC_OP1(comp, ~, )
 			break;                                                                                 \
 		},                                                                                         \
 		zero_check)
+// NOLINTEND(cppcoreguidelines-macro-usage)
 
 // The binary operations supported for vectors.
 VEC_OP2_WITH_DOUBLE(add, +, 0)
@@ -184,6 +190,7 @@ VEC_OP2_WITH_INT(rshift, >>, , 0)
 
 // A version of VEC_OP2 that instead supports relational operations, so
 // the result type is always vector-of-bool.
+// NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
 #define VEC_REL_OP(name, op)                                                                                           \
     VectorValPtr vec_op_##name##__CPP(const VectorValPtr& v1, const VectorValPtr& v2) {                                \
         if ( ! check_vec_sizes__CPP(v1, v2) )                                                                          \

src/script_opt/ZAM/ZBody.cc

@@ -188,6 +188,8 @@ static void vec_exec(ZOp op, TypePtr t, VectorVal*& v1, const VectorVal* v2, con
 
 static void vec_exec(ZOp op, TypePtr t, VectorVal*& v1, const VectorVal* v2, const VectorVal* v3, const ZInst& z);
 
+auto false_func = [](double x) { return false; };
+
 // Vector coercion.
 #define VEC_COERCE(tag, lhs_type, cast, rhs_accessor, ov_check, ov_err)                                                \
     VectorVal* vec_coerce_##tag(VectorVal* vec, std::shared_ptr<ZAMLocInfo> z_loc) {                                   \
@@ -216,8 +218,6 @@ static void vec_exec(ZOp op, TypePtr t, VectorVal*& v1, const VectorVal* v2, con
         return res_zv;                                                                                                 \
     }
 
-#define false_func(x) false
-
 VEC_COERCE(DI, TYPE_DOUBLE, double, AsInt(), false_func, "")
 VEC_COERCE(DU, TYPE_DOUBLE, double, AsCount(), false_func, "")
 VEC_COERCE(ID, TYPE_INT, zeek_int_t, AsDouble(), double_to_int_would_overflow, "double to signed")

src/spicy/file-analyzer.cc

@@ -13,11 +13,13 @@ using namespace zeek;
 using namespace zeek::spicy;
 using namespace zeek::spicy::rt;
 
+// NOLINTBEGIN(cppcoreguidelines-macro-usage)
 #ifdef DEBUG
 #define STATE_DEBUG_MSG(...) DebugMsg(__VA_ARGS__)
 #else
 #define STATE_DEBUG_MSG(...)
 #endif
+// NOLINTEND(cppcoreguidelines-macro-usage)
 
 void FileState::debug(const std::string& msg) { spicy::rt::debug(_cookie, msg); }
 

src/spicy/packet-analyzer.cc

@@ -9,11 +9,13 @@ using namespace zeek;
 using namespace zeek::spicy;
 using namespace zeek::spicy::rt;
 
+// NOLINTBEGIN(cppcoreguidelines-macro-usage)
 #ifdef DEBUG
 #define STATE_DEBUG_MSG(...) DebugMsg(__VA_ARGS__)
 #else
 #define STATE_DEBUG_MSG(...)
 #endif
+// NOLINTEND(cppcoreguidelines-macro-usage)
 
 void PacketState::debug(const std::string& msg) { spicy::rt::debug(_cookie, msg); }
 

src/spicy/protocol-analyzer.cc

@@ -9,11 +9,13 @@ using namespace zeek;
 using namespace zeek::spicy;
 using namespace zeek::spicy::rt;
 
+// NOLINTBEGIN(cppcoreguidelines-macro-usage)
 #ifdef DEBUG
 #define STATE_DEBUG_MSG(...) DebugMsg(__VA_ARGS__)
 #else
 #define STATE_DEBUG_MSG(...)
 #endif
+// NOLINTEND(cppcoreguidelines-macro-usage)
 
 void EndpointState::debug(const std::string& msg) { spicy::rt::debug(_cookie, msg); }
 

src/supervisor/Supervisor.cc

@@ -44,11 +44,13 @@ extern "C" {
 #include "zeek/util.h"
 #include "zeek/zeek-affinity.h"
 
+// NOLINTBEGIN(cppcoreguidelines-macro-usage)
 #ifdef DEBUG
 #define DBG_STEM(...) stem->LogDebug(__VA_ARGS__);
 #else
 #define DBG_STEM(...)
 #endif
+// NOLINTEND(cppcoreguidelines-macro-usage)
 
 using namespace zeek;
 using zeek::detail::SupervisedNode;