From: Ron Dempster (rdempste) <rdempste@cisco.com>
Date: Tue, 16 Jan 2024 23:09:53 +0000 (+0000)
Subject: Pull request #4132: appid: update peg counts to be thread safe
X-Git-Tag: 3.1.79.0~14
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b9be2f9a62c9e15e75bdf69fcf67566994d757a0;p=thirdparty%2Fsnort3.git

Pull request #4132: appid: update peg counts to be thread safe

Merge in SNORT/snort3 from ~RDEMPSTE/snort3:appid_stats to master

Squashed commit of the following:

commit ccc7a16e0fb0e7ce4eb244ca6f19a41bca00dde4
Author: Ron Dempster (rdempste) <rdempste@cisco.com>
Date:   Wed Dec 6 15:11:21 2023 -0500

    appid: update peg counts to be thread safe
---

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 706bdb7f8..c094dc045 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -11,7 +11,7 @@ if (DEFINED VERSION_BUILD)
     set (BUILD "${VERSION_BUILD}")
 endif ()
 
-set (CMAKE_CXX_STANDARD 14)
+set (CMAKE_CXX_STANDARD 17)
 set (CMAKE_CXX_STANDARD_REQUIRED ON)
 set (CMAKE_CXX_EXTENSIONS OFF)
 
diff --git a/src/network_inspectors/appid/app_info_table.cc b/src/network_inspectors/appid/app_info_table.cc
index 3e6c979b9..ffc3361aa 100644
--- a/src/network_inspectors/appid/app_info_table.cc
+++ b/src/network_inspectors/appid/app_info_table.cc
@@ -30,10 +30,12 @@
 #include <string>
 #include <unistd.h>
 
+#include "log/messages.h"
 #include "log/unified2.h"
 #include "main/snort_config.h"
 #include "target_based/snort_protocols.h"
 #include "utils/util_cstring.h"
+
 #include "appid_api.h"
 #include "appid_config.h"
 #include "appid_debug.h"
diff --git a/src/network_inspectors/appid/appid_config.cc b/src/network_inspectors/appid/appid_config.cc
index a825a8231..51d772853 100644
--- a/src/network_inspectors/appid/appid_config.cc
+++ b/src/network_inspectors/appid/appid_config.cc
@@ -28,23 +28,25 @@
 #include <glob.h>
 #include <climits>
 
+#include "log/messages.h"
+#include "main/snort_config.h"
+#include "utils/util.h"
+#include "target_based/snort_protocols.h"
+
 #include "app_info_table.h"
 #include "appid_debug.h"
 #include "appid_discovery.h"
 #include "appid_http_session.h"
 #include "appid_inspector.h"
 #include "appid_session.h"
+#include "detector_plugins/detector_dns.h"
 #include "detector_plugins/detector_imap.h"
 #include "detector_plugins/detector_kerberos.h"
 #include "detector_plugins/detector_pattern.h"
 #include "detector_plugins/detector_pop3.h"
 #include "detector_plugins/detector_smtp.h"
 #include "host_port_app_cache.h"
-#include "main/snort_config.h"
-#include "utils/util.h"
 #include "service_plugins/service_ssl.h"
-#include "detector_plugins/detector_dns.h"
-#include "target_based/snort_protocols.h"
 #include "tp_appid_utils.h"
 #include "tp_lib_handler.h"
 
diff --git a/src/network_inspectors/appid/appid_data_decrypt_event_handler.cc b/src/network_inspectors/appid/appid_data_decrypt_event_handler.cc
index 2e1536380..9c0e4c2cb 100644
--- a/src/network_inspectors/appid/appid_data_decrypt_event_handler.cc
+++ b/src/network_inspectors/appid/appid_data_decrypt_event_handler.cc
@@ -24,6 +24,8 @@
 
 #include "appid_data_decrypt_event_handler.h"
 
+#include "detection/detection_engine.h"
+#include "log/messages.h"
 #include "pub_sub/data_decrypt_event.h"
 
 #include "app_info_table.h"
@@ -33,7 +35,6 @@
 #include "appid_inspector.h"
 #include "appid_session.h"
 #include "appid_session_api.h"
