Skip to content

Commit

Permalink
sample: fix issue with memory leak on events
Browse files Browse the repository at this point in the history 
Some events have context allocated on heap,
but it was not freed in all states.

Signed-off-by: Robert Gałat <[email protected]>
  • Loading branch information
@RobertGalatNordic
RobertGalatNordic committed Jun 27, 2024
1 parent 435c341 commit 3e5348f
Show file tree
Hide file tree
Showing 9 changed files with 118 additions and 41 deletions.
4 changes: 3 additions & 1 deletion samples/sid_end_device/include/sidewalk.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,11 @@ typedef enum {
SID_EVENT_LAST,
} sidewalk_event_t;

typedef void (*ctx_free)(void *ctx);
typedef struct {
sidewalk_event_t id;
void *ctx;
ctx_free ctx_free;
} sidewalk_ctx_event_t;

typedef struct {
Expand Down Expand Up @@ -56,7 +58,7 @@ typedef struct {

void sidewalk_start(sidewalk_ctx_t *context);

int sidewalk_event_send(sidewalk_event_t event, void *ctx);
int sidewalk_event_send(sidewalk_event_t event, void *ctx, ctx_free free);

sidewalk_msg_t *get_message_buffer(uint16_t message_id);

Expand Down
4 changes: 2 additions & 2 deletions samples/sid_end_device/src/cli/app.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ static uint32_t persistent_link_mask;

static void on_sidewalk_event(bool in_isr, void *context)
{
int err = sidewalk_event_send(SID_EVENT_SIDEWALK, NULL);
int err = sidewalk_event_send(SID_EVENT_SIDEWALK, NULL, NULL);
if (err) {
LOG_ERR("Send event err %d", err);
};
Expand Down Expand Up @@ -81,7 +81,7 @@ static void on_sidewalk_status_changed(const struct sid_status *status, void *co
} else {
memcpy(new_status, status, sizeof(struct sid_status));
}
sidewalk_event_send(SID_EVENT_NEW_STATUS, new_status);
sidewalk_event_send(SID_EVENT_NEW_STATUS, new_status, sid_hal_free);

switch (status->state) {
case SID_STATE_READY:
Expand Down
46 changes: 34 additions & 12 deletions samples/sid_end_device/src/cli/app_shell.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,18 @@ static int cmd_sid_option_handle_set_link3_profile(const char *value,
return 0;
}

static void free_sid_option_event_ctx(void *ctx)
{
sidewalk_option_t *p_opt = (sidewalk_option_t *)ctx;
if (p_opt == NULL) {
return;
}
if (p_opt->data) {
sid_hal_free(p_opt->data);
}
sid_hal_free(p_opt);
}

static int cmd_sid_option(cli_event_t event, enum sid_option option, void *data, size_t len)
{
sidewalk_option_t *p_opt = sid_hal_malloc(sizeof(sidewalk_option_t));
Expand All @@ -227,12 +239,9 @@ static int cmd_sid_option(cli_event_t event, enum sid_option option, void *data,
p_opt->data = NULL;
}

int err = sidewalk_event_send((sidewalk_event_t)event, p_opt);
int err = sidewalk_event_send((sidewalk_event_t)event, p_opt, free_sid_option_event_ctx);
if (err) {
if (p_opt->data) {
sid_hal_free(p_opt->data);
}
sid_hal_free(p_opt);
free_sid_option_event_ctx(p_opt);
err = -ENOMSG;
}

Expand Down Expand Up @@ -262,7 +271,7 @@ static int cmd_sid_simple_param(cli_event_t event, uint32_t *data)
}
memcpy(event_ctx, data, sizeof(uint32_t));

int err = sidewalk_event_send((sidewalk_event_t)event, event_ctx);
int err = sidewalk_event_send((sidewalk_event_t)event, event_ctx, sid_hal_free);
if (err) {
sid_hal_free(event_ctx);
return -ENOMSG;
Expand Down Expand Up @@ -299,7 +308,7 @@ int cmd_sid_deinit(const struct shell *shell, int32_t argc, const char **argv)
{
CHECK_ARGUMENT_COUNT(argc, CMD_SID_DEINIT_ARG_REQUIRED, CMD_SID_DEINIT_ARG_OPTIONAL);

return sidewalk_event_send((sidewalk_event_t)DUT_EVENT_DEINIT, NULL);
return sidewalk_event_send((sidewalk_event_t)DUT_EVENT_DEINIT, NULL, NULL);
}

int cmd_sid_start(const struct shell *shell, int32_t argc, const char **argv)
Expand Down Expand Up @@ -336,6 +345,18 @@ int cmd_sid_stop(const struct shell *shell, int32_t argc, const char **argv)
return cmd_sid_simple_param(DUT_EVENT_STOP, &link_type);
}

static void free_sid_send_event_ctx(void *ctx)
{
sidewalk_msg_t *send = (sidewalk_msg_t *)ctx;
if (send == NULL) {
return;
}
if (send->msg.data) {
sid_hal_free(send->msg.data);
}
sid_hal_free(send);
}

int cmd_sid_send(const struct shell *shell, int32_t argc, const char **argv)
{
CHECK_ARGUMENT_COUNT(argc, CMD_SID_SEND_ARG_REQUIRED, CMD_SID_SEND_ARG_OPTIONAL);
Expand Down Expand Up @@ -496,16 +517,17 @@ int cmd_sid_send(const struct shell *shell, int32_t argc, const char **argv)

sidewalk_msg_t *send = sid_hal_malloc(sizeof(sidewalk_msg_t));
if (!send) {
sid_hal_free(msg.data);
return -ENOMEM;
}
memset(send, 0x0, sizeof(*send));
memcpy(&send->msg, &msg, sizeof(struct sid_msg));
memcpy(&send->desc, &desc, sizeof(struct sid_msg_desc));

int err = sidewalk_event_send((sidewalk_event_t)SID_EVENT_SEND_MSG, send);
int err = sidewalk_event_send((sidewalk_event_t)SID_EVENT_SEND_MSG, send,
free_sid_send_event_ctx);
if (err) {
sid_hal_free(send->msg.data);
sid_hal_free(send);
free_sid_send_event_ctx(send);
return -ENOMSG;
}

Expand All @@ -517,7 +539,7 @@ int cmd_sid_factory_reset(const struct shell *shell, int32_t argc, const char **
CHECK_ARGUMENT_COUNT(argc, CMD_SID_FACTORY_RESET_ARG_REQUIRED,
CMD_SID_FACTORY_RESET_ARG_OPTIONAL);

int err = sidewalk_event_send((sidewalk_event_t)SID_EVENT_FACTORY_RESET, NULL);
int err = sidewalk_event_send((sidewalk_event_t)SID_EVENT_FACTORY_RESET, NULL, NULL);
if (err) {
shell_error(shell, "event err %d", err);
}
Expand Down Expand Up @@ -911,7 +933,7 @@ int cmd_sid_last_status(const struct shell *shell, int32_t argc, const char **ar
CHECK_ARGUMENT_COUNT(argc, CMD_SID_LAST_STATUS_ARG_REQUIRED,
CMD_SID_LAST_STATUS_ARG_OPTIONAL);

if (0 != sidewalk_event_send((sidewalk_event_t)DUT_EVENT_GET_STATUS, NULL)) {
if (0 != sidewalk_event_send((sidewalk_event_t)DUT_EVENT_GET_STATUS, NULL, NULL)) {
shell_error(shell, "Failed to send Event");
}
return 0;
Expand Down
39 changes: 30 additions & 9 deletions samples/sid_end_device/src/hello/app.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,24 @@ static uint32_t persistent_link_mask;

static void on_sidewalk_event(bool in_isr, void *context)
{
int err = sidewalk_event_send(SID_EVENT_SIDEWALK, NULL);
int err = sidewalk_event_send(SID_EVENT_SIDEWALK, NULL, NULL);
if (err) {
LOG_ERR("Send event err %d", err);
};
}

static void free_sid_echo_event_ctx(void *ctx)
{
sidewalk_msg_t *echo = (sidewalk_msg_t *)ctx;
if (echo == NULL) {
return;
}
if (echo->msg.data) {
sid_hal_free(echo->msg.data);
}
sid_hal_free(echo);
}

static void on_sidewalk_msg_received(const struct sid_msg_desc *msg_desc, const struct sid_msg *msg,
void *context)
{
Expand Down Expand Up @@ -70,10 +82,9 @@ static void on_sidewalk_msg_received(const struct sid_msg_desc *msg_desc, const
echo->desc.link_type = SID_LINK_TYPE_ANY;
echo->desc.link_mode = SID_LINK_MODE_CLOUD;

int err = sidewalk_event_send(SID_EVENT_SEND_MSG, echo);
int err = sidewalk_event_send(SID_EVENT_SEND_MSG, echo, free_sid_echo_event_ctx);
if (err) {
sid_hal_free(echo->msg.data);
sid_hal_free(echo);
free_sid_echo_event_ctx(echo);
LOG_ERR("Send event err %d", err);
} else {
application_state_sending(&global_state_notifier, true);
Expand Down Expand Up @@ -141,7 +152,7 @@ static void on_sidewalk_status_changed(const struct sid_status *status, void *co
} else {
memcpy(new_status, status, sizeof(struct sid_status));
}
err = sidewalk_event_send(SID_EVENT_NEW_STATUS, new_status);
err = sidewalk_event_send(SID_EVENT_NEW_STATUS, new_status, sid_hal_free);

switch (status->state) {
case SID_STATE_READY:
Expand Down Expand Up @@ -190,6 +201,17 @@ static void on_sidewalk_status_changed(const struct sid_status *status, void *co
}
}
}
static void free_sid_hello_event_ctx(void *ctx)
{
sidewalk_msg_t *hello = (sidewalk_msg_t *)ctx;
if (hello == NULL) {
return;
}
if (hello->msg.data) {
sid_hal_free(hello->msg.data);
}
sid_hal_free(hello->msg.data);
}

static void app_button_handler(uint32_t event)
{
Expand All @@ -216,18 +238,17 @@ static void app_button_handler(uint32_t event)
hello->desc.link_type = SID_LINK_TYPE_ANY;
hello->desc.link_mode = SID_LINK_MODE_CLOUD;

int err = sidewalk_event_send(SID_EVENT_SEND_MSG, hello);
int err = sidewalk_event_send(SID_EVENT_SEND_MSG, hello, free_sid_hello_event_ctx);
if (err) {
sid_hal_free(hello->msg.data);
sid_hal_free(hello);
free_sid_hello_event_ctx(hello);
LOG_ERR("Send event err %d", err);
} else {
application_state_sending(&global_state_notifier, true);
}
return;
}

sidewalk_event_send((sidewalk_event_t)event, NULL);
sidewalk_event_send((sidewalk_event_t)event, NULL, NULL);
}

static int app_buttons_init(void)
Expand Down
4 changes: 2 additions & 2 deletions samples/sid_end_device/src/sbdt/file_transfer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ static void on_data_received(const struct sid_bulk_data_transfer_desc *const des
transfer->data = buffer->data;
transfer->data_size = buffer->size;

int err = sidewalk_event_send(SID_EVENT_FILE_TRANSFER, transfer);
int err = sidewalk_event_send(SID_EVENT_FILE_TRANSFER, transfer, sid_hal_free);
if (err) {
LOG_ERR("Event transfer err %d", err);
LOG_INF("Cancelig file transfer");
Expand Down Expand Up @@ -117,7 +117,7 @@ static void on_finalize_request(uint32_t file_id, void *context)
LOG_ERR("dfu image finalize fail %d", err);
}

err = sidewalk_event_send(SID_EVENT_REBOOT, NULL);
err = sidewalk_event_send(SID_EVENT_REBOOT, NULL, NULL);
if (err) {
LOG_ERR("reboot event send ret %d", err);
}
Expand Down
6 changes: 3 additions & 3 deletions samples/sid_end_device/src/sensor_monitoring/app.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ static sidewalk_ctx_t sid_ctx;

static void on_sidewalk_event(bool in_isr, void *context)
{
int err = sidewalk_event_send(SID_EVENT_SIDEWALK, NULL);
int err = sidewalk_event_send(SID_EVENT_SIDEWALK, NULL, NULL);
if (err) {
LOG_ERR("Send event err %d", err);
};
Expand Down Expand Up @@ -112,7 +112,7 @@ static void on_sidewalk_status_changed(const struct sid_status *status, void *co
} else {
memcpy(new_status, status, sizeof(struct sid_status));
}
sidewalk_event_send(SID_EVENT_NEW_STATUS, new_status);
sidewalk_event_send(SID_EVENT_NEW_STATUS, new_status, sid_hal_free);

int err = 0;
switch (status->state) {
Expand Down Expand Up @@ -165,7 +165,7 @@ static void on_sidewalk_status_changed(const struct sid_status *status, void *co

static void sidewalk_btn_handler(uint32_t event)
{
int err = sidewalk_event_send((sidewalk_event_t)event, NULL);
int err = sidewalk_event_send((sidewalk_event_t)event, NULL, NULL);
if (err) {
LOG_ERR("Send event err %d", err);
return;
Expand Down
17 changes: 14 additions & 3 deletions samples/sid_end_device/src/sensor_monitoring/app_tx.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,18 @@ static uint32_t last_link_mask_get(void)
return last_link_mask;
}

static void free_sid_msg_event_ctx(void *ctx)
{
sidewalk_msg_t *sid_msg = (sidewalk_msg_t *)ctx;
if (sid_msg == NULL) {
return;
}
if (sid_msg->msg.data) {
sid_hal_free(sid_msg->msg.data);
}
sid_hal_free(sid_msg);
}

static int app_tx_demo_msg_send(struct sid_parse_state *state, uint8_t *buffer,
struct sid_demo_msg_desc *demo_desc, struct sid_msg_desc *sid_desc)
{
Expand Down Expand Up @@ -111,10 +123,9 @@ static int app_tx_demo_msg_send(struct sid_parse_state *state, uint8_t *buffer,
memcpy(sid_msg->msg.data, msg_buffer, sid_msg->msg.size);
memcpy(&sid_msg->desc, sid_desc, sizeof(struct sid_msg_desc));

int err = sidewalk_event_send(SID_EVENT_SEND_MSG, sid_msg);
int err = sidewalk_event_send(SID_EVENT_SEND_MSG, sid_msg, free_sid_msg_event_ctx);
if (err) {
sid_hal_free(sid_msg->msg.data);
sid_hal_free(sid_msg);
free_sid_msg_event_ctx(sid_msg);
LOG_ERR("Event send err %d", err);
return -EIO;
};
Expand Down
Loading

0 comments on commit 3e5348f

Please sign in to comment.