Skip to content

Commit

Permalink
chore(userspace/libsinsp): avoid possible issues with references usage.
Browse files Browse the repository at this point in the history 
Moreover, improved sinsp_observer tests.

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
  • Loading branch information
@FedeDP
FedeDP committed Jan 10, 2025
1 parent fefb4fa commit 5e420c1
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 19 deletions.
23 changes: 15 additions & 8 deletions userspace/libsinsp/parsers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1281,7 +1281,7 @@ void sinsp_parser::parse_clone_exit_caller(sinsp_evt *evt, int64_t child_tid) {
//
if(m_inspector->get_observer()) {
m_inspector->m_post_process_cbs.emplace(
[&new_child, tid_collision](sinsp_observer *observer, sinsp_evt *evt) {
[new_child, tid_collision](sinsp_observer *observer, sinsp_evt *evt) {
observer->on_clone(evt, new_child.get(), tid_collision);
});
}
Expand Down Expand Up @@ -1768,7 +1768,7 @@ void sinsp_parser::parse_clone_exit_child(sinsp_evt *evt) {
//
if(m_inspector->get_observer()) {
m_inspector->m_post_process_cbs.emplace(
[&new_child, tid_collision](sinsp_observer *observer, sinsp_evt *evt) {
[new_child, tid_collision](sinsp_observer *observer, sinsp_evt *evt) {
observer->on_clone(evt, new_child.get(), tid_collision);

Check warning on line 1772 in userspace/libsinsp/parsers.cpp

View check run for this annotation

Codecov / codecov/patch

userspace/libsinsp/parsers.cpp#L1772 

Added line #L1772 was not covered by tests
});
}
Expand Down Expand Up @@ -3057,7 +3057,7 @@ void sinsp_parser::parse_connect_exit(sinsp_evt *evt) {
//
if(m_inspector->get_observer()) {
m_inspector->m_post_process_cbs.emplace(
[&packed_data](sinsp_observer *observer, sinsp_evt *evt) {
[packed_data](sinsp_observer *observer, sinsp_evt *evt) {
observer->on_connect(evt, packed_data);

Check warning on line 3061 in userspace/libsinsp/parsers.cpp

View check run for this annotation

Codecov / codecov/patch

userspace/libsinsp/parsers.cpp#L3059-L3061 

Added lines #L3059 - L3061 were not covered by tests
});
}
Expand Down Expand Up @@ -3152,7 +3152,7 @@ void sinsp_parser::parse_accept_exit(sinsp_evt *evt) {
//
if(m_inspector->get_observer()) {
m_inspector->m_post_process_cbs.emplace(
[fd, &packed_data](sinsp_observer *observer, sinsp_evt *evt) {
[fd, packed_data](sinsp_observer *observer, sinsp_evt *evt) {
observer->on_accept(evt, fd, packed_data, evt->get_fd_info());

Check warning on line 3156 in userspace/libsinsp/parsers.cpp

View check run for this annotation

Codecov / codecov/patch

userspace/libsinsp/parsers.cpp#L3155-L3156 

Added lines #L3155 - L3156 were not covered by tests
});
}
Expand Down Expand Up @@ -3218,9 +3218,16 @@ void sinsp_parser::erase_fd(erase_fd_params *params) {
// If there's a listener, add a callback to later invoke it.
//
if(m_inspector->get_observer()) {
auto ts = params->m_ts;
auto remove_from_table = params->m_remove_from_table;
auto fd = params->m_fd;
auto tinfo = params->m_tinfo;
auto fdinfo = params->m_fdinfo;
m_inspector->m_post_process_cbs.emplace(
[&params](sinsp_observer *observer, sinsp_evt *evt) {
observer->on_erase_fd(params);
[ts, remove_from_table, fd, tinfo, fdinfo](sinsp_observer *observer,
sinsp_evt *evt) {
erase_fd_params p = {remove_from_table, fd, tinfo, fdinfo, ts};
observer->on_erase_fd(&p);
});
}
}
Expand Down Expand Up @@ -3750,7 +3757,7 @@ void sinsp_parser::parse_rw_exit(sinsp_evt *evt) {
//
if(m_inspector->get_observer()) {
m_inspector->m_post_process_cbs.emplace(
[tid, &data, retval, datalen](sinsp_observer *observer, sinsp_evt *evt) {
[tid, data, retval, datalen](sinsp_observer *observer, sinsp_evt *evt) {
observer->on_read(evt,
tid,
evt->get_tinfo()->m_lastevent_fd,
Expand Down Expand Up @@ -3871,7 +3878,7 @@ void sinsp_parser::parse_rw_exit(sinsp_evt *evt) {
//
if(m_inspector->get_observer()) {
m_inspector->m_post_process_cbs.emplace(
[tid, &data, retval, datalen](sinsp_observer *observer, sinsp_evt *evt) {
[tid, data, retval, datalen](sinsp_observer *observer, sinsp_evt *evt) {
observer->on_write(evt,

Check warning on line 3882 in userspace/libsinsp/parsers.cpp

View check run for this annotation

Codecov / codecov/patch

userspace/libsinsp/parsers.cpp#L3880-L3882 

Added lines #L3880 - L3882 were not covered by tests
tid,
evt->get_tinfo()->m_lastevent_fd,

Check warning on line 3884 in userspace/libsinsp/parsers.cpp

View check run for this annotation

Codecov / codecov/patch

userspace/libsinsp/parsers.cpp#L3884 

Added line #L3884 was not covered by tests
Expand Down
93 changes: 82 additions & 11 deletions userspace/libsinsp/test/classes/sinsp_observer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,24 @@ limitations under the License.

class test_observer : public sinsp_observer {

Check warning on line 22 in userspace/libsinsp/test/classes/sinsp_observer.cpp

View check run for this annotation

Codecov / codecov/patch

userspace/libsinsp/test/classes/sinsp_observer.cpp#L22 

Added line #L22 was not covered by tests
public:
test_observer(): clone_counter(0), execve_counter(0) {}
test_observer():
m_clone_ctr(0),
m_execve_ctr(0),
m_open_ctr(0),
m_read_ctr(0),
m_close_ctr(0) {}

void on_read(sinsp_evt* evt,
int64_t tid,
int64_t fd,
sinsp_fdinfo* fdinfo,
const char* data,
uint32_t original_len,
uint32_t len) override {}
uint32_t len) override {
std::cerr << "[ ] read = " << evt->get_name() << " fd " << fdinfo->m_fd << " data "
<< data << std::endl;
m_read_ctr++;
}

void on_write(sinsp_evt* evt,

Check warning on line 43 in userspace/libsinsp/test/classes/sinsp_observer.cpp

View check run for this annotation

Codecov / codecov/patch

userspace/libsinsp/test/classes/sinsp_observer.cpp#L43 

Added line #L43 was not covered by tests
int64_t tid,
Expand All @@ -47,14 +56,29 @@ class test_observer : public sinsp_observer {
uint8_t* packed_data,
sinsp_fdinfo* new_fdinfo) override {}

Check warning on line 57 in userspace/libsinsp/test/classes/sinsp_observer.cpp

View check run for this annotation

Codecov / codecov/patch

userspace/libsinsp/test/classes/sinsp_observer.cpp#L57 

Added line #L57 was not covered by tests

void on_file_open(sinsp_evt* evt, const std::string& fullpath, uint32_t flags) override {}
void on_file_open(sinsp_evt* evt, const std::string& fullpath, uint32_t flags) override {
std::cerr << "[ ] open = " << evt->get_name() << " path " << fullpath << std::endl;
m_open_ctr++;
}
void on_error(sinsp_evt* evt) override {}

Check warning on line 63 in userspace/libsinsp/test/classes/sinsp_observer.cpp

View check run for this annotation

Codecov / codecov/patch

userspace/libsinsp/test/classes/sinsp_observer.cpp#L63 

Added line #L63 was not covered by tests
void on_erase_fd(erase_fd_params* params) override {}
void on_erase_fd(erase_fd_params* params) override {
// Test that sanitizer does not complain about these ones, ie that params pointer is correct
std::cerr << "[ ] closed fd = " << params->m_fd << " at " << params->m_ts
<< " tid " << params->m_tinfo->m_tid << std::endl;
m_close_ctr++;
}
void on_socket_shutdown(sinsp_evt* evt) override {}

Check warning on line 70 in userspace/libsinsp/test/classes/sinsp_observer.cpp

View check run for this annotation

Codecov / codecov/patch

userspace/libsinsp/test/classes/sinsp_observer.cpp#L70 

Added line #L70 was not covered by tests
void on_execve(sinsp_evt* evt) override { execve_counter++; }
void on_execve(sinsp_evt* evt) override {
// Test that sanitizer does not complain about these ones, ie that evt pointer is correct
std::cerr << "[ ] execve = " << evt->get_name() << std::endl;
m_execve_ctr++;
}

void on_clone(sinsp_evt* evt, sinsp_threadinfo* newtinfo, int64_t tid_collision) override {
clone_counter++;
// Test that sanitizer does not complain about these ones, ie that evt pointer is correct
std::cerr << "[ ] clone = " << evt->get_name() << " created " << newtinfo->m_tid
<< std::endl;
m_clone_ctr++;
}

void on_bind(sinsp_evt* evt) override {}

Check warning on line 84 in userspace/libsinsp/test/classes/sinsp_observer.cpp

View check run for this annotation

Codecov / codecov/patch

userspace/libsinsp/test/classes/sinsp_observer.cpp#L84 

Added line #L84 was not covered by tests
Expand All @@ -67,13 +91,18 @@ class test_observer : public sinsp_observer {

void on_socket_status_changed(sinsp_evt* evt) override {}

Check warning on line 92 in userspace/libsinsp/test/classes/sinsp_observer.cpp

View check run for this annotation

Codecov / codecov/patch

userspace/libsinsp/test/classes/sinsp_observer.cpp#L92 

Added line #L92 was not covered by tests

int get_clone_counter() const { return clone_counter; }

int get_execve_counter() const { return execve_counter; };
int get_clone_counter() const { return m_clone_ctr; }
int get_execve_counter() const { return m_execve_ctr; };
int get_open_counter() const { return m_open_ctr; }
int get_read_counter() const { return m_read_ctr; }
int get_close_ctr() const { return m_close_ctr; }

private:
int clone_counter;
int execve_counter;
int m_clone_ctr;
int m_execve_ctr;
int m_open_ctr;
int m_read_ctr;
int m_close_ctr;
};

TEST_F(sinsp_with_test_input, sinsp_observer) {
Expand All @@ -88,9 +117,51 @@ TEST_F(sinsp_with_test_input, sinsp_observer) {
generate_clone_x_event(22, INIT_TID, INIT_PID, INIT_PTID);
ASSERT_EQ(observer.get_clone_counter(), 1);
ASSERT_EQ(observer.get_execve_counter(), 0);
ASSERT_EQ(observer.get_open_counter(), 0);
ASSERT_EQ(observer.get_read_counter(), 0);
ASSERT_EQ(observer.get_close_ctr(), 0);

/* execve exit event */
generate_execve_enter_and_exit_event(0, 11, 11, 11, 11);
ASSERT_EQ(observer.get_clone_counter(), 1);
ASSERT_EQ(observer.get_execve_counter(), 1);
ASSERT_EQ(observer.get_open_counter(), 0);
ASSERT_EQ(observer.get_read_counter(), 0);
ASSERT_EQ(observer.get_close_ctr(), 0);

generate_open_x_event();
ASSERT_EQ(observer.get_clone_counter(), 1);
ASSERT_EQ(observer.get_execve_counter(), 1);
ASSERT_EQ(observer.get_open_counter(), 1);
ASSERT_EQ(observer.get_read_counter(), 0);
ASSERT_EQ(observer.get_close_ctr(), 0);

std::string data = "hello";
uint32_t size = data.size();
add_event_advance_ts(increasing_ts(),
INIT_TID,
PPME_SYSCALL_READ_X,
4,
(int64_t)size,
scap_const_sized_buffer{data.c_str(), size},
sinsp_test_input::open_params::default_fd,
size);
ASSERT_EQ(observer.get_clone_counter(), 1);
ASSERT_EQ(observer.get_execve_counter(), 1);
ASSERT_EQ(observer.get_open_counter(), 1);
ASSERT_EQ(observer.get_read_counter(), 1);
ASSERT_EQ(observer.get_close_ctr(), 0);

// Close the opened FD
add_event_advance_ts(increasing_ts(),
INIT_TID,
PPME_SYSCALL_CLOSE_E,
1,
(int64_t)4); // default sinsp_test_input::open_params fd)
add_event_advance_ts(increasing_ts(), INIT_TID, PPME_SYSCALL_CLOSE_X, 1, (int64_t)0);
ASSERT_EQ(observer.get_clone_counter(), 1);
ASSERT_EQ(observer.get_execve_counter(), 1);
ASSERT_EQ(observer.get_open_counter(), 1);
ASSERT_EQ(observer.get_read_counter(), 1);
ASSERT_EQ(observer.get_close_ctr(), 1);
}

0 comments on commit 5e420c1

Please sign in to comment.