-#include "detection/detection_engine.h"
 
 using namespace snort;
 
diff --git a/src/network_inspectors/appid/appid_inspector.cc b/src/network_inspectors/appid/appid_inspector.cc
index 9a2a5c28b..c98004d5a 100644
--- a/src/network_inspectors/appid/appid_inspector.cc
+++ b/src/network_inspectors/appid/appid_inspector.cc
@@ -259,12 +259,14 @@ static void appid_inspector_pinit()
 {
     AppIdSession::init();
     TPLibHandler::get();
+    AppIdPegCounts::init_peg_info();
 }
 
 static void appid_inspector_pterm()
 {
     AppIdContext::pterm();
     TPLibHandler::pfini();
+    AppIdPegCounts::cleanup_peg_info();
 }
 
 static void appid_inspector_tinit()
diff --git a/src/network_inspectors/appid/appid_module.cc b/src/network_inspectors/appid/appid_module.cc
index e97fa28d4..c920b2ffc 100644
--- a/src/network_inspectors/appid/appid_module.cc
+++ b/src/network_inspectors/appid/appid_module.cc
@@ -404,6 +404,7 @@ static int reload_detectors(lua_State* L)
     ServiceDiscovery::clear_ftp_service_state();
     clear_dynamic_host_cache_services();
     AppIdPegCounts::cleanup_peg_info();
+    AppIdPegCounts::init_peg_info();
     LuaDetectorManager::clear_lua_detector_mgrs();
     ctxt.create_odp_ctxt();
     assert(odp_thread_local_ctxt);
@@ -474,14 +475,7 @@ static const PegInfo appid_pegs[] =
 };
 
 AppIdModule::AppIdModule() : Module(MOD_NAME, MOD_HELP, s_params)
-{
-    config = nullptr;
-}
-
-AppIdModule::~AppIdModule()
-{
-    AppIdPegCounts::cleanup_peg_info();
-}
+{ config = nullptr; }
 
 void AppIdModule::set_trace(const Trace* trace) const
 { appid_trace = trace; }
