From 460fe24a9a42ee0f24d46353019d89fad9bd868a Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Fri, 9 May 2025 11:59:04 -0700 Subject: [PATCH] Fix clang-tidy cppcoreguidelines-macro-usage findings (macro functions) --- .clang-tidy | 2 ++ src/Expr.cc | 7 +++-- src/Type.cc | 1 + src/Val.cc | 4 +++ .../protocol/bittorrent/BitTorrentTracker.cc | 20 +++++++------- src/analyzer/protocol/login/NVT.cc | 27 ++++++++++--------- src/analyzer/protocol/ncp/NCP.cc | 5 ---- src/analyzer/protocol/netbios/NetbiosSSN.cc | 15 ++++++----- src/analyzer/protocol/pop3/POP3.cc | 2 -- src/analyzer/protocol/smtp/SMTP.cc | 6 +++-- src/broker/Manager.cc | 2 -- src/broker/WebSocketShim.cc | 1 + src/cluster/backend/zeromq/ZeroMQ.cc | 4 +++ .../binary-serialization-format/Serializer.cc | 1 + src/cluster/websocket/WebSocket.cc | 2 +- src/net_util.cc | 2 +- src/packet_analysis/protocol/tcp/Stats.cc | 2 ++ src/script_opt/CPP/RuntimeVec.cc | 7 +++++ src/script_opt/ZAM/ZBody.cc | 4 +-- src/spicy/file-analyzer.cc | 2 ++ src/spicy/packet-analyzer.cc | 2 ++ src/spicy/protocol-analyzer.cc | 2 ++ src/supervisor/Supervisor.cc | 2 ++ 23 files changed, 75 insertions(+), 47 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index afe9219e49..34b33b6e0f 100644 --- a/.clang-tidy +++ b/.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 diff --git a/src/Expr.cc b/src/Expr.cc index 35391eea34..06f142100f 100644 --- a/src/Expr.cc +++ b/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(<=) diff --git a/src/Type.cc b/src/Type.cc index b504dfe59f..c00f0951af 100644 --- a/src/Type.cc +++ b/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 { diff --git a/src/Val.cc b/src/Val.cc index b15e502f60..19dd67fcd2 100644 --- a/src/Val.cc +++ b/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) diff --git a/src/analyzer/protocol/bittorrent/BitTorrentTracker.cc b/src/analyzer/protocol/bittorrent/BitTorrentTracker.cc index 178687623c..dc4b8acce4 100644 --- a/src/analyzer/protocol/bittorrent/BitTorrentTracker.cc +++ b/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 \ - { \ - unsigned int count = benc_count.back(); \ - benc_count.pop_back(); \ - benc_count.push_back(count + 1); \ - } +int BitTorrentTracker_Analyzer::ResponseParseBenc(void) { + auto INC_COUNT = [this]() { + unsigned int count = benc_count.back(); + benc_count.pop_back(); + 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; diff --git a/src/analyzer/protocol/login/NVT.cc b/src/analyzer/protocol/login/NVT.cc index bdb7336157..6de963e323 100644 --- a/src/analyzer/protocol/login/NVT.cc +++ b/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 ) - EMIT_LINE + if ( CRLFAsEOL() & CR_as_EOL ) { + 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 ) - EMIT_LINE + else if ( CRLFAsEOL() & LF_as_EOL ) { + buf[offset] = '\0'; + ForwardStream(offset, buf, IsOrig()); + offset = 0; + } else { if ( Conn()->FlagEvent(SINGULAR_LF) ) diff --git a/src/analyzer/protocol/ncp/NCP.cc b/src/analyzer/protocol/ncp/NCP.cc index 03c5a4486d..7cd518d14f 100644 --- a/src/analyzer/protocol/ncp/NCP.cc +++ b/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 { diff --git a/src/analyzer/protocol/netbios/NetbiosSSN.cc b/src/analyzer/protocol/netbios/NetbiosSSN.cc index c9106537b2..33ef723531 100644 --- a/src/analyzer/protocol/netbios/NetbiosSSN.cc +++ b/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) \ - (dest) = *(src); \ - (dest) <<= 8; \ - (src)++; \ - (dest) |= *(src); \ - (src)++; +static constexpr void MAKE_INT16(uint16_t& dest, const u_char*& src) { + dest = *src; + dest <= 8; + src++; + dest |= *src; + src++; +} namespace zeek::analyzer::netbios_ssn { namespace detail { diff --git a/src/analyzer/protocol/pop3/POP3.cc b/src/analyzer/protocol/pop3/POP3.cc index d9d2466ae7..e1a3e240aa 100644 --- a/src/analyzer/protocol/pop3/POP3.cc +++ b/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; diff --git a/src/analyzer/protocol/smtp/SMTP.cc b/src/analyzer/protocol/smtp/SMTP.cc index 8edc14b286..06a4d10b4f 100644 --- a/src/analyzer/protocol/smtp/SMTP.cc +++ b/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 { diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index 22c70981d7..e182764085 100644 --- a/src/broker/Manager.cc +++ b/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 { diff --git a/src/broker/WebSocketShim.cc b/src/broker/WebSocketShim.cc index 0bc88158f3..552edb749f 100644 --- a/src/broker/WebSocketShim.cc +++ b/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__); \ diff --git a/src/cluster/backend/zeromq/ZeroMQ.cc b/src/cluster/backend/zeromq/ZeroMQ.cc index 0479a8bf06..5c1104afbd 100644 --- a/src/cluster/backend/zeromq/ZeroMQ.cc +++ b/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(x & static_cast(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 es, std::unique_ptr ls, std::unique_ptr ehs) : ThreadedBackend("ZeroMQ", std::move(es), std::move(ls), std::move(ehs)) { diff --git a/src/cluster/serializer/binary-serialization-format/Serializer.cc b/src/cluster/serializer/binary-serialization-format/Serializer.cc index 888b4bdf22..7d9ede592d 100644 --- a/src/cluster/serializer/binary-serialization-format/Serializer.cc +++ b/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, diff --git a/src/cluster/websocket/WebSocket.cc b/src/cluster/websocket/WebSocket.cc index 2d602180c4..d9797aa0ed 100644 --- a/src/cluster/websocket/WebSocket.cc +++ b/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 { diff --git a/src/net_util.cc b/src/net_util.cc index eceda9befd..83a8db6dd0 100644 --- a/src/net_util.cc +++ b/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) ) diff --git a/src/packet_analysis/protocol/tcp/Stats.cc b/src/packet_analysis/protocol/tcp/Stats.cc index 3599313852..a19d0ca09d 100644 --- a/src/packet_analysis/protocol/tcp/Stats.cc +++ b/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; diff --git a/src/script_opt/CPP/RuntimeVec.cc b/src/script_opt/CPP/RuntimeVec.cc index e7d61b223d..d93b5c485a 100644 --- a/src/script_opt/CPP/RuntimeVec.cc +++ b/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) ) \ diff --git a/src/script_opt/ZAM/ZBody.cc b/src/script_opt/ZAM/ZBody.cc index 4cb96716ce..8ce9d041ab 100644 --- a/src/script_opt/ZAM/ZBody.cc +++ b/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 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") diff --git a/src/spicy/file-analyzer.cc b/src/spicy/file-analyzer.cc index 732ae68d68..0a7df9ae96 100644 --- a/src/spicy/file-analyzer.cc +++ b/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); } diff --git a/src/spicy/packet-analyzer.cc b/src/spicy/packet-analyzer.cc index f69c301bb4..bccc3275c6 100644 --- a/src/spicy/packet-analyzer.cc +++ b/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); } diff --git a/src/spicy/protocol-analyzer.cc b/src/spicy/protocol-analyzer.cc index 274c29643a..6ab424a8b5 100644 --- a/src/spicy/protocol-analyzer.cc +++ b/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); } diff --git a/src/supervisor/Supervisor.cc b/src/supervisor/Supervisor.cc index f063a8c447..38e02c8073 100644 --- a/src/supervisor/Supervisor.cc +++ b/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;