From d14f2c5fca99fa6c9af3b3036ea566646c6d6322 Mon Sep 17 00:00:00 2001 From: David Cermak Date: Fri, 31 May 2019 08:29:55 +0200 Subject: [PATCH 1/2] esp_event: extend register/unregister test case to cover overwriting existing handler works as expected --- components/esp_event/test/test_event.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/components/esp_event/test/test_event.c b/components/esp_event/test/test_event.c index 9805bd2e0b..d706db942f 100644 --- a/components/esp_event/test/test_event.c +++ b/components/esp_event/test/test_event.c @@ -126,6 +126,9 @@ static esp_event_loop_args_t test_event_get_default_loop_args() static void test_event_simple_handler(void* event_handler_arg, esp_event_base_t event_base, int32_t event_id, void* event_data) { + if (!event_handler_arg) { + return; + } simple_arg_t* arg = (simple_arg_t*) event_handler_arg; xSemaphoreTake(arg->mutex, portMAX_DELAY); @@ -345,7 +348,13 @@ TEST_CASE("can register/unregister handlers for all events/all events for a spec loop_args.task_name = NULL; TEST_ASSERT_EQUAL(ESP_OK, esp_event_loop_create(&loop_args, &loop)); + /* Register the handler twice to the same base and id but with a different argument (expects to return ESP_OK and log a warning) + * This aims to verify: 1) Handler's argument to be updated + * 2) Registration not to leak memory + */ + TEST_ASSERT_EQUAL(ESP_OK, esp_event_handler_register_with(loop, ESP_EVENT_ANY_BASE, ESP_EVENT_ANY_ID, test_event_simple_handler, NULL)); TEST_ASSERT_EQUAL(ESP_OK, esp_event_handler_register_with(loop, ESP_EVENT_ANY_BASE, ESP_EVENT_ANY_ID, test_event_simple_handler, &arg)); + TEST_ASSERT_EQUAL(ESP_OK, esp_event_handler_register_with(loop, s_test_base1, ESP_EVENT_ANY_ID, test_event_simple_handler, &arg)); TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG, esp_event_handler_register_with(loop, ESP_EVENT_ANY_BASE, TEST_EVENT_BASE1_EV1, test_event_simple_handler, &arg)); TEST_ASSERT_EQUAL(ESP_OK, esp_event_handler_register_with(loop, s_test_base1, TEST_EVENT_BASE1_EV1, test_event_simple_handler, &arg)); From b1f264be695176115656918548b4e58ca6eb593a Mon Sep 17 00:00:00 2001 From: David Cermak Date: Thu, 30 May 2019 16:59:24 +0200 Subject: [PATCH 2/2] esp_event: fix minor memory leak when overwriting alredy registered handler --- components/esp_event/esp_event.c | 1 + 1 file changed, 1 insertion(+) diff --git a/components/esp_event/esp_event.c b/components/esp_event/esp_event.c index d9f65c4a2f..e0a0f04480 100644 --- a/components/esp_event/esp_event.c +++ b/components/esp_event/esp_event.c @@ -164,6 +164,7 @@ static esp_err_t handler_instances_add(esp_event_handler_instances_t* handlers, if (handler == it->handler) { it->arg = handler_arg; ESP_LOGW(TAG, "handler already registered, overwriting"); + free(handler_instance); return ESP_OK; } last = it;