@@ -497,9 +491,9 @@ const TraceOption* AppIdModule::get_trace_options() const
 snort::ProfileStats* AppIdModule::get_profile(
         unsigned i, const char*& name, const char*& parent) const
 {
-    switch (i) 
+    switch (i)
     {
-    
+
         case 0:
             name = get_name();
             parent = nullptr;
diff --git a/src/network_inspectors/appid/appid_module.h b/src/network_inspectors/appid/appid_module.h
index 91d3dc1fd..1e08d5563 100644
--- a/src/network_inspectors/appid/appid_module.h
+++ b/src/network_inspectors/appid/appid_module.h
@@ -77,7 +77,7 @@ class AppIdModule : public snort::Module
 {
 public:
     AppIdModule();
-    ~AppIdModule() override;
+    ~AppIdModule() override = default;
 
     bool begin(const char*, int, snort::SnortConfig*) override;
     bool set(const char*, snort::Value&, snort::SnortConfig*) override;
diff --git a/src/network_inspectors/appid/appid_peg_counts.cc b/src/network_inspectors/appid/appid_peg_counts.cc
index 0a8bb1066..a5ff699ce 100644
--- a/src/network_inspectors/appid/appid_peg_counts.cc
+++ b/src/network_inspectors/appid/appid_peg_counts.cc
@@ -26,46 +26,115 @@
 #include "appid_peg_counts.h"
 
 #include <algorithm>
-#include <string>
-
-#include "framework/inspector.h"
-#include "main/thread_config.h"
+#include <cassert>
+#include <cstring>
+#include <memory>
+#include <mutex>
+#include <shared_mutex>
+#include <unordered_map>
+#include <vector>
+
+#include "log/messages.h"
+#include "main/thread.h"
 #include "utils/stats.h"
 
+#include "app_info_table.h"
+
 using namespace snort;
 
-THREAD_LOCAL AppIdPegCounts::AppIdDynamicPeg* AppIdPegCounts::unknown_appids_peg;
-THREAD_LOCAL std::unordered_map<AppId, AppIdPegCounts::AppIdDynamicPeg>* AppIdPegCounts::appid_peg_counts;
-std::unordered_map<AppId, std::pair<std::string, uint32_t>> AppIdPegCounts::appid_peg_ids;
-AppIdPegCounts::AppIdDynamicPeg AppIdPegCounts::appid_dynamic_sum[SF_APPID_MAX + 1];
-AppIdPegCounts::AppIdDynamicPeg AppIdPegCounts::zeroed_peg;
-PegCount AppIdPegCounts::all_zeroed_peg[DetectorPegs::NUM_APPID_DETECTOR_PEGS] = {};
+enum DetectorPegs
+{
+    SERVICE_DETECTS = 0,
+    CLIENT_DETECTS,
+    USER_DETECTS,
+    PAYLOAD_DETECTS,
+    MISC_DETECTS,
+    REFERRED_DETECTS,
+    NUM_APPID_DETECTOR_PEGS
+};
+
+static PegCount all_zeroed_peg[DetectorPegs::NUM_APPID_DETECTOR_PEGS] = {};
+
+class AppIdDynamicPeg
+{
+public:
+    PegCount stats[DetectorPegs::NUM_APPID_DETECTOR_PEGS] = { };
+
+    bool all_zeros()
+    {
+        return !memcmp(stats, &all_zeroed_peg, sizeof(stats));
+    }
+
+    void zero_out()
+    {
+        memcpy(stats, &all_zeroed_peg, sizeof(stats));
+    }
+
+    void print(const char* app, char* buf, int buf_size)
+    {
+        snprintf(buf, buf_size, "%25.25s: " FMTu64("-10") " " FMTu64("-10") " " FMTu64("-10")
+            " " FMTu64("-10") " " FMTu64("-10") " " FMTu64("-10"), app,
+            stats[SERVICE_DETECTS], stats[CLIENT_DETECTS], stats[USER_DETECTS],
+            stats[PAYLOAD_DETECTS], stats[MISC_DETECTS], stats[REFERRED_DETECTS]);
+    }
+};
+
+using AppIdPegCountIdInfo = std::unordered_map<AppId, std::pair<std::string, uint32_t>>;
+using AppIdPegCountIdMap = std::unordered_map<AppId, AppIdDynamicPeg>;
+
+class AppIdThreadPegs
+{
+public:
+    AppIdThreadPegs() = delete;
+    explicit AppIdThreadPegs(std::shared_ptr<AppIdPegCountIdInfo>& the_peg_ids) : peg_ids(the_peg_ids)
+    {
+        if (!peg_ids)
+        {
+            WarningMessage("AppId peg counts thread created with no ids\n");
+            peg_ids = std::make_shared<AppIdPegCountIdInfo>();
+        }
+    }
+    ~AppIdThreadPegs() = default;
+    AppIdDynamicPeg unknown_appids_peg;
+    AppIdPegCountIdMap peg_counts;
+    std::shared_ptr<AppIdPegCountIdInfo> peg_ids;
+};
+
+static THREAD_LOCAL AppIdThreadPegs* appid_thread_pegs;
+static std::shared_ptr<AppIdPegCountIdInfo> appid_peg_ids;
+static AppIdDynamicPeg appid_dynamic_sum[SF_APPID_MAX + 1];
+static AppIdDynamicPeg zeroed_peg;
 
 static std::mutex dynamic_stats_mutex;
+static std::shared_mutex appid_peg_ids_mutex;
 
 void AppIdPegCounts::init_pegs()
 {
-    assert(!appid_peg_counts);
-    appid_peg_counts = new std::unordered_map<AppId, AppIdPegCounts::AppIdDynamicPeg>();
+    assert(!appid_thread_pegs);
+    const std::shared_lock<std::shared_mutex> _lock(appid_peg_ids_mutex);
+    appid_thread_pegs = new AppIdThreadPegs(appid_peg_ids);
 
-    for (auto& appid : appid_peg_ids)
-    {
-        appid_peg_counts->emplace(appid.first, zeroed_peg);
-    }
-    unknown_appids_peg = new AppIdDynamicPeg(zeroed_peg);
+    for (auto& appid : *appid_thread_pegs->peg_ids)
+        appid_thread_pegs->peg_counts.emplace(appid.first, zeroed_peg);
 }
 
 void AppIdPegCounts::cleanup_pegs()
 {
-    delete appid_peg_counts;
-    appid_peg_counts = nullptr;
-    delete unknown_appids_peg;
-    unknown_appids_peg = nullptr;
+    const std::shared_lock<std::shared_mutex> _lock(appid_peg_ids_mutex);
+    delete appid_thread_pegs;
+    appid_thread_pegs = nullptr;
+}
+
+void AppIdPegCounts::init_peg_info()
+{
+    const std::unique_lock<std::shared_mutex> _lock(appid_peg_ids_mutex);
+    appid_peg_ids = std::make_shared<AppIdPegCountIdInfo>();
 }
 
 void AppIdPegCounts::cleanup_peg_info()
 {
-    appid_peg_ids.clear();
+    const std::unique_lock<std::shared_mutex> _lock(appid_peg_ids_mutex);
+    appid_peg_ids.reset();
 }
 
 void AppIdPegCounts::cleanup_dynamic_sum()
@@ -73,44 +142,39 @@ void AppIdPegCounts::cleanup_dynamic_sum()
     const std::lock_guard<std::mutex> _lock(dynamic_stats_mutex);
 
     for (unsigned app_num = 0; app_num < SF_APPID_MAX; app_num++)
-    {
-        memset(appid_dynamic_sum[app_num].stats, 0, sizeof(PegCount) *
-            DetectorPegs::NUM_APPID_DETECTOR_PEGS);
-    }
+        memset(appid_dynamic_sum[app_num].stats, 0, sizeof(PegCount) * DetectorPegs::NUM_APPID_DETECTOR_PEGS);
 
-    if (appid_peg_counts)
+    if (appid_thread_pegs)
     {
-        for (auto& peg : (*appid_peg_counts))
-        {
+        for (auto& peg : appid_thread_pegs->peg_counts)
             memset(&peg.second.stats, 0, sizeof(PegCount) * DetectorPegs::NUM_APPID_DETECTOR_PEGS);
-        }
+        memset(&appid_thread_pegs->unknown_appids_peg.stats, 0, sizeof(PegCount) * DetectorPegs::NUM_APPID_DETECTOR_PEGS);
     }
 
     // reset unknown_app stats
-    memset(appid_dynamic_sum[SF_APPID_MAX].stats, 0, sizeof(PegCount) *
-        DetectorPegs::NUM_APPID_DETECTOR_PEGS);
-    if (unknown_appids_peg)
-        memset(&unknown_appids_peg->stats, 0, sizeof(PegCount) * DetectorPegs::NUM_APPID_DETECTOR_PEGS);
-
+    memset(appid_dynamic_sum[SF_APPID_MAX].stats, 0, sizeof(PegCount) * DetectorPegs::NUM_APPID_DETECTOR_PEGS);
 }
 
 void AppIdPegCounts::add_app_peg_info(std::string app_name, AppId app_id)
 {
     std::replace(app_name.begin(), app_name.end(), ' ', '_');
 
-    appid_peg_ids.emplace(app_id, std::make_pair(app_name, appid_peg_ids.size()));
+    assert(appid_peg_ids);
+    const std::unique_lock<std::shared_mutex> _lock(appid_peg_ids_mutex);
+    appid_peg_ids->emplace(app_id, std::make_pair(app_name, appid_peg_ids->size()));
 }
 
 void AppIdPegCounts::sum_stats()
 {
-    if (!appid_peg_counts or !unknown_appids_peg)
+    if (!appid_thread_pegs)
         return;
 
+    const std::shared_lock<std::shared_mutex> _ids_lock(appid_peg_ids_mutex);
     const std::lock_guard<std::mutex> _lock(dynamic_stats_mutex);
 
-    for (auto& peg : (*appid_peg_counts))
+    for (auto& peg : appid_thread_pegs->peg_counts)
     {
-        auto dyn_indx = appid_peg_ids[peg.first].second;
+        auto dyn_indx = appid_thread_pegs->peg_ids->at(peg.first).second;
         for (unsigned j = 0; j < DetectorPegs::NUM_APPID_DETECTOR_PEGS; ++j)
             appid_dynamic_sum[dyn_indx].stats[j] += peg.second.stats[j];
 
@@ -119,78 +183,81 @@ void AppIdPegCounts::sum_stats()
 
     // unknown_app stats
     for (unsigned j = 0; j < DetectorPegs::NUM_APPID_DETECTOR_PEGS; ++j)
-        appid_dynamic_sum[SF_APPID_MAX].stats[j] += unknown_appids_peg->stats[j];
+        appid_dynamic_sum[SF_APPID_MAX].stats[j] += appid_thread_pegs->unknown_appids_peg.stats[j];
 
-    unknown_appids_peg->zero_out();
+    appid_thread_pegs->unknown_appids_peg.zero_out();
 }
 
 void AppIdPegCounts::inc_service_count(AppId id)
 {
-    auto peg = appid_peg_counts->find(id);
-    if (peg != appid_peg_counts->end())
+    auto peg = appid_thread_pegs->peg_counts.find(id);
+    if (peg != appid_thread_pegs->peg_counts.end())
         peg->second.stats[DetectorPegs::SERVICE_DETECTS]++;
     else
-        unknown_appids_peg->stats[DetectorPegs::SERVICE_DETECTS]++;
+        appid_thread_pegs->unknown_appids_peg.stats[DetectorPegs::SERVICE_DETECTS]++;
 }
 
 void AppIdPegCounts::inc_client_count(AppId id)
 {
-    auto peg = appid_peg_counts->find(id);
-    if (peg != appid_peg_counts->end())
+    auto peg = appid_thread_pegs->peg_counts.find(id);
+    if (peg != appid_thread_pegs->peg_counts.end())
         peg->second.stats[DetectorPegs::CLIENT_DETECTS]++;
     else
-        unknown_appids_peg->stats[DetectorPegs::CLIENT_DETECTS]++;
+        appid_thread_pegs->unknown_appids_peg.stats[DetectorPegs::CLIENT_DETECTS]++;
 }
 
 void AppIdPegCounts::inc_payload_count(AppId id)
 {
-    auto peg = appid_peg_counts->find(id);
-    if (peg != appid_peg_counts->end())
+    auto peg = appid_thread_pegs->peg_counts.find(id);
+    if (peg != appid_thread_pegs->peg_counts.end())
         peg->second.stats[DetectorPegs::PAYLOAD_DETECTS]++;
     else
-        unknown_appids_peg->stats[DetectorPegs::PAYLOAD_DETECTS]++;
+        appid_thread_pegs->unknown_appids_peg.stats[DetectorPegs::PAYLOAD_DETECTS]++;
 }
 
 void AppIdPegCounts::inc_user_count(AppId id)
 {
-    auto peg = appid_peg_counts->find(id);
-    if (peg != appid_peg_counts->end())
+    auto peg = appid_thread_pegs->peg_counts.find(id);
+    if (peg != appid_thread_pegs->peg_counts.end())
         peg->second.stats[DetectorPegs::USER_DETECTS]++;
     else
-        unknown_appids_peg->stats[DetectorPegs::USER_DETECTS]++;
+        appid_thread_pegs->unknown_appids_peg.stats[DetectorPegs::USER_DETECTS]++;
 }
 
 void AppIdPegCounts::inc_misc_count(AppId id)
 {
-    auto peg = appid_peg_counts->find(id);
-    if (peg != appid_peg_counts->end())
+    auto peg = appid_thread_pegs->peg_counts.find(id);
+    if (peg != appid_thread_pegs->peg_counts.end())
         peg->second.stats[DetectorPegs::MISC_DETECTS]++;
     else
-        unknown_appids_peg->stats[DetectorPegs::MISC_DETECTS]++;
+        appid_thread_pegs->unknown_appids_peg.stats[DetectorPegs::MISC_DETECTS]++;
 }
 
 void AppIdPegCounts::inc_referred_count(AppId id)
 {
-    auto peg = appid_peg_counts->find(id);
-    if (peg != appid_peg_counts->end())
+    auto peg = appid_thread_pegs->peg_counts.find(id);
+    if (peg != appid_thread_pegs->peg_counts.end())
         peg->second.stats[DetectorPegs::REFERRED_DETECTS]++;
     else
-        unknown_appids_peg->stats[DetectorPegs::REFERRED_DETECTS]++;
+        appid_thread_pegs->unknown_appids_peg.stats[DetectorPegs::REFERRED_DETECTS]++;
 }
 
 void AppIdPegCounts::print()
 {
+    if (!appid_peg_ids)
+        return;
+
     bool print = false;
-    unsigned app_num = AppIdPegCounts::appid_peg_ids.size();
+    unsigned app_num = appid_peg_ids->size();
 
     std::unordered_map<uint32_t, std::string*> tmp_sorting_map;
 
-    for (auto& det_info : AppIdPegCounts::appid_peg_ids)
+    for (auto& det_info : *appid_peg_ids)
     {
         AppIdDynamicPeg* pegs = &appid_dynamic_sum[det_info.second.second];
         if (!pegs->all_zeros())
         {
-            print |= true;
+            print = true;
         }
         tmp_sorting_map.emplace(det_info.second.second, &det_info.second.first);
     }
diff --git a/src/network_inspectors/appid/appid_peg_counts.h b/src/network_inspectors/appid/appid_peg_counts.h
index 0b21cf2b3..6134f959b 100644
--- a/src/network_inspectors/appid/appid_peg_counts.h
+++ b/src/network_inspectors/appid/appid_peg_counts.h
@@ -32,15 +32,10 @@
 // array index.
 // Only the main thread is using a static array.
 
-#include <unordered_map>
-#include <vector>
+#include <string>
 
 #include "application_ids.h"
-#include "app_info_table.h"
 #include "framework/counts.h"
-#include "log/messages.h"
-#include "main/thread.h"
-#include "utils/util.h"
 
 struct AppIdStats
 {
@@ -60,44 +55,10 @@ struct AppIdStats
 class AppIdPegCounts
 {
 public:
-    enum DetectorPegs
-    {
-        SERVICE_DETECTS = 0,
-        CLIENT_DETECTS,
-        USER_DETECTS,
-        PAYLOAD_DETECTS,
-        MISC_DETECTS,
-        REFERRED_DETECTS,
-        NUM_APPID_DETECTOR_PEGS
-    };
-
-    class AppIdDynamicPeg
-    {
-    public:
-        PegCount stats[DetectorPegs::NUM_APPID_DETECTOR_PEGS] = { };
-
-        bool all_zeros()
-        {
-            return !memcmp(stats, &all_zeroed_peg, sizeof(stats));
-        }
-
-        void zero_out()
-        {
-            memcpy(stats, &all_zeroed_peg, sizeof(stats));
-        }
-
-        void print(const char* app, char* buf, int buf_size)
-        {
-            snprintf(buf, buf_size, "%25.25s: " FMTu64("-10") " " FMTu64("-10") " " FMTu64("-10")
-                " " FMTu64("-10") " " FMTu64("-10") " " FMTu64("-10"), app,
-                stats[SERVICE_DETECTS], stats[CLIENT_DETECTS], stats[USER_DETECTS],
-                stats[PAYLOAD_DETECTS], stats[MISC_DETECTS], stats[REFERRED_DETECTS]);
-        }
-    };
-
     static void add_app_peg_info(std::string app_name, AppId);
     static void init_pegs();
     static void cleanup_pegs();
+    static void init_peg_info();
     static void cleanup_peg_info();
     static void cleanup_dynamic_sum();
 
@@ -110,14 +71,6 @@ public:
 
     static void sum_stats();
     static void print();
-
-private:
-    static AppIdDynamicPeg appid_dynamic_sum[SF_APPID_MAX+1];
-    static THREAD_LOCAL AppIdDynamicPeg* unknown_appids_peg;
-    static THREAD_LOCAL std::unordered_map<AppId, AppIdPegCounts::AppIdDynamicPeg>* appid_peg_counts;
-    static std::unordered_map<AppId, std::pair<std::string, uint32_t>> appid_peg_ids;
-    static AppIdDynamicPeg zeroed_peg;
-    static PegCount all_zeroed_peg[DetectorPegs::NUM_APPID_DETECTOR_PEGS];
 };
 #endif
 
diff --git a/src/network_inspectors/appid/detector_plugins/test/detector_plugins_mock.h b/src/network_inspectors/appid/detector_plugins/test/detector_plugins_mock.h
index b9b7f01f2..99f58ff3d 100644
--- a/src/network_inspectors/appid/detector_plugins/test/detector_plugins_mock.h
+++ b/src/network_inspectors/appid/detector_plugins/test/detector_plugins_mock.h
@@ -19,10 +19,13 @@
 
 #ifndef DETECTOR_PLUGINS_MOCK_H
 #define DETECTOR_PLUGINS_MOCK_H
+
+#include "log/messages.h"
+#include "utils/stats.h"
+
 #include "appid_detector.h"
 #include "appid_module.h"
 #include "appid_peg_counts.h"
-#include "utils/stats.h"
 
 namespace snort
 {
@@ -107,7 +110,6 @@ private:
 AppIdConfig::~AppIdConfig() = default;
 AppIdModule::AppIdModule()
     : Module("a", "b") { }
-AppIdModule::~AppIdModule() = default;
 
 // LCOV_EXCL_START
 bool AppIdModule::begin(const char*, int, snort::SnortConfig*)
diff --git a/src/network_inspectors/appid/lua_detector_module.cc b/src/network_inspectors/appid/lua_detector_module.cc
index 3fc55d121..554f810d8 100644
--- a/src/network_inspectors/appid/lua_detector_module.cc
+++ b/src/network_inspectors/appid/lua_detector_module.cc
@@ -32,15 +32,14 @@
 #include <cstring>
 #include <fstream>
 
+#include "log/messages.h"
+
 #include "appid_config.h"
 #include "appid_debug.h"
 #include "appid_inspector.h"
 #include "lua_detector_util.h"
 #include "lua_detector_api.h"
 #include "lua_detector_flow_api.h"
-#include "main/snort_config.h"
-#include "utils/util.h"
-#include "utils/sflsq.h"
 
 using namespace snort;
 using namespace std;
diff --git a/src/network_inspectors/appid/test/appid_discovery_test.cc b/src/network_inspectors/appid/test/appid_discovery_test.cc
index f002e289e..2df6a4dd7 100644
--- a/src/network_inspectors/appid/test/appid_discovery_test.cc
+++ b/src/network_inspectors/appid/test/appid_discovery_test.cc
@@ -55,6 +55,7 @@ THREAD_LOCAL PacketTracer* s_pkt_trace = nullptr;
 THREAD_LOCAL Stopwatch<SnortClock>* pt_timer = nullptr;
 void PacketTracer::daq_log(const char*, ...) { }
 void FatalError(const char* fmt, ...) { (void)fmt; exit(1); }
+void WarningMessage(const char*,...) { }
 
 // Stubs for packet
 Packet::Packet(bool) {}
@@ -160,7 +161,6 @@ void ClientAppDescriptor::update_user(AppId, const char*, AppidChangeBits&){}
 
 // Stubs for AppIdModule
 AppIdModule::AppIdModule(): Module("appid_mock", "appid_mock_help") {}
-AppIdModule::~AppIdModule() = default;
 void AppIdModule::sum_stats(bool) {}
 void AppIdModule::show_dynamic_stats() {}
 bool AppIdModule::begin(char const*, int, SnortConfig*) { return true; }
@@ -169,8 +169,7 @@ bool AppIdModule::set(char const*, Value&, SnortConfig*) { return true; }
 const Command* AppIdModule::get_commands() const { return nullptr; }
 const PegInfo* AppIdModule::get_pegs() const { return nullptr; }
 PegCount* AppIdModule::get_counts() const { return nullptr; }
-ProfileStats* AppIdModule::get_profile(
-        unsigned, const char*&, const char*&) const { return nullptr; }
+ProfileStats* AppIdModule::get_profile(unsigned, const char*&, const char*&) const { return nullptr; }
 void AppIdModule::set_trace(const Trace*) const { }
 const TraceOption* AppIdModule::get_trace_options() const { return nullptr; }
 THREAD_LOCAL bool ThirdPartyAppIdContext::tp_reload_in_progress = false;
diff --git a/src/network_inspectors/appid/test/appid_mock_definitions.h b/src/network_inspectors/appid/test/appid_mock_definitions.h
index 2a930de51..e4f3ba217 100644
--- a/src/network_inspectors/appid/test/appid_mock_definitions.h
+++ b/src/network_inspectors/appid/test/appid_mock_definitions.h
@@ -21,11 +21,13 @@
 #ifndef APPID_MOCK_DEFINITIONS_H
 #define APPID_MOCK_DEFINITIONS_H
 
+#include "log/messages.h"
+#include "utils/stats.h"
+
 #include "appid_detector.h"
 #include "appid_module.h"
 #include "appid_peg_counts.h"
 #include "service_inspectors/http_inspect/http_msg_header.h"
-#include "utils/stats.h"
 
 class ThirdPartyAppIdContext;
 
diff --git a/src/network_inspectors/appid/test/appid_mock_inspector.h b/src/network_inspectors/appid/test/appid_mock_inspector.h
index ca8d514d3..63a95af2a 100644
--- a/src/network_inspectors/appid/test/appid_mock_inspector.h
+++ b/src/network_inspectors/appid/test/appid_mock_inspector.h
@@ -47,7 +47,6 @@ PegCount Module::get_global_count(char const*) const { return 0; }
 }
 
 AppIdModule::AppIdModule(): snort::Module("appid_mock", "appid_mock_help") {}
-AppIdModule::~AppIdModule() = default;
 void AppIdModule::sum_stats(bool) {}
 void AppIdModule::show_dynamic_stats() {}
 bool AppIdModule::begin(char const*, int, snort::SnortConfig*) { return true; }
diff --git a/src/network_inspectors/appid/test/service_state_test.cc b/src/network_inspectors/appid/test/service_state_test.cc
index 892142073..f0d52560c 100644
--- a/src/network_inspectors/appid/test/service_state_test.cc
+++ b/src/network_inspectors/appid/test/service_state_test.cc
@@ -52,6 +52,8 @@ void LogMessage(const char* format,...)
     LogMessage(format, args);
     va_end(args);
 }
+void WarningMessage(const char*,...)
+{ }
 
 void LogLabel(const char*, FILE*) {}
 void LogText(const char* s, FILE*) { LogMessage("%s\n", s); }
diff --git a/tools/snort2lua/helpers/s2l_util.cc b/tools/snort2lua/helpers/s2l_util.cc
index 3825bd9b3..00e896da0 100644
--- a/tools/snort2lua/helpers/s2l_util.cc
+++ b/tools/snort2lua/helpers/s2l_util.cc
@@ -26,6 +26,7 @@
 #include <sys/stat.h>
 
 #include <algorithm>
+#include <functional>
 
 #include "conversion_state.h"
 #include "data/data_types/dt_table.h"
@@ -102,15 +103,13 @@ Table* find_table(const std::vector<Table*>& vec, const std::string& name)
 
 std::string& ltrim(std::string& s)
 {
-    s.erase(s.begin(), std::find_if(s.begin(), s.end(), std::not1(std::ptr_fun<int, int>(
-        std::isspace))));
+    s.erase(s.begin(), std::find_if(s.begin(), s.end(), std::not_fn(std::ptr_fun<int, int>(std::isspace))));
     return s;
 }
 
 std::string& rtrim(std::string& s)
 {
-    s.erase(std::find_if(s.rbegin(), s.rend(), std::not1(std::ptr_fun<int, int>(
-        std::isspace))).base(), s.end());
+    s.erase(std::find_if(s.rbegin(), s.rend(), std::not_fn(std::ptr_fun<int, int>(std::isspace))).base(), s.end());
     return s;
 }