From adf3cbda57aa7bceecb35df33c006a87424dd86e Mon Sep 17 00:00:00 2001 From: Jason Dellaluce Date: Fri, 9 Feb 2024 17:21:06 +0000 Subject: [PATCH 01/10] refactor(userspace/libsinsp)!: use unique_ptr in filtercheck allocate_new Signed-off-by: Jason Dellaluce --- userspace/libsinsp/plugin_filtercheck.cpp | 4 ++-- userspace/libsinsp/plugin_filtercheck.h | 2 +- userspace/libsinsp/sinsp_filtercheck.h | 3 ++- userspace/libsinsp/sinsp_filtercheck_container.cpp | 4 ++-- userspace/libsinsp/sinsp_filtercheck_container.h | 2 +- userspace/libsinsp/sinsp_filtercheck_event.cpp | 4 ++-- userspace/libsinsp/sinsp_filtercheck_event.h | 2 +- userspace/libsinsp/sinsp_filtercheck_evtin.cpp | 4 ++-- userspace/libsinsp/sinsp_filtercheck_evtin.h | 2 +- userspace/libsinsp/sinsp_filtercheck_fd.cpp | 4 ++-- userspace/libsinsp/sinsp_filtercheck_fd.h | 2 +- userspace/libsinsp/sinsp_filtercheck_fdlist.cpp | 4 ++-- userspace/libsinsp/sinsp_filtercheck_fdlist.h | 2 +- userspace/libsinsp/sinsp_filtercheck_fspath.cpp | 4 ++-- userspace/libsinsp/sinsp_filtercheck_fspath.h | 2 +- userspace/libsinsp/sinsp_filtercheck_gen_event.cpp | 4 ++-- userspace/libsinsp/sinsp_filtercheck_gen_event.h | 2 +- userspace/libsinsp/sinsp_filtercheck_group.cpp | 4 ++-- userspace/libsinsp/sinsp_filtercheck_group.h | 2 +- userspace/libsinsp/sinsp_filtercheck_k8s.cpp | 4 ++-- userspace/libsinsp/sinsp_filtercheck_k8s.h | 2 +- userspace/libsinsp/sinsp_filtercheck_mesos.cpp | 4 ++-- userspace/libsinsp/sinsp_filtercheck_mesos.h | 2 +- userspace/libsinsp/sinsp_filtercheck_rawstring.cpp | 4 ++-- userspace/libsinsp/sinsp_filtercheck_rawstring.h | 2 +- userspace/libsinsp/sinsp_filtercheck_reference.cpp | 4 ++-- userspace/libsinsp/sinsp_filtercheck_reference.h | 2 +- userspace/libsinsp/sinsp_filtercheck_syslog.cpp | 4 ++-- userspace/libsinsp/sinsp_filtercheck_syslog.h | 2 +- userspace/libsinsp/sinsp_filtercheck_thread.cpp | 4 ++-- userspace/libsinsp/sinsp_filtercheck_thread.h | 2 +- userspace/libsinsp/sinsp_filtercheck_tracer.cpp | 4 ++-- userspace/libsinsp/sinsp_filtercheck_tracer.h | 2 +- userspace/libsinsp/sinsp_filtercheck_user.cpp | 4 ++-- userspace/libsinsp/sinsp_filtercheck_user.h | 2 +- userspace/libsinsp/sinsp_filtercheck_utils.cpp | 4 ++-- userspace/libsinsp/sinsp_filtercheck_utils.h | 2 +- 37 files changed, 56 insertions(+), 55 deletions(-) diff --git a/userspace/libsinsp/plugin_filtercheck.cpp b/userspace/libsinsp/plugin_filtercheck.cpp index 27583c501c..510c848bb4 100755 --- a/userspace/libsinsp/plugin_filtercheck.cpp +++ b/userspace/libsinsp/plugin_filtercheck.cpp @@ -128,9 +128,9 @@ int32_t sinsp_filter_check_plugin::parse_field_name(const char* str, bool alloc_ return res; } -sinsp_filter_check* sinsp_filter_check_plugin::allocate_new() +std::unique_ptr sinsp_filter_check_plugin::allocate_new() { - return new sinsp_filter_check_plugin(*this); + return std::make_unique(*this); } bool sinsp_filter_check_plugin::extract(sinsp_evt *evt, OUT vector& values, bool sanitize_strings) diff --git a/userspace/libsinsp/plugin_filtercheck.h b/userspace/libsinsp/plugin_filtercheck.h index c7ee362c9b..aa56718469 100755 --- a/userspace/libsinsp/plugin_filtercheck.h +++ b/userspace/libsinsp/plugin_filtercheck.h @@ -41,7 +41,7 @@ class sinsp_filter_check_plugin : public sinsp_filter_check virtual ~sinsp_filter_check_plugin() = default; - sinsp_filter_check* allocate_new() override; + std::unique_ptr allocate_new() override; int32_t parse_field_name( const char* str, diff --git a/userspace/libsinsp/sinsp_filtercheck.h b/userspace/libsinsp/sinsp_filtercheck.h index 8b507b1cd8..12b99760de 100644 --- a/userspace/libsinsp/sinsp_filtercheck.h +++ b/userspace/libsinsp/sinsp_filtercheck.h @@ -20,6 +20,7 @@ limitations under the License. #include #include +#include #include #include #include @@ -163,7 +164,7 @@ class sinsp_filter_check // Allocate a new check of the same type. // Every filtercheck plugin must implement this. // - virtual sinsp_filter_check* allocate_new() + virtual std::unique_ptr allocate_new() { throw sinsp_exception("can't clone abstract sinsp_filter_check"); } diff --git a/userspace/libsinsp/sinsp_filtercheck_container.cpp b/userspace/libsinsp/sinsp_filtercheck_container.cpp index 2e507595e5..02cdf37853 100644 --- a/userspace/libsinsp/sinsp_filtercheck_container.cpp +++ b/userspace/libsinsp/sinsp_filtercheck_container.cpp @@ -69,9 +69,9 @@ sinsp_filter_check_container::sinsp_filter_check_container() m_info.m_flags = filter_check_info::FL_NONE; } -sinsp_filter_check* sinsp_filter_check_container::allocate_new() +std::unique_ptr sinsp_filter_check_container::allocate_new() { - return (sinsp_filter_check*) new sinsp_filter_check_container(); + return std::make_unique(); } int32_t sinsp_filter_check_container::extract_arg(const string &val, size_t basepos) diff --git a/userspace/libsinsp/sinsp_filtercheck_container.h b/userspace/libsinsp/sinsp_filtercheck_container.h index c4c47584f2..b3445764d0 100644 --- a/userspace/libsinsp/sinsp_filtercheck_container.h +++ b/userspace/libsinsp/sinsp_filtercheck_container.h @@ -55,7 +55,7 @@ class sinsp_filter_check_container : public sinsp_filter_check sinsp_filter_check_container(); virtual ~sinsp_filter_check_container() = default; - sinsp_filter_check* allocate_new() override; + std::unique_ptr allocate_new() override; int32_t parse_field_name(const char* str, bool alloc_state, bool needed_for_filtering) override; const std::string& get_argstr() const; diff --git a/userspace/libsinsp/sinsp_filtercheck_event.cpp b/userspace/libsinsp/sinsp_filtercheck_event.cpp index 28d3787ef0..01e73db264 100644 --- a/userspace/libsinsp/sinsp_filtercheck_event.cpp +++ b/userspace/libsinsp/sinsp_filtercheck_event.cpp @@ -143,9 +143,9 @@ sinsp_filter_check_event::~sinsp_filter_check_event() } } -sinsp_filter_check* sinsp_filter_check_event::allocate_new() +std::unique_ptr sinsp_filter_check_event::allocate_new() { - return (sinsp_filter_check*) new sinsp_filter_check_event(); + return std::make_unique(); } int32_t sinsp_filter_check_event::extract_arg(string fldname, string val, OUT const ppm_param_info** parinfo) diff --git a/userspace/libsinsp/sinsp_filtercheck_event.h b/userspace/libsinsp/sinsp_filtercheck_event.h index 2064bf7668..87a1c4aa9e 100644 --- a/userspace/libsinsp/sinsp_filtercheck_event.h +++ b/userspace/libsinsp/sinsp_filtercheck_event.h @@ -89,7 +89,7 @@ class sinsp_filter_check_event : public sinsp_filter_check sinsp_filter_check_event(); virtual ~sinsp_filter_check_event(); - sinsp_filter_check* allocate_new() override; + std::unique_ptr allocate_new() override; int32_t parse_field_name(const char* str, bool alloc_state, bool needed_for_filtering) override; size_t parse_filter_value(const char* str, uint32_t len, uint8_t* storage, uint32_t storage_len) override; const filtercheck_field_info* get_field_info() const override; diff --git a/userspace/libsinsp/sinsp_filtercheck_evtin.cpp b/userspace/libsinsp/sinsp_filtercheck_evtin.cpp index f7664c3156..ff8444fc50 100644 --- a/userspace/libsinsp/sinsp_filtercheck_evtin.cpp +++ b/userspace/libsinsp/sinsp_filtercheck_evtin.cpp @@ -189,9 +189,9 @@ int32_t sinsp_filter_check_evtin::parse_field_name(const char* str, bool alloc_s return res; } -sinsp_filter_check* sinsp_filter_check_evtin::allocate_new() +std::unique_ptr sinsp_filter_check_evtin::allocate_new() { - return (sinsp_filter_check*) new sinsp_filter_check_evtin(); + return std::make_unique(); } uint8_t* sinsp_filter_check_evtin::extract(sinsp_evt *evt, OUT uint32_t* len, bool sanitize_strings) diff --git a/userspace/libsinsp/sinsp_filtercheck_evtin.h b/userspace/libsinsp/sinsp_filtercheck_evtin.h index b57fa75757..d551c895e9 100644 --- a/userspace/libsinsp/sinsp_filtercheck_evtin.h +++ b/userspace/libsinsp/sinsp_filtercheck_evtin.h @@ -58,7 +58,7 @@ class sinsp_filter_check_evtin : public sinsp_filter_check sinsp_filter_check_evtin(); virtual ~sinsp_filter_check_evtin() = default; - sinsp_filter_check* allocate_new() override; + std::unique_ptr allocate_new() override; int32_t parse_field_name(const char* str, bool alloc_state, bool needed_for_filtering) override; protected: diff --git a/userspace/libsinsp/sinsp_filtercheck_fd.cpp b/userspace/libsinsp/sinsp_filtercheck_fd.cpp index 41b3f2cecc..c9eaa6dc4f 100644 --- a/userspace/libsinsp/sinsp_filtercheck_fd.cpp +++ b/userspace/libsinsp/sinsp_filtercheck_fd.cpp @@ -109,9 +109,9 @@ sinsp_filter_check_fd::sinsp_filter_check_fd() m_info.m_flags = filter_check_info::FL_NONE; } -sinsp_filter_check* sinsp_filter_check_fd::allocate_new() +std::unique_ptr sinsp_filter_check_fd::allocate_new() { - return (sinsp_filter_check*) new sinsp_filter_check_fd(); + return std::make_unique(); } int32_t sinsp_filter_check_fd::extract_arg(string fldname, string val) diff --git a/userspace/libsinsp/sinsp_filtercheck_fd.h b/userspace/libsinsp/sinsp_filtercheck_fd.h index 990329dc48..0d400634d1 100644 --- a/userspace/libsinsp/sinsp_filtercheck_fd.h +++ b/userspace/libsinsp/sinsp_filtercheck_fd.h @@ -74,7 +74,7 @@ class sinsp_filter_check_fd : public sinsp_filter_check sinsp_filter_check_fd(); virtual ~sinsp_filter_check_fd() = default; - sinsp_filter_check* allocate_new() override; + std::unique_ptr allocate_new() override; int32_t parse_field_name(const char* str, bool alloc_state, bool needed_for_filtering) override; bool extract(sinsp_evt*, OUT std::vector& values, bool sanitize_strings = true) override; diff --git a/userspace/libsinsp/sinsp_filtercheck_fdlist.cpp b/userspace/libsinsp/sinsp_filtercheck_fdlist.cpp index 703ff3264e..d309df39f0 100644 --- a/userspace/libsinsp/sinsp_filtercheck_fdlist.cpp +++ b/userspace/libsinsp/sinsp_filtercheck_fdlist.cpp @@ -46,9 +46,9 @@ sinsp_filter_check_fdlist::sinsp_filter_check_fdlist() m_info.m_flags = filter_check_info::FL_NONE; } -sinsp_filter_check* sinsp_filter_check_fdlist::allocate_new() +std::unique_ptr sinsp_filter_check_fdlist::allocate_new() { - return (sinsp_filter_check*) new sinsp_filter_check_fdlist(); + return std::make_unique(); } uint8_t* sinsp_filter_check_fdlist::extract(sinsp_evt *evt, OUT uint32_t* len, bool sanitize_strings) diff --git a/userspace/libsinsp/sinsp_filtercheck_fdlist.h b/userspace/libsinsp/sinsp_filtercheck_fdlist.h index e6d4af206c..b80b1bf4cd 100644 --- a/userspace/libsinsp/sinsp_filtercheck_fdlist.h +++ b/userspace/libsinsp/sinsp_filtercheck_fdlist.h @@ -36,7 +36,7 @@ class sinsp_filter_check_fdlist : public sinsp_filter_check sinsp_filter_check_fdlist(); virtual ~sinsp_filter_check_fdlist() = default; - sinsp_filter_check* allocate_new() override; + std::unique_ptr allocate_new() override; protected: uint8_t* extract(sinsp_evt*, OUT uint32_t* len, bool sanitize_strings = true) override; diff --git a/userspace/libsinsp/sinsp_filtercheck_fspath.cpp b/userspace/libsinsp/sinsp_filtercheck_fspath.cpp index eab6d59033..198a8adc35 100644 --- a/userspace/libsinsp/sinsp_filtercheck_fspath.cpp +++ b/userspace/libsinsp/sinsp_filtercheck_fspath.cpp @@ -219,7 +219,7 @@ void sinsp_filter_check_fspath::set_fspath_checks(std::shared_ptr sinsp_filter_check_fspath::allocate_new() { // If not yet populated, do so now. The maps will be empty // *only* for the initial filtercheck created in @@ -229,7 +229,7 @@ sinsp_filter_check* sinsp_filter_check_fspath::allocate_new() create_fspath_checks(); } - sinsp_filter_check_fspath* ret = new sinsp_filter_check_fspath(); + auto ret = std::make_unique(); ret->set_fspath_checks(m_success_checks, m_path_checks, m_source_checks, m_target_checks); diff --git a/userspace/libsinsp/sinsp_filtercheck_fspath.h b/userspace/libsinsp/sinsp_filtercheck_fspath.h index 46bfd00392..3a2abe44e1 100644 --- a/userspace/libsinsp/sinsp_filtercheck_fspath.h +++ b/userspace/libsinsp/sinsp_filtercheck_fspath.h @@ -36,7 +36,7 @@ class sinsp_filter_check_fspath : public sinsp_filter_check sinsp_filter_check_fspath(); virtual ~sinsp_filter_check_fspath() = default; - sinsp_filter_check* allocate_new() override; + std::unique_ptr allocate_new() override; protected: uint8_t* extract(sinsp_evt*, OUT uint32_t* len, bool sanitize_strings = true) override; diff --git a/userspace/libsinsp/sinsp_filtercheck_gen_event.cpp b/userspace/libsinsp/sinsp_filtercheck_gen_event.cpp index 6dff2c1596..571ca2e235 100644 --- a/userspace/libsinsp/sinsp_filtercheck_gen_event.cpp +++ b/userspace/libsinsp/sinsp_filtercheck_gen_event.cpp @@ -75,9 +75,9 @@ sinsp_filter_check_gen_event::sinsp_filter_check_gen_event() m_u64val = 0; } -sinsp_filter_check* sinsp_filter_check_gen_event::allocate_new() +std::unique_ptr sinsp_filter_check_gen_event::allocate_new() { - return (sinsp_filter_check*) new sinsp_filter_check_gen_event(); + return std::make_unique(); } Json::Value sinsp_filter_check_gen_event::extract_as_js(sinsp_evt *evt, OUT uint32_t* len) diff --git a/userspace/libsinsp/sinsp_filtercheck_gen_event.h b/userspace/libsinsp/sinsp_filtercheck_gen_event.h index a5e764e4b2..34ed019b57 100644 --- a/userspace/libsinsp/sinsp_filtercheck_gen_event.h +++ b/userspace/libsinsp/sinsp_filtercheck_gen_event.h @@ -48,7 +48,7 @@ class sinsp_filter_check_gen_event : public sinsp_filter_check sinsp_filter_check_gen_event(); virtual ~sinsp_filter_check_gen_event() = default; - sinsp_filter_check* allocate_new() override; + std::unique_ptr allocate_new() override; protected: uint8_t* extract(sinsp_evt*, OUT uint32_t* len, bool sanitize_strings = true) override; diff --git a/userspace/libsinsp/sinsp_filtercheck_group.cpp b/userspace/libsinsp/sinsp_filtercheck_group.cpp index 81fe3230f9..84151a7cc3 100644 --- a/userspace/libsinsp/sinsp_filtercheck_group.cpp +++ b/userspace/libsinsp/sinsp_filtercheck_group.cpp @@ -50,9 +50,9 @@ sinsp_filter_check_group::sinsp_filter_check_group() m_info.m_flags = filter_check_info::FL_NONE; } -sinsp_filter_check* sinsp_filter_check_group::allocate_new() +std::unique_ptr sinsp_filter_check_group::allocate_new() { - return (sinsp_filter_check*) new sinsp_filter_check_group(); + return std::make_unique(); } uint8_t* sinsp_filter_check_group::extract(sinsp_evt *evt, OUT uint32_t* len, bool sanitize_strings) diff --git a/userspace/libsinsp/sinsp_filtercheck_group.h b/userspace/libsinsp/sinsp_filtercheck_group.h index 6c46b212d3..7b91b078c1 100644 --- a/userspace/libsinsp/sinsp_filtercheck_group.h +++ b/userspace/libsinsp/sinsp_filtercheck_group.h @@ -32,7 +32,7 @@ class sinsp_filter_check_group : public sinsp_filter_check sinsp_filter_check_group(); virtual ~sinsp_filter_check_group() = default; - sinsp_filter_check* allocate_new() override; + std::unique_ptr allocate_new() override; protected: uint8_t* extract(sinsp_evt*, OUT uint32_t* len, bool sanitize_strings = true) override; diff --git a/userspace/libsinsp/sinsp_filtercheck_k8s.cpp b/userspace/libsinsp/sinsp_filtercheck_k8s.cpp index 60a7e84865..6bc082604d 100644 --- a/userspace/libsinsp/sinsp_filtercheck_k8s.cpp +++ b/userspace/libsinsp/sinsp_filtercheck_k8s.cpp @@ -76,9 +76,9 @@ sinsp_filter_check_k8s::sinsp_filter_check_k8s() m_info.m_flags = filter_check_info::FL_NONE; } -sinsp_filter_check* sinsp_filter_check_k8s::allocate_new() +std::unique_ptr sinsp_filter_check_k8s::allocate_new() { - return (sinsp_filter_check*) new sinsp_filter_check_k8s(); + return std::make_unique(); } int32_t sinsp_filter_check_k8s::parse_field_name(const char* str, bool alloc_state, bool needed_for_filtering) diff --git a/userspace/libsinsp/sinsp_filtercheck_k8s.h b/userspace/libsinsp/sinsp_filtercheck_k8s.h index 01ca64031f..d50d5766ca 100644 --- a/userspace/libsinsp/sinsp_filtercheck_k8s.h +++ b/userspace/libsinsp/sinsp_filtercheck_k8s.h @@ -60,7 +60,7 @@ class sinsp_filter_check_k8s : public sinsp_filter_check sinsp_filter_check_k8s(); virtual ~sinsp_filter_check_k8s() = default; - sinsp_filter_check* allocate_new() override; + std::unique_ptr allocate_new() override; int32_t parse_field_name(const char* str, bool alloc_state, bool needed_for_filtering) override; protected: diff --git a/userspace/libsinsp/sinsp_filtercheck_mesos.cpp b/userspace/libsinsp/sinsp_filtercheck_mesos.cpp index 99492e26c1..08872d374f 100644 --- a/userspace/libsinsp/sinsp_filtercheck_mesos.cpp +++ b/userspace/libsinsp/sinsp_filtercheck_mesos.cpp @@ -54,9 +54,9 @@ sinsp_filter_check_mesos::sinsp_filter_check_mesos() m_info.m_flags = filter_check_info::FL_NONE; } -sinsp_filter_check* sinsp_filter_check_mesos::allocate_new() +std::unique_ptr sinsp_filter_check_mesos::allocate_new() { - return (sinsp_filter_check*) new sinsp_filter_check_mesos(); + return std::make_unique(); } int32_t sinsp_filter_check_mesos::parse_field_name(const char* str, bool alloc_state, bool needed_for_filtering) diff --git a/userspace/libsinsp/sinsp_filtercheck_mesos.h b/userspace/libsinsp/sinsp_filtercheck_mesos.h index 282cb7e5a7..d144ade6a2 100644 --- a/userspace/libsinsp/sinsp_filtercheck_mesos.h +++ b/userspace/libsinsp/sinsp_filtercheck_mesos.h @@ -42,7 +42,7 @@ class sinsp_filter_check_mesos : public sinsp_filter_check sinsp_filter_check_mesos(); virtual ~sinsp_filter_check_mesos() = default; - sinsp_filter_check* allocate_new() override; + std::unique_ptr allocate_new() override; int32_t parse_field_name(const char* str, bool alloc_state, bool needed_for_filtering) override; protected: diff --git a/userspace/libsinsp/sinsp_filtercheck_rawstring.cpp b/userspace/libsinsp/sinsp_filtercheck_rawstring.cpp index b05597a318..8857f16a49 100644 --- a/userspace/libsinsp/sinsp_filtercheck_rawstring.cpp +++ b/userspace/libsinsp/sinsp_filtercheck_rawstring.cpp @@ -34,10 +34,10 @@ rawstring_check::rawstring_check(string text) set_text(text); } -sinsp_filter_check* rawstring_check::allocate_new() +std::unique_ptr rawstring_check::allocate_new() { ASSERT(false); - return NULL; + return nullptr; } void rawstring_check::set_text(string text) diff --git a/userspace/libsinsp/sinsp_filtercheck_rawstring.h b/userspace/libsinsp/sinsp_filtercheck_rawstring.h index e007debeb0..04d22f049d 100644 --- a/userspace/libsinsp/sinsp_filtercheck_rawstring.h +++ b/userspace/libsinsp/sinsp_filtercheck_rawstring.h @@ -26,7 +26,7 @@ class rawstring_check : public sinsp_filter_check rawstring_check(std::string text); virtual ~rawstring_check() = default; - sinsp_filter_check* allocate_new() override; + std::unique_ptr allocate_new() override; int32_t parse_field_name(const char* str, bool alloc_state, bool needed_for_filtering) override; uint8_t* extract(sinsp_evt*, OUT uint32_t* len, bool sanitize_strings = true) override; diff --git a/userspace/libsinsp/sinsp_filtercheck_reference.cpp b/userspace/libsinsp/sinsp_filtercheck_reference.cpp index e5b11a6fa9..ef8ea08fe2 100644 --- a/userspace/libsinsp/sinsp_filtercheck_reference.cpp +++ b/userspace/libsinsp/sinsp_filtercheck_reference.cpp @@ -35,10 +35,10 @@ sinsp_filter_check_reference::sinsp_filter_check_reference() m_field = &m_finfo; } -sinsp_filter_check* sinsp_filter_check_reference::allocate_new() +std::unique_ptr sinsp_filter_check_reference::allocate_new() { ASSERT(false); - return NULL; + return nullptr; } int32_t sinsp_filter_check_reference::parse_field_name(const char* str, bool alloc_state, bool needed_for_filtering) diff --git a/userspace/libsinsp/sinsp_filtercheck_reference.h b/userspace/libsinsp/sinsp_filtercheck_reference.h index 1da08a1da8..dfd5637e92 100644 --- a/userspace/libsinsp/sinsp_filtercheck_reference.h +++ b/userspace/libsinsp/sinsp_filtercheck_reference.h @@ -32,7 +32,7 @@ class sinsp_filter_check_reference : public sinsp_filter_check sinsp_filter_check_reference(); virtual ~sinsp_filter_check_reference() = default; - sinsp_filter_check* allocate_new() override; + std::unique_ptr allocate_new() override; int32_t parse_field_name(const char* str, bool alloc_state, bool needed_for_filtering) override; uint8_t* extract(sinsp_evt*, OUT uint32_t* len, bool sanitize_strings = true) override; diff --git a/userspace/libsinsp/sinsp_filtercheck_syslog.cpp b/userspace/libsinsp/sinsp_filtercheck_syslog.cpp index 2a2b7dafab..8ba47dca7b 100644 --- a/userspace/libsinsp/sinsp_filtercheck_syslog.cpp +++ b/userspace/libsinsp/sinsp_filtercheck_syslog.cpp @@ -57,9 +57,9 @@ sinsp_filter_check_syslog::sinsp_filter_check_syslog() m_info.m_nfields = sizeof(sinsp_filter_check_syslog_fields) / sizeof(sinsp_filter_check_syslog_fields[0]); } -sinsp_filter_check* sinsp_filter_check_syslog::allocate_new() +std::unique_ptr sinsp_filter_check_syslog::allocate_new() { - return (sinsp_filter_check*) new sinsp_filter_check_syslog(); + return std::make_unique(); } uint8_t* sinsp_filter_check_syslog::extract(sinsp_evt *evt, OUT uint32_t* len, bool sanitize_strings) diff --git a/userspace/libsinsp/sinsp_filtercheck_syslog.h b/userspace/libsinsp/sinsp_filtercheck_syslog.h index 707da263c9..310dc90ce0 100644 --- a/userspace/libsinsp/sinsp_filtercheck_syslog.h +++ b/userspace/libsinsp/sinsp_filtercheck_syslog.h @@ -35,7 +35,7 @@ class sinsp_filter_check_syslog : public sinsp_filter_check sinsp_filter_check_syslog(); virtual ~sinsp_filter_check_syslog() = default; - sinsp_filter_check* allocate_new() override; + std::unique_ptr allocate_new() override; protected: uint8_t* extract(sinsp_evt*, OUT uint32_t* len, bool sanitize_strings = true) override; diff --git a/userspace/libsinsp/sinsp_filtercheck_thread.cpp b/userspace/libsinsp/sinsp_filtercheck_thread.cpp index c3aa3815d2..41f5754de9 100644 --- a/userspace/libsinsp/sinsp_filtercheck_thread.cpp +++ b/userspace/libsinsp/sinsp_filtercheck_thread.cpp @@ -139,9 +139,9 @@ sinsp_filter_check_thread::sinsp_filter_check_thread() m_u64val = 0; } -sinsp_filter_check* sinsp_filter_check_thread::allocate_new() +std::unique_ptr sinsp_filter_check_thread::allocate_new() { - return (sinsp_filter_check*) new sinsp_filter_check_thread(); + return std::make_unique(); } int32_t sinsp_filter_check_thread::extract_arg(std::string fldname, std::string val, OUT const ppm_param_info** parinfo) diff --git a/userspace/libsinsp/sinsp_filtercheck_thread.h b/userspace/libsinsp/sinsp_filtercheck_thread.h index 3db3c94248..7f519edb28 100644 --- a/userspace/libsinsp/sinsp_filtercheck_thread.h +++ b/userspace/libsinsp/sinsp_filtercheck_thread.h @@ -111,7 +111,7 @@ class sinsp_filter_check_thread : public sinsp_filter_check sinsp_filter_check_thread(); virtual ~sinsp_filter_check_thread() = default; - sinsp_filter_check* allocate_new() override; + std::unique_ptr allocate_new() override; int32_t parse_field_name(const char* str, bool alloc_state, bool needed_for_filtering) override; int32_t get_argid() const; diff --git a/userspace/libsinsp/sinsp_filtercheck_tracer.cpp b/userspace/libsinsp/sinsp_filtercheck_tracer.cpp index b65dab32cd..924b4493fe 100644 --- a/userspace/libsinsp/sinsp_filtercheck_tracer.cpp +++ b/userspace/libsinsp/sinsp_filtercheck_tracer.cpp @@ -66,9 +66,9 @@ sinsp_filter_check_tracer::sinsp_filter_check_tracer() m_info.m_nfields = sizeof(sinsp_filter_check_tracer_fields) / sizeof(sinsp_filter_check_tracer_fields[0]); } -sinsp_filter_check* sinsp_filter_check_tracer::allocate_new() +std::unique_ptr sinsp_filter_check_tracer::allocate_new() { - return (sinsp_filter_check*) new sinsp_filter_check_tracer(); + return std::make_unique(); } int32_t sinsp_filter_check_tracer::extract_arg(string fldname, string val, OUT const ppm_param_info** parinfo) diff --git a/userspace/libsinsp/sinsp_filtercheck_tracer.h b/userspace/libsinsp/sinsp_filtercheck_tracer.h index 643c7470b2..6f5207bb4c 100644 --- a/userspace/libsinsp/sinsp_filtercheck_tracer.h +++ b/userspace/libsinsp/sinsp_filtercheck_tracer.h @@ -50,7 +50,7 @@ class sinsp_filter_check_tracer : public sinsp_filter_check sinsp_filter_check_tracer(); virtual ~sinsp_filter_check_tracer() = default; - sinsp_filter_check* allocate_new() override; + std::unique_ptr allocate_new() override; int32_t parse_field_name(const char* str, bool alloc_state, bool needed_for_filtering) override; protected: diff --git a/userspace/libsinsp/sinsp_filtercheck_user.cpp b/userspace/libsinsp/sinsp_filtercheck_user.cpp index b226b3a99b..20a89f2ccc 100644 --- a/userspace/libsinsp/sinsp_filtercheck_user.cpp +++ b/userspace/libsinsp/sinsp_filtercheck_user.cpp @@ -59,9 +59,9 @@ sinsp_filter_check_user::sinsp_filter_check_user() m_info.m_flags = filter_check_info::FL_NONE; } -sinsp_filter_check* sinsp_filter_check_user::allocate_new() +std::unique_ptr sinsp_filter_check_user::allocate_new() { - return (sinsp_filter_check*) new sinsp_filter_check_user(); + return std::make_unique(); } uint8_t* sinsp_filter_check_user::extract(sinsp_evt *evt, OUT uint32_t* len, bool sanitize_strings) diff --git a/userspace/libsinsp/sinsp_filtercheck_user.h b/userspace/libsinsp/sinsp_filtercheck_user.h index c92da418f8..bb323e6732 100644 --- a/userspace/libsinsp/sinsp_filtercheck_user.h +++ b/userspace/libsinsp/sinsp_filtercheck_user.h @@ -36,7 +36,7 @@ class sinsp_filter_check_user : public sinsp_filter_check sinsp_filter_check_user(); virtual ~sinsp_filter_check_user() = default; - sinsp_filter_check* allocate_new() override; + std::unique_ptr allocate_new() override; protected: uint8_t* extract(sinsp_evt*, OUT uint32_t* len, bool sanitize_strings = true) override; diff --git a/userspace/libsinsp/sinsp_filtercheck_utils.cpp b/userspace/libsinsp/sinsp_filtercheck_utils.cpp index 39c79e1bdf..68c4f9857a 100644 --- a/userspace/libsinsp/sinsp_filtercheck_utils.cpp +++ b/userspace/libsinsp/sinsp_filtercheck_utils.cpp @@ -42,9 +42,9 @@ sinsp_filter_check_utils::sinsp_filter_check_utils() m_cnt = 0; } -sinsp_filter_check* sinsp_filter_check_utils::allocate_new() +std::unique_ptr sinsp_filter_check_utils::allocate_new() { - return (sinsp_filter_check*) new sinsp_filter_check_utils(); + return std::make_unique(); } uint8_t* sinsp_filter_check_utils::extract(sinsp_evt *evt, OUT uint32_t* len, bool sanitize_strings) diff --git a/userspace/libsinsp/sinsp_filtercheck_utils.h b/userspace/libsinsp/sinsp_filtercheck_utils.h index 32785048e4..68fb687e00 100644 --- a/userspace/libsinsp/sinsp_filtercheck_utils.h +++ b/userspace/libsinsp/sinsp_filtercheck_utils.h @@ -31,7 +31,7 @@ class sinsp_filter_check_utils : public sinsp_filter_check sinsp_filter_check_utils(); virtual ~sinsp_filter_check_utils() = default; - sinsp_filter_check* allocate_new() override; + std::unique_ptr allocate_new() override; protected: uint8_t* extract(sinsp_evt*, OUT uint32_t* len, bool sanitize_strings = true) override; From 0ff6cb8d52efc5e99b410651eb5d2bd29f9deb3d Mon Sep 17 00:00:00 2001 From: Jason Dellaluce Date: Fri, 9 Feb 2024 17:52:08 +0000 Subject: [PATCH 02/10] refactor(userspace/libsinsp)!: adopt unique_ptrs in filtercheck lists Signed-off-by: Jason Dellaluce --- userspace/chisel/chisel_api.cpp | 4 +- userspace/chisel/chisel_table.cpp | 8 +-- userspace/libsinsp/eventformatter.cpp | 24 +++----- userspace/libsinsp/eventformatter.h | 4 +- userspace/libsinsp/filter.cpp | 2 +- userspace/libsinsp/filter_check_list.cpp | 57 ++++++++----------- userspace/libsinsp/filter_check_list.h | 9 +-- userspace/libsinsp/plugin.cpp | 4 +- userspace/libsinsp/plugin.h | 2 +- userspace/libsinsp/sinsp.cpp | 4 +- userspace/libsinsp/sinsp.h | 2 +- .../libsinsp/test/sinsp_with_test_input.cpp | 1 - 12 files changed, 51 insertions(+), 70 deletions(-) diff --git a/userspace/chisel/chisel_api.cpp b/userspace/chisel/chisel_api.cpp index 55c30ef7e3..66f32589ed 100644 --- a/userspace/chisel/chisel_api.cpp +++ b/userspace/chisel/chisel_api.cpp @@ -319,9 +319,9 @@ int lua_cbacks::request_field(lua_State *ls) throw sinsp_exception("chisel error"); } - sinsp_filter_check* chk = s_filterlist.new_filter_check_from_fldname(fld, + auto chk = s_filterlist.new_filter_check_from_fldname(fld, inspector, - false); + false).release(); if(chk == NULL) { diff --git a/userspace/chisel/chisel_table.cpp b/userspace/chisel/chisel_table.cpp index 49937ab123..9dac28487e 100644 --- a/userspace/chisel/chisel_table.cpp +++ b/userspace/chisel/chisel_table.cpp @@ -167,9 +167,9 @@ void chisel_table::configure(vector* entries, const str for(auto vit : *entries) { - sinsp_filter_check* chk = s_filterlist.new_filter_check_from_fldname(vit.get_field(m_view_depth), + auto chk = s_filterlist.new_filter_check_from_fldname(vit.get_field(m_view_depth), m_inspector, - false); + false).release(); if(chk == NULL) { @@ -209,9 +209,9 @@ void chisel_table::configure(vector* entries, const str } else { - sinsp_filter_check* chk = s_filterlist.new_filter_check_from_fldname("util.cnt", + auto chk = s_filterlist.new_filter_check_from_fldname("util.cnt", m_inspector, - false); + false).release(); if(chk == NULL) { diff --git a/userspace/libsinsp/eventformatter.cpp b/userspace/libsinsp/eventformatter.cpp index b89855e517..bf899ce409 100644 --- a/userspace/libsinsp/eventformatter.cpp +++ b/userspace/libsinsp/eventformatter.cpp @@ -53,16 +53,6 @@ sinsp_evt_formatter::sinsp_evt_formatter(sinsp* inspector, set_format(of, fmt); } -sinsp_evt_formatter::~sinsp_evt_formatter() -{ - uint32_t j; - - for(j = 0; j < m_chks_to_free.size(); j++) - { - delete m_chks_to_free[j]; - } -} - void sinsp_evt_formatter::set_format(output_format of, const std::string& fmt) { uint32_t j; @@ -109,7 +99,7 @@ void sinsp_evt_formatter::set_format(output_format of, const std::string& fmt) rawstring_check* newtkn = new rawstring_check(lfmt.substr(last_nontoken_str_start, j - last_nontoken_str_start)); m_tokens.emplace_back(std::make_pair("", newtkn)); m_tokenlens.push_back(0); - m_chks_to_free.push_back(newtkn); + m_chks_to_free.push_back(std::unique_ptr(newtkn)); } if(j == lfmtlen - 1) @@ -148,26 +138,26 @@ void sinsp_evt_formatter::set_format(output_format of, const std::string& fmt) } } - sinsp_filter_check* chk = m_available_checks.new_filter_check_from_fldname(std::string(cfmt + j + 1), + auto chk = m_available_checks.new_filter_check_from_fldname(std::string(cfmt + j + 1), m_inspector, false); - if(chk == NULL) + if(chk == nullptr) { throw sinsp_exception("invalid formatting token " + std::string(cfmt + j + 1)); } - m_chks_to_free.push_back(chk); - const char * fstart = cfmt + j + 1; uint32_t fsize = chk->parse_field_name(fstart, true, false); j += fsize; ASSERT(j <= lfmt.length()); - m_tokens.emplace_back(std::make_pair(std::string(fstart, fsize), chk)); + m_tokens.emplace_back(std::make_pair(std::string(fstart, fsize), chk.get())); m_tokenlens.push_back(toklen); + m_chks_to_free.push_back(std::move(chk)); + last_nontoken_str_start = j + 1; } } @@ -176,7 +166,7 @@ void sinsp_evt_formatter::set_format(output_format of, const std::string& fmt) { sinsp_filter_check * chk = new rawstring_check(lfmt.substr(last_nontoken_str_start, j - last_nontoken_str_start)); m_tokens.emplace_back(std::make_pair("", chk)); - m_chks_to_free.push_back(chk); + m_chks_to_free.emplace_back(chk); m_tokenlens.push_back(0); } } diff --git a/userspace/libsinsp/eventformatter.h b/userspace/libsinsp/eventformatter.h index 78f73684b1..8c1202a409 100644 --- a/userspace/libsinsp/eventformatter.h +++ b/userspace/libsinsp/eventformatter.h @@ -55,7 +55,7 @@ class SINSP_PUBLIC sinsp_evt_formatter sinsp_evt_formatter(sinsp* inspector, const std::string& fmt, filter_check_list &available_checks); - virtual ~sinsp_evt_formatter(); + virtual ~sinsp_evt_formatter() = default; /*! \brief Resolve all the formatted tokens and return them in a key/value @@ -119,7 +119,7 @@ class SINSP_PUBLIC sinsp_evt_formatter sinsp* m_inspector; filter_check_list &m_available_checks; bool m_require_all_values; - std::vector m_chks_to_free; + std::vector> m_chks_to_free; Json::Value m_root; Json::FastWriter m_writer; diff --git a/userspace/libsinsp/filter.cpp b/userspace/libsinsp/filter.cpp index 7becb014db..a4d345957e 100644 --- a/userspace/libsinsp/filter.cpp +++ b/userspace/libsinsp/filter.cpp @@ -531,7 +531,7 @@ sinsp_filter_check *sinsp_filter_factory::new_filtercheck(const char *fldname) { return m_available_checks.new_filter_check_from_fldname(fldname, m_inspector, - true); + true).release(); } std::list sinsp_filter_factory::get_fields() const diff --git a/userspace/libsinsp/filter_check_list.cpp b/userspace/libsinsp/filter_check_list.cpp index 456e3de321..f3768e5df7 100644 --- a/userspace/libsinsp/filter_check_list.cpp +++ b/userspace/libsinsp/filter_check_list.cpp @@ -31,48 +31,39 @@ using namespace std; // sinsp_filter_check_list implementation /////////////////////////////////////////////////////////////////////////////// -filter_check_list::~filter_check_list() -{ - for(auto *chk : m_check_list) - { - delete chk; - } -} - -void filter_check_list::add_filter_check(sinsp_filter_check* filter_check) +void filter_check_list::add_filter_check(std::unique_ptr filter_check) { // If a filtercheck already exists with this name and // shortdesc, don't add it--this can occur when plugins are // loaded and set up sinsp_filter_checks to handle plugin // events. - for(auto *chk : m_check_list) + for(const auto& chk : m_check_list) { if(chk->get_fields()->m_name == filter_check->get_fields()->m_name && chk->get_fields()->m_shortdesc == filter_check->get_fields()->m_shortdesc) { - delete filter_check; return; } } - m_check_list.push_back(filter_check); + m_check_list.push_back(std::move(filter_check)); } void filter_check_list::get_all_fields(std::vector& list) const { - for(auto *chk : m_check_list) + for(const auto& chk : m_check_list) { list.push_back(chk->get_fields()); } } /* Craft a new filter check from the field name */ -sinsp_filter_check* filter_check_list::new_filter_check_from_fldname(const std::string& name, +std::unique_ptr filter_check_list::new_filter_check_from_fldname(const std::string& name, sinsp* inspector, - bool do_exact_check) + bool do_exact_check) const { - for(auto *chk : m_check_list) + for(const auto& chk : m_check_list) { chk->m_inspector = inspector; @@ -88,7 +79,7 @@ sinsp_filter_check* filter_check_list::new_filter_check_from_fldname(const std:: } } - sinsp_filter_check* newchk = chk->allocate_new(); + auto newchk = chk->allocate_new(); newchk->set_inspector(inspector); return newchk; } @@ -99,7 +90,7 @@ sinsp_filter_check* filter_check_list::new_filter_check_from_fldname(const std:: // it's very likely that you've forgotten to add your filter to the list in // the constructor // - return NULL; + return nullptr; } sinsp_filter_check_list::sinsp_filter_check_list() @@ -107,19 +98,19 @@ sinsp_filter_check_list::sinsp_filter_check_list() ////////////////////////////////////////////////////////////////////////////// // ADD NEW FILTER CHECK CLASSES HERE ////////////////////////////////////////////////////////////////////////////// - add_filter_check(new sinsp_filter_check_gen_event()); - add_filter_check(new sinsp_filter_check_event()); - add_filter_check(new sinsp_filter_check_thread()); - add_filter_check(new sinsp_filter_check_user()); - add_filter_check(new sinsp_filter_check_group()); - add_filter_check(new sinsp_filter_check_container()); - add_filter_check(new sinsp_filter_check_fd()); - add_filter_check(new sinsp_filter_check_fspath()); - add_filter_check(new sinsp_filter_check_syslog()); - add_filter_check(new sinsp_filter_check_utils()); - add_filter_check(new sinsp_filter_check_fdlist()); - add_filter_check(new sinsp_filter_check_k8s()); - add_filter_check(new sinsp_filter_check_mesos()); - add_filter_check(new sinsp_filter_check_tracer()); - add_filter_check(new sinsp_filter_check_evtin()); + add_filter_check(std::make_unique()); + add_filter_check(std::make_unique()); + add_filter_check(std::make_unique()); + add_filter_check(std::make_unique()); + add_filter_check(std::make_unique()); + add_filter_check(std::make_unique()); + add_filter_check(std::make_unique()); + add_filter_check(std::make_unique()); + add_filter_check(std::make_unique()); + add_filter_check(std::make_unique()); + add_filter_check(std::make_unique()); + add_filter_check(std::make_unique()); + add_filter_check(std::make_unique()); + add_filter_check(std::make_unique()); + add_filter_check(std::make_unique()); } diff --git a/userspace/libsinsp/filter_check_list.h b/userspace/libsinsp/filter_check_list.h index fad0fccb85..d3f4166156 100644 --- a/userspace/libsinsp/filter_check_list.h +++ b/userspace/libsinsp/filter_check_list.h @@ -33,13 +33,14 @@ class filter_check_list { public: filter_check_list() = default; - virtual ~filter_check_list(); - void add_filter_check(sinsp_filter_check* filter_check); + virtual ~filter_check_list() = default; + + void add_filter_check(std::unique_ptr filter_check); void get_all_fields(std::vector&) const; - sinsp_filter_check* new_filter_check_from_fldname(const std::string& name, sinsp* inspector, bool do_exact_check); + std::unique_ptr new_filter_check_from_fldname(const std::string& name, sinsp* inspector, bool do_exact_check) const; protected: - std::vector m_check_list; + std::vector> m_check_list; }; // diff --git a/userspace/libsinsp/plugin.cpp b/userspace/libsinsp/plugin.cpp index 3fc5bc452b..2d73e72b5c 100755 --- a/userspace/libsinsp/plugin.cpp +++ b/userspace/libsinsp/plugin.cpp @@ -872,9 +872,9 @@ std::vector sinsp_plugin::list_open_params() const /** Field Extraction CAP **/ -sinsp_filter_check* sinsp_plugin::new_filtercheck(std::shared_ptr plugin) +std::unique_ptr sinsp_plugin::new_filtercheck(std::shared_ptr plugin) { - return new sinsp_filter_check_plugin(plugin); + return std::make_unique(plugin); } bool sinsp_plugin::extract_fields(sinsp_evt* evt, uint32_t num_fields, ss_plugin_extract_field *fields) const diff --git a/userspace/libsinsp/plugin.h b/userspace/libsinsp/plugin.h index ec0d371138..d6361c4537 100755 --- a/userspace/libsinsp/plugin.h +++ b/userspace/libsinsp/plugin.h @@ -80,7 +80,7 @@ class sinsp_plugin * * todo(jasondellaluce): make this return a unique_ptr */ - static sinsp_filter_check* new_filtercheck(std::shared_ptr plugin); + static std::unique_ptr new_filtercheck(std::shared_ptr plugin); /** * @brief Returns true if the source is compatible with the given set diff --git a/userspace/libsinsp/sinsp.cpp b/userspace/libsinsp/sinsp.cpp index 84648eb9ad..57b82b0884 100644 --- a/userspace/libsinsp/sinsp.cpp +++ b/userspace/libsinsp/sinsp.cpp @@ -1784,9 +1784,9 @@ std::shared_ptr sinsp::get_sinsp_stats_v2() const return m_sinsp_stats_v2; } -sinsp_filter_check* sinsp::new_generic_filtercheck() +std::unique_ptr sinsp::new_generic_filtercheck() { - return new sinsp_filter_check_gen_event(); + return std::make_unique(); } void sinsp::get_capture_stats(scap_stats* stats) const diff --git a/userspace/libsinsp/sinsp.h b/userspace/libsinsp/sinsp.h index ad4687911d..c2815f1127 100644 --- a/userspace/libsinsp/sinsp.h +++ b/userspace/libsinsp/sinsp.h @@ -441,7 +441,7 @@ class SINSP_PUBLIC sinsp : public capture_stats_source \brief Returns a new instance of a filtercheck supporting fields for a generic event source (e.g. evt.num, evt.time, evt.pluginname...) */ - static sinsp_filter_check* new_generic_filtercheck(); + static std::unique_ptr new_generic_filtercheck(); /*! \brief Return information about the machine generating the events. diff --git a/userspace/libsinsp/test/sinsp_with_test_input.cpp b/userspace/libsinsp/test/sinsp_with_test_input.cpp index f72c869cd3..6168bf6ce7 100644 --- a/userspace/libsinsp/test/sinsp_with_test_input.cpp +++ b/userspace/libsinsp/test/sinsp_with_test_input.cpp @@ -431,7 +431,6 @@ bool sinsp_with_test_input::field_exists(sinsp_evt* evt, const std::string& fiel if(new_fl != nullptr) { // if we can create a filter check it means that the field exists - delete new_fl; return true; } else From 909b47b1b3fb872212ad8a3a343d498b23019beb Mon Sep 17 00:00:00 2001 From: Jason Dellaluce Date: Fri, 9 Feb 2024 18:57:48 +0000 Subject: [PATCH 03/10] refactor(userspace/libsinsp)!: adopt unique_ptrs in filters Signed-off-by: Jason Dellaluce --- userspace/chisel/chisel.cpp | 2 +- userspace/chisel/chisel_api.cpp | 11 +-- userspace/chisel/chisel_table.cpp | 2 +- userspace/libsinsp/eventformatter.cpp | 2 +- userspace/libsinsp/filter.cpp | 76 ++++++------------- userspace/libsinsp/filter.h | 28 +++---- userspace/libsinsp/sinsp.cpp | 10 +-- userspace/libsinsp/sinsp.h | 4 +- userspace/libsinsp/test/filter_compiler.h | 3 +- .../libsinsp/test/filter_compiler.ut.cpp | 10 +-- 10 files changed, 55 insertions(+), 93 deletions(-) diff --git a/userspace/chisel/chisel.cpp b/userspace/chisel/chisel.cpp index e4021fcc88..d17be46b91 100644 --- a/userspace/chisel/chisel.cpp +++ b/userspace/chisel/chisel.cpp @@ -211,7 +211,7 @@ void chiselinfo::set_filter(string filterstr) if(filterstr != "") { - m_filter = compiler.compile(); + m_filter = compiler.compile().release(); } } diff --git a/userspace/chisel/chisel_api.cpp b/userspace/chisel/chisel_api.cpp index 66f32589ed..ac81c865ff 100644 --- a/userspace/chisel/chisel_api.cpp +++ b/userspace/chisel/chisel_api.cpp @@ -710,8 +710,8 @@ int lua_cbacks::get_machine_info(lua_State *ls) int lua_cbacks::get_thread_table_int(lua_State *ls, bool include_fds, bool barebone) { uint32_t j; - sinsp_filter_compiler* compiler = NULL; - sinsp_filter* filter = NULL; + std::unique_ptr compiler; + std::unique_ptr filter; sinsp_evt tevt; scap_evt tscapevt; @@ -739,7 +739,7 @@ int lua_cbacks::get_thread_table_int(lua_State *ls, bool include_fds, bool bareb { try { - compiler = new sinsp_filter_compiler(ch->m_inspector, filterstr); + compiler = std::make_unique(ch->m_inspector, filterstr); filter = compiler->compile(); } catch(const sinsp_exception& e) @@ -1079,11 +1079,6 @@ int lua_cbacks::get_thread_table_int(lua_State *ls, bool include_fds, bool bareb return true; }); - if(filter) - { - delete filter; - } - return 1; } diff --git a/userspace/chisel/chisel_table.cpp b/userspace/chisel/chisel_table.cpp index 9dac28487e..dc1132ce06 100644 --- a/userspace/chisel/chisel_table.cpp +++ b/userspace/chisel/chisel_table.cpp @@ -157,7 +157,7 @@ void chisel_table::configure(vector* entries, const str if(filter != "") { sinsp_filter_compiler compiler(m_inspector, filter); - m_filter = compiler.compile(); + m_filter = compiler.compile().release(); } ////////////////////////////////////////////////////////////////////////////////////// diff --git a/userspace/libsinsp/eventformatter.cpp b/userspace/libsinsp/eventformatter.cpp index bf899ce409..26c2c8eb00 100644 --- a/userspace/libsinsp/eventformatter.cpp +++ b/userspace/libsinsp/eventformatter.cpp @@ -164,7 +164,7 @@ void sinsp_evt_formatter::set_format(output_format of, const std::string& fmt) if(last_nontoken_str_start != j) { - sinsp_filter_check * chk = new rawstring_check(lfmt.substr(last_nontoken_str_start, j - last_nontoken_str_start)); + auto chk = new rawstring_check(lfmt.substr(last_nontoken_str_start, j - last_nontoken_str_start)); m_tokens.emplace_back(std::make_pair("", chk)); m_chks_to_free.emplace_back(chk); m_tokenlens.push_back(0); diff --git a/userspace/libsinsp/filter.cpp b/userspace/libsinsp/filter.cpp index a4d345957e..9061657d1f 100644 --- a/userspace/libsinsp/filter.cpp +++ b/userspace/libsinsp/filter.cpp @@ -39,24 +39,9 @@ limitations under the License. /////////////////////////////////////////////////////////////////////////////// // sinsp_filter_expression implementation /////////////////////////////////////////////////////////////////////////////// -sinsp_filter_expression::sinsp_filter_expression() +void sinsp_filter_expression::add_check(std::unique_ptr chk) { - m_parent = NULL; -} - -sinsp_filter_expression::~sinsp_filter_expression() -{ - uint32_t j; - - for(j = 0; j < m_checks.size(); j++) - { - delete m_checks[j]; - } -} - -void sinsp_filter_expression::add_check(sinsp_filter_check* chk) -{ - m_checks.push_back(chk); + m_checks.push_back(std::move(chk)); } bool sinsp_filter_expression::compare(sinsp_evt *evt) @@ -68,7 +53,7 @@ bool sinsp_filter_expression::compare(sinsp_evt *evt) auto size = m_checks.size(); for(size_t j = 0; j < size; j++) { - chk = m_checks[j]; + chk = m_checks[j].get(); ASSERT(chk != NULL); if(j == 0) @@ -165,16 +150,8 @@ int32_t sinsp_filter_expression::get_expr_boolop() const sinsp_filter::sinsp_filter(sinsp *inspector) { m_inspector = inspector; - m_filter = new sinsp_filter_expression(); - m_curexpr = m_filter; -} - -sinsp_filter::~sinsp_filter() -{ - if(m_filter) - { - delete m_filter; - } + m_filter = std::make_unique(); + m_curexpr = m_filter.get(); } void sinsp_filter::push_expression(boolop op) @@ -183,7 +160,7 @@ void sinsp_filter::push_expression(boolop op) newexpr->m_boolop = op; newexpr->m_parent = m_curexpr; - add_check((sinsp_filter_check*)newexpr); + add_check(std::unique_ptr(newexpr)); m_curexpr = newexpr; } @@ -204,9 +181,9 @@ bool sinsp_filter::run(sinsp_evt *evt) return m_filter->compare(evt); } -void sinsp_filter::add_check(sinsp_filter_check* chk) +void sinsp_filter::add_check(std::unique_ptr chk) { - m_curexpr->add_check(chk); + m_curexpr->add_check(std::move(chk)); } /////////////////////////////////////////////////////////////////////////////// @@ -241,7 +218,7 @@ sinsp_filter_compiler::sinsp_filter_compiler( m_flt_ast = fltast; } -sinsp_filter* sinsp_filter_compiler::compile() +std::unique_ptr sinsp_filter_compiler::compile() { // parse filter string on-the-fly if not pre-parsed AST is provided if (m_flt_ast == NULL) @@ -259,11 +236,9 @@ sinsp_filter* sinsp_filter_compiler::compile() } } - // create new filter using factory - auto new_filter = m_factory->new_filter(); - + // create new filter using factory, // setup compiler state and start compilation - m_filter = new_filter; + m_filter = m_factory->new_filter(); m_last_boolop = BO_NONE; m_expect_values = false; try @@ -272,14 +247,12 @@ sinsp_filter* sinsp_filter_compiler::compile() } catch (const sinsp_exception& e) { - delete new_filter; - m_filter = NULL; + m_filter = nullptr; throw e; } // return compiled filter - m_filter = NULL; - return new_filter; + return std::move(m_filter); } void sinsp_filter_compiler::visit(const libsinsp::filter::ast::and_expr* e) @@ -336,11 +309,11 @@ void sinsp_filter_compiler::visit(const libsinsp::filter::ast::unary_check_expr* { m_pos = e->get_pos(); std::string field = create_filtercheck_name(e->field, e->arg); - sinsp_filter_check *check = create_filtercheck(field); - m_filter->add_check(check); + auto check = create_filtercheck(field); check->m_cmpop = str_to_cmpop(e->op); check->m_boolop = m_last_boolop; check->parse_field_name(field.c_str(), true, true); + m_filter->add_check(std::move(check)); } static void add_filtercheck_value(sinsp_filter_check *chk, size_t idx, const std::string& value) @@ -366,8 +339,7 @@ void sinsp_filter_compiler::visit(const libsinsp::filter::ast::binary_check_expr { m_pos = e->get_pos(); std::string field = create_filtercheck_name(e->field, e->arg); - sinsp_filter_check *check = create_filtercheck(field); - m_filter->add_check(check); + auto check = create_filtercheck(field); check->m_cmpop = str_to_cmpop(e->op); check->m_boolop = m_last_boolop; check->parse_field_name(field.c_str(), true, true); @@ -382,8 +354,9 @@ void sinsp_filter_compiler::visit(const libsinsp::filter::ast::binary_check_expr m_expect_values = false; for (size_t i = 0; i < m_field_values.size(); i++) { - add_filtercheck_value(check, i, m_field_values[i]); + add_filtercheck_value(check.get(), i, m_field_values[i]); } + m_filter->add_check(std::move(check)); } void sinsp_filter_compiler::visit(const libsinsp::filter::ast::value_expr* e) @@ -425,9 +398,9 @@ std::string sinsp_filter_compiler::create_filtercheck_name(const std::string& na return fld; } -sinsp_filter_check* sinsp_filter_compiler::create_filtercheck(std::string& field) +std::unique_ptr sinsp_filter_compiler::create_filtercheck(std::string& field) { - sinsp_filter_check *chk = m_factory->new_filtercheck(field.c_str()); + auto chk = m_factory->new_filtercheck(field.c_str()); if(chk == NULL) { throw sinsp_exception("filter_check called with nonexistent field " + field); @@ -521,17 +494,16 @@ sinsp_filter_factory::sinsp_filter_factory(sinsp *inspector, { } -sinsp_filter *sinsp_filter_factory::new_filter() +std::unique_ptr sinsp_filter_factory::new_filter() const { - return new sinsp_filter(m_inspector); + return std::make_unique(m_inspector); } - -sinsp_filter_check *sinsp_filter_factory::new_filtercheck(const char *fldname) +std::unique_ptr sinsp_filter_factory::new_filtercheck(const char *fldname) const { return m_available_checks.new_filter_check_from_fldname(fldname, m_inspector, - true).release(); + true); } std::list sinsp_filter_factory::get_fields() const diff --git a/userspace/libsinsp/filter.h b/userspace/libsinsp/filter.h index 767e53ebbc..90d03b7db3 100644 --- a/userspace/libsinsp/filter.h +++ b/userspace/libsinsp/filter.h @@ -39,8 +39,8 @@ limitations under the License. class sinsp_filter_expression : public sinsp_filter_check { public: - sinsp_filter_expression(); - virtual ~sinsp_filter_expression(); + sinsp_filter_expression() = default; + virtual ~sinsp_filter_expression() = default; // // The following methods are part of the filter check interface but are irrelevant @@ -59,7 +59,7 @@ class sinsp_filter_expression : public sinsp_filter_check bool compare(sinsp_evt*) override; bool extract(sinsp_evt*, std::vector& values, bool sanitize_strings = true) override; - void add_check(sinsp_filter_check* chk); + void add_check(std::unique_ptr chk); // // An expression is consistent if all its checks are of the same type (or/and). @@ -69,8 +69,8 @@ class sinsp_filter_expression : public sinsp_filter_check // int32_t get_expr_boolop() const; - sinsp_filter_expression* m_parent; - std::vector m_checks; + sinsp_filter_expression* m_parent = nullptr; + std::vector> m_checks; }; @@ -81,15 +81,15 @@ class SINSP_PUBLIC sinsp_filter { public: sinsp_filter(sinsp* inspector); - virtual ~sinsp_filter(); + virtual ~sinsp_filter() = default; bool run(sinsp_evt *evt); void push_expression(boolop op); void pop_expression(); - void add_check(sinsp_filter_check* chk); + void add_check(std::unique_ptr chk); - sinsp_filter_expression* m_filter; + std::unique_ptr m_filter; private: sinsp_filter_expression* m_curexpr; @@ -162,9 +162,9 @@ class sinsp_filter_factory virtual ~sinsp_filter_factory() = default; - virtual sinsp_filter* new_filter(); + virtual std::unique_ptr new_filter() const; - virtual sinsp_filter_check* new_filtercheck(const char* fldname); + virtual std::unique_ptr new_filtercheck(const char* fldname) const; virtual std::list get_fields() const; @@ -230,7 +230,9 @@ class SINSP_PUBLIC sinsp_filter_compiler: by it. The pointer is automatically deleted in case of exception. \note Throws a sinsp_exception if the filter syntax is not valid */ - sinsp_filter* compile(); + std::unique_ptr compile(); + + std::shared_ptr get_filter_ast() const { return m_internal_flt_ast; } std::shared_ptr get_filter_ast() { return m_internal_flt_ast; } @@ -246,13 +248,13 @@ class SINSP_PUBLIC sinsp_filter_compiler: void visit(const libsinsp::filter::ast::binary_check_expr*) override; cmpop str_to_cmpop(const std::string& str); std::string create_filtercheck_name(const std::string& name, const std::string& arg); - sinsp_filter_check* create_filtercheck(std::string& field); + std::unique_ptr create_filtercheck(std::string& field); libsinsp::filter::ast::pos_info m_pos; bool m_expect_values; boolop m_last_boolop; std::string m_flt_str; - sinsp_filter* m_filter; + std::unique_ptr m_filter; std::vector m_field_values; std::shared_ptr m_internal_flt_ast; const libsinsp::filter::ast::expr* m_flt_ast; diff --git a/userspace/libsinsp/sinsp.cpp b/userspace/libsinsp/sinsp.cpp index 57b82b0884..7377174100 100644 --- a/userspace/libsinsp/sinsp.cpp +++ b/userspace/libsinsp/sinsp.cpp @@ -785,11 +785,7 @@ void sinsp::close() deinit_state(); - if(m_filter != NULL) - { - delete m_filter; - m_filter = NULL; - } + m_filter.reset(); // unset the meta-event callback to all plugins that support it if (!is_capture() && m_mode != SINSP_MODE_NONE) @@ -1706,7 +1702,7 @@ void sinsp::start_dropping_mode(uint32_t sampling_ratio) } #endif // _WIN32 -void sinsp::set_filter(sinsp_filter* filter) +void sinsp::set_filter(std::unique_ptr filter) { if(m_filter != NULL) { @@ -1714,7 +1710,7 @@ void sinsp::set_filter(sinsp_filter* filter) throw sinsp_exception("filter can only be set once"); } - m_filter = filter; + m_filter = std::move(filter); } void sinsp::set_filter(const std::string& filter) diff --git a/userspace/libsinsp/sinsp.h b/userspace/libsinsp/sinsp.h index c2815f1127..3575373177 100644 --- a/userspace/libsinsp/sinsp.h +++ b/userspace/libsinsp/sinsp.h @@ -300,7 +300,7 @@ class SINSP_PUBLIC sinsp : public capture_stats_source \param filter the runtime filter object */ - void set_filter(sinsp_filter* filter); + void set_filter(std::unique_ptr filter); /*! \brief Return the filter set for this capture. @@ -1173,7 +1173,7 @@ class SINSP_PUBLIC sinsp : public capture_stats_source bool m_print_container_data; uint64_t m_firstevent_ts; - sinsp_filter* m_filter; + std::unique_ptr m_filter; std::string m_filterstring; std::shared_ptr m_internal_flt_ast; diff --git a/userspace/libsinsp/test/filter_compiler.h b/userspace/libsinsp/test/filter_compiler.h index 51b9421ecd..ef703c807c 100644 --- a/userspace/libsinsp/test/filter_compiler.h +++ b/userspace/libsinsp/test/filter_compiler.h @@ -32,12 +32,11 @@ static void filter_compile(sinsp_filter **out, std::string filter) auto f = compiler.compile(); if (!out) { - delete f; FAIL() << "Unexpected successful compilation for: " << filter; } else { - *out = f; + *out = f.release(); } } catch(const sinsp_exception& e) diff --git a/userspace/libsinsp/test/filter_compiler.ut.cpp b/userspace/libsinsp/test/filter_compiler.ut.cpp index 2053b25174..ca07049f5d 100644 --- a/userspace/libsinsp/test/filter_compiler.ut.cpp +++ b/userspace/libsinsp/test/filter_compiler.ut.cpp @@ -57,14 +57,14 @@ class mock_compiler_filter_factory: public sinsp_filter_factory public: mock_compiler_filter_factory(sinsp *inspector): sinsp_filter_factory(inspector, m_filterlist) {} - inline sinsp_filter *new_filter() override + inline std::unique_ptr new_filter() const override { - return new sinsp_filter(m_inspector); + return std::make_unique(m_inspector); } - inline sinsp_filter_check *new_filtercheck(const char *fldname) override + inline std::unique_ptr new_filtercheck(const char *fldname) const override { - return new mock_compiler_filter_check(); + return std::make_unique(); } inline list get_fields() const override @@ -92,7 +92,6 @@ void test_filter_run(bool result, string filter_str) { FAIL() << filter_str << " -> unexpected '" << (result ? "false" : "true") << "' result"; } - delete filter; } catch(const sinsp_exception& e) { @@ -109,7 +108,6 @@ void test_filter_compile( try { auto filter = compiler.compile(); - delete filter; if (expect_fail) { FAIL() << filter_str << " -> expected failure but compilation was successful"; From 3a8fae7a4d6404e9c2cdf1d18cc9adec94078ce9 Mon Sep 17 00:00:00 2001 From: Jason Dellaluce Date: Fri, 9 Feb 2024 19:39:25 +0000 Subject: [PATCH 04/10] chore(userspace/libsinsp)!: remove unused method Signed-off-by: Jason Dellaluce --- userspace/libsinsp/container.h | 8 -------- 1 file changed, 8 deletions(-) diff --git a/userspace/libsinsp/container.h b/userspace/libsinsp/container.h index 18faef5e0d..aa13e07532 100644 --- a/userspace/libsinsp/container.h +++ b/userspace/libsinsp/container.h @@ -222,14 +222,6 @@ class sinsp_container_manager : return engine_lookup == container_lookups->second.end(); } - /** - * \brief get the list of container engines in the inspector - * - * @return a pointer to the list of container engines - */ - std::list>* get_container_engines() { - return &m_container_engines; - } uint64_t m_last_flush_time_ns; std::string container_to_json(const sinsp_container_info& container_info); From 21ed490eb88ab9c2353c0403327ade9399f64eeb Mon Sep 17 00:00:00 2001 From: Jason Dellaluce Date: Fri, 9 Feb 2024 19:40:06 +0000 Subject: [PATCH 05/10] refactor(userspace/libsinsp)!: adopt unique_ptr for heap-allocated class members Signed-off-by: Jason Dellaluce --- userspace/libsinsp/sinsp.cpp | 23 +++---------------- userspace/libsinsp/sinsp.h | 11 +++++---- .../libsinsp/sinsp_filtercheck_event.cpp | 10 +------- userspace/libsinsp/sinsp_filtercheck_event.h | 4 ++-- userspace/libsinsp/test/eventformatter.ut.cpp | 6 ++--- userspace/libsinsp/test/sinsp_stats.ut.cpp | 2 +- userspace/libsinsp/test/state.ut.cpp | 2 +- userspace/libsinsp/user.cpp | 9 +------- userspace/libsinsp/user.h | 4 ++-- 9 files changed, 21 insertions(+), 50 deletions(-) diff --git a/userspace/libsinsp/sinsp.cpp b/userspace/libsinsp/sinsp.cpp index 7377174100..6d1850f694 100644 --- a/userspace/libsinsp/sinsp.cpp +++ b/userspace/libsinsp/sinsp.cpp @@ -84,8 +84,8 @@ sinsp::sinsp(bool static_container, const std::string &static_id, const std::str m_h = NULL; m_parser = NULL; m_is_dumping = false; - m_parser = new sinsp_parser(this); - m_thread_manager = new sinsp_thread_manager(this); + m_parser = std::make_unique(this); + m_thread_manager = std::make_unique(this); m_max_fdtable_size = MAX_FD_TABLE_SIZE; m_containers_purging_scan_time_ns = DEFAULT_INACTIVE_CONTAINER_SCAN_TIME_S * ONE_SECOND_IN_NS; m_usergroups_purging_scan_time_ns = DEFAULT_DELETED_USERS_GROUPS_SCAN_TIME_S * ONE_SECOND_IN_NS; @@ -130,25 +130,13 @@ sinsp::sinsp(bool static_container, const std::string &static_id, const std::str // create state tables registry m_table_registry = std::make_shared(); - m_table_registry->add_table(m_thread_manager); + m_table_registry->add_table(m_thread_manager.get()); } sinsp::~sinsp() { close(); - if(m_parser) - { - delete m_parser; - m_parser = NULL; - } - - if(m_thread_manager) - { - delete m_thread_manager; - m_thread_manager = NULL; - } - m_container_manager.cleanup(); #if !defined(MINIMAL_BUILD) && !defined(__EMSCRIPTEN__) @@ -1931,11 +1919,6 @@ void sinsp::set_max_evt_output_len(uint32_t len) m_max_evt_output_len = len; } -sinsp_parser* sinsp::get_parser() -{ - return m_parser; -} - double sinsp::get_read_progress_file() const { if(m_input_fd != 0) diff --git a/userspace/libsinsp/sinsp.h b/userspace/libsinsp/sinsp.h index 3575373177..2b41714496 100644 --- a/userspace/libsinsp/sinsp.h +++ b/userspace/libsinsp/sinsp.h @@ -857,11 +857,14 @@ class SINSP_PUBLIC sinsp : public capture_stats_source m_get_procs_cpu_from_driver = get_procs_cpu_from_driver; } - sinsp_parser* get_parser(); + sinsp_parser* get_parser() + { + return m_parser.get(); + } inline const sinsp_parser* get_parser() const { - return m_parser; + return m_parser.get(); } /*=============================== PPM_SC set related (ppm_sc.cpp) ===============================*/ @@ -1140,7 +1143,7 @@ class SINSP_PUBLIC sinsp : public capture_stats_source std::vector m_fds_to_remove; uint64_t m_lastevent_ts; // the parsing engine - sinsp_parser* m_parser; + std::unique_ptr m_parser; // the statistics analysis engine std::unique_ptr m_dumper; bool m_is_dumping; @@ -1160,7 +1163,7 @@ class SINSP_PUBLIC sinsp : public capture_stats_source int32_t m_quantization_interval = -1; public: - sinsp_thread_manager* m_thread_manager; + std::unique_ptr m_thread_manager; sinsp_container_manager m_container_manager; diff --git a/userspace/libsinsp/sinsp_filtercheck_event.cpp b/userspace/libsinsp/sinsp_filtercheck_event.cpp index 01e73db264..504839e331 100644 --- a/userspace/libsinsp/sinsp_filtercheck_event.cpp +++ b/userspace/libsinsp/sinsp_filtercheck_event.cpp @@ -132,15 +132,7 @@ sinsp_filter_check_event::sinsp_filter_check_event() m_info.m_fields = sinsp_filter_check_event_fields; m_info.m_nfields = sizeof(sinsp_filter_check_event_fields) / sizeof(sinsp_filter_check_event_fields[0]); m_u64val = 0; - m_converter = new sinsp_filter_check_reference(); -} - -sinsp_filter_check_event::~sinsp_filter_check_event() -{ - if(m_converter != NULL) - { - delete m_converter; - } + m_converter = std::make_unique(); } std::unique_ptr sinsp_filter_check_event::allocate_new() diff --git a/userspace/libsinsp/sinsp_filtercheck_event.h b/userspace/libsinsp/sinsp_filtercheck_event.h index 87a1c4aa9e..3bb348dfc5 100644 --- a/userspace/libsinsp/sinsp_filtercheck_event.h +++ b/userspace/libsinsp/sinsp_filtercheck_event.h @@ -87,7 +87,7 @@ class sinsp_filter_check_event : public sinsp_filter_check }; sinsp_filter_check_event(); - virtual ~sinsp_filter_check_event(); + virtual ~sinsp_filter_check_event() = default; std::unique_ptr allocate_new() override; int32_t parse_field_name(const char* str, bool alloc_state, bool needed_for_filtering) override; @@ -125,5 +125,5 @@ class sinsp_filter_check_event : public sinsp_filter_check // filtercheck_field_info m_customfield; bool m_is_compare; - sinsp_filter_check_reference* m_converter; + std::unique_ptr m_converter; }; diff --git a/userspace/libsinsp/test/eventformatter.ut.cpp b/userspace/libsinsp/test/eventformatter.ut.cpp index 1ac3737218..68ba16e482 100644 --- a/userspace/libsinsp/test/eventformatter.ut.cpp +++ b/userspace/libsinsp/test/eventformatter.ut.cpp @@ -21,6 +21,7 @@ limitations under the License. #include +#include #include #include #include @@ -29,15 +30,14 @@ using namespace std; TEST(eventformatter, get_field_names) { - auto inspector = new sinsp(); + auto inspector = std::make_unique(); sinsp_filter_check_list filterlist; string output = "this is a sample output %proc.name %fd.type %proc.pid"; - sinsp_evt_formatter fmt(inspector, output, filterlist); + sinsp_evt_formatter fmt(inspector.get(), output, filterlist); vector output_fields; fmt.get_field_names(output_fields); ASSERT_EQ(output_fields.size(), 3); ASSERT_NE(find(output_fields.begin(), output_fields.end(), "proc.name"), output_fields.end()); ASSERT_NE(find(output_fields.begin(), output_fields.end(), "fd.type"), output_fields.end()); ASSERT_NE(find(output_fields.begin(), output_fields.end(), "proc.pid"), output_fields.end()); - delete inspector; } diff --git a/userspace/libsinsp/test/sinsp_stats.ut.cpp b/userspace/libsinsp/test/sinsp_stats.ut.cpp index 0615e037e0..64caaa8bf3 100644 --- a/userspace/libsinsp/test/sinsp_stats.ut.cpp +++ b/userspace/libsinsp/test/sinsp_stats.ut.cpp @@ -38,7 +38,7 @@ TEST_F(sinsp_with_test_input, sinsp_stats_v2_resource_utilization) const scap_stats_v2* sinsp_stats_v2_snapshot; auto buffer = m_inspector.get_sinsp_stats_v2_buffer(); auto sinsp_stats_v2_counters = m_inspector.get_sinsp_stats_v2(); - sinsp_thread_manager* thread_manager = m_inspector.m_thread_manager; + sinsp_thread_manager* thread_manager = m_inspector.m_thread_manager.get(); uint32_t flags = 0; sinsp_stats_v2_snapshot = libsinsp::stats::get_sinsp_stats_v2(flags, agent_info, thread_manager, sinsp_stats_v2_counters, buffer, &nstats, &rc); ASSERT_EQ(nstats, 0); diff --git a/userspace/libsinsp/test/state.ut.cpp b/userspace/libsinsp/test/state.ut.cpp index 0ebdfaf722..3dd21001f6 100644 --- a/userspace/libsinsp/test/state.ut.cpp +++ b/userspace/libsinsp/test/state.ut.cpp @@ -305,7 +305,7 @@ TEST(thread_manager, table_access) static const int s_threadinfo_static_fields_count = 20; sinsp inspector; - auto table = static_cast*>(inspector.m_thread_manager); + auto table = static_cast*>(inspector.m_thread_manager.get()); // empty table state and info ASSERT_EQ(table->name(), "threads"); diff --git a/userspace/libsinsp/user.cpp b/userspace/libsinsp/user.cpp index d398736095..4b07b20f83 100644 --- a/userspace/libsinsp/user.cpp +++ b/userspace/libsinsp/user.cpp @@ -123,7 +123,7 @@ sinsp_usergroup_manager::sinsp_usergroup_manager(sinsp* inspector) , m_inspector(inspector) , m_host_root(m_inspector->get_host_root()) #if defined(__linux__) && (defined(HAVE_PWD_H) || defined(HAVE_GRP_H)) - , m_ns_helper(new libsinsp::procfs_utils::ns_helper(m_host_root)) + , m_ns_helper(std::make_unique(m_host_root)) #else , m_ns_helper(nullptr) #endif @@ -133,13 +133,6 @@ sinsp_usergroup_manager::sinsp_usergroup_manager(sinsp* inspector) strlcpy(m_fallback_user.shell, "", sizeof(m_fallback_user.shell)); strlcpy(m_fallback_grp.name, "", sizeof(m_fallback_grp.name)); } - -sinsp_usergroup_manager::~sinsp_usergroup_manager() -{ -#if defined(__linux__) && (defined(HAVE_PWD_H) || defined(HAVE_GRP_H)) - delete m_ns_helper; -#endif -} // clang-format on void sinsp_usergroup_manager::subscribe_container_mgr() diff --git a/userspace/libsinsp/user.h b/userspace/libsinsp/user.h index 48b1d7b02c..a5f4123222 100644 --- a/userspace/libsinsp/user.h +++ b/userspace/libsinsp/user.h @@ -64,7 +64,7 @@ class sinsp_usergroup_manager { public: explicit sinsp_usergroup_manager(sinsp* inspector); - ~sinsp_usergroup_manager(); + ~sinsp_usergroup_manager() = default; // Do not call subscribe_container_mgr() in capture mode, because // events shall not be sent as they will be loaded from capture file. @@ -175,7 +175,7 @@ class sinsp_usergroup_manager scap_groupinfo m_fallback_grp; const std::string &m_host_root; - libsinsp::procfs_utils::ns_helper *m_ns_helper; + std::unique_ptr m_ns_helper; }; #endif // FALCOSECURITY_LIBS_USER_H From 8c2f438690443573073094a3128ea0b77329c740 Mon Sep 17 00:00:00 2001 From: Jason Dellaluce Date: Fri, 9 Feb 2024 20:09:49 +0000 Subject: [PATCH 06/10] refactor(userspace/libsinsp)!: clear up ownership model of thread manager Signed-off-by: Jason Dellaluce --- userspace/libsinsp/container_info.cpp | 2 +- userspace/libsinsp/parsers.cpp | 73 +++++++++---------- userspace/libsinsp/sinsp.cpp | 40 ++++------ userspace/libsinsp/sinsp.h | 6 +- userspace/libsinsp/sinsp_external_processor.h | 2 +- userspace/libsinsp/threadinfo.cpp | 16 ++-- userspace/libsinsp/threadinfo.h | 5 +- 7 files changed, 64 insertions(+), 80 deletions(-) diff --git a/userspace/libsinsp/container_info.cpp b/userspace/libsinsp/container_info.cpp index d16491a320..4608de9a3c 100644 --- a/userspace/libsinsp/container_info.cpp +++ b/userspace/libsinsp/container_info.cpp @@ -150,7 +150,7 @@ const sinsp_container_info::container_mount_info *sinsp_container_info::mount_by std::shared_ptr sinsp_container_info::get_tinfo(sinsp* inspector) const { - std::shared_ptr tinfo(inspector->build_threadinfo()); + std::shared_ptr tinfo(inspector->build_threadinfo().release()); tinfo->m_tid = -1; tinfo->m_pid = -1; tinfo->m_vtid = -2; diff --git a/userspace/libsinsp/parsers.cpp b/userspace/libsinsp/parsers.cpp index af83388f0c..2c67dcfd08 100644 --- a/userspace/libsinsp/parsers.cpp +++ b/userspace/libsinsp/parsers.cpp @@ -1097,15 +1097,15 @@ void sinsp_parser::parse_clone_exit_caller(sinsp_evt *evt, int64_t child_tid) /*=============================== CHILD ALREADY THERE ===========================*/ /* See if the child is already there, if yes and it is valid we return immediately */ - sinsp_threadinfo* child_tinfo = m_inspector->get_thread_ref(child_tid, false, true).get(); - if(child_tinfo != nullptr) + sinsp_threadinfo* existing_child_tinfo = m_inspector->get_thread_ref(child_tid, false, true).get(); + if(existing_child_tinfo != nullptr) { /* If this was an inverted clone, all is fine, we've already taken care * of adding the thread table entry in the child. * Otherwise, we assume that the entry is there because we missed the proc exit event * for a previous thread and we replace the tinfo. */ - if(child_tinfo->m_flags & PPM_CL_CLONE_INVERTED) + if(existing_child_tinfo->m_flags & PPM_CL_CLONE_INVERTED) { return; } @@ -1125,7 +1125,7 @@ void sinsp_parser::parse_clone_exit_caller(sinsp_evt *evt, int64_t child_tid) /* Allocate the new thread info and initialize it. * We avoid `malloc` here and get the item from a preallocated list. */ - child_tinfo = m_inspector->build_threadinfo(); + auto child_tinfo = m_inspector->build_threadinfo(); /* Initialise last exec time to zero (can be overridden in the case of a * thread clone) @@ -1157,7 +1157,7 @@ void sinsp_parser::parse_clone_exit_caller(sinsp_evt *evt, int64_t child_tid) if(fd_table_ptr != NULL) { child_tinfo->get_fdtable().clear(); - fd_table_ptr->const_loop([child_tinfo](int64_t fd, const sinsp_fdinfo& info) { + fd_table_ptr->const_loop([&child_tinfo](int64_t fd, const sinsp_fdinfo& info) { /* Track down that those are cloned fds */ auto newinfo = info.clone(); newinfo->set_is_cloned(); @@ -1355,7 +1355,7 @@ void sinsp_parser::parse_clone_exit_caller(sinsp_evt *evt, int64_t child_tid) case PPME_SYSCALL_CLONE3_X: parinfo = evt->get_param(14); child_tinfo->set_cgroups(parinfo->m_val, parinfo->m_len); - m_inspector->m_container_manager.resolve_container(child_tinfo, m_inspector->is_live() || m_inspector->is_syscall_plugin()); + m_inspector->m_container_manager.resolve_container(child_tinfo.get(), m_inspector->is_live() || m_inspector->is_syscall_plugin()); break; } @@ -1419,20 +1419,25 @@ void sinsp_parser::parse_clone_exit_caller(sinsp_evt *evt, int64_t child_tid) /*=============================== ADD THREAD TO THE TABLE ===========================*/ /* Until we use the shared pointer we need it here, after we can move it at the end */ - bool thread_added = m_inspector->add_thread(child_tinfo); + auto new_child = m_inspector->add_thread(std::move(child_tinfo)); + if (!new_child) + { + // note: we expect the thread manager to log a warning already + return; + } /* Refresh user / loginuser / group */ - if(child_tinfo->m_container_id.empty() == false) + if(new_child->m_container_id.empty() == false) { - child_tinfo->set_user(child_tinfo->m_user.uid); - child_tinfo->set_loginuser(child_tinfo->m_loginuser.uid); - child_tinfo->set_group(child_tinfo->m_group.gid); + new_child->set_user(new_child->m_user.uid); + new_child->set_loginuser(new_child->m_loginuser.uid); + new_child->set_group(new_child->m_group.gid); } /* If there's a listener, invoke it */ if(m_inspector->get_observer()) { - m_inspector->get_observer()->on_clone(evt, child_tinfo, tid_collision); + m_inspector->get_observer()->on_clone(evt, new_child.get(), tid_collision); } /* If we had to erase a previous entry for this tid and rebalance the table, @@ -1444,14 +1449,8 @@ void sinsp_parser::parse_clone_exit_caller(sinsp_evt *evt, int64_t child_tid) reset(evt); DBG_SINSP_INFO("tid collision for %" PRIu64 "(%s)", tid_collision, - child_tinfo->m_comm.c_str()); - } - - if(!thread_added) - { - delete child_tinfo; + new_child->m_comm.c_str()); } - /*=============================== ADD THREAD TO THE TABLE ===========================*/ return; @@ -1510,7 +1509,7 @@ void sinsp_parser::parse_clone_exit_child(sinsp_evt *evt) /* Allocate the new thread info and initialize it. * We must avoid `malloc` here and get the item from a preallocated list. */ - sinsp_threadinfo *child_tinfo = m_inspector->build_threadinfo(); + auto child_tinfo = m_inspector->build_threadinfo(); /* Initialise last exec time to zero (can be overridden in the case of a * thread clone) @@ -1626,7 +1625,6 @@ void sinsp_parser::parse_clone_exit_child(sinsp_evt *evt) { /* Invalidate the thread_info associated with this event */ evt->set_tinfo(nullptr); - delete child_tinfo; return; } @@ -1751,7 +1749,7 @@ void sinsp_parser::parse_clone_exit_child(sinsp_evt *evt) if(fd_table_ptr != NULL) { child_tinfo->get_fdtable().clear(); - fd_table_ptr->loop([child_tinfo](int64_t fd, const sinsp_fdinfo& info) { + fd_table_ptr->loop([&child_tinfo](int64_t fd, const sinsp_fdinfo& info) { /* Track down that those are cloned fds. * This flag `FLAGS_IS_CLONED` seems to be never used... */ @@ -1906,7 +1904,7 @@ void sinsp_parser::parse_clone_exit_child(sinsp_evt *evt) case PPME_SYSCALL_CLONE3_X: parinfo = evt->get_param(14); child_tinfo->set_cgroups(parinfo->m_val, parinfo->m_len); - m_inspector->m_container_manager.resolve_container(child_tinfo, m_inspector->is_live()); + m_inspector->m_container_manager.resolve_container(child_tinfo.get(), m_inspector->is_live()); break; } @@ -1933,20 +1931,26 @@ void sinsp_parser::parse_clone_exit_child(sinsp_evt *evt) /*=============================== CREATE NEW THREAD-INFO ===========================*/ /* Add the new thread to the table */ - bool thread_added = m_inspector->add_thread(child_tinfo); + auto new_child = m_inspector->add_thread(std::move(child_tinfo)); + if (!new_child) + { + // note: we expect the thread manager to log a warning already + evt->set_tinfo(nullptr); + return; + } /* Update the evt->get_tinfo() of the child. * We update it here, in this way the `on_clone` * callback will use updated info. */ - evt->set_tinfo(child_tinfo); + evt->set_tinfo(new_child.get()); /* Refresh user / loginuser / group */ - if(child_tinfo->m_container_id.empty() == false) + if(new_child->m_container_id.empty() == false) { - child_tinfo->set_user(child_tinfo->m_user.uid); - child_tinfo->set_loginuser(child_tinfo->m_loginuser.uid); - child_tinfo->set_group(child_tinfo->m_group.gid); + new_child->set_user(new_child->m_user.uid); + new_child->set_loginuser(new_child->m_loginuser.uid); + new_child->set_group(new_child->m_group.gid); } // @@ -1954,7 +1958,7 @@ void sinsp_parser::parse_clone_exit_child(sinsp_evt *evt) // if(m_inspector->get_observer()) { - m_inspector->get_observer()->on_clone(evt, child_tinfo, tid_collision); + m_inspector->get_observer()->on_clone(evt, new_child.get(), tid_collision); } /* If we had to erase a previous entry for this tid and rebalance the table, @@ -1966,17 +1970,10 @@ void sinsp_parser::parse_clone_exit_child(sinsp_evt *evt) { reset(evt); /* Right now we have collisions only on the clone() caller */ - DBG_SINSP_INFO("tid collision for %" PRIu64 "(%s)", tid_collision, child_tinfo->m_comm.c_str()); - } - - if(!thread_added) - { - evt->set_tinfo(nullptr); - delete child_tinfo; + DBG_SINSP_INFO("tid collision for %" PRIu64 "(%s)", tid_collision, new_child->m_comm.c_str()); } /*=============================== CREATE NEW THREAD-INFO ===========================*/ - return; } diff --git a/userspace/libsinsp/sinsp.cpp b/userspace/libsinsp/sinsp.cpp index 6d1850f694..404854eb54 100644 --- a/userspace/libsinsp/sinsp.cpp +++ b/userspace/libsinsp/sinsp.cpp @@ -846,29 +846,23 @@ void sinsp::on_new_entry_from_proc(void* context, { ASSERT(tinfo != NULL); - bool thread_added = false; - sinsp_threadinfo* newti = build_threadinfo(); + threadinfo_map_t::ptr_t sinsp_tinfo; + auto newti = build_threadinfo(); newti->init(tinfo); if(is_nodriver()) { - auto sinsp_tinfo = find_thread(tid, true); - if(sinsp_tinfo == nullptr || newti->m_clone_ts > sinsp_tinfo->m_clone_ts) + auto existing_tinfo = find_thread(tid, true); + if(existing_tinfo == nullptr || newti->m_clone_ts > existing_tinfo->m_clone_ts) { - thread_added = m_thread_manager->add_thread(newti, true); + sinsp_tinfo = m_thread_manager->add_thread(std::move(newti), true); } } else { - thread_added = m_thread_manager->add_thread(newti, true); + sinsp_tinfo = m_thread_manager->add_thread(std::move(newti), true); } - if (!thread_added) + if (sinsp_tinfo) { - delete newti; - } - else - { - auto sinsp_tinfo = find_thread(tid, true); - // in case the inspector is configured with an internal filter, // we filter out thread infos in case we determine them not passing // the given filter. Filtered out thread infos will not be dumped @@ -932,17 +926,11 @@ void sinsp::on_new_entry_from_proc(void* context, { ASSERT(tinfo != NULL); - sinsp_threadinfo* newti = build_threadinfo(); + auto newti = build_threadinfo(); newti->init(tinfo); - if (!m_thread_manager->add_thread(newti, true)) { - ASSERT(false); - delete newti; - return; - } - - sinsp_tinfo = find_thread(tid, true); - if (!sinsp_tinfo) { + sinsp_tinfo = m_thread_manager->add_thread(std::move(newti), true); + if (sinsp_tinfo == nullptr) { ASSERT(false); return; } @@ -1409,9 +1397,9 @@ threadinfo_map_t::ptr_t sinsp::get_thread_ref(int64_t tid, bool query_os_if_not_ return m_thread_manager->get_thread_ref(tid, query_os_if_not_found, lookup_only, main_thread); } -bool sinsp::add_thread(const sinsp_threadinfo *ptinfo) +std::shared_ptr sinsp::add_thread(std::unique_ptr ptinfo) { - return m_thread_manager->add_thread((sinsp_threadinfo*)ptinfo, false); + return m_thread_manager->add_thread(std::move(ptinfo), false); } void sinsp::remove_thread(int64_t tid) @@ -2103,10 +2091,10 @@ bool sinsp_thread_manager::remove_inactive_threads() return false; } -sinsp_threadinfo* +std::unique_ptr libsinsp::event_processor::build_threadinfo(sinsp* inspector) { - return new sinsp_threadinfo(inspector); + return std::make_unique(inspector); } std::unique_ptr diff --git a/userspace/libsinsp/sinsp.h b/userspace/libsinsp/sinsp.h index 2b41714496..d5f11d0dec 100644 --- a/userspace/libsinsp/sinsp.h +++ b/userspace/libsinsp/sinsp.h @@ -526,10 +526,10 @@ class SINSP_PUBLIC sinsp : public capture_stats_source libsinsp::event_processor* m_external_event_processor; - inline sinsp_threadinfo* build_threadinfo() + inline std::unique_ptr build_threadinfo() { return m_external_event_processor ? m_external_event_processor->build_threadinfo(this) - : m_thread_manager->new_threadinfo().release(); + : m_thread_manager->new_threadinfo(); } inline std::unique_ptr build_fdinfo() @@ -998,7 +998,7 @@ class SINSP_PUBLIC sinsp : public capture_stats_source bool remove_inactive_threads(); - bool add_thread(const sinsp_threadinfo *ptinfo); + std::shared_ptr add_thread(std::unique_ptr ptinfo); void set_mode(sinsp_mode_t value) { diff --git a/userspace/libsinsp/sinsp_external_processor.h b/userspace/libsinsp/sinsp_external_processor.h index f7f5b9c2ab..57bd3b0123 100644 --- a/userspace/libsinsp/sinsp_external_processor.h +++ b/userspace/libsinsp/sinsp_external_processor.h @@ -52,7 +52,7 @@ class event_processor * If this is overridden by the event processor, the processor MUST be registered * before the sinsp object is init-ed */ - virtual sinsp_threadinfo* build_threadinfo(sinsp* inspector); + virtual std::unique_ptr build_threadinfo(sinsp* inspector); /** * Some event processors allocate different fd info types with extra data. diff --git a/userspace/libsinsp/threadinfo.cpp b/userspace/libsinsp/threadinfo.cpp index 94cd17117c..7b1a5be769 100644 --- a/userspace/libsinsp/threadinfo.cpp +++ b/userspace/libsinsp/threadinfo.cpp @@ -1482,7 +1482,7 @@ std::unique_ptr sinsp_thread_manager::new_fdinfo() const * 2. We are doing a proc scan with a callback or without. (`from_scap_proctable==true`) * 3. We are trying to obtain thread info from /proc through `get_thread_ref` */ -bool sinsp_thread_manager::add_thread(sinsp_threadinfo *threadinfo, bool from_scap_proctable) +std::shared_ptr sinsp_thread_manager::add_thread(std::unique_ptr threadinfo, bool from_scap_proctable) { /* We have no more space */ @@ -1500,10 +1500,10 @@ bool sinsp_thread_manager::add_thread(sinsp_threadinfo *threadinfo, bool from_sc } m_inspector->get_sinsp_stats_v2()->m_n_drops_full_threadtable++; } - return false; + return nullptr; } - auto tinfo_shared_ptr = std::shared_ptr(threadinfo); + auto tinfo_shared_ptr = std::shared_ptr(std::move(threadinfo)); if(!from_scap_proctable) { @@ -1520,14 +1520,14 @@ bool sinsp_thread_manager::add_thread(sinsp_threadinfo *threadinfo, bool from_sc } tinfo_shared_ptr->compute_program_hash(); - m_threadtable.put(std::move(tinfo_shared_ptr)); + m_threadtable.put(tinfo_shared_ptr); if (m_inspector != nullptr && m_inspector->get_sinsp_stats_v2()) { m_inspector->get_sinsp_stats_v2()->m_n_added_threads++; } - return true; + return tinfo_shared_ptr; } /* Taken from `find_new_reaper` kernel function: @@ -2055,7 +2055,7 @@ threadinfo_map_t::ptr_t sinsp_thread_manager::get_thread_ref(int64_t tid, bool q scap_proc.ptid = -1; // unfortunately, sinsp owns the threade factory - sinsp_threadinfo* newti = m_inspector->build_threadinfo(); + auto newti = m_inspector->build_threadinfo(); m_n_proc_lookups++; @@ -2118,8 +2118,8 @@ threadinfo_map_t::ptr_t sinsp_thread_manager::get_thread_ref(int64_t tid, bool q // // Done. Add the new thread to the list. // - add_thread(newti, false); - sinsp_proc = find_thread(tid, lookup_only); + add_thread(std::move(newti), false); + sinsp_proc = find_thread(tid, lookup_only); } return sinsp_proc; diff --git a/userspace/libsinsp/threadinfo.h b/userspace/libsinsp/threadinfo.h index 9a682f78d6..a6897b9bb3 100644 --- a/userspace/libsinsp/threadinfo.h +++ b/userspace/libsinsp/threadinfo.h @@ -785,7 +785,7 @@ class SINSP_PUBLIC sinsp_thread_manager: public libsinsp::state::table std::unique_ptr new_fdinfo() const; - bool add_thread(sinsp_threadinfo *threadinfo, bool from_scap_proctable); + threadinfo_map_t::ptr_t add_thread(std::unique_ptr threadinfo, bool from_scap_proctable); sinsp_threadinfo* find_new_reaper(sinsp_threadinfo*); void remove_thread(int64_t tid); // Returns true if the table is actually scanned @@ -887,8 +887,7 @@ class SINSP_PUBLIC sinsp_thread_manager: public libsinsp::state::table } entry.release(); tinfo->m_tid = key; - add_thread(tinfo, false); - return get_entry(key); + return add_thread(std::unique_ptr(tinfo), false); } bool erase_entry(const int64_t& key) override From 27fcdea408401ebce8bd32881056d0953519a82b Mon Sep 17 00:00:00 2001 From: Jason Dellaluce Date: Mon, 19 Feb 2024 17:32:30 +0000 Subject: [PATCH 07/10] chore(userspace/libsinsp): further minor improvements Signed-off-by: Jason Dellaluce --- .../container_engine/container_engine_base.h | 2 ++ userspace/libsinsp/dns_manager.h | 1 + userspace/libsinsp/event.h | 1 + userspace/libsinsp/eventformatter.cpp | 12 ++++++------ userspace/libsinsp/eventformatter.h | 1 + userspace/libsinsp/filter.h | 1 + userspace/libsinsp/filter_check_list.h | 1 + userspace/libsinsp/state/table.h | 1 + userspace/libsinsp/test/eventformatter.ut.cpp | 4 ++-- userspace/libsinsp/threadinfo.cpp | 2 +- userspace/libsinsp/user.h | 1 + 11 files changed, 18 insertions(+), 9 deletions(-) diff --git a/userspace/libsinsp/container_engine/container_engine_base.h b/userspace/libsinsp/container_engine/container_engine_base.h index 0798dd9042..0aa669d2f2 100644 --- a/userspace/libsinsp/container_engine/container_engine_base.h +++ b/userspace/libsinsp/container_engine/container_engine_base.h @@ -18,6 +18,8 @@ limitations under the License. #pragma once +#include + #include class sinsp_threadinfo; diff --git a/userspace/libsinsp/dns_manager.h b/userspace/libsinsp/dns_manager.h index 0a64258169..b03d8afab1 100644 --- a/userspace/libsinsp/dns_manager.h +++ b/userspace/libsinsp/dns_manager.h @@ -28,6 +28,7 @@ limitations under the License. #include #include #include +#include #if !defined(__EMSCRIPTEN__) #include "tbb/concurrent_unordered_map.h" #endif diff --git a/userspace/libsinsp/event.h b/userspace/libsinsp/event.h index 3c8130980a..4475bd985b 100644 --- a/userspace/libsinsp/event.h +++ b/userspace/libsinsp/event.h @@ -21,6 +21,7 @@ limitations under the License. #include #include #include +#include #include diff --git a/userspace/libsinsp/eventformatter.cpp b/userspace/libsinsp/eventformatter.cpp index 26c2c8eb00..0025c67e00 100644 --- a/userspace/libsinsp/eventformatter.cpp +++ b/userspace/libsinsp/eventformatter.cpp @@ -96,10 +96,10 @@ void sinsp_evt_formatter::set_format(output_format of, const std::string& fmt) if(last_nontoken_str_start != j) { - rawstring_check* newtkn = new rawstring_check(lfmt.substr(last_nontoken_str_start, j - last_nontoken_str_start)); - m_tokens.emplace_back(std::make_pair("", newtkn)); + auto newtkn = std::make_unique(lfmt.substr(last_nontoken_str_start, j - last_nontoken_str_start)); + m_tokens.emplace_back(std::make_pair("", newtkn.get())); m_tokenlens.push_back(0); - m_chks_to_free.push_back(std::unique_ptr(newtkn)); + m_chks_to_free.push_back(std::move(newtkn)); } if(j == lfmtlen - 1) @@ -164,9 +164,9 @@ void sinsp_evt_formatter::set_format(output_format of, const std::string& fmt) if(last_nontoken_str_start != j) { - auto chk = new rawstring_check(lfmt.substr(last_nontoken_str_start, j - last_nontoken_str_start)); - m_tokens.emplace_back(std::make_pair("", chk)); - m_chks_to_free.emplace_back(chk); + auto chk = std::make_unique(lfmt.substr(last_nontoken_str_start, j - last_nontoken_str_start)); + m_tokens.emplace_back(std::make_pair("", chk.get())); + m_chks_to_free.emplace_back(std::move(chk)); m_tokenlens.push_back(0); } } diff --git a/userspace/libsinsp/eventformatter.h b/userspace/libsinsp/eventformatter.h index 8c1202a409..49261fb2b6 100644 --- a/userspace/libsinsp/eventformatter.h +++ b/userspace/libsinsp/eventformatter.h @@ -20,6 +20,7 @@ limitations under the License. #include #include #include +#include #include #include diff --git a/userspace/libsinsp/filter.h b/userspace/libsinsp/filter.h index 90d03b7db3..734cf7a00c 100644 --- a/userspace/libsinsp/filter.h +++ b/userspace/libsinsp/filter.h @@ -25,6 +25,7 @@ limitations under the License. #include #include #include +#include /** @defgroup filter Filtering events * Filtering infrastructure. diff --git a/userspace/libsinsp/filter_check_list.h b/userspace/libsinsp/filter_check_list.h index d3f4166156..6ef866eab9 100644 --- a/userspace/libsinsp/filter_check_list.h +++ b/userspace/libsinsp/filter_check_list.h @@ -20,6 +20,7 @@ limitations under the License. #include #include +#include class sinsp_filter_check; class filter_check_info; diff --git a/userspace/libsinsp/state/table.h b/userspace/libsinsp/state/table.h index 001be330da..f60c6efa94 100644 --- a/userspace/libsinsp/state/table.h +++ b/userspace/libsinsp/state/table.h @@ -20,6 +20,7 @@ limitations under the License. #include #include +#include namespace libsinsp { namespace state { diff --git a/userspace/libsinsp/test/eventformatter.ut.cpp b/userspace/libsinsp/test/eventformatter.ut.cpp index 68ba16e482..22f953b3f7 100644 --- a/userspace/libsinsp/test/eventformatter.ut.cpp +++ b/userspace/libsinsp/test/eventformatter.ut.cpp @@ -30,10 +30,10 @@ using namespace std; TEST(eventformatter, get_field_names) { - auto inspector = std::make_unique(); + sinsp inspector; sinsp_filter_check_list filterlist; string output = "this is a sample output %proc.name %fd.type %proc.pid"; - sinsp_evt_formatter fmt(inspector.get(), output, filterlist); + sinsp_evt_formatter fmt(&inspector, output, filterlist); vector output_fields; fmt.get_field_names(output_fields); ASSERT_EQ(output_fields.size(), 3); diff --git a/userspace/libsinsp/threadinfo.cpp b/userspace/libsinsp/threadinfo.cpp index 7b1a5be769..7557fca4e6 100644 --- a/userspace/libsinsp/threadinfo.cpp +++ b/userspace/libsinsp/threadinfo.cpp @@ -2119,7 +2119,7 @@ threadinfo_map_t::ptr_t sinsp_thread_manager::get_thread_ref(int64_t tid, bool q // Done. Add the new thread to the list. // add_thread(std::move(newti), false); - sinsp_proc = find_thread(tid, lookup_only); + sinsp_proc = find_thread(tid, lookup_only); } return sinsp_proc; diff --git a/userspace/libsinsp/user.h b/userspace/libsinsp/user.h index a5f4123222..fa0280ea77 100644 --- a/userspace/libsinsp/user.h +++ b/userspace/libsinsp/user.h @@ -21,6 +21,7 @@ limitations under the License. #include #include +#include #include #include #include From a614e836f2edce6427c17ffbe254cccd0ec50b9f Mon Sep 17 00:00:00 2001 From: Jason Dellaluce Date: Tue, 20 Feb 2024 16:24:53 +0000 Subject: [PATCH 08/10] fix(userspace/libsinsp): solve link issues in sinsp-example Signed-off-by: Jason Dellaluce --- userspace/libsinsp/examples/CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/userspace/libsinsp/examples/CMakeLists.txt b/userspace/libsinsp/examples/CMakeLists.txt index d2278326b3..25a671d0ad 100644 --- a/userspace/libsinsp/examples/CMakeLists.txt +++ b/userspace/libsinsp/examples/CMakeLists.txt @@ -15,6 +15,8 @@ # limitations under the License. # +include(jsoncpp) + add_executable(sinsp-example util.cpp test.cpp @@ -22,6 +24,7 @@ add_executable(sinsp-example target_link_libraries(sinsp-example sinsp + "${JSONCPP_LIB}" ) if (EMSCRIPTEN) From 8b155264cbd8d63bff10f2091bdebf7ac19d94ce Mon Sep 17 00:00:00 2001 From: Jason Dellaluce Date: Wed, 21 Feb 2024 10:32:52 +0000 Subject: [PATCH 09/10] update(userspace/libsinsp): apply reviews suggestions Co-authored-by: Federico Di Pierro Co-authored-by: Mauro Ezequiel Moltrasio Signed-off-by: Jason Dellaluce --- userspace/libsinsp/container.h | 8 ++++++++ userspace/libsinsp/eventformatter.cpp | 6 +++--- userspace/libsinsp/eventformatter.h | 2 +- userspace/libsinsp/sinsp.h | 2 +- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/userspace/libsinsp/container.h b/userspace/libsinsp/container.h index aa13e07532..18faef5e0d 100644 --- a/userspace/libsinsp/container.h +++ b/userspace/libsinsp/container.h @@ -222,6 +222,14 @@ class sinsp_container_manager : return engine_lookup == container_lookups->second.end(); } + /** + * \brief get the list of container engines in the inspector + * + * @return a pointer to the list of container engines + */ + std::list>* get_container_engines() { + return &m_container_engines; + } uint64_t m_last_flush_time_ns; std::string container_to_json(const sinsp_container_info& container_info); diff --git a/userspace/libsinsp/eventformatter.cpp b/userspace/libsinsp/eventformatter.cpp index 0025c67e00..97bfd9d7c4 100644 --- a/userspace/libsinsp/eventformatter.cpp +++ b/userspace/libsinsp/eventformatter.cpp @@ -99,7 +99,7 @@ void sinsp_evt_formatter::set_format(output_format of, const std::string& fmt) auto newtkn = std::make_unique(lfmt.substr(last_nontoken_str_start, j - last_nontoken_str_start)); m_tokens.emplace_back(std::make_pair("", newtkn.get())); m_tokenlens.push_back(0); - m_chks_to_free.push_back(std::move(newtkn)); + m_checks.push_back(std::move(newtkn)); } if(j == lfmtlen - 1) @@ -156,7 +156,7 @@ void sinsp_evt_formatter::set_format(output_format of, const std::string& fmt) m_tokens.emplace_back(std::make_pair(std::string(fstart, fsize), chk.get())); m_tokenlens.push_back(toklen); - m_chks_to_free.push_back(std::move(chk)); + m_checks.push_back(std::move(chk)); last_nontoken_str_start = j + 1; } @@ -166,7 +166,7 @@ void sinsp_evt_formatter::set_format(output_format of, const std::string& fmt) { auto chk = std::make_unique(lfmt.substr(last_nontoken_str_start, j - last_nontoken_str_start)); m_tokens.emplace_back(std::make_pair("", chk.get())); - m_chks_to_free.emplace_back(std::move(chk)); + m_checks.emplace_back(std::move(chk)); m_tokenlens.push_back(0); } } diff --git a/userspace/libsinsp/eventformatter.h b/userspace/libsinsp/eventformatter.h index 49261fb2b6..e28d86b32c 100644 --- a/userspace/libsinsp/eventformatter.h +++ b/userspace/libsinsp/eventformatter.h @@ -120,7 +120,7 @@ class SINSP_PUBLIC sinsp_evt_formatter sinsp* m_inspector; filter_check_list &m_available_checks; bool m_require_all_values; - std::vector> m_chks_to_free; + std::vector> m_checks; Json::Value m_root; Json::FastWriter m_writer; diff --git a/userspace/libsinsp/sinsp.h b/userspace/libsinsp/sinsp.h index d5f11d0dec..eab132b1b7 100644 --- a/userspace/libsinsp/sinsp.h +++ b/userspace/libsinsp/sinsp.h @@ -857,7 +857,7 @@ class SINSP_PUBLIC sinsp : public capture_stats_source m_get_procs_cpu_from_driver = get_procs_cpu_from_driver; } - sinsp_parser* get_parser() + inline sinsp_parser* get_parser() { return m_parser.get(); } From 578d50ce25c104695cbce1ea64e08315280f8e47 Mon Sep 17 00:00:00 2001 From: Jason Dellaluce Date: Wed, 21 Feb 2024 16:17:17 +0000 Subject: [PATCH 10/10] fix(test/libsinsp_e2e): solve compilation issues Signed-off-by: Jason Dellaluce --- test/libsinsp_e2e/sys_call_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/libsinsp_e2e/sys_call_test.cpp b/test/libsinsp_e2e/sys_call_test.cpp index e1e98239a4..872dc94077 100644 --- a/test/libsinsp_e2e/sys_call_test.cpp +++ b/test/libsinsp_e2e/sys_call_test.cpp @@ -999,7 +999,7 @@ TEST_F(sys_call_test32, execve_ia32_emulation) { sinsp_filter_compiler compiler(inspector, "evt.type=execve and proc.apid=" + std::to_string(getpid())); - is_subprocess_execve.reset(compiler.compile()); + is_subprocess_execve = compiler.compile(); }; event_filter_t filter = [&](sinsp_evt* evt) { return is_subprocess_execve->run(evt); };