Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(libsinsp): optimize sinsp_split, modify set_env/args #1962

Merged
merged 1 commit into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion userspace/libsinsp/event.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,14 @@ inline std::string get_event_param_as<std::string>(const sinsp_evt_param& param)
template<>
inline std::vector<std::string> get_event_param_as<std::vector<std::string>>(const sinsp_evt_param& param)
{
return sinsp_split(param.m_val, static_cast<size_t>(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<std::string_view::size_type>(len)}, '\0');
}

/*!
Expand Down
2 changes: 0 additions & 2 deletions userspace/libsinsp/sinsp.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,6 @@ class sinsp_plugin;
class sinsp_plugin_manager;
class sinsp_observer;

std::vector<std::string> 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
*/
Expand Down
75 changes: 59 additions & 16 deletions userspace/libsinsp/test/sinsp_utils.ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string> 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);
LucaGuerra marked this conversation as resolved.
Show resolved Hide resolved

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");

}
3 changes: 0 additions & 3 deletions userspace/libsinsp/test/sinsp_with_test_input.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -358,21 +358,18 @@ scap_threadinfo sinsp_with_test_input::create_threadinfo(
if (!args.empty())
{
argsv = test_utils::to_null_delimited(args);
argsv.push_back('\0');
LucaGuerra marked this conversation as resolved.
Show resolved Hide resolved
}

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());
Expand Down
21 changes: 18 additions & 3 deletions userspace/libsinsp/threadinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(const std::vector<std::string>& args)
Expand All @@ -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() {
Expand Down Expand Up @@ -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(const std::vector<std::string>& cgroups)
Expand Down
38 changes: 13 additions & 25 deletions userspace/libsinsp/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1371,43 +1371,31 @@ const char* print_format_to_string(ppm_print_format fmt)
//
// String split
//
std::vector<std::string> sinsp_split(const std::string &s, char delim)
std::vector<std::string> sinsp_split(std::string_view sv, char delim)
{
std::vector<std::string> 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<std::string> 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;
}

//
Expand Down
6 changes: 4 additions & 2 deletions userspace/libsinsp/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,10 @@ const char* print_format_to_string(ppm_print_format fmt);
///////////////////////////////////////////////////////////////////////////////
// String helpers
///////////////////////////////////////////////////////////////////////////////
std::vector<std::string> sinsp_split(const std::string& s, char delim);
std::vector<std::string> 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<std::string> sinsp_split(std::string_view sv, char delim);

template<typename It>
std::string sinsp_join(It begin, It end, char delim)
Expand Down
Loading