From 55eb6543cbfbf753e3d66a123e3392b317fa2ada Mon Sep 17 00:00:00 2001 From: Luca Guerra Date: Wed, 17 Jul 2024 15:41:28 +0000 Subject: [PATCH] fix(libsinsp): simplify sinsp_split, modify set_env/args Signed-off-by: Luca Guerra --- userspace/libsinsp/event.h | 9 ++- userspace/libsinsp/sinsp.h | 2 - userspace/libsinsp/test/sinsp_utils.ut.cpp | 75 +++++++++++++++---- .../libsinsp/test/sinsp_with_test_input.cpp | 3 - userspace/libsinsp/threadinfo.cpp | 21 +++++- userspace/libsinsp/utils.cpp | 38 ++++------ userspace/libsinsp/utils.h | 6 +- 7 files changed, 102 insertions(+), 52 deletions(-) diff --git a/userspace/libsinsp/event.h b/userspace/libsinsp/event.h index 0cff2b5470..afd96066e4 100644 --- a/userspace/libsinsp/event.h +++ b/userspace/libsinsp/event.h @@ -164,7 +164,14 @@ inline std::string get_event_param_as(const sinsp_evt_param& param) template<> inline std::vector get_event_param_as>(const sinsp_evt_param& param) { - return sinsp_split(param.m_val, static_cast(param.m_len), '\0'); + // vector string parameters coming from the driver may be NUL-terminated or not. Either way, remove the NUL terminator + uint32_t len = param.m_len; + if (len > 0 && param.m_val[param.m_len - 1] == '\0') + { + len--; + } + + return sinsp_split({param.m_val, static_cast(len)}, '\0'); } /*! diff --git a/userspace/libsinsp/sinsp.h b/userspace/libsinsp/sinsp.h index de9c394f31..b82667e6be 100644 --- a/userspace/libsinsp/sinsp.h +++ b/userspace/libsinsp/sinsp.h @@ -92,8 +92,6 @@ class sinsp_plugin; class sinsp_plugin_manager; class sinsp_observer; -std::vector sinsp_split(const std::string &s, char delim); - /*! \brief The user agent string to use for any libsinsp connection, can be changed at compile time */ diff --git a/userspace/libsinsp/test/sinsp_utils.ut.cpp b/userspace/libsinsp/test/sinsp_utils.ut.cpp index 021ebfb799..8779e0b0fe 100644 --- a/userspace/libsinsp/test/sinsp_utils.ut.cpp +++ b/userspace/libsinsp/test/sinsp_utils.ut.cpp @@ -199,28 +199,71 @@ TEST(sinsp_utils_test, concatenate_paths) EXPECT_EQ("/root/c:/hello/world", res); */ } -TEST(sinsp_utils_test, sinsp_split_buf) +TEST(sinsp_utils_test, sinsp_split) { const char *in = "hello\0world\0"; - size_t len = 12; - auto split = sinsp_split(in, len, '\0'); + size_t len = 11; + std::vector split = sinsp_split({in, len}, '\0'); EXPECT_EQ(split.size(), 2); EXPECT_EQ(split[0], "hello"); EXPECT_EQ(split[1], "world"); -} -TEST(sinsp_utils_test, sinsp_split_check_terminator) -{ - // check that the null terminator is enforced - const char *in = "hello\0worlddd"; - size_t len = 13; -#ifdef _DEBUG - EXPECT_THROW(sinsp_split(in, len, '\0'), sinsp_exception); -#else - auto split = sinsp_split(in, len, '\0'); + std::string str; + + str = "A,B,C"; + split = sinsp_split(str, ','); + EXPECT_EQ(split.size(), 3); + EXPECT_EQ(split[0], "A"); + EXPECT_EQ(split[1], "B"); + EXPECT_EQ(split[2], "C"); + + str = ",B,C"; + split = sinsp_split(str, ','); + EXPECT_EQ(split.size(), 3); + EXPECT_EQ(split[0], ""); + EXPECT_EQ(split[1], "B"); + EXPECT_EQ(split[2], "C"); + + str = "A,B,"; + split = sinsp_split(str, ','); + EXPECT_EQ(split.size(), 3); + EXPECT_EQ(split[0], "A"); + EXPECT_EQ(split[1], "B"); + EXPECT_EQ(split[2], ""); + + str = ""; + split = sinsp_split(str, ','); + EXPECT_EQ(split.size(), 0); + + str = "A"; + split = sinsp_split(str, ','); + EXPECT_EQ(split.size(), 1); + EXPECT_EQ(split[0], "A"); + + str = ","; + split = sinsp_split(str, ','); EXPECT_EQ(split.size(), 2); - EXPECT_EQ(split[0], "hello"); - EXPECT_EQ(split[1], "worldd"); -#endif + EXPECT_EQ(split[0], ""); + EXPECT_EQ(split[1], ""); + + str = ",,"; + split = sinsp_split(str, ','); + EXPECT_EQ(split.size(), 3); + EXPECT_EQ(split[0], ""); + EXPECT_EQ(split[1], ""); + EXPECT_EQ(split[2], ""); + + str = "A,"; + split = sinsp_split(str, ','); + EXPECT_EQ(split.size(), 2); + EXPECT_EQ(split[0], "A"); + EXPECT_EQ(split[1], ""); + + str = ",B"; + split = sinsp_split(str, ','); + EXPECT_EQ(split.size(), 2); + EXPECT_EQ(split[0], ""); + EXPECT_EQ(split[1], "B"); + } diff --git a/userspace/libsinsp/test/sinsp_with_test_input.cpp b/userspace/libsinsp/test/sinsp_with_test_input.cpp index 61c20416d1..96ddb05449 100644 --- a/userspace/libsinsp/test/sinsp_with_test_input.cpp +++ b/userspace/libsinsp/test/sinsp_with_test_input.cpp @@ -356,21 +356,18 @@ scap_threadinfo sinsp_with_test_input::create_threadinfo( if (!args.empty()) { argsv = test_utils::to_null_delimited(args); - argsv.push_back('\0'); } std::string envv; if (!env.empty()) { envv = test_utils::to_null_delimited(env); - envv.push_back('\0'); } std::string cgroupsv; if (!cgroups.empty()) { cgroupsv = test_utils::to_null_delimited(cgroups); - cgroupsv.push_back('\0'); } memcpy(tinfo.args, argsv.data(), argsv.size()); diff --git a/userspace/libsinsp/threadinfo.cpp b/userspace/libsinsp/threadinfo.cpp index a856098d77..fffa949cd7 100644 --- a/userspace/libsinsp/threadinfo.cpp +++ b/userspace/libsinsp/threadinfo.cpp @@ -625,7 +625,12 @@ std::string sinsp_threadinfo::get_exepath() const void sinsp_threadinfo::set_args(const char* args, size_t len) { - set_args(sinsp_split(args, len, '\0')); + if (len > 0 && args[len - 1] == '\0') + { + len--; + } + + set_args(sinsp_split({args, len}, '\0')); } void sinsp_threadinfo::set_args(std::vector args) @@ -648,7 +653,12 @@ void sinsp_threadinfo::set_env(const char* env, size_t len) } } - m_env = sinsp_split(env, len, '\0'); + if (len > 0 && env[len - 1] == '\0') + { + len--; + } + + m_env = sinsp_split({env, len}, '\0'); } bool sinsp_threadinfo::set_env_from_proc() { @@ -739,7 +749,12 @@ std::string sinsp_threadinfo::concatenate_all_env() void sinsp_threadinfo::set_cgroups(const char* cgroups, size_t len) { - set_cgroups(sinsp_split(cgroups, len, '\0')); + if (len > 0 && cgroups[len - 1] == '\0') + { + len--; + } + + set_cgroups(sinsp_split({cgroups, len}, '\0')); } void sinsp_threadinfo::set_cgroups(std::vector cgroups) diff --git a/userspace/libsinsp/utils.cpp b/userspace/libsinsp/utils.cpp index b14c8209b8..d1768d5da7 100644 --- a/userspace/libsinsp/utils.cpp +++ b/userspace/libsinsp/utils.cpp @@ -1371,43 +1371,31 @@ const char* print_format_to_string(ppm_print_format fmt) // // String split // -std::vector sinsp_split(const std::string &s, char delim) +std::vector sinsp_split(std::string_view sv, char delim) { std::vector res; - std::istringstream f(s); - std::string ts; - while(getline(f, ts, delim)) + if(sv.length() == 0) { - res.push_back(ts); + return {}; } - return res; -} - -std::vector sinsp_split(const char *buf, size_t len, char delim) -{ - if(len == 0) + std::string_view::size_type start = 0; + for (std::string_view::size_type i = 0; i < sv.size(); i++) { - return {}; + if (sv[i] == delim) + { + res.push_back(std::string(sv.substr(start, i - start))); + start = i + 1; + } } - std::string s {buf, len - 1}; - - if(buf[len - 1] != '\0') + if (start <= sv.length()) { -#ifdef _DEBUG - throw sinsp_exception("expected a NUL-terminated buffer of size " + - std::to_string(len) + ", which instead ends with " + - std::to_string(buf[len - 1])); -#else - libsinsp_logger()->format(sinsp_logger::SEV_WARNING, "expected a NUL-terminated buffer of size '%ld' which instead ends with '%c'", len, buf[len - 1]); - // enforce the null terminator - s.replace(len-1, 1, "\0"); -#endif + res.push_back(std::string(sv.substr(start))); } - return sinsp_split(s, delim); + return res; } // diff --git a/userspace/libsinsp/utils.h b/userspace/libsinsp/utils.h index 54ed2e73cd..6f4d664d42 100644 --- a/userspace/libsinsp/utils.h +++ b/userspace/libsinsp/utils.h @@ -263,8 +263,10 @@ const char* print_format_to_string(ppm_print_format fmt); /////////////////////////////////////////////////////////////////////////////// // String helpers /////////////////////////////////////////////////////////////////////////////// -std::vector sinsp_split(const std::string& s, char delim); -std::vector sinsp_split(const char *buf, size_t len, char delim); + +// split a string into components separated by delim. +// An empty string in input will produce a vector with no elements. +std::vector sinsp_split(std::string_view sv, char delim); template std::string sinsp_join(It begin, It end, char delim)