From e0d10c3cb83eaeb2915209aa7efd3dc65a907123 Mon Sep 17 00:00:00 2001 From: Eric Leblond Date: Mon, 23 Dec 2024 22:15:50 +0100 Subject: [PATCH 01/16] devel: add .clang-tidy file By adding this file, code analysis by clang-tidy is now available in LSP compatible editor using clangd. The CustomFunctions option that adds Suricata banned functions in the error list is only available with the (at the time of writing) future clang 20. Ticket: 3837 --- .clang-tidy | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 .clang-tidy diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 000000000000..eb604220676f --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,30 @@ +--- +Checks: " + bugprone-*, + cert-*, + clang-analyzer-*, + concurrency-*, + misc-*, + modernize-*, + performance-*, + portability-*, + readability-*, + -readability-identifier-length, + -readability-braces-around-statements, + -readability-avoid-const-params-in-decls, + -bugprone-easily-swappable-parameters, +" +WarningsAsErrors: " + bugprone-unsafe-functions, +" +HeaderFileExtensions: + - '' + - 'h' +ImplementationFileExtensions: + - 'c' +FormatStyle: none +CheckOptions: + bugprone-unsafe-functions.ReportDefaultFunctions: 'true' + bugprone-unsafe-functions.ReportMoreUnsafeFunctions: 'true' + bugprone-unsafe-functions.CustomFunctions: '^strtok$,strtok_r;^sprintf$,snprintf;^strcat$,strlcat;^strcpy$,strlcpy;^strncpy$,strlcat;^strncat$,strlcpy;^strndup$,,is OS specific;^strchrnul$;^rand$;^rand_r$;^index$;^rindex$;^bzero$,^memset$' + readability-function-cognitive-complexity.Threshold: 50 From 0029166bebb6825ca736cd32720772d1d9d50a9d Mon Sep 17 00:00:00 2001 From: Eric Leblond Date: Fri, 27 Dec 2024 09:01:43 +0100 Subject: [PATCH 02/16] util/debug: exit is not thread safe The exit() function is not thread safe and triggers a warning from clang tidy for all FatalError() and FatalErrorAtInit() calls. This patch uses quick_exit instead to only flush the critical IO and not call the static destructors (which are the non thread safe part). --- configure.ac | 5 +++++ src/util-debug.h | 8 ++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index ca964d9039a0..0623de52446c 100644 --- a/configure.ac +++ b/configure.ac @@ -224,6 +224,11 @@ [], [ #include ]) + AC_CHECK_DECL([quick_exit], + AC_DEFINE([HAVE_QUICK_EXIT], [1], [Use quick_exit]), + [], [ + #include + ]) AC_CHECK_HEADERS([malloc.h]) AC_CHECK_DECL([malloc_trim], diff --git a/src/util-debug.h b/src/util-debug.h index cb22e9097389..4fba832ea652 100644 --- a/src/util-debug.h +++ b/src/util-debug.h @@ -499,10 +499,14 @@ void SCLogErr(int x, const char *file, const char *func, const int line, const c #endif /* DEBUG */ +#if !HAVE_QUICK_EXIT +#define quick_exit exit +#endif + #define FatalError(...) \ do { \ SCLogError(__VA_ARGS__); \ - exit(EXIT_FAILURE); \ + quick_exit(EXIT_FAILURE); \ } while (0) /** \brief Fatal error IF we're starting up, and configured to consider @@ -515,7 +519,7 @@ void SCLogErr(int x, const char *file, const char *func, const int line, const c (void)ConfGetBool("engine.init-failure-fatal", &init_errors_fatal); \ if (init_errors_fatal && (SC_ATOMIC_GET(engine_stage) == SURICATA_INIT)) { \ SCLogError(__VA_ARGS__); \ - exit(EXIT_FAILURE); \ + quick_exit(EXIT_FAILURE); \ } \ SCLogWarning(__VA_ARGS__); \ } while (0) From ca67296a82e0d851fa00120d0aa9205842ee4a6c Mon Sep 17 00:00:00 2001 From: Eric Leblond Date: Sat, 11 Jan 2025 17:51:21 +0100 Subject: [PATCH 03/16] gitignore: add compile database This is generated at compile time so let's ignore. --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 66416e27d14e..f7e183ff1989 100644 --- a/.gitignore +++ b/.gitignore @@ -69,3 +69,4 @@ test.sh /libsuricata-config !/libsuricata-config.in !/.readthedocs.yaml +compile_commands.json From e41d2999f155bc133563d797857000ab7fa8ab2f Mon Sep 17 00:00:00 2001 From: Eric Leblond Date: Sat, 11 Jan 2025 20:05:39 +0100 Subject: [PATCH 04/16] devel: silent a clang-tidy warning Direct call to clang-tidy have more checks than through clangd and directly including headers is not in Suricata code style so it triggers a lot of unwanted errors. --- .clang-tidy | 1 + 1 file changed, 1 insertion(+) diff --git a/.clang-tidy b/.clang-tidy index eb604220676f..1e4a967ffd56 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -13,6 +13,7 @@ Checks: " -readability-braces-around-statements, -readability-avoid-const-params-in-decls, -bugprone-easily-swappable-parameters, + -misc-include-cleaner, " WarningsAsErrors: " bugprone-unsafe-functions, From 49e80bccb13673aa5e964af213bc159b7a625257 Mon Sep 17 00:00:00 2001 From: Eric Leblond Date: Sat, 11 Jan 2025 20:16:02 +0100 Subject: [PATCH 05/16] af-packet: code cleaning clang-tidy did detect the -1 return value was not compatible with TmEcode enum. --- src/source-af-packet.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/source-af-packet.c b/src/source-af-packet.c index e3c83da49de4..10213c573879 100644 --- a/src/source-af-packet.c +++ b/src/source-af-packet.c @@ -1849,21 +1849,21 @@ static int SockFanoutSeteBPF(AFPThreadVars *ptv) return 0; } -static int SetEbpfFilter(AFPThreadVars *ptv) +static TmEcode SetEbpfFilter(AFPThreadVars *ptv) { int pfd = ptv->ebpf_filter_fd; if (pfd == -1) { SCLogError("Filter file descriptor is invalid"); - return -1; + return TM_ECODE_FAILED; } if (setsockopt(ptv->socket, SOL_SOCKET, SO_ATTACH_BPF, &pfd, sizeof(pfd))) { SCLogError("Error setting ebpf: %s", strerror(errno)); - return -1; + return TM_ECODE_FAILED; } SCLogInfo("Activated eBPF filter on socket"); - return 0; + return TM_ECODE_OK; } #endif From d713906c514275a913f99a731b67e1969e64c329 Mon Sep 17 00:00:00 2001 From: Eric Leblond Date: Sat, 11 Jan 2025 20:28:44 +0100 Subject: [PATCH 06/16] util/memcp: silent clang-tidy error It is complaining about circular import that are handled at build time. We have one single warning of the sort so let's silent it. --- src/util-memcmp.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util-memcmp.h b/src/util-memcmp.h index f9fc75357c17..bc126b912aba 100644 --- a/src/util-memcmp.h +++ b/src/util-memcmp.h @@ -29,6 +29,7 @@ #ifndef SURICATA_UTIL_MEMCMP_H #define SURICATA_UTIL_MEMCMP_H +// NOLINTNEXTLINE(misc-header-include-cycle) #include "suricata-common.h" #include "util-optimize.h" From 86f85aca0898532f2eb979b96f5de9bc8c4bb9a5 Mon Sep 17 00:00:00 2001 From: Eric Leblond Date: Sat, 11 Jan 2025 20:30:04 +0100 Subject: [PATCH 07/16] util/mem: add missing variables to declaration This is reported by clang-tidy as non standard. --- src/util-memcmp.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util-memcmp.h b/src/util-memcmp.h index bc126b912aba..578dd752d4bd 100644 --- a/src/util-memcmp.h +++ b/src/util-memcmp.h @@ -36,7 +36,7 @@ /** \brief compare two patterns, converting the 2nd to lowercase * \warning *ONLY* the 2nd pattern is converted to lowercase */ -static inline int SCMemcmpLowercase(const void *, const void *, size_t); +static inline int SCMemcmpLowercase(const void *s1, const void *s2, size_t n); void MemcmpRegisterTests(void); From 3c03346f25f3006688b7a96d9a9cb894724b99dc Mon Sep 17 00:00:00 2001 From: Eric Leblond Date: Sat, 11 Jan 2025 20:44:09 +0100 Subject: [PATCH 08/16] util/ebpf: fix memory leak in error handling bpf_map_data was allocated in the function and not freed. Found by clang-tidy. --- src/util-ebpf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util-ebpf.c b/src/util-ebpf.c index ea0811b89a15..8fe378591a7a 100644 --- a/src/util-ebpf.c +++ b/src/util-ebpf.c @@ -280,6 +280,7 @@ static int EBPFLoadPinnedMaps(LiveDevice *livedev, struct ebpf_timeout_config *c } bpf_map_data->last = 0; SCLogError("Can't allocate bpf map name"); + SCFree(bpf_map_data); return -1; } From f41076e3195c977ed74ed060406a9ece842f22e8 Mon Sep 17 00:00:00 2001 From: Eric Leblond Date: Sat, 11 Jan 2025 20:47:38 +0100 Subject: [PATCH 09/16] util/lua-sandbox: fix duplicate include Spotted by clang-tidy. --- src/util-lua-sandbox.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/util-lua-sandbox.c b/src/util-lua-sandbox.c index c3596f97c543..4bdc2f034a51 100644 --- a/src/util-lua-sandbox.c +++ b/src/util-lua-sandbox.c @@ -26,7 +26,6 @@ #include "lua.h" #include "lauxlib.h" #include "lualib.h" -#include "util-debug.h" #include "util-debug.h" #include "util-validate.h" From c4d79b79fdfc6c69ed1d9691ab3c9ffddb12ac46 Mon Sep 17 00:00:00 2001 From: Eric Leblond Date: Sat, 11 Jan 2025 21:00:00 +0100 Subject: [PATCH 10/16] detect/http2: fix readability named parameter Patched via `clang-tidy --quiet src/detect-http2.c --fix` --- src/detect-http2.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/detect-http2.c b/src/detect-http2.c index 4d954a5ac96a..b06444020289 100644 --- a/src/detect-http2.c +++ b/src/detect-http2.c @@ -53,38 +53,44 @@ void DetectHTTP2sizeUpdateRegisterTests (void); static int DetectHTTP2frametypeMatch(DetectEngineThreadCtx *det_ctx, Flow *f, uint8_t flags, void *state, void *txv, const Signature *s, const SigMatchCtx *ctx); -static int DetectHTTP2frametypeSetup (DetectEngineCtx *, Signature *, const char *); -void DetectHTTP2frametypeFree (DetectEngineCtx *, void *); +static int DetectHTTP2frametypeSetup( + DetectEngineCtx * /*de_ctx*/, Signature * /*s*/, const char * /*str*/); +void DetectHTTP2frametypeFree(DetectEngineCtx * /*de_ctx*/, void * /*ptr*/); static int DetectHTTP2errorcodeMatch(DetectEngineThreadCtx *det_ctx, Flow *f, uint8_t flags, void *state, void *txv, const Signature *s, const SigMatchCtx *ctx); -static int DetectHTTP2errorcodeSetup (DetectEngineCtx *, Signature *, const char *); -void DetectHTTP2errorcodeFree (DetectEngineCtx *, void *); +static int DetectHTTP2errorcodeSetup( + DetectEngineCtx * /*de_ctx*/, Signature * /*s*/, const char * /*str*/); +void DetectHTTP2errorcodeFree(DetectEngineCtx * /*de_ctx*/, void * /*ptr*/); static int DetectHTTP2priorityMatch(DetectEngineThreadCtx *det_ctx, Flow *f, uint8_t flags, void *state, void *txv, const Signature *s, const SigMatchCtx *ctx); -static int DetectHTTP2prioritySetup (DetectEngineCtx *, Signature *, const char *); -void DetectHTTP2priorityFree (DetectEngineCtx *, void *); +static int DetectHTTP2prioritySetup( + DetectEngineCtx * /*de_ctx*/, Signature * /*s*/, const char * /*str*/); +void DetectHTTP2priorityFree(DetectEngineCtx * /*de_ctx*/, void * /*ptr*/); static int DetectHTTP2windowMatch(DetectEngineThreadCtx *det_ctx, Flow *f, uint8_t flags, void *state, void *txv, const Signature *s, const SigMatchCtx *ctx); -static int DetectHTTP2windowSetup (DetectEngineCtx *, Signature *, const char *); -void DetectHTTP2windowFree (DetectEngineCtx *, void *); +static int DetectHTTP2windowSetup( + DetectEngineCtx * /*de_ctx*/, Signature * /*s*/, const char * /*str*/); +void DetectHTTP2windowFree(DetectEngineCtx * /*de_ctx*/, void * /*ptr*/); static int DetectHTTP2sizeUpdateMatch(DetectEngineThreadCtx *det_ctx, Flow *f, uint8_t flags, void *state, void *txv, const Signature *s, const SigMatchCtx *ctx); -static int DetectHTTP2sizeUpdateSetup (DetectEngineCtx *, Signature *, const char *); -void DetectHTTP2sizeUpdateFree (DetectEngineCtx *, void *); +static int DetectHTTP2sizeUpdateSetup( + DetectEngineCtx * /*de_ctx*/, Signature * /*s*/, const char * /*str*/); +void DetectHTTP2sizeUpdateFree(DetectEngineCtx * /*de_ctx*/, void * /*ptr*/); static int DetectHTTP2settingsMatch(DetectEngineThreadCtx *det_ctx, Flow *f, uint8_t flags, void *state, void *txv, const Signature *s, const SigMatchCtx *ctx); -static int DetectHTTP2settingsSetup (DetectEngineCtx *, Signature *, const char *); -void DetectHTTP2settingsFree (DetectEngineCtx *, void *); +static int DetectHTTP2settingsSetup( + DetectEngineCtx * /*de_ctx*/, Signature * /*s*/, const char * /*str*/); +void DetectHTTP2settingsFree(DetectEngineCtx * /*de_ctx*/, void * /*ptr*/); static int DetectHTTP2headerNameSetup(DetectEngineCtx *de_ctx, Signature *s, const char *arg); From 707f6a54fc521b46a6a8a166df60dede9827aaba Mon Sep 17 00:00:00 2001 From: Eric Leblond Date: Sat, 11 Jan 2025 21:04:33 +0100 Subject: [PATCH 11/16] detect/http2: remove not directly used include --- src/detect-http2.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/detect-http2.c b/src/detect-http2.c index b06444020289..b632d047a1e0 100644 --- a/src/detect-http2.c +++ b/src/detect-http2.c @@ -26,19 +26,11 @@ #include "detect.h" #include "detect-parse.h" -#include "detect-content.h" - #include "detect-engine.h" #include "detect-engine-uint.h" -#include "detect-engine-mpm.h" -#include "detect-engine-prefilter.h" -#include "detect-engine-content-inspection.h" #include "detect-engine-helper.h" - #include "detect-http2.h" #include "util-byte.h" -#include "rust.h" -#include "util-profiling.h" #ifdef UNITTESTS void DetectHTTP2frameTypeRegisterTests (void); From d6e9eb34998fe949d7c985b83ba96a788c0b3b75 Mon Sep 17 00:00:00 2001 From: Eric Leblond Date: Sat, 11 Jan 2025 21:38:05 +0100 Subject: [PATCH 12/16] detect/http2: silent clang-tidy warning Including a C file is bug prone. --- src/detect-http2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/detect-http2.c b/src/detect-http2.c index b632d047a1e0..6986044d684f 100644 --- a/src/detect-http2.c +++ b/src/detect-http2.c @@ -595,5 +595,5 @@ static int DetectHTTP2headerNameSetup(DetectEngineCtx *de_ctx, Signature *s, con } #ifdef UNITTESTS -#include "tests/detect-http2.c" +#include "tests/detect-http2.c" // NOLINT(bugprone-suspicious-include) #endif From ae037dd63cc1580863fb39edddd9e1c7be4a430e Mon Sep 17 00:00:00 2001 From: Eric Leblond Date: Sat, 11 Jan 2025 21:45:38 +0100 Subject: [PATCH 13/16] clang-tidy: make sure clang format is applied Default format style is not using clang-format so switching to 'file' to use clang-format. --- .clang-tidy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.clang-tidy b/.clang-tidy index 1e4a967ffd56..193454805d7d 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -23,7 +23,7 @@ HeaderFileExtensions: - 'h' ImplementationFileExtensions: - 'c' -FormatStyle: none +FormatStyle: 'file' CheckOptions: bugprone-unsafe-functions.ReportDefaultFunctions: 'true' bugprone-unsafe-functions.ReportMoreUnsafeFunctions: 'true' From 124b610ea7113ee123292ae3520f29280faf2400 Mon Sep 17 00:00:00 2001 From: Eric Leblond Date: Sat, 11 Jan 2025 21:50:48 +0100 Subject: [PATCH 14/16] reputation: apply clangd-tidy proposal Patch obtained via `clang-tidy --quiet src/reputation.c --fix`. On modification has not been applied: reputation.c:326:9: warning: macro 'SREP_SHORTNAME_LEN' defines an integral constant; prefer an enum instead [modernize-macro-to-enum] 326 | #define SREP_SHORTNAME_LEN 32 The change is not really making sense for a single value. --- src/reputation.c | 82 +++++++++++++++++++++++------------------------- 1 file changed, 40 insertions(+), 42 deletions(-) diff --git a/src/reputation.c b/src/reputation.c index 417b60a52480..9a25fef01381 100644 --- a/src/reputation.c +++ b/src/reputation.c @@ -297,7 +297,8 @@ static int SRepSplitLine(SRepCIDRTree *cidr_ctx, char *line, Address *ip, uint8_ if (strcmp(ptrs[0], "ip") == 0) return 1; - uint8_t c, v; + uint8_t c; + uint8_t v; if (StringParseU8RangeCheck(&c, 10, 0, (const char *)ptrs[1], 0, SREP_MAX_CATS - 1) < 0) return -1; @@ -307,19 +308,18 @@ static int SRepSplitLine(SRepCIDRTree *cidr_ctx, char *line, Address *ip, uint8_ if (strchr(ptrs[0], '/') != NULL) { SRepCIDRAddNetblock(cidr_ctx, ptrs[0], c, v); return 1; + } + if (inet_pton(AF_INET, ptrs[0], &ip->address) == 1) { + ip->family = AF_INET; + } else if (inet_pton(AF_INET6, ptrs[0], &ip->address) == 1) { + ip->family = AF_INET6; } else { - if (inet_pton(AF_INET, ptrs[0], &ip->address) == 1) { - ip->family = AF_INET; - } else if (inet_pton(AF_INET6, ptrs[0], &ip->address) == 1) { - ip->family = AF_INET6; - } else { - return -1; - } - - *cat = c; - *value = v; + return -1; } + *cat = c; + *value = v; + return 0; } @@ -448,7 +448,8 @@ int SRepLoadFileFromFD(SRepCIDRTree *cidr_ctx, FILE *fp) memset(&a, 0x00, sizeof(a)); a.family = AF_INET; - uint8_t cat = 0, value = 0; + uint8_t cat = 0; + uint8_t value = 0; int r = SRepSplitLine(cidr_ctx, line, &a, &cat, &value); if (r < 0) { SCLogError("bad line \"%s\"", line); @@ -467,45 +468,42 @@ int SRepLoadFileFromFD(SRepCIDRTree *cidr_ctx, FILE *fp) if (h == NULL) { SCLogError("failed to get a host, increase host.memcap"); break; - } else { - //SCLogInfo("host %p", h); + } // SCLogInfo("host %p", h); - if (h->iprep == NULL) { - h->iprep = SCCalloc(1, sizeof(SReputation)); - if (h->iprep != NULL) { - HostIncrUsecnt(h); - } - } + if (h->iprep == NULL) { + h->iprep = SCCalloc(1, sizeof(SReputation)); if (h->iprep != NULL) { - SReputation *rep = h->iprep; + HostIncrUsecnt(h); + } + } + if (h->iprep != NULL) { + SReputation *rep = h->iprep; - /* if version is outdated, it's an older entry that we'll - * now replace. */ - if (rep->version != SRepGetVersion()) { - memset(rep, 0x00, sizeof(SReputation)); - } + /* if version is outdated, it's an older entry that we'll + * now replace. */ + if (rep->version != SRepGetVersion()) { + memset(rep, 0x00, sizeof(SReputation)); + } - rep->version = SRepGetVersion(); - rep->rep[cat] = value; + rep->version = SRepGetVersion(); + rep->rep[cat] = value; - SCLogDebug("host %p iprep %p setting cat %u to value %u", - h, h->iprep, cat, value); + SCLogDebug("host %p iprep %p setting cat %u to value %u", h, h->iprep, cat, value); #ifdef DEBUG - if (SCLogDebugEnabled()) { - int i; - for (i = 0; i < SREP_MAX_CATS; i++) { - if (rep->rep[i] == 0) - continue; - - SCLogDebug("--> host %p iprep %p cat %d to value %u", - h, h->iprep, i, rep->rep[i]); - } + if (SCLogDebugEnabled()) { + int i; + for (i = 0; i < SREP_MAX_CATS; i++) { + if (rep->rep[i] == 0) + continue; + + SCLogDebug("--> host %p iprep %p cat %d to value %u", h, h->iprep, i, + rep->rep[i]); } -#endif } - - HostRelease(h); +#endif } + + HostRelease(h); } } From 75eb6fbf1649b2f7334830502f84a0effeaaffc4 Mon Sep 17 00:00:00 2001 From: Eric Leblond Date: Sat, 11 Jan 2025 22:17:55 +0100 Subject: [PATCH 15/16] app-layer/ftp: apply clang-tidy Patch generated by clang-tidy. Most hunks are `else after return` but there is also an unused parameter removed from FTPDataParse() function. --- src/app-layer-ftp.c | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/src/app-layer-ftp.c b/src/app-layer-ftp.c index a6a1f632fcb4..4f8486086453 100644 --- a/src/app-layer-ftp.c +++ b/src/app-layer-ftp.c @@ -111,7 +111,7 @@ uint32_t ftp_max_line_len = 4096; SC_ATOMIC_DECLARE(uint64_t, ftp_memuse); SC_ATOMIC_DECLARE(uint64_t, ftp_memcap); -static FTPTransaction *FTPGetOldestTx(const FtpState *, FTPTransaction *); +static FTPTransaction *FTPGetOldestTx(const FtpState * /*ftp_state*/, FTPTransaction * /*starttx*/); static void FTPParseMemcap(void) { @@ -375,7 +375,8 @@ static AppLayerResult FTPGetLineForDirection( SCReturnStruct(APP_LAYER_OK); } SCReturnStruct(APP_LAYER_INCOMPLETE(input->consumed, input->len + 1)); - } else if (*current_line_truncated) { + } + if (*current_line_truncated) { // Whatever came in with first LF should also get discarded *current_line_truncated = false; line->len = 0; @@ -507,7 +508,8 @@ static AppLayerResult FTPParseRequest(Flow *f, void *ftp_state, AppLayerParserSt if (input == NULL && AppLayerParserStateIssetFlag(pstate, APP_LAYER_PARSER_EOF_TS)) { SCReturnStruct(APP_LAYER_OK); - } else if (input == NULL || input_len == 0) { + } + if (input == NULL || input_len == 0) { SCReturnStruct(APP_LAYER_ERROR); } @@ -520,7 +522,8 @@ static AppLayerResult FTPParseRequest(Flow *f, void *ftp_state, AppLayerParserSt res = FTPGetLineForDirection(&line, &ftpi, &state->current_line_truncated_ts); if (res.status == 1) { return res; - } else if (res.status == -1) { + } + if (res.status == -1) { break; } const FtpCommand *cmd_descriptor; @@ -617,11 +620,9 @@ static AppLayerResult FTPParseRequest(Flow *f, void *ftp_state, AppLayerParserSt FtpTransferCmdFree(data); SCLogDebug("No expectation created."); SCReturnStruct(APP_LAYER_ERROR); - } else { - SCLogDebug("Expectation created [direction: %s, dynamic port %"PRIu16"].", - state->active ? "to server" : "to client", - state->dyn_port); } + SCLogDebug("Expectation created [direction: %s, dynamic port %" PRIu16 "].", + state->active ? "to server" : "to client", state->dyn_port); /* reset the dyn port to avoid duplicate */ state->dyn_port = 0; @@ -713,7 +714,8 @@ static AppLayerResult FTPParseResponse(Flow *f, void *ftp_state, AppLayerParserS res = FTPGetLineForDirection(&line, &ftpi, &state->current_line_truncated_tc); if (res.status == 1) { return res; - } else if (res.status == -1) { + } + if (res.status == -1) { break; } FTPTransaction *tx = FTPGetOldestTx(state, lasttx); @@ -929,7 +931,7 @@ static void FTPStateTransactionFree(void *state, uint64_t tx_id) TAILQ_FOREACH(tx, &ftp_state->tx_list, next) { if (tx_id < tx->tx_id) break; - else if (tx_id > tx->tx_id) + if (tx_id > tx->tx_id) continue; if (tx == ftp_state->curr_tx) @@ -1046,7 +1048,7 @@ static StreamingBufferConfig sbcfg = STREAMING_BUFFER_CONFIG_INITIALIZER; * \retval 1 when the command is parsed, 0 otherwise */ static AppLayerResult FTPDataParse(Flow *f, FtpDataState *ftpdata_state, - AppLayerParserState *pstate, StreamSlice stream_slice, void *local_data, uint8_t direction) + AppLayerParserState *pstate, StreamSlice stream_slice, uint8_t direction) { const uint8_t *input = StreamSliceGetData(&stream_slice); uint32_t input_len = StreamSliceGetDataLen(&stream_slice); @@ -1116,10 +1118,8 @@ static AppLayerResult FTPDataParse(Flow *f, FtpDataState *ftpdata_state, /* open with fixed track_id 0 as we can have just one * file per ftp-data flow. */ - if (FileOpenFileWithId(ftpdata_state->files, &sbcfg, - 0ULL, (uint8_t *) ftpdata_state->file_name, - ftpdata_state->file_len, - input, input_len, flags) != 0) { + if (FileOpenFileWithId(ftpdata_state->files, &sbcfg, 0ULL, ftpdata_state->file_name, + ftpdata_state->file_len, input, input_len, flags) != 0) { SCLogDebug("Can't open file"); ret = -1; } @@ -1171,13 +1171,13 @@ static AppLayerResult FTPDataParse(Flow *f, FtpDataState *ftpdata_state, static AppLayerResult FTPDataParseRequest(Flow *f, void *ftp_state, AppLayerParserState *pstate, StreamSlice stream_slice, void *local_data) { - return FTPDataParse(f, ftp_state, pstate, stream_slice, local_data, STREAM_TOSERVER); + return FTPDataParse(f, ftp_state, pstate, stream_slice, STREAM_TOSERVER); } static AppLayerResult FTPDataParseResponse(Flow *f, void *ftp_state, AppLayerParserState *pstate, StreamSlice stream_slice, void *local_data) { - return FTPDataParse(f, ftp_state, pstate, stream_slice, local_data, STREAM_TOCLIENT); + return FTPDataParse(f, ftp_state, pstate, stream_slice, STREAM_TOCLIENT); } #ifdef DEBUG @@ -1260,8 +1260,7 @@ static int FTPDataGetAlstateProgress(void *tx, uint8_t direction) FtpDataState *ftpdata_state = (FtpDataState *)tx; if (direction == ftpdata_state->direction) return ftpdata_state->state; - else - return FTPDATA_STATE_FINISHED; + return FTPDATA_STATE_FINISHED; } static AppLayerGetFileState FTPDataStateGetTxFiles(void *tx, uint8_t direction) From 918fa00a2512944bdd4c307dac909dd60071b0b2 Mon Sep 17 00:00:00 2001 From: Eric Leblond Date: Sat, 11 Jan 2025 22:58:26 +0100 Subject: [PATCH 16/16] clang-tidy: error on Malloc check This cause clang-tidy to exit with error if we encounter the warning about a potential memory leak. --- .clang-tidy | 1 + 1 file changed, 1 insertion(+) diff --git a/.clang-tidy b/.clang-tidy index 193454805d7d..c503a6b4f0a1 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -17,6 +17,7 @@ Checks: " " WarningsAsErrors: " bugprone-unsafe-functions, + clang-analyzer-unix.Malloc, " HeaderFileExtensions: - ''