From 574acdcacfbd5febf3c73c1c369dccd29f495c21 Mon Sep 17 00:00:00 2001 From: "Steve Chew (stechew)" Date: Wed, 11 Mar 2020 19:53:38 +0000 Subject: [PATCH] Merge pull request #1990 in SNORT/snort3 from ~OSHUMEIK/snort3:trace_all_builds to master Squashed commit of the following: commit d82d981dd4e40793bb741e8cfd8c2ac053b908cf Author: Oleksii Shumeiko Date: Tue Mar 3 12:05:28 2020 +0200 build: refactor trace logs Changes follow: * move on/off check before forming va_list * delete unused trace_debug trace_debugf * delete unused code FileMemPool::verify() flush_policy_names in src/stream/tcp/tcp_stream_session.cc --- src/detection/detect_trace.cc | 6 +- src/file_api/file_mempool.h | 3 - src/main/snort_debug.cc | 86 ++----------------- src/main/snort_debug.h | 76 ++++++++-------- src/service_inspectors/dce_rpc/CMakeLists.txt | 42 ++++----- src/stream/tcp/tcp_stream_session.cc | 9 -- src/stream/tcp/tcp_stream_session.h | 4 - 7 files changed, 68 insertions(+), 158 deletions(-) diff --git a/src/detection/detect_trace.cc b/src/detection/detect_trace.cc index 047b58de5..4387db604 100644 --- a/src/detection/detect_trace.cc +++ b/src/detection/detect_trace.cc @@ -100,7 +100,7 @@ void print_pattern(const PatternMatchData* pmd) void dump_buffer(const uint8_t* buff, unsigned len, Packet* p) { - if (!trace_enabled(detection_trace, trace_buffer)) + if (!trace_enabled(TRACE_NAME(detection), trace_buffer)) return; if (len == 0) @@ -129,10 +129,10 @@ void node_eval_trace(const detection_option_tree_node_t* node, const Cursor& cur name, pos); } - if (!trace_enabled(detection_trace, trace_buffer)) + if (!trace_enabled(TRACE_NAME(detection), trace_buffer)) return; - if (trace_enabled(detection_trace, TRACE_BUFFER_VERBOSE)) + if (trace_enabled(TRACE_NAME(detection), TRACE_BUFFER_VERBOSE)) { dump_buffer(cursor.buffer() + pos, cursor.length(), p); } diff --git a/src/file_api/file_mempool.h b/src/file_api/file_mempool.h index 13f940d47..39cbae14f 100644 --- a/src/file_api/file_mempool.h +++ b/src/file_api/file_mempool.h @@ -73,9 +73,6 @@ private: void free_pools(); int remove(CircularBuffer* cb, void* obj); -#ifdef DEBUG_MSGS - void verify(); -#endif void** datapool; /* memory buffer */ uint64_t total; diff --git a/src/main/snort_debug.cc b/src/main/snort_debug.cc index e7b415dde..72ec34559 100644 --- a/src/main/snort_debug.cc +++ b/src/main/snort_debug.cc @@ -35,39 +35,16 @@ using namespace snort; -bool trace_enabled(Trace mask, Trace flags) -{ return mask & flags; } - -bool trace_enabled(Trace mask) -{ return mask; } - template -static inline void trace_vprintf(const char* name, Trace mask, const char* file, int line, - Trace flags, const char* fmt, va_list ap) +static inline void trace_vprintf(const char* name, const char* fmt, va_list ap) { - if ( !trace_enabled(mask, flags) ) - return; - char buf[STD_BUF]; int buf_len = sizeof(buf); char* buf_ptr = buf; - int size; if (name) { - size = snprintf(buf, buf_len, "%s: ", name); - if ( size >= buf_len ) - size = buf_len - 1; - if ( size > 0 ) - { - buf_ptr += size; - buf_len -= size; - } - } - - if ( file ) - { - size = snprintf(buf_ptr, buf_len, "%s:%d: ", file, line); + int size = snprintf(buf, buf_len, "%s: ", name); if ( size >= buf_len ) size = buf_len - 1; if ( size > 0 ) @@ -85,10 +62,9 @@ static inline void trace_vprintf(const char* name, Trace mask, const char* file, output(buf, stdout); } -void trace_vprintf(const char* name, Trace mask, const char* file, int line, - Trace flags, const char* fmt, va_list ap) +void trace_vprintf(const char* name, const char* fmt, va_list ap) { - trace_vprintf(name, mask, file, line, flags, fmt, ap); + trace_vprintf(name, fmt, ap); } #ifdef UNIT_TEST @@ -125,31 +101,19 @@ TEST_CASE("macros", "[trace]") { { sx(trace_log(testing, "my message")), - "trace_print(\"testing\", testing_trace, nullptr, 0, \"my message\")" + "trace_print(\"testing\", testing_trace, \"my message\")" }, { sx(trace_log(testing, my_flags, "my message")), - "trace_print(\"testing\", testing_trace, nullptr, 0, my_flags, \"my message\")" + "trace_print(\"testing\", testing_trace, my_flags, \"my message\")" }, { sx(trace_logf(testing, "%s %s", "my", "message")), - "trace_printf(\"testing\", testing_trace, nullptr, 0, \"%s %s\", \"my\", \"message\")" + "trace_printf(\"testing\", testing_trace, \"%s %s\", \"my\", \"message\")" }, { sx(trace_logf(testing, my_flags, "%s %s", "my", "message")), - "trace_printf(\"testing\", testing_trace, nullptr, 0, my_flags, \"%s %s\", \"my\", \"message\")" - }, - { - sx(trace_debug(testing, "my message")), "trace_print(\"testing\", testing_trace, " sx(__FILE__) ", " sx(__LINE__) ", \"my message\")" - }, - { - sx(trace_debug(testing, my_flags, "my message")), "trace_print(\"testing\", testing_trace, " sx(__FILE__) ", " sx(__LINE__) ", my_flags, \"my message\")" - }, - { - sx(trace_debugf(testing, "%s %s", "my", "message")), "trace_printf(\"testing\", testing_trace, " sx(__FILE__) ", " sx(__LINE__) ", \"%s %s\", \"my\", \"message\")" - }, - { - sx(trace_debugf(testing, my_flags, "%s %s", "my", "message")), "trace_printf(\"testing\", testing_trace, " sx(__FILE__) ", " sx(__LINE__) ", my_flags, \"%s %s\", \"my\", \"message\")" + "trace_printf(\"testing\", testing_trace, my_flags, \"%s %s\", \"my\", \"message\")" } }; @@ -157,10 +121,6 @@ TEST_CASE("macros", "[trace]") CHECK( !strcmp(cases[1].expected, cases[1].test) ); CHECK( !strcmp(cases[2].expected, cases[2].test) ); CHECK( !strcmp(cases[3].expected, cases[3].test) ); - CHECK( !strcmp(cases[4].expected, cases[4].test) ); - CHECK( !strcmp(cases[5].expected, cases[5].test) ); - CHECK( !strcmp(cases[6].expected, cases[6].test) ); - CHECK( !strcmp(cases[7].expected, cases[7].test) ); } #undef trace_print @@ -206,36 +166,6 @@ TEST_CASE("trace_logf", "[trace]") CHECK( !strcmp(testing_dump, "testing: my other masked message") ); } -TEST_CASE("trace_debug", "[trace]") -{ - Trace TRACE_NAME(testing) = TRACE_SECTION_2 | TRACE_SECTION_3; - - testing_dump[0] = '\0'; - trace_debug(testing, "my message"); CHECK( !strcmp(testing_dump, "testing: " __FILE__ ":" sx(__LINE__) ": my message") ); - - testing_dump[0] = '\0'; - trace_debug(testing, TRACE_SECTION_1, "my masked message"); - CHECK( testing_dump[0] == '\0' ); - - testing_dump[0] = '\0'; - trace_debug(testing, TRACE_SECTION_2, "my other masked message"); CHECK( !strcmp(testing_dump, "testing: " __FILE__ ":" sx(__LINE__) ": my other masked message") ); -} - -TEST_CASE("trace_debugf", "[trace]") -{ - Trace TRACE_NAME(testing) = TRACE_SECTION_2 | TRACE_SECTION_3; - - testing_dump[0] = '\0'; - trace_debugf(testing, "%s %s", "my", "message"); CHECK( !strcmp(testing_dump, "testing: " __FILE__ ":" sx(__LINE__) ": my message") ); - - testing_dump[0] = '\0'; - trace_debugf(testing, TRACE_SECTION_1, "%s %s %s", "my", "masked", "message"); - CHECK( testing_dump[0] == '\0' ); - - testing_dump[0] = '\0'; - trace_debugf(testing, TRACE_SECTION_2, "%s %s %s %s", "my", "other", "masked", "message"); CHECK( !strcmp(testing_dump, "testing: " __FILE__ ":" sx(__LINE__) ": my other masked message") ); -} - TEST_CASE("safety", "[trace]") { Trace TRACE_NAME(testing) = TRACE_SECTION_2 | TRACE_SECTION_3; diff --git a/src/main/snort_debug.h b/src/main/snort_debug.h index 8e19045d2..ddae34c7c 100644 --- a/src/main/snort_debug.h +++ b/src/main/snort_debug.h @@ -31,92 +31,88 @@ typedef uint64_t Trace; -bool trace_enabled(Trace mask); -bool trace_enabled(Trace mask, Trace flags); - #define TRACE_NAME(name) name##_trace #ifdef DEBUG_MSGS -void trace_vprintf(const char* name, Trace mask, const char* file, int line, - Trace flags, const char* fmt, va_list); +void trace_vprintf(const char* name, const char* fmt, va_list); + +static inline bool trace_enabled(Trace mask, Trace flags) +{ return mask & flags; } + +static inline bool trace_enabled(Trace mask) +{ return mask; } -template -static inline void trace_printf(const char* name, Trace mask, const char* file, int line, - Trace flags, const char* fmt, ...) __attribute__((format (printf, 6, 7))); +template +static inline void trace_printf(const char* name, Trace mask, Trace flags, const char* fmt, ...) + __attribute__((format (printf, 4, 5))); -template -static inline void trace_printf(const char* name, Trace mask, const char* file, int line, - Trace flags, const char* fmt, ...) +template +static inline void trace_printf(const char* name, Trace mask, Trace flags, const char* fmt, ...) { + if (!trace_enabled(mask, flags)) + return; + va_list ap; va_start(ap, fmt); - trace_vprintf(name, mask, file, line, flags, fmt, ap); + trace_vprintf(name, fmt, ap); va_end(ap); } -template -static inline void trace_printf(const char* name, Trace mask, const char* file, - int line, const char* fmt, ...) __attribute__((format (printf, 5, 6))); +template +static inline void trace_printf(const char* name, Trace mask, const char* fmt, ...) + __attribute__((format (printf, 3, 4))); -template -static inline void trace_printf(const char* name, Trace mask, const char* file, - int line, const char* fmt, ...) +template +static inline void trace_printf(const char* name, Trace mask, const char* fmt, ...) { + if (!trace_enabled(mask)) + return; + va_list ap; va_start(ap, fmt); - trace_vprintf(name, mask, file, line, UINT64_MAX, fmt, ap); + trace_vprintf(name, fmt, ap); va_end(ap); } -template -static inline void trace_print(const char* name, Trace mask, const char* file, - int line, const char* msg) +template +static inline void trace_print(const char* name, Trace mask, const char* msg) { - trace_printf(name, mask, file, line, UINT64_MAX, "%s", msg); + trace_printf(name, mask, UINT64_MAX, "%s", msg); } -template -static inline void trace_print(const char* name, Trace mask, const char* file, - int line, Trace flags, const char* msg) +template +static inline void trace_print(const char* name, Trace mask, Trace flags, const char* msg) { - trace_printf(name, mask, file, line, flags, "%s", msg); + trace_printf(name, mask, flags, "%s", msg); } #define trace_print trace_print #define trace_printf trace_printf #define trace_log(tracer, ...) \ - trace_print(#tracer, tracer##_trace, nullptr, 0, __VA_ARGS__) + trace_print(#tracer, tracer##_trace, __VA_ARGS__) #define trace_log_wo_name(tracer, ...) \ - trace_print(nullptr, tracer##_trace, nullptr, 0, __VA_ARGS__) + trace_print(nullptr, tracer##_trace, __VA_ARGS__) #define trace_logf(tracer, ...) \ - trace_printf(#tracer, tracer##_trace, nullptr, 0, __VA_ARGS__) + trace_printf(#tracer, tracer##_trace, __VA_ARGS__) #define trace_logf_wo_name(tracer, ...) \ - trace_printf(nullptr, tracer##_trace, nullptr, 0, __VA_ARGS__) - -#define trace_debug(tracer, ...) \ - trace_print(#tracer, tracer##_trace, __FILE__, __LINE__, __VA_ARGS__) - -#define trace_debugf(tracer, ...) \ - trace_printf(#tracer, tracer##_trace, __FILE__, __LINE__, __VA_ARGS__) + trace_printf(nullptr, tracer##_trace, __VA_ARGS__) #else + #define trace_log(tracer, ...) #define trace_log_wo_name(tracer, ...) #define trace_logf(tracer, ...) #define trace_logf_wo_name(tracer, ...) -#define trace_debug(tracer, ...) -#define trace_debugf(tracer, ...) #endif #endif - diff --git a/src/service_inspectors/dce_rpc/CMakeLists.txt b/src/service_inspectors/dce_rpc/CMakeLists.txt index 416673663..aa055282e 100644 --- a/src/service_inspectors/dce_rpc/CMakeLists.txt +++ b/src/service_inspectors/dce_rpc/CMakeLists.txt @@ -1,32 +1,32 @@ set( FILE_LIST dce_co.cc - dce_co.h - dce_common.cc - dce_common.h - dce_context_data.cc + dce_co.h + dce_common.cc + dce_common.h + dce_context_data.cc dce_context_data.h dce_http_proxy.cc - dce_http_proxy_module.cc - dce_http_proxy_module.h - dce_http_proxy_splitter.cc + dce_http_proxy_module.cc + dce_http_proxy_module.h + dce_http_proxy_splitter.cc dce_http_proxy_splitter.h dce_http_server.cc - dce_http_server_module.cc - dce_http_server_module.h - dce_http_server_splitter.cc + dce_http_server_module.cc + dce_http_server_module.h + dce_http_server_splitter.cc dce_http_server_splitter.h dce_list.h dce_list.cc - dce_smb.cc + dce_smb.cc dce_smb.h dce_smb2.cc dce_smb2.h dce_smb_commands.cc - dce_smb_commands.h - dce_smb_module.cc - dce_smb_module.h - dce_smb_paf.cc + dce_smb_commands.h + dce_smb_module.cc + dce_smb_module.h + dce_smb_paf.cc dce_smb_paf.h dce_smb_transaction.cc dce_smb_transaction.h @@ -35,14 +35,14 @@ set( FILE_LIST dce_smb_utils.cc dce_smb_utils.h dce_tcp.cc - dce_tcp.h - dce_tcp_module.cc - dce_tcp_module.h - dce_tcp_paf.cc + dce_tcp.h + dce_tcp_module.cc + dce_tcp_module.h + dce_tcp_paf.cc dce_tcp_paf.h dce_udp.cc - dce_udp.h - dce_udp_module.cc + dce_udp.h + dce_udp_module.cc dce_udp_module.h dce_udp_processing.cc dce_utils.cc diff --git a/src/stream/tcp/tcp_stream_session.cc b/src/stream/tcp/tcp_stream_session.cc index c0a0f6675..eb36a9dc8 100644 --- a/src/stream/tcp/tcp_stream_session.cc +++ b/src/stream/tcp/tcp_stream_session.cc @@ -30,15 +30,6 @@ using namespace snort; -#ifdef DEBUG_MSGS -const char* const flush_policy_names[] = -{ - "ignore", - "on-ack", - "on-data" -}; -#endif - TcpStreamSession::TcpStreamSession(Flow* f) : Session(f), client(true), server(false) { } diff --git a/src/stream/tcp/tcp_stream_session.h b/src/stream/tcp/tcp_stream_session.h index c86300523..027fcfaaf 100644 --- a/src/stream/tcp/tcp_stream_session.h +++ b/src/stream/tcp/tcp_stream_session.h @@ -29,10 +29,6 @@ #include "tcp_stream_config.h" #include "tcp_stream_tracker.h" -#ifdef DEBUG_MSGS -extern const char* const flush_policy_names[]; -#endif - // FIXIT-L session tracking must be split from reassembly // into a separate module a la ip_session.cc and ip_defrag.cc // (of course defrag should also be cleaned up) -- 2.47.3