Skip to content

Commit

Permalink
fix(libsinsp): simplify sinsp_split, modify set_env/args
Browse files Browse the repository at this point in the history
Signed-off-by: Luca Guerra <[email protected]>
  • Loading branch information
LucaGuerra committed Jul 22, 2024
1 parent f7b1055 commit 55eb654
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 52 deletions.
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);

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 @@ -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());
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(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(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

0 comments on commit 55eb654

Please sign in to comment.