From 50131fb9e21f579b8eda12203f25a21e83de42dd Mon Sep 17 00:00:00 2001 From: Federico Aponte Date: Mon, 4 Dec 2023 17:21:05 +0100 Subject: [PATCH] build: address some PR review concerns Signed-off-by: Federico Aponte --- .../libsinsp/sinsp_filtercheck_event.cpp | 7 ++++--- .../libsinsp/sinsp_filtercheck_syslog.cpp | 9 ++++++--- userspace/libsinsp/sinsp_filtercheck_syslog.h | 1 + userspace/libsinsp/sinsp_syslog.cpp | 20 ++++++++----------- userspace/libsinsp/sinsp_syslog.h | 6 +++--- userspace/libsinsp/test/container_info.ut.cpp | 2 +- 6 files changed, 23 insertions(+), 22 deletions(-) diff --git a/userspace/libsinsp/sinsp_filtercheck_event.cpp b/userspace/libsinsp/sinsp_filtercheck_event.cpp index 4388ee2a2d..0e0e291b9a 100644 --- a/userspace/libsinsp/sinsp_filtercheck_event.cpp +++ b/userspace/libsinsp/sinsp_filtercheck_event.cpp @@ -1122,7 +1122,8 @@ uint8_t* sinsp_filter_check_event::extract(sinsp_evt *evt, OUT uint32_t* len, bo if(m_inspector->m_parser->get_syslog_decoder().is_data_valid()) { // syslog is actually the only info line we support up until now - RETURN_EXTRACT_STRING(m_inspector->m_parser->get_syslog_decoder().get_info_line()); + m_strstorage = m_inspector->m_parser->get_syslog_decoder().get_info_line(); + RETURN_EXTRACT_STRING(m_strstorage); } } // @@ -1703,7 +1704,7 @@ uint8_t* sinsp_filter_check_event::extract(sinsp_evt *evt, OUT uint32_t* len, bo if(m_field_id == TYPE_ISOPEN_CREATE) { - // If PPM_O_F_CREATED is set the file is created + // If PPM_O_F_CREATED is set the file is created if(flags & PPM_O_F_CREATED) { m_u32val = 1; @@ -1736,7 +1737,7 @@ uint8_t* sinsp_filter_check_event::extract(sinsp_evt *evt, OUT uint32_t* len, bo RETURN_EXTRACT_VAR(m_u32val); } - + break; case TYPE_INFRA_DOCKER_NAME: case TYPE_INFRA_DOCKER_CONTAINER_ID: diff --git a/userspace/libsinsp/sinsp_filtercheck_syslog.cpp b/userspace/libsinsp/sinsp_filtercheck_syslog.cpp index cad51d0fb1..103848e6ec 100644 --- a/userspace/libsinsp/sinsp_filtercheck_syslog.cpp +++ b/userspace/libsinsp/sinsp_filtercheck_syslog.cpp @@ -77,14 +77,17 @@ uint8_t* sinsp_filter_check_syslog::extract(sinsp_evt *evt, OUT uint32_t* len, b m_storageu32 = decoder.get_facility(); RETURN_EXTRACT_VAR(m_storageu32); case TYPE_FACILITY_STR: - RETURN_EXTRACT_STRING(decoder.get_facility_str()); + mstrstorage = decoder.get_facility_str(); + RETURN_EXTRACT_STRING(mstrstorage); case TYPE_SEVERITY: m_storageu32 = decoder.get_severity(); RETURN_EXTRACT_VAR(m_storageu32); case TYPE_SEVERITY_STR: - RETURN_EXTRACT_STRING(decoder.get_severity_str()); + mstrstorage = decoder.get_severity_str(); + RETURN_EXTRACT_STRING(mstrstorage); case TYPE_MESSAGE: - RETURN_EXTRACT_STRING(decoder.get_msg()); + mstrstorage = decoder.get_msg(); + RETURN_EXTRACT_STRING(mstrstorage); default: ASSERT(false); return NULL; diff --git a/userspace/libsinsp/sinsp_filtercheck_syslog.h b/userspace/libsinsp/sinsp_filtercheck_syslog.h index 63cef55dfd..75b4d48b9b 100644 --- a/userspace/libsinsp/sinsp_filtercheck_syslog.h +++ b/userspace/libsinsp/sinsp_filtercheck_syslog.h @@ -38,5 +38,6 @@ class sinsp_filter_check_syslog : public sinsp_filter_check uint8_t* extract(sinsp_evt*, OUT uint32_t* len, bool sanitize_strings = true) override; uint32_t m_storageu32; + std::string mstrstorage; std::string m_name; }; diff --git a/userspace/libsinsp/sinsp_syslog.cpp b/userspace/libsinsp/sinsp_syslog.cpp index b082098137..ed11411907 100644 --- a/userspace/libsinsp/sinsp_syslog.cpp +++ b/userspace/libsinsp/sinsp_syslog.cpp @@ -21,7 +21,6 @@ limitations under the License. #define PRI_BUF_SIZE 16 -static const std::string s_syslog_severity_not_available = ""; static const std::string s_syslog_severity_strings[] = { "emerg", "alert", "crit", "err", "warn", "notice", "info", "debug" @@ -73,11 +72,11 @@ void sinsp_syslog_decoder::parse_data(const char *data, uint32_t len) decode_message(data, len, pri, j); } -const std::string& sinsp_syslog_decoder::get_severity_str() const +std::string sinsp_syslog_decoder::get_severity_str() const { if(!is_data_valid() || m_severity >= sizeof(s_syslog_severity_strings) / sizeof(s_syslog_severity_strings[0])) { - return s_syslog_severity_not_available; + return ""; } else { @@ -85,11 +84,11 @@ const std::string& sinsp_syslog_decoder::get_severity_str() const } } -const std::string& sinsp_syslog_decoder::get_facility_str() const +std::string sinsp_syslog_decoder::get_facility_str() const { if(!is_data_valid() || m_facility >= sizeof(s_syslog_facility_strings) / sizeof(s_syslog_facility_strings[0])) { - return s_syslog_severity_not_available; + return ""; } else { @@ -119,15 +118,12 @@ void sinsp_syslog_decoder::decode_message(const char *data, uint32_t len, char* m_msg.assign(data + pristrlen + 2, len - pristrlen - 2); } -const std::string& sinsp_syslog_decoder::get_info_line() +std::string sinsp_syslog_decoder::get_info_line() const { if (!is_data_valid()) { - m_infostr = s_syslog_severity_not_available; + return ""; } - else - { - m_infostr = std::string("syslog sev=") + get_severity_str() + " msg=" + m_msg; - } - return m_infostr; + + return "syslog sev=" + get_severity_str() + " msg=" + m_msg; } diff --git a/userspace/libsinsp/sinsp_syslog.h b/userspace/libsinsp/sinsp_syslog.h index 7e53a582f0..29d3fc5f10 100644 --- a/userspace/libsinsp/sinsp_syslog.h +++ b/userspace/libsinsp/sinsp_syslog.h @@ -29,9 +29,9 @@ class sinsp_syslog_decoder public: void parse_data(const char *data, uint32_t len); - const std::string& get_info_line(); - const std::string& get_severity_str() const; - const std::string& get_facility_str() const; + std::string get_info_line() const; + std::string get_severity_str() const; + std::string get_facility_str() const; inline void reset() { diff --git a/userspace/libsinsp/test/container_info.ut.cpp b/userspace/libsinsp/test/container_info.ut.cpp index 24f8337924..b922b64250 100644 --- a/userspace/libsinsp/test/container_info.ut.cpp +++ b/userspace/libsinsp/test/container_info.ut.cpp @@ -59,7 +59,7 @@ TEST_P(sinsp_container_lookup_test, delays_match) } } -INSTANTIATE_TEST_SUITE_P(sinsp_container_lookup, +INSTANTIATE_TEST_CASE_P(sinsp_container_lookup, sinsp_container_lookup_test, ::testing::Values( std::tuple>{3, 500, {125, 250, 500}},