From ed0be53bae11062a2c3f30bd7c96abc609229253 Mon Sep 17 00:00:00 2001 From: Jason Dellaluce Date: Fri, 9 Feb 2024 19:40:06 +0000 Subject: [PATCH] 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 73771741005..6d1850f694c 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 3575373177a..2b417144963 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 01e73db2646..504839e3310 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 87a1c4aa9ec..3bb348dfc52 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 1ac37372181..68ba16e4829 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 0615e037e08..64caaa8bf35 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 0ebdfaf7226..3dd21001f68 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 d398736095a..4b07b20f837 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 48b1d7b02ce..a5f41232226 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