Skip to content

Commit 5e420c1

Browse files
FedeDPFedeDP
committed
chore(userspace/libsinsp): avoid possible issues with references usage.
Moreover, improved sinsp_observer tests. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
1 parent fefb4fa commit 5e420c1

File tree

2 files changed

+97
-19
lines changed

2 files changed

+97
-19
lines changed

‎userspace/libsinsp/parsers.cpp‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1281,7 +1281,7 @@ void sinsp_parser::parse_clone_exit_caller(sinsp_evt *evt, int64_t child_tid) {
12811281
//
12821282
if(m_inspector->get_observer()) {
12831283
m_inspector->m_post_process_cbs.emplace(
1284-
[&new_child, tid_collision](sinsp_observer *observer, sinsp_evt *evt) {
1284+
[new_child, tid_collision](sinsp_observer *observer, sinsp_evt *evt) {
12851285
observer->on_clone(evt, new_child.get(), tid_collision);
12861286
});
12871287
}
@@ -1768,7 +1768,7 @@ void sinsp_parser::parse_clone_exit_child(sinsp_evt *evt) {
17681768
//
17691769
if(m_inspector->get_observer()) {
17701770
m_inspector->m_post_process_cbs.emplace(
1771-
[&new_child, tid_collision](sinsp_observer *observer, sinsp_evt *evt) {
1771+
[new_child, tid_collision](sinsp_observer *observer, sinsp_evt *evt) {
17721772
observer->on_clone(evt, new_child.get(), tid_collision);
17731773
});
17741774
}
@@ -3057,7 +3057,7 @@ void sinsp_parser::parse_connect_exit(sinsp_evt *evt) {
30573057
//
30583058
if(m_inspector->get_observer()) {
30593059
m_inspector->m_post_process_cbs.emplace(
3060-
[&packed_data](sinsp_observer *observer, sinsp_evt *evt) {
3060+
[packed_data](sinsp_observer *observer, sinsp_evt *evt) {
30613061
observer->on_connect(evt, packed_data);
30623062
});
30633063
}
@@ -3152,7 +3152,7 @@ void sinsp_parser::parse_accept_exit(sinsp_evt *evt) {
31523152
//
31533153
if(m_inspector->get_observer()) {
31543154
m_inspector->m_post_process_cbs.emplace(
3155-
[fd, &packed_data](sinsp_observer *observer, sinsp_evt *evt) {
3155+
[fd, packed_data](sinsp_observer *observer, sinsp_evt *evt) {
31563156
observer->on_accept(evt, fd, packed_data, evt->get_fd_info());
31573157
});
31583158
}
@@ -3218,9 +3218,16 @@ void sinsp_parser::erase_fd(erase_fd_params *params) {
32183218
// If there's a listener, add a callback to later invoke it.
32193219
//
32203220
if(m_inspector->get_observer()) {
3221+
auto ts = params->m_ts;
3222+
auto remove_from_table = params->m_remove_from_table;
3223+
auto fd = params->m_fd;
3224+
auto tinfo = params->m_tinfo;
3225+
auto fdinfo = params->m_fdinfo;
32213226
m_inspector->m_post_process_cbs.emplace(
3222-
[&params](sinsp_observer *observer, sinsp_evt *evt) {
3223-
observer->on_erase_fd(params);
3227+
[ts, remove_from_table, fd, tinfo, fdinfo](sinsp_observer *observer,
3228+
sinsp_evt *evt) {
3229+
erase_fd_params p = {remove_from_table, fd, tinfo, fdinfo, ts};
3230+
observer->on_erase_fd(&p);
32243231
});
32253232
}
32263233
}
@@ -3750,7 +3757,7 @@ void sinsp_parser::parse_rw_exit(sinsp_evt *evt) {
37503757
//
37513758
if(m_inspector->get_observer()) {
37523759
m_inspector->m_post_process_cbs.emplace(
3753-
[tid, &data, retval, datalen](sinsp_observer *observer, sinsp_evt *evt) {
3760+
[tid, data, retval, datalen](sinsp_observer *observer, sinsp_evt *evt) {
37543761
observer->on_read(evt,
37553762
tid,
37563763
evt->get_tinfo()->m_lastevent_fd,
@@ -3871,7 +3878,7 @@ void sinsp_parser::parse_rw_exit(sinsp_evt *evt) {
38713878
//
38723879
if(m_inspector->get_observer()) {
38733880
m_inspector->m_post_process_cbs.emplace(
3874-
[tid, &data, retval, datalen](sinsp_observer *observer, sinsp_evt *evt) {
3881+
[tid, data, retval, datalen](sinsp_observer *observer, sinsp_evt *evt) {
38753882
observer->on_write(evt,
38763883
tid,
38773884
evt->get_tinfo()->m_lastevent_fd,

‎userspace/libsinsp/test/classes/sinsp_observer.cpp‎

Lines changed: 82 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,24 @@ limitations under the License.
2121

2222
class test_observer : public sinsp_observer {
2323
public:
24-
test_observer(): clone_counter(0), execve_counter(0) {}
24+
test_observer():
25+
m_clone_ctr(0),
26+
m_execve_ctr(0),
27+
m_open_ctr(0),
28+
m_read_ctr(0),
29+
m_close_ctr(0) {}
2530

2631
void on_read(sinsp_evt* evt,
2732
int64_t tid,
2833
int64_t fd,
2934
sinsp_fdinfo* fdinfo,
3035
const char* data,
3136
uint32_t original_len,
32-
uint32_t len) override {}
37+
uint32_t len) override {
38+
std::cerr << "[ ] read = " << evt->get_name() << " fd " << fdinfo->m_fd << " data "
39+
<< data << std::endl;
40+
m_read_ctr++;
41+
}
3342

3443
void on_write(sinsp_evt* evt,
3544
int64_t tid,
@@ -47,14 +56,29 @@ class test_observer : public sinsp_observer {
4756
uint8_t* packed_data,
4857
sinsp_fdinfo* new_fdinfo) override {}
4958

50-
void on_file_open(sinsp_evt* evt, const std::string& fullpath, uint32_t flags) override {}
59+
void on_file_open(sinsp_evt* evt, const std::string& fullpath, uint32_t flags) override {
60+
std::cerr << "[ ] open = " << evt->get_name() << " path " << fullpath << std::endl;
61+
m_open_ctr++;
62+
}
5163
void on_error(sinsp_evt* evt) override {}
52-
void on_erase_fd(erase_fd_params* params) override {}
64+
void on_erase_fd(erase_fd_params* params) override {
65+
// Test that sanitizer does not complain about these ones, ie that params pointer is correct
66+
std::cerr << "[ ] closed fd = " << params->m_fd << " at " << params->m_ts
67+
<< " tid " << params->m_tinfo->m_tid << std::endl;
68+
m_close_ctr++;
69+
}
5370
void on_socket_shutdown(sinsp_evt* evt) override {}
54-
void on_execve(sinsp_evt* evt) override { execve_counter++; }
71+
void on_execve(sinsp_evt* evt) override {
72+
// Test that sanitizer does not complain about these ones, ie that evt pointer is correct
73+
std::cerr << "[ ] execve = " << evt->get_name() << std::endl;
74+
m_execve_ctr++;
75+
}
5576

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

6084
void on_bind(sinsp_evt* evt) override {}
@@ -67,13 +91,18 @@ class test_observer : public sinsp_observer {
6791

6892
void on_socket_status_changed(sinsp_evt* evt) override {}
6993

70-
int get_clone_counter() const { return clone_counter; }
71-
72-
int get_execve_counter() const { return execve_counter; };
94+
int get_clone_counter() const { return m_clone_ctr; }
95+
int get_execve_counter() const { return m_execve_ctr; };
96+
int get_open_counter() const { return m_open_ctr; }
97+
int get_read_counter() const { return m_read_ctr; }
98+
int get_close_ctr() const { return m_close_ctr; }
7399

74100
private:
75-
int clone_counter;
76-
int execve_counter;
101+
int m_clone_ctr;
102+
int m_execve_ctr;
103+
int m_open_ctr;
104+
int m_read_ctr;
105+
int m_close_ctr;
77106
};
78107

79108
TEST_F(sinsp_with_test_input, sinsp_observer) {
@@ -88,9 +117,51 @@ TEST_F(sinsp_with_test_input, sinsp_observer) {
88117
generate_clone_x_event(22, INIT_TID, INIT_PID, INIT_PTID);
89118
ASSERT_EQ(observer.get_clone_counter(), 1);
90119
ASSERT_EQ(observer.get_execve_counter(), 0);
120+
ASSERT_EQ(observer.get_open_counter(), 0);
121+
ASSERT_EQ(observer.get_read_counter(), 0);
122+
ASSERT_EQ(observer.get_close_ctr(), 0);
91123

92124
/* execve exit event */
93125
generate_execve_enter_and_exit_event(0, 11, 11, 11, 11);
94126
ASSERT_EQ(observer.get_clone_counter(), 1);
95127
ASSERT_EQ(observer.get_execve_counter(), 1);
128+
ASSERT_EQ(observer.get_open_counter(), 0);
129+
ASSERT_EQ(observer.get_read_counter(), 0);
130+
ASSERT_EQ(observer.get_close_ctr(), 0);
131+
132+
generate_open_x_event();
133+
ASSERT_EQ(observer.get_clone_counter(), 1);
134+
ASSERT_EQ(observer.get_execve_counter(), 1);
135+
ASSERT_EQ(observer.get_open_counter(), 1);
136+
ASSERT_EQ(observer.get_read_counter(), 0);
137+
ASSERT_EQ(observer.get_close_ctr(), 0);
138+
139+
std::string data = "hello";
140+
uint32_t size = data.size();
141+
add_event_advance_ts(increasing_ts(),
142+
INIT_TID,
143+
PPME_SYSCALL_READ_X,
144+
4,
145+
(int64_t)size,
146+
scap_const_sized_buffer{data.c_str(), size},
147+
sinsp_test_input::open_params::default_fd,
148+
size);
149+
ASSERT_EQ(observer.get_clone_counter(), 1);
150+
ASSERT_EQ(observer.get_execve_counter(), 1);
151+
ASSERT_EQ(observer.get_open_counter(), 1);
152+
ASSERT_EQ(observer.get_read_counter(), 1);
153+
ASSERT_EQ(observer.get_close_ctr(), 0);
154+
155+
// Close the opened FD
156+
add_event_advance_ts(increasing_ts(),
157+
INIT_TID,
158+
PPME_SYSCALL_CLOSE_E,
159+
1,
160+
(int64_t)4); // default sinsp_test_input::open_params fd)
161+
add_event_advance_ts(increasing_ts(), INIT_TID, PPME_SYSCALL_CLOSE_X, 1, (int64_t)0);
162+
ASSERT_EQ(observer.get_clone_counter(), 1);
163+
ASSERT_EQ(observer.get_execve_counter(), 1);
164+
ASSERT_EQ(observer.get_open_counter(), 1);
165+
ASSERT_EQ(observer.get_read_counter(), 1);
166+
ASSERT_EQ(observer.get_close_ctr(), 1);
96167
}

0 commit comments

Comments
 (0)