Skip to content

Commit

Permalink
Fix leaks and other memory access issues
Browse files Browse the repository at this point in the history
  • Loading branch information
sashacmc committed Nov 26, 2024
1 parent 0cee152 commit 4c7b117
Show file tree
Hide file tree
Showing 14 changed files with 40 additions and 23 deletions.
1 change: 0 additions & 1 deletion include/zenoh-pico/collections/bytes.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ z_result_t _z_bytes_copy(_z_bytes_t *dst, const _z_bytes_t *src);
_z_bytes_t _z_bytes_duplicate(const _z_bytes_t *src);
void _z_bytes_move(_z_bytes_t *dst, _z_bytes_t *src);
void _z_bytes_drop(_z_bytes_t *bytes);
void _z_bytes_aliased_drop(_z_bytes_t *bytes);
void _z_bytes_free(_z_bytes_t **bs);
size_t _z_bytes_num_slices(const _z_bytes_t *bs);
_z_arc_slice_t *_z_bytes_get_slice(const _z_bytes_t *bs, size_t i);
Expand Down
7 changes: 5 additions & 2 deletions include/zenoh-pico/collections/vec.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,15 @@ typedef struct {
size_t _capacity;
size_t _len;
void *_val;
bool _aliased;
} _z_svec_t;

static inline _z_svec_t _z_svec_null(void) { return (_z_svec_t){0}; }
static inline _z_svec_t _z_svec_alias(const _z_svec_t *src) { return *src; }
static inline _z_svec_t _z_svec_alias(const _z_svec_t *src) {
return (_z_svec_t){._capacity = src->_capacity, ._len = src->_len, ._val = src->_val, ._aliased = true};
}
static inline _z_svec_t _z_svec_alias_element(void *element) {
return (_z_svec_t){._capacity = 1, ._len = 1, ._val = element};
return (_z_svec_t){._capacity = 1, ._len = 1, ._val = element, ._aliased = true};
}
void _z_svec_init(_z_svec_t *v, size_t offset, size_t element_size);
_z_svec_t _z_svec_make(size_t capacity, size_t element_size);
Expand Down
2 changes: 1 addition & 1 deletion src/api/liveliness.c
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ z_result_t z_liveliness_get(const z_loaned_session_t *zs, const z_loaned_keyexpr
opt = *options;
}

_z_keyexpr_t ke = _z_keyexpr_duplicate(keyexpr);
_z_keyexpr_t ke = _z_keyexpr_alias_from_user_defined(*keyexpr, true);
ret = _z_liveliness_query(_Z_RC_IN_VAL(zs), &ke, callback->_this._val.call, callback->_this._val.drop, ctx,
opt.timeout_ms);

Expand Down
2 changes: 0 additions & 2 deletions src/collections/bytes.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ _z_arc_slice_t *_z_bytes_get_slice(const _z_bytes_t *bs, size_t i) {

void _z_bytes_drop(_z_bytes_t *bytes) { _z_arc_slice_svec_clear(&bytes->_slices); }

void _z_bytes_aliased_drop(_z_bytes_t *bytes) { _z_arc_slice_svec_reset(&bytes->_slices); }

void _z_bytes_free(_z_bytes_t **bs) {
_z_bytes_t *ptr = *bs;

Expand Down
2 changes: 1 addition & 1 deletion src/collections/slice.c
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ _z_slice_t _z_slice_copy_from_buf(const uint8_t *p, size_t len) {
}

void _z_slice_clear(_z_slice_t *bs) {
if ((bs->start != NULL)) {
if (bs->start != NULL) {
_z_delete_context_delete(&bs->_delete_context, (void *)bs->start);
}
_z_slice_reset(bs);
Expand Down
6 changes: 4 additions & 2 deletions src/collections/vec.c
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ static inline void __z_svec_move_inner(void *dst, void *src, z_element_move_f mo
offset += element_size;
}
} else {
memcpy(dst, src, num_elements * element_size);
memmove(dst, src, num_elements * element_size);
}
}

Expand Down Expand Up @@ -205,7 +205,9 @@ void _z_svec_reset(_z_svec_t *v, z_element_clear_f clear, size_t element_size) {

void _z_svec_clear(_z_svec_t *v, z_element_clear_f clear_f, size_t element_size) {
_z_svec_reset(v, clear_f, element_size);
_z_svec_release(v);
if (!v->_aliased) {
_z_svec_release(v);
}
}

void _z_svec_release(_z_svec_t *v) {
Expand Down
4 changes: 2 additions & 2 deletions src/protocol/definitions/message.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
void _z_msg_reply_clear(_z_msg_reply_t *msg) { _z_push_body_clear(&msg->_body); }

void _z_msg_put_clear(_z_msg_put_t *msg) {
_z_bytes_aliased_drop(&msg->_payload);
_z_bytes_drop(&msg->_payload);
_z_bytes_drop(&msg->_attachment);
_z_encoding_clear(&msg->_encoding);
_z_timestamp_clear(&msg->_commons._timestamp);
Expand All @@ -44,5 +44,5 @@ void _z_msg_query_clear(_z_msg_query_t *msg) {
}
void _z_msg_err_clear(_z_msg_err_t *err) {
_z_encoding_clear(&err->_encoding);
_z_bytes_aliased_drop(&err->_payload);
_z_bytes_drop(&err->_payload);
}
7 changes: 6 additions & 1 deletion src/protocol/definitions/transport.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,12 @@ void _z_t_msg_close_clear(_z_t_msg_close_t *msg) { (void)(msg); }

void _z_t_msg_keep_alive_clear(_z_t_msg_keep_alive_t *msg) { (void)(msg); }

void _z_t_msg_frame_clear(_z_t_msg_frame_t *msg) { (void)(msg); }
void _z_t_msg_frame_clear(_z_t_msg_frame_t *msg) {
// TODO (sashacmc): make in more correct way
if (!msg->_messages._aliased) {
_z_network_message_svec_clear(&msg->_messages);
}
}

void _z_t_msg_fragment_clear(_z_t_msg_fragment_t *msg) { _z_slice_clear(&msg->_payload); }

Expand Down
4 changes: 1 addition & 3 deletions src/protocol/keyexpr.c
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,7 @@ void _z_keyexpr_move(_z_keyexpr_t *dst, _z_keyexpr_t *src) { *dst = _z_keyexpr_s

void _z_keyexpr_clear(_z_keyexpr_t *rk) {
rk->_id = 0;
if (_z_keyexpr_has_suffix(rk)) {
_z_string_clear(&rk->_suffix);
}
_z_string_clear(&rk->_suffix);
}

void _z_keyexpr_free(_z_keyexpr_t **rk) {
Expand Down
4 changes: 2 additions & 2 deletions src/session/query.c
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ z_result_t _z_trigger_query_reply_partial(_z_session_t *zn, const _z_zint_t id,
z_result_t ret = _z_trigger_query_reply_partial_inner(zn, id, keyexpr, msg, kind);
// Clean up
_z_keyexpr_clear(keyexpr);
_z_bytes_aliased_drop(&msg->_payload);
_z_bytes_drop(&msg->_payload);
_z_bytes_drop(&msg->_attachment);
_z_encoding_clear(&msg->_encoding);
return ret;
Expand All @@ -209,7 +209,7 @@ z_result_t _z_trigger_query_reply_err(_z_session_t *zn, _z_zint_t id, _z_msg_err
pen_qry->_callback(&reply, pen_qry->_arg);
}
// Clean up
_z_bytes_aliased_drop(&msg->_payload);
_z_bytes_drop(&msg->_payload);
_z_encoding_clear(&msg->_encoding);
return ret;
}
Expand Down
2 changes: 2 additions & 0 deletions src/session/rx.c
Original file line number Diff line number Diff line change
Expand Up @@ -164,5 +164,7 @@ z_result_t _z_handle_network_message(_z_session_rc_t *zsrc, _z_zenoh_message_t *
}
}
}
// TODO (sashacmc): why it was removed???
_z_msg_clear(msg);
return ret;
}
9 changes: 7 additions & 2 deletions src/session/subscription.c
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ static z_result_t _z_trigger_subscriptions_inner(_z_session_t *zn, _z_subscriber
_z_string_data(&key._suffix));
if (sub_nb == 0) {
_z_keyexpr_clear(&key);
_z_subscription_infos_svec_release(&subs);
return _Z_RES_OK;
}
// Create sample
Expand All @@ -271,9 +272,13 @@ static z_result_t _z_trigger_subscriptions_inner(_z_session_t *zn, _z_subscriber
}
// Clean up
_z_keyexpr_clear(&key);
if (sub_kind == _Z_SUBSCRIBER_KIND_LIVELINESS_SUBSCRIBER) {
_z_subscription_infos_svec_release(&subs);
} else {
#if Z_FEATURE_RX_CACHE != 1
_z_subscription_infos_svec_release(&subs); // Otherwise it's released with cache
_z_subscription_infos_svec_release(&subs); // Otherwise it's released with cache
#endif
}
return _Z_RES_OK;
}

Expand All @@ -286,7 +291,7 @@ z_result_t _z_trigger_subscriptions_impl(_z_session_t *zn, _z_subscriber_kind_t
// Clean up
_z_keyexpr_clear(keyexpr);
_z_encoding_clear(encoding);
_z_bytes_aliased_drop(payload);
_z_bytes_drop(payload);
_z_bytes_drop(attachment);
return ret;
}
Expand Down
11 changes: 7 additions & 4 deletions tests/z_msgcodec_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ void payload_field(void) {

// Free
_z_bytes_drop(&e_pld);
_z_bytes_aliased_drop(&d_pld);
_z_bytes_drop(&d_pld);
_z_zbuf_clear(&zbf);
_z_wbuf_clear(&wbf);
}
Expand Down Expand Up @@ -1799,9 +1799,8 @@ void assert_eq_net_msg(const _z_network_message_t *left, const _z_network_messag
_z_network_message_svec_t gen_net_msgs(size_t n) {
_z_network_message_svec_t ret = _z_network_message_svec_make(n);
for (size_t i = 0; i < n; i++) {
_z_network_message_t *msg = _z_network_message_svec_get_mut(&ret, i);
memset(msg, 0, sizeof(_z_network_message_t));
*msg = gen_net_msg();
_z_network_message_t msg = gen_net_msg();
_z_network_message_svec_append(&ret, &msg, false);
}
return ret;
}
Expand Down Expand Up @@ -1829,6 +1828,8 @@ void frame_message(void) {
z_result_t ret = _z_frame_decode(&decoded, &zbf, expected._header, &arcs, &msg);
assert(_Z_RES_OK == ret);
assert_eq_frame(&expected._body._frame, &decoded);
_z_network_message_svec_clear(&msg);
_z_arc_slice_svec_release(&arcs);
_z_t_msg_frame_clear(&decoded);
_z_t_msg_clear(&expected);
_z_zbuf_clear(&zbf);
Expand Down Expand Up @@ -1924,6 +1925,8 @@ void transport_message(void) {
z_result_t ret = _z_transport_message_decode(&decoded, &zbf, &arcs, &msg);
assert(_Z_RES_OK == ret);
assert_eq_transport(&expected, &decoded);
_z_network_message_svec_clear(&msg);
_z_arc_slice_svec_release(&arcs);
_z_t_msg_clear(&decoded);
_z_t_msg_clear(&expected);
_z_zbuf_clear(&zbf);
Expand Down
2 changes: 2 additions & 0 deletions tests/z_refcount_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,8 @@ void test_simple_rc_decr(void) {
_dummy_simple_rc_t drc2 = _dummy_simple_rc_clone(&drc1);
assert(!_dummy_simple_rc_decr(&drc2));
assert(_dummy_simple_rc_decr(&drc1));
// free manualy, to make asan happy, because counter already zero
z_free(drc1._val);
}

int main(void) {
Expand Down

0 comments on commit 4c7b117

Please sign in to comment.