From bde2dec6856f7c3eb17e00b492431a644ee9ce54 Mon Sep 17 00:00:00 2001
From: Tim Wojtulewicz <tim@corelight.com>
Date: Fri, 4 Apr 2025 15:11:40 -0700
Subject: [PATCH] Minor fixes in POP3 analyzer based on flycheck warnings

This also adds an implementation of unreachable() to use for default
cases where it shouldn't ever actually cause the default.
---
 src/analyzer/protocol/pop3/POP3.cc | 10 ++++++----
 src/analyzer/protocol/pop3/POP3.h  |  2 +-
 src/util.h                         | 15 +++++++++++++++
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/src/analyzer/protocol/pop3/POP3.cc b/src/analyzer/protocol/pop3/POP3.cc
index 295e195d5b..9379fe738a 100644
--- a/src/analyzer/protocol/pop3/POP3.cc
+++ b/src/analyzer/protocol/pop3/POP3.cc
@@ -24,7 +24,7 @@ static const char* pop3_cmd_word[] = {
 #include "POP3_cmd.def"
 };
 
-#define POP3_CMD_WORD(code) ((code >= 0) ? pop3_cmd_word[code] : "(UNKNOWN)")
+#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;
@@ -78,7 +78,7 @@ void POP3_Analyzer::DeliverStream(int len, const u_char* data, bool orig) {
 }
 
 static std::string trim_whitespace(const char* in) {
-    int n = strlen(in);
+    size_t n = strlen(in);
     char* out = new char[n + 1];
     char* out_p = out;
 
@@ -673,6 +673,7 @@ void POP3_Analyzer::ProcessReply(int length, const char* line) {
                         masterState = detail::POP3_UPDATE;
 
                     break;
+                default: util::unreachable();
             }
 
             POP3Event(pop3_reply, false, cmd, message);
@@ -727,6 +728,7 @@ void POP3_Analyzer::ProcessReply(int length, const char* line) {
                          masterState == detail::POP3_START )
                         masterState = detail::POP3_FINISHED;
                     break;
+                default: util::unreachable();
             }
 
             POP3Event(pop3_reply, false, cmd, message);
@@ -809,7 +811,7 @@ std::vector<std::string> POP3_Analyzer::TokenizeLine(const std::string& input, c
         return tokens;
     }
 
-    if ( (splitPos = input.find(split, 0)) < input.size() ) {
+    if ( splitPos = input.find(split, 0); splitPos < input.size() ) {
         token = input.substr(start, splitPos);
         if ( token.size() > 0 && token[0] != split )
             tokens.push_back(token);
@@ -821,7 +823,7 @@ std::vector<std::string> POP3_Analyzer::TokenizeLine(const std::string& input, c
     return tokens;
 }
 
-void POP3_Analyzer::POP3Event(EventHandlerPtr event, bool is_orig, const char* arg1, const char* arg2) {
+void POP3_Analyzer::POP3Event(const EventHandlerPtr& event, bool is_orig, const char* arg1, const char* arg2) {
     if ( ! event )
         return;
 
diff --git a/src/analyzer/protocol/pop3/POP3.h b/src/analyzer/protocol/pop3/POP3.h
index 3191df8038..d5384a050e 100644
--- a/src/analyzer/protocol/pop3/POP3.h
+++ b/src/analyzer/protocol/pop3/POP3.h
@@ -102,7 +102,7 @@ protected:
     std::vector<std::string> TokenizeLine(const std::string& input, char split);
     int ParseCmd(std::string cmd);
     void AuthSuccessful();
-    void POP3Event(EventHandlerPtr event, bool is_orig, const char* arg1 = nullptr, const char* arg2 = nullptr);
+    void POP3Event(const EventHandlerPtr& event, bool is_orig, const char* arg1 = nullptr, const char* arg2 = nullptr);
 
     analyzer::mime::MIME_Mail* mail;
     std::list<std::string> cmds;
diff --git a/src/util.h b/src/util.h
index d950c9df17..bfe1fdae5e 100644
--- a/src/util.h
+++ b/src/util.h
@@ -661,5 +661,20 @@ inline std::vector<std::wstring_view> split(const wchar_t* s, const wchar_t* del
     return split(std::wstring_view(s), std::wstring_view(delim));
 }
 
+/**
+ * Implementation of std::unreachable. Once C++23 is supported this can be replaced with
+ * an alias. This implementation is taken from cppreference.
+ */
+[[noreturn]] inline void unreachable() {
+    // Uses compiler specific extensions if possible.  Even if no extension is used,
+    // undefined behavior is still raised by an empty function body and the noreturn
+    // attribute.
+#if defined(_MSC_VER) && ! defined(__clang__) // MSVC
+    __assume(false);
+#else // GCC, Clang
+    __builtin_unreachable();
+#endif
+}
+
 } // namespace util
 } // namespace zeek