From 2460203ad4c64f0dcdb716e833abee10a79ec092 Mon Sep 17 00:00:00 2001 From: Francis Deslauriers Date: Wed, 23 Jun 2021 12:33:18 -0400 Subject: [PATCH] actions: list: Add `for_each_action_{const, mutable}()` macros MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Accessing all the inner actions of a action list in a loop is a common access pattern. This commit adds 2 `for_each` macros to iterate over all elements either using a const or a mutable pointer. Add a few unit tests for the list action to test these macros. Signed-off-by: Francis Deslauriers Signed-off-by: Jérémie Galarneau Change-Id: I9aff0b81e1f782b5d20c3fcb82ee7028da8dd810 --- .clang-format | 2 + include/lttng/action/list-internal.hpp | 18 ++++ src/bin/lttng-sessiond/action-executor.cpp | 11 +-- .../notification-thread-events.cpp | 13 +-- src/bin/lttng/commands/list_triggers.cpp | 19 ++-- .../tools/trigger/utils/Makefile.am | 2 +- ...ation-client.c => notification-client.cpp} | 16 +--- tests/unit/test_action.cpp | 94 ++++++++++++++++++- 8 files changed, 130 insertions(+), 45 deletions(-) rename tests/regression/tools/trigger/utils/{notification-client.c => notification-client.cpp} (94%) diff --git a/.clang-format b/.clang-format index fd362abba..f854e32c9 100644 --- a/.clang-format +++ b/.clang-format @@ -52,6 +52,8 @@ ForEachMacros: - 'cds_lfht_for_each_duplicate' - 'cds_list_for_each_entry' - 'cds_list_for_each_entry_safe' + - 'for_each_action_mutable' + - 'for_each_action_const' IncludeBlocks: Regroup IncludeCategories: diff --git a/include/lttng/action/list-internal.hpp b/include/lttng/action/list-internal.hpp index 7282ab7e4..53cab5897 100644 --- a/include/lttng/action/list-internal.hpp +++ b/include/lttng/action/list-internal.hpp @@ -10,6 +10,9 @@ #include +#include + +#include #include struct lttng_action; @@ -38,4 +41,19 @@ lttng_action_list_mi_serialize(const struct lttng_trigger *trigger, const struct mi_lttng_error_query_callbacks *error_query_callbacks, struct lttng_dynamic_array *action_path_indexes); +#define for_each_action_const(__action_element, __action_list) \ + assert(lttng_action_get_type(__action_list) == LTTNG_ACTION_TYPE_LIST); \ + \ + for (unsigned int __action_idx = 0; \ + (__action_element = lttng_action_list_get_at_index(__action_list, __action_idx)); \ + __action_idx++) + +#define for_each_action_mutable(__action_element, __action_list) \ + assert(lttng_action_get_type(__action_list) == LTTNG_ACTION_TYPE_LIST); \ + \ + for (unsigned int __action_idx = 0; \ + (__action_element = \ + lttng_action_list_borrow_mutable_at_index(__action_list, __action_idx)); \ + __action_idx++) + #endif /* LTTNG_ACTION_LIST_INTERNAL_H */ diff --git a/src/bin/lttng-sessiond/action-executor.cpp b/src/bin/lttng-sessiond/action-executor.cpp index 31683547b..a9476bd31 100644 --- a/src/bin/lttng-sessiond/action-executor.cpp +++ b/src/bin/lttng-sessiond/action-executor.cpp @@ -992,16 +992,11 @@ static int add_action_to_subitem_array(struct lttng_action *action, LTTNG_ASSERT(subitems); if (type == LTTNG_ACTION_TYPE_LIST) { - unsigned int count, i; + struct lttng_action *inner_action = NULL; - status = lttng_action_list_get_count(action, &count); - LTTNG_ASSERT(status == LTTNG_ACTION_STATUS_OK); - - for (i = 0; i < count; i++) { - struct lttng_action *inner_action = nullptr; - - inner_action = lttng_action_list_borrow_mutable_at_index(action, i); + for_each_action_mutable (inner_action, action) { LTTNG_ASSERT(inner_action); + ret = add_action_to_subitem_array(inner_action, subitems); if (ret) { goto end; diff --git a/src/bin/lttng-sessiond/notification-thread-events.cpp b/src/bin/lttng-sessiond/notification-thread-events.cpp index 58a38cce1..ab0fa84ce 100644 --- a/src/bin/lttng-sessiond/notification-thread-events.cpp +++ b/src/bin/lttng-sessiond/notification-thread-events.cpp @@ -2502,9 +2502,8 @@ end: static bool is_trigger_action_notify(const struct lttng_trigger *trigger) { bool is_notify = false; - unsigned int i, count; - enum lttng_action_status action_status; const struct lttng_action *action = lttng_trigger_get_const_action(trigger); + const struct lttng_action *inner_action; enum lttng_action_type action_type; LTTNG_ASSERT(action); @@ -2516,14 +2515,8 @@ static bool is_trigger_action_notify(const struct lttng_trigger *trigger) goto end; } - action_status = lttng_action_list_get_count(action, &count); - LTTNG_ASSERT(action_status == LTTNG_ACTION_STATUS_OK); - - for (i = 0; i < count; i++) { - const struct lttng_action *inner_action = lttng_action_list_get_at_index(action, i); - - action_type = lttng_action_get_type(inner_action); - if (action_type == LTTNG_ACTION_TYPE_NOTIFY) { + for_each_action_const (inner_action, action) { + if (lttng_action_get_type(inner_action) == LTTNG_ACTION_TYPE_NOTIFY) { is_notify = true; goto end; } diff --git a/src/bin/lttng/commands/list_triggers.cpp b/src/bin/lttng/commands/list_triggers.cpp index d39615390..c9c6c265d 100644 --- a/src/bin/lttng/commands/list_triggers.cpp +++ b/src/bin/lttng/commands/list_triggers.cpp @@ -10,14 +10,16 @@ #include "common/argpar/argpar.h" #include "common/dynamic-array.hpp" #include "common/mi-lttng.hpp" +#include "lttng/action/list-internal.hpp" -#include /* For lttng_condition_type_str(). */ #include "lttng/condition/condition-internal.hpp" #include "lttng/condition/event-rule-matches-internal.hpp" #include "lttng/condition/event-rule-matches.h" + /* For lttng_domain_type_str(). */ #include "lttng/domain-internal.hpp" + /* For lttng_event_rule_kernel_syscall_emission_site_str() */ #include "../loglevel.hpp" #include "lttng/event-rule/kernel-syscall-internal.hpp" @@ -1022,21 +1024,14 @@ static void print_one_trigger(const struct lttng_trigger *trigger) action = lttng_trigger_get_const_action(trigger); action_type = lttng_action_get_type(action); if (action_type == LTTNG_ACTION_TYPE_LIST) { - unsigned int count, i; - enum lttng_action_status action_status; + const struct lttng_action *subaction; + uint64_t action_path_index = 0; MSG(" actions:"); - - action_status = lttng_action_list_get_count(action, &count); - LTTNG_ASSERT(action_status == LTTNG_ACTION_STATUS_OK); - - for (i = 0; i < count; i++) { - const uint64_t action_path_index = i; - const struct lttng_action *subaction = - lttng_action_list_get_at_index(action, i); - + for_each_action_const (subaction, action) { _MSG(" "); print_one_action(trigger, subaction, &action_path_index, 1); + action_path_index++; } } else { _MSG(" action:"); diff --git a/tests/regression/tools/trigger/utils/Makefile.am b/tests/regression/tools/trigger/utils/Makefile.am index a6a0258d4..3522aaec3 100644 --- a/tests/regression/tools/trigger/utils/Makefile.am +++ b/tests/regression/tools/trigger/utils/Makefile.am @@ -7,7 +7,7 @@ noinst_PROGRAMS = \ notification-client \ register-some-triggers -notification_client_SOURCES = notification-client.c +notification_client_SOURCES = notification-client.cpp notification_client_LDADD = $(LIBLTTNG_CTL) \ $(top_builddir)/tests/utils/libtestutils.la diff --git a/tests/regression/tools/trigger/utils/notification-client.c b/tests/regression/tools/trigger/utils/notification-client.cpp similarity index 94% rename from tests/regression/tools/trigger/utils/notification-client.c rename to tests/regression/tools/trigger/utils/notification-client.cpp index b656d726b..02dc06260 100644 --- a/tests/regression/tools/trigger/utils/notification-client.c +++ b/tests/regression/tools/trigger/utils/notification-client.cpp @@ -7,6 +7,7 @@ #include "utils.h" +#include #include #include @@ -37,19 +38,10 @@ static struct option long_options[] = { static bool action_list_contains_notify(const struct lttng_action *action_list) { - unsigned int i, count; - enum lttng_action_status status = lttng_action_list_get_count(action_list, &count); + const struct lttng_action *sub_action; - if (status != LTTNG_ACTION_STATUS_OK) { - printf("Failed to get action count from action list\n"); - exit(1); - } - - for (i = 0; i < count; i++) { - const struct lttng_action *action = lttng_action_list_get_at_index(action_list, i); - const enum lttng_action_type action_type = lttng_action_get_type(action); - - if (action_type == LTTNG_ACTION_TYPE_NOTIFY) { + for_each_action_const (sub_action, action_list) { + if (lttng_action_get_type(sub_action) == LTTNG_ACTION_TYPE_NOTIFY) { return true; } } diff --git a/tests/unit/test_action.cpp b/tests/unit/test_action.cpp index c54b0278d..131a3af1d 100644 --- a/tests/unit/test_action.cpp +++ b/tests/unit/test_action.cpp @@ -14,6 +14,7 @@ #include #include +#include #include #include #include @@ -33,7 +34,7 @@ int lttng_opt_quiet = 1; int lttng_opt_verbose; int lttng_opt_mi; -#define NUM_TESTS 60 +#define NUM_TESTS 71 static void test_action_notify() { @@ -98,7 +99,95 @@ static void test_action_notify() lttng_payload_reset(&payload); } -static void test_action_rotate_session() +static void test_action_list(void) +{ + int ret, action_idx; + struct lttng_action *list_action = NULL, *list_action_from_buffer = NULL, + *mut_inner_action = NULL, *stop_session_action = NULL, + *notify_action = NULL, *start_session_action = NULL; + const struct lttng_action *const_inner_action; + struct lttng_payload payload; + + lttng_payload_init(&payload); + + list_action = lttng_action_list_create(); + ok(list_action, "Create list action"); + ok(lttng_action_get_type(list_action) == LTTNG_ACTION_TYPE_LIST, + "Action has type LTTNG_ACTION_TYPE_LIST"); + + start_session_action = lttng_action_start_session_create(); + (void) lttng_action_start_session_set_session_name(start_session_action, "une-session"); + + stop_session_action = lttng_action_stop_session_create(); + (void) lttng_action_stop_session_set_session_name(stop_session_action, "une-autre-session"); + notify_action = lttng_action_notify_create(); + + lttng_action_list_add_action(list_action, start_session_action); + lttng_action_list_add_action(list_action, stop_session_action); + lttng_action_list_add_action(list_action, notify_action); + + ret = lttng_action_serialize(list_action, &payload); + ok(ret == 0, "Action list serialized"); + + { + struct lttng_payload_view view = lttng_payload_view_from_payload(&payload, 0, -1); + (void) lttng_action_create_from_payload(&view, &list_action_from_buffer); + } + ok(list_action_from_buffer, "Notify action created from payload is non-null"); + + ok(lttng_action_is_equal(list_action, list_action_from_buffer), + "Serialized and de-serialized list action are equal"); + + action_idx = 0; + for_each_action_const (const_inner_action, list_action) { + enum lttng_action_type inner_action_type = + lttng_action_get_type(const_inner_action); + switch (action_idx) { + case 0: + ok(inner_action_type == LTTNG_ACTION_TYPE_START_SESSION, + "First inner action of action list is `start-session` action"); + break; + case 1: + ok(inner_action_type == LTTNG_ACTION_TYPE_STOP_SESSION, + "Second inner action of action list is `stop-session` action"); + break; + case 2: + ok(inner_action_type == LTTNG_ACTION_TYPE_NOTIFY, + "Third inner action of action list is `notify` action"); + break; + } + action_idx++; + } + + action_idx = 0; + for_each_action_mutable (mut_inner_action, list_action) { + enum lttng_action_type inner_action_type = lttng_action_get_type(mut_inner_action); + switch (action_idx) { + case 0: + ok(inner_action_type == LTTNG_ACTION_TYPE_START_SESSION, + "First inner action of action list is `start-session` action"); + break; + case 1: + ok(inner_action_type == LTTNG_ACTION_TYPE_STOP_SESSION, + "Second inner action of action list is `stop-session` action"); + break; + case 2: + ok(inner_action_type == LTTNG_ACTION_TYPE_NOTIFY, + "Third inner action of action list is `notify` action"); + break; + } + action_idx++; + } + + lttng_action_destroy(list_action); + lttng_action_destroy(list_action_from_buffer); + lttng_action_destroy(start_session_action); + lttng_action_destroy(stop_session_action); + lttng_action_destroy(notify_action); + lttng_payload_reset(&payload); +} + +static void test_action_rotate_session(void) { int ret; enum lttng_action_status status; @@ -458,6 +547,7 @@ int main() { plan_tests(NUM_TESTS); test_action_notify(); + test_action_list(); test_action_rotate_session(); test_action_start_session(); test_action_stop_session(); -- 2.34.1