From ad99a6821e0a197ca7bb03483590b9323487108e Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Fri, 9 May 2025 11:57:58 -0700 Subject: [PATCH] Fix clang-tidy cppcoreguidelines-macro-usage findings (macros as constants) --- src/Desc.cc | 4 +- src/Val.cc | 1 + src/analyzer/protocol/login/Login.cc | 3 +- src/analyzer/protocol/login/NVT.cc | 75 ++++++++++++------------- src/analyzer/protocol/login/NVT.h | 25 ++++----- src/analyzer/protocol/rpc/Portmap.cc | 14 +++-- src/analyzer/protocol/rpc/RPC.cc | 2 +- src/file_analysis/analyzer/x509/OCSP.cc | 2 +- src/iosource/Manager.cc | 9 +-- src/net_util.cc | 10 ++-- src/packet_analysis/protocol/arp/ARP.cc | 4 ++ src/script_opt/FuncInfo.cc | 12 ++-- src/script_opt/ScriptOpt.cc | 12 +++- src/supervisor/Supervisor.cc | 1 + src/telemetry/Manager.cc | 1 + src/threading/formatters/JSON.cc | 1 + 16 files changed, 96 insertions(+), 80 deletions(-) diff --git a/src/Desc.cc b/src/Desc.cc index 3e001ce4e9..d11c9f4ab6 100644 --- a/src/Desc.cc +++ b/src/Desc.cc @@ -11,8 +11,8 @@ #include "zeek/IPAddr.h" #include "zeek/Reporter.h" -#define DEFAULT_SIZE 128 -#define SLOP 10 +constexpr unsigned int DEFAULT_SIZE = 128; +constexpr int SLOP = 10; namespace zeek { diff --git a/src/Val.cc b/src/Val.cc index d2543ed375..b15e502f60 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -6,6 +6,7 @@ #include #include +// NOLINTNEXTLINE(cppcoreguidelines-macro-usage) #define RAPIDJSON_HAS_STDSTRING 1 #include #include diff --git a/src/analyzer/protocol/login/Login.cc b/src/analyzer/protocol/login/Login.cc index 6fb360bb25..ee06843d5d 100644 --- a/src/analyzer/protocol/login/Login.cc +++ b/src/analyzer/protocol/login/Login.cc @@ -133,10 +133,11 @@ void Login_Analyzer::NewLine(bool orig, char* line) { } } +constexpr char VMS_REPEAT_SEQ[] = "\x1b[A"; + void Login_Analyzer::AuthenticationDialog(bool orig, char* line) { if ( orig ) { if ( is_VMS ) { -#define VMS_REPEAT_SEQ "\x1b[A" char* repeat_prev_line = strstr(line, VMS_REPEAT_SEQ); if ( repeat_prev_line ) { if ( repeat_prev_line[strlen(VMS_REPEAT_SEQ)] ) { diff --git a/src/analyzer/protocol/login/NVT.cc b/src/analyzer/protocol/login/NVT.cc index 1543bdd79d..bdb7336157 100644 --- a/src/analyzer/protocol/login/NVT.cc +++ b/src/analyzer/protocol/login/NVT.cc @@ -11,27 +11,22 @@ #define IS_3_BYTE_OPTION(c) ((c) >= 251 && (c) <= 254) -#define TELNET_OPT_SB 250 -#define TELNET_OPT_SE 240 +static constexpr uint8_t TELNET_OPT_SB = 250; +static constexpr uint8_t TELNET_OPT_SE = 240; -#define TELNET_OPT_IS 0 -#define TELNET_OPT_SEND 1 +static constexpr uint8_t TELNET_OPT_IS = 0; +static constexpr uint8_t TELNET_OPT_SEND = 1; -#define TELNET_OPT_WILL 251 -#define TELNET_OPT_WONT 252 -#define TELNET_OPT_DO 253 -#define TELNET_OPT_DONT 254 +static constexpr uint8_t TELNET_OPT_WILL = 251; +static constexpr uint8_t TELNET_OPT_WONT = 252; +static constexpr uint8_t TELNET_OPT_DO = 253; +static constexpr uint8_t TELNET_OPT_DONT = 254; -#define TELNET_IAC 255 +static constexpr uint8_t TELNET_IAC = 255; namespace zeek::analyzer::login { -TelnetOption::TelnetOption(NVT_Analyzer* arg_endp, unsigned int arg_code) { - endp = arg_endp; - code = arg_code; - flags = 0; - active = 0; -} +TelnetOption::TelnetOption(NVT_Analyzer* arg_endp, unsigned int arg_code) : endp(arg_endp), code(arg_code) {} void TelnetOption::RecvOption(unsigned int type) { TelnetOption* peer = endp->FindPeerOption(code); @@ -114,15 +109,17 @@ void TelnetTerminalOption::RecvSubOption(u_char* data, int len) { endp->SetTerminal(data + 1, len - 1); } -#define ENCRYPT_SET_ALGORITHM 0 -#define ENCRYPT_SUPPORT_ALGORITHM 1 -#define ENCRYPT_REPLY 2 -#define ENCRYPT_STARTING_TO_ENCRYPT 3 -#define ENCRYPT_NO_LONGER_ENCRYPTING 4 -#define ENCRYPT_REQUEST_START_TO_ENCRYPT 5 -#define ENCRYPT_REQUEST_NO_LONGER_ENCRYPT 6 -#define ENCRYPT_ENCRYPT_KEY 7 -#define ENCRYPT_DECRYPT_KEY 8 +enum EncryptOptions : uint8_t { + ENCRYPT_SET_ALGORITHM = 0, + ENCRYPT_SUPPORT_ALGORITHM = 1, + ENCRYPT_REPLY = 2, + ENCRYPT_STARTING_TO_ENCRYPT = 3, + ENCRYPT_NO_LONGER_ENCRYPTING = 4, + ENCRYPT_REQUEST_START_TO_ENCRYPT = 5, + ENCRYPT_REQUEST_NO_LONGER_ENCRYPT = 6, + ENCRYPT_ENCRYPT_KEY = 7, + ENCRYPT_DECRYPT_KEY = 8, +}; void TelnetEncryptOption::RecvSubOption(u_char* data, int len) { if ( ! active ) { @@ -157,13 +154,15 @@ void TelnetEncryptOption::RecvSubOption(u_char* data, int len) { } } -#define HERE_IS_AUTHENTICATION 0 -#define SEND_ME_AUTHENTICATION 1 -#define AUTHENTICATION_STATUS 2 -#define AUTHENTICATION_NAME 3 +enum AuthOptions : uint8_t { + HERE_IS_AUTHENTICATION = 0, + SEND_ME_AUTHENTICATION = 1, + AUTHENTICATION_STATUS = 2, + AUTHENTICATION_NAME = 3, +}; -#define AUTH_REJECT 1 -#define AUTH_ACCEPT 2 +constexpr int AUTH_REJECT = 1; +constexpr int AUTH_ACCEPT = 2; void TelnetAuthenticateOption::RecvSubOption(u_char* data, int len) { if ( len <= 0 ) { @@ -212,14 +211,14 @@ void TelnetAuthenticateOption::RecvSubOption(u_char* data, int len) { } } -#define ENVIRON_IS 0 -#define ENVIRON_SEND 1 -#define ENVIRON_INFO 2 +constexpr uint8_t ENVIRON_IS = 0; +constexpr uint8_t ENVIRON_SEND = 1; +constexpr uint8_t ENVIRON_INFO = 2; -#define ENVIRON_VAR 0 -#define ENVIRON_VAL 1 -#define ENVIRON_ESC 2 -#define ENVIRON_USERVAR 3 +constexpr uint8_t ENVIRON_VAR = 0; +constexpr uint8_t ENVIRON_VAL = 1; +constexpr uint8_t ENVIRON_ESC = 2; +constexpr uint8_t ENVIRON_USERVAR = 3; void TelnetEnvironmentOption::RecvSubOption(u_char* data, int len) { if ( len <= 0 ) { @@ -386,7 +385,7 @@ void NVT_Analyzer::SetEncrypting(int mode) { Event(activating_encryption); } -#define MAX_DELIVER_UNIT 128 +constexpr int MAX_DELIVER_UNIT = 128; void NVT_Analyzer::DoDeliver(int len, const u_char* data) { while ( len > 0 ) { diff --git a/src/analyzer/protocol/login/NVT.h b/src/analyzer/protocol/login/NVT.h index 4ef15d4351..3f8a99af88 100644 --- a/src/analyzer/protocol/login/NVT.h +++ b/src/analyzer/protocol/login/NVT.h @@ -4,12 +4,12 @@ #include "zeek/analyzer/protocol/tcp/ContentLine.h" -#define TELNET_OPTION_BINARY 0 -#define TELNET_OPTION_TERMINAL 24 -#define TELNET_OPTION_AUTHENTICATE 37 -#define TELNET_OPTION_ENCRYPT 38 -#define TELNET_OPTION_ENVIRON 39 -#define NUM_TELNET_OPTIONS 5 +constexpr uint8_t TELNET_OPTION_BINARY = 0; +constexpr uint8_t TELNET_OPTION_TERMINAL = 24; +constexpr uint8_t TELNET_OPTION_AUTHENTICATE = 37; +constexpr uint8_t TELNET_OPTION_ENCRYPT = 38; +constexpr uint8_t TELNET_OPTION_ENVIRON = 39; +constexpr uint8_t NUM_TELNET_OPTIONS = 5; namespace zeek::analyzer::login { @@ -20,11 +20,8 @@ public: TelnetOption(NVT_Analyzer* endp, unsigned int code); virtual ~TelnetOption() {} -// Whether we told the other side WILL/WONT/DO/DONT. -#define OPT_SAID_WILL 0x1 -#define OPT_SAID_WONT 0x2 -#define OPT_SAID_DO 0x4 -#define OPT_SAID_DONT 0x8 + // Whether we told the other side WILL/WONT/DO/DONT. + enum SaidOptions : uint8_t { OPT_SAID_WILL = 0x1, OPT_SAID_WONT = 0x2, OPT_SAID_DO = 0x4, OPT_SAID_DONT = 0x8 }; unsigned int Code() const { return code; } @@ -52,10 +49,10 @@ protected: virtual void InconsistentOption(unsigned int type); virtual void BadOption(); - NVT_Analyzer* endp; + NVT_Analyzer* endp = nullptr; unsigned int code; - int flags; - int active; + int flags = 0; + bool active = false; }; namespace detail { diff --git a/src/analyzer/protocol/rpc/Portmap.cc b/src/analyzer/protocol/rpc/Portmap.cc index d3ccf6fa5f..4676699941 100644 --- a/src/analyzer/protocol/rpc/Portmap.cc +++ b/src/analyzer/protocol/rpc/Portmap.cc @@ -5,12 +5,14 @@ #include "zeek/analyzer/protocol/rpc/XDR.h" #include "zeek/analyzer/protocol/rpc/events.bif.h" -#define PMAPPROC_NULL 0 -#define PMAPPROC_SET 1 -#define PMAPPROC_UNSET 2 -#define PMAPPROC_GETPORT 3 -#define PMAPPROC_DUMP 4 -#define PMAPPROC_CALLIT 5 +enum PortmapperProcs : uint8_t { + PMAPPROC_NULL = 0, + PMAPPROC_SET = 1, + PMAPPROC_UNSET = 2, + PMAPPROC_GETPORT = 3, + PMAPPROC_DUMP = 4, + PMAPPROC_CALLIT = 5, +}; namespace zeek::analyzer::rpc { namespace detail { diff --git a/src/analyzer/protocol/rpc/RPC.cc b/src/analyzer/protocol/rpc/RPC.cc index 8ca5e37271..f7a1eb67a1 100644 --- a/src/analyzer/protocol/rpc/RPC.cc +++ b/src/analyzer/protocol/rpc/RPC.cc @@ -19,7 +19,7 @@ const bool DEBUG_rpc_resync = false; // TODO: Should we add start_time and last_time to the rpc_* events?? // TODO: make this configurable -#define MAX_RPC_LEN 65536 +constexpr uint32_t MAX_RPC_LEN = 65536; namespace zeek::analyzer::rpc { namespace detail { diff --git a/src/file_analysis/analyzer/x509/OCSP.cc b/src/file_analysis/analyzer/x509/OCSP.cc index 9728732243..8fead1edf4 100644 --- a/src/file_analysis/analyzer/x509/OCSP.cc +++ b/src/file_analysis/analyzer/x509/OCSP.cc @@ -23,7 +23,7 @@ X509* helper_sk_X509_value(const STACK_OF(X509) * certs, int i) { return sk_X509 namespace zeek::file_analysis::detail { -#define OCSP_STRING_BUF_SIZE 2048 +static constexpr size_t OCSP_STRING_BUF_SIZE = 2048; static bool OCSP_RESPID_bio(OCSP_BASICRESP* basic_resp, BIO* bio) { #if ( OPENSSL_VERSION_NUMBER < 0x10100000L ) || defined(LIBRESSL_VERSION_NUMBER) diff --git a/src/iosource/Manager.cc b/src/iosource/Manager.cc index 12e072fb27..8e0f4d106d 100644 --- a/src/iosource/Manager.cc +++ b/src/iosource/Manager.cc @@ -19,8 +19,6 @@ #include "zeek/iosource/PktSrc.h" #include "zeek/plugin/Manager.h" -#define DEFAULT_PREFIX "pcap" - extern int signal_val; namespace zeek::iosource { @@ -368,6 +366,10 @@ void Manager::Register(PktSrc* src) { poll_interval = 1; } +/** + * Checks if the path comes with a prefix telling us which type of PktSrc to use. If no + * prefix exists, return "pcap" as a default. + */ static std::pair split_prefix(std::string path) { // See if the path comes with a prefix telling us which type of // PktSrc to use. If not, choose default. @@ -378,9 +380,8 @@ static std::pair split_prefix(std::string path) { prefix = path.substr(0, i); path = path.substr(i + 2, std::string::npos); } - else - prefix = DEFAULT_PREFIX; + prefix = "pcap"; return std::make_pair(prefix, path); } diff --git a/src/net_util.cc b/src/net_util.cc index 34a33bccf7..eceda9befd 100644 --- a/src/net_util.cc +++ b/src/net_util.cc @@ -123,11 +123,11 @@ int icmp6_checksum(const struct icmp* icmpp, const IP_Hdr* ip, int len) { len); } -#define CLASS_A 0x00000000 -#define CLASS_B 0x80000000 -#define CLASS_C 0xc0000000 -#define CLASS_D 0xe0000000 -#define CLASS_E 0xf0000000 +constexpr uint32_t CLASS_A = 0x00000000; +constexpr uint32_t CLASS_B = 0x80000000; +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) { diff --git a/src/packet_analysis/protocol/arp/ARP.cc b/src/packet_analysis/protocol/arp/ARP.cc index d92d003c37..f334e26418 100644 --- a/src/packet_analysis/protocol/arp/ARP.cc +++ b/src/packet_analysis/protocol/arp/ARP.cc @@ -25,6 +25,8 @@ ARPAnalyzer::ARPAnalyzer() : zeek::packet_analysis::Analyzer("ARP") {} // ... and on Solaris we are missing half of the ARPOP codes, so define // them here as necessary: +// NOLINTBEGIN(cppcoreguidelines-macro-usage) + #ifndef ARPOP_REQUEST #define ARPOP_REQUEST 1 // ARP request. #endif @@ -84,6 +86,8 @@ ARPAnalyzer::ARPAnalyzer() : zeek::packet_analysis::Analyzer("ARP") {} #define ARPHRD_IEEE802 6 #endif +// NOLINTEND(cppcoreguidelines-macro-usage) + bool ARPAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) { packet->l3_proto = L3_ARP; diff --git a/src/script_opt/FuncInfo.cc b/src/script_opt/FuncInfo.cc index a06ef289ea..f99d060557 100644 --- a/src/script_opt/FuncInfo.cc +++ b/src/script_opt/FuncInfo.cc @@ -20,29 +20,29 @@ namespace zeek::detail { // to the event engine. // Does not change script-level state (though may change internal state). -#define ATTR_NO_SCRIPT_SIDE_EFFECTS 0x1 +constexpr unsigned int ATTR_NO_SCRIPT_SIDE_EFFECTS = 0x1; // Does not change any Zeek state, internal or external. (May change // state outside of Zeek, such as file system elements.) Implies // ATTR_NO_SCRIPT_SIDE_EFFECTS. -#define ATTR_NO_ZEEK_SIDE_EFFECTS 0x2 +constexpr unsigned int ATTR_NO_ZEEK_SIDE_EFFECTS = 0x2; // Calls made with the same arguments yield the same results, if made // after full Zeek initialization. Implies ATTR_NO_ZEEK_SIDE_EFFECTS. -#define ATTR_IDEMPOTENT 0x4 +constexpr unsigned int ATTR_IDEMPOTENT = 0x4; // Calls with constant arguments can always be folded, even prior to // full Zeek initialization. Such functions must not have the potential // to generate errors. Implies ATTR_IDEMPOTENT. -#define ATTR_FOLDABLE 0x8 +constexpr unsigned int ATTR_FOLDABLE = 0x8; // The event engine knows about this script function and may call it // during its processing. -#define ATTR_SPECIAL_SCRIPT_FUNC 0x10 +constexpr unsigned int ATTR_SPECIAL_SCRIPT_FUNC = 0x10; // ZAM knows about this script function and will replace it with specialized // instructions. -#define ATTR_ZAM_REPLACEABLE_SCRIPT_FUNC 0x20 +constexpr unsigned int ATTR_ZAM_REPLACEABLE_SCRIPT_FUNC = 0x20; static std::unordered_map func_attrs = { // Script functions. diff --git a/src/script_opt/ScriptOpt.cc b/src/script_opt/ScriptOpt.cc index 2a3148bb5a..4fe4a1e291 100644 --- a/src/script_opt/ScriptOpt.cc +++ b/src/script_opt/ScriptOpt.cc @@ -718,8 +718,12 @@ bool has_AST_node_unknown_to_script_opt(const ProfileFunc* prof, bool /* is_ZAM STMT_ASSERT, // STMT_EXTERN, // STMT_STD_FUNCTION, -#define SCRIPT_OPT_NUM_STMTS 24 }; + + // This should be the total number of entries in the set above, including + // the commented values. + constexpr int SCRIPT_OPT_NUM_STMTS = 24; + // clang-format on // Fail compilation if NUM_STMT in StmtEnums.h changes. @@ -803,8 +807,12 @@ bool has_AST_node_unknown_to_script_opt(const ProfileFunc* prof, bool /* is_ZAM // EXPR_ANY_INDEX, // EXPR_SCRIPT_OPT_BUILTIN, // EXPR_NOP, -#define SCRIPT_OPT_NUM_EXPRS 70 }; + + // This should be the total number of entries in the set above, including + // the commented values. + constexpr int SCRIPT_OPT_NUM_EXPRS = 70; + // clang-format on // Fail compilation if NUM_EXPRS in Expr.h changes. diff --git a/src/supervisor/Supervisor.cc b/src/supervisor/Supervisor.cc index f9c8ccc23b..f063a8c447 100644 --- a/src/supervisor/Supervisor.cc +++ b/src/supervisor/Supervisor.cc @@ -16,6 +16,7 @@ #include #include +// NOLINTNEXTLINE(cppcoreguidelines-macro-usage) #define RAPIDJSON_HAS_STDSTRING 1 #include #include diff --git a/src/telemetry/Manager.cc b/src/telemetry/Manager.cc index 0772879581..51424fe7a3 100644 --- a/src/telemetry/Manager.cc +++ b/src/telemetry/Manager.cc @@ -2,6 +2,7 @@ #include "zeek/telemetry/Manager.h" +// NOLINTNEXTLINE(cppcoreguidelines-macro-usage) #define RAPIDJSON_HAS_STDSTRING 1 // CivetServer is from the civetweb submodule in prometheus-cpp diff --git a/src/threading/formatters/JSON.cc b/src/threading/formatters/JSON.cc index 13ed2eb2a4..38b8dcc4c6 100644 --- a/src/threading/formatters/JSON.cc +++ b/src/threading/formatters/JSON.cc @@ -6,6 +6,7 @@ #define __STDC_LIMIT_MACROS #endif +// NOLINTNEXTLINE(cppcoreguidelines-macro-usage) #define RAPIDJSON_HAS_STDSTRING 1 #include