From b03a81fbbc65c1cf2e9abc67c56465137c788322 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Mon, 19 Apr 2021 22:13:28 -0400 Subject: [PATCH] lttng add-trigger: replace --exclude-names with --exclude-name MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Implement the proposal to make the --exclude-names option singular. The user can specify it multiple times to specify multiple exclusions, rather than specifying a comma-separated list. Change-Id: Id95313bb5576a8d6cecb73b61b537ae190554cd0 Signed-off-by: Simon Marchi Signed-off-by: Jérémie Galarneau --- src/bin/lttng/commands/add_trigger.c | 77 ++++++++++++------- src/bin/lttng/commands/enable_events.c | 50 ++++++++---- .../tools/trigger/test_add_trigger_cli | 15 +++- .../tools/trigger/test_list_triggers_cli | 2 +- 4 files changed, 98 insertions(+), 46 deletions(-) diff --git a/src/bin/lttng/commands/add_trigger.c b/src/bin/lttng/commands/add_trigger.c index ceaf3e807..ee5c353e4 100644 --- a/src/bin/lttng/commands/add_trigger.c +++ b/src/bin/lttng/commands/add_trigger.c @@ -46,7 +46,7 @@ enum { OPT_NAME, OPT_FILTER, - OPT_EXCLUDE_NAMES, + OPT_EXCLUDE_NAME, OPT_EVENT_NAME, OPT_LOG_LEVEL, @@ -66,7 +66,7 @@ enum { static const struct argpar_opt_descr event_rule_opt_descrs[] = { { OPT_FILTER, 'f', "filter", true }, { OPT_NAME, 'n', "name", true }, - { OPT_EXCLUDE_NAMES, 'x', "exclude-names", true }, + { OPT_EXCLUDE_NAME, 'x', "exclude-name", true }, { OPT_LOG_LEVEL, 'l', "log-level", true }, { OPT_EVENT_NAME, 'E', "event-name", true }, @@ -211,9 +211,8 @@ error: /* This is defined in enable_events.c. */ LTTNG_HIDDEN -int create_exclusion_list_and_validate(const char *event_name, - const char *exclusions_arg, - char ***exclusion_list); +int validate_exclusion_list( + const char *event_name, const char *const *exclusions); /* * Parse `str` as a log level in domain `domain_type`. @@ -661,8 +660,8 @@ struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv) /* Tracepoint and syscall options. */ char *name = NULL; - char *exclude_names = NULL; - char **exclusion_list = NULL; + /* Array of strings. */ + struct lttng_dynamic_pointer_array exclude_names; /* For userspace / kernel probe and function. */ char *location = NULL; @@ -676,6 +675,9 @@ struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv) lttng_dynamic_pointer_array_init(&res.capture_descriptors, destroy_event_expr); + + lttng_dynamic_pointer_array_init(&exclude_names, free); + state = argpar_state_create(*argc, *argv, event_rule_opt_descrs); if (!state) { ERR("Failed to allocate an argpar state."); @@ -756,14 +758,20 @@ struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv) } break; - case OPT_EXCLUDE_NAMES: - if (!assign_string(&exclude_names, - item_opt->arg, - "--exclude-names/-x")) { + case OPT_EXCLUDE_NAME: + { + int ret; + + ret = lttng_dynamic_pointer_array_add_pointer( + &exclude_names, + strdup(item_opt->arg)); + if (ret != 0) { + ERR("Failed to add pointer to dynamic pointer array."); goto error; } break; + } case OPT_LOG_LEVEL: if (!assign_string(&log_level_str, item_opt->arg, "--log-level/-l")) { @@ -849,8 +857,8 @@ struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv) goto error; } - if (exclude_names) { - ERR("Can't use --exclude-names/-x with %s event rules.", + if (lttng_dynamic_pointer_array_get_count(&exclude_names) > 0) { + ERR("Can't use --exclude-name/-x with %s event rules.", lttng_event_rule_type_str( event_rule_type)); goto error; @@ -945,17 +953,23 @@ struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv) } } - /* If --exclude/-x was passed, split it into an exclusion list. */ - if (exclude_names) { + /* If --exclude-name/-x was passed, split it into an exclusion list. */ + if (lttng_dynamic_pointer_array_get_count(&exclude_names) > 0) { if (domain_type != LTTNG_DOMAIN_UST) { ERR("Event name exclusions are not yet implemented for %s event rules.", get_domain_str(domain_type)); goto error; } - if (create_exclusion_list_and_validate(name, - exclude_names, &exclusion_list) != 0) { - ERR("Failed to create exclusion list."); + if (validate_exclusion_list(name, + (const char **) exclude_names.array.buffer + .data + + ) != 0) { + /* + * Assume validate_exclusion_list already prints an + * error message. + */ goto error; } } @@ -1005,17 +1019,25 @@ struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv) } /* Set exclusion list. */ - if (exclusion_list) { + if (lttng_dynamic_pointer_array_get_count(&exclude_names) > 0) { int n; - - for (n = 0; exclusion_list[n]; n++) { - event_rule_status = lttng_event_rule_tracepoint_add_exclusion( - res.er, - exclusion_list[n]); + int count = lttng_dynamic_pointer_array_get_count( + &exclude_names); + + for (n = 0; n < count; n++) { + const char *exclude_name = + lttng_dynamic_pointer_array_get_pointer( + &exclude_names, + n); + + event_rule_status = + lttng_event_rule_tracepoint_add_exclusion( + res.er, + exclude_name); if (event_rule_status != LTTNG_EVENT_RULE_STATUS_OK) { ERR("Failed to set tracepoint exclusion list element '%s'", - exclusion_list[n]); + exclude_name); goto error; } } @@ -1178,13 +1200,12 @@ end: argpar_state_destroy(state); free(filter); free(name); - free(exclude_names); + lttng_dynamic_pointer_array_reset(&exclude_names); free(log_level_str); free(location); free(event_name); free(event_rule_type_str); - strutils_free_null_terminated_array_of_strings(exclusion_list); lttng_kernel_probe_location_destroy(kernel_probe_location); lttng_userspace_probe_location_destroy(userspace_probe_location); lttng_log_level_rule_destroy(log_level_rule); @@ -1491,7 +1512,7 @@ static struct lttng_action *handle_action_simple_session_with_policy(int *argc, { OPT_RATE_POLICY, '\0', "rate-policy", true }, ARGPAR_OPT_DESCR_SENTINEL }; - + state = argpar_state_create(*argc, *argv, rate_policy_opt_descrs); if (!state) { ERR("Failed to allocate an argpar state."); diff --git a/src/bin/lttng/commands/enable_events.c b/src/bin/lttng/commands/enable_events.c index 627b7ebf9..056977412 100644 --- a/src/bin/lttng/commands/enable_events.c +++ b/src/bin/lttng/commands/enable_events.c @@ -327,18 +327,14 @@ end: * FIXME: find a good place to declare this since add trigger also uses it */ LTTNG_HIDDEN -int create_exclusion_list_and_validate(const char *event_name, - const char *exclusions_arg, - char ***exclusion_list); - +int validate_exclusion_list( + const char *event_name, const char *const *exclusions); LTTNG_HIDDEN -int create_exclusion_list_and_validate(const char *event_name, - const char *exclusions_arg, - char ***exclusion_list) +int validate_exclusion_list( + const char *event_name, const char *const *exclusions) { - int ret = 0; - char **exclusions = NULL; + int ret; /* Event name must be a valid globbing pattern to allow exclusions. */ if (!strutils_is_star_glob_pattern(event_name)) { @@ -347,19 +343,13 @@ int create_exclusion_list_and_validate(const char *event_name, goto error; } - /* Split exclusions. */ - exclusions = strutils_split(exclusions_arg, ',', true); - if (!exclusions) { - goto error; - } - /* * If the event name is a star-at-end only globbing pattern, * then we can validate the individual exclusions. Otherwise * all exclusions are passed to the session daemon. */ if (strutils_is_star_at_the_end_only_glob_pattern(event_name)) { - char * const *exclusion; + const char *const *exclusion; for (exclusion = exclusions; *exclusion; exclusion++) { if (!strutils_is_star_glob_pattern(*exclusion) || @@ -372,6 +362,34 @@ int create_exclusion_list_and_validate(const char *event_name, } } + ret = 0; + goto end; + +error: + ret = -1; + +end: + return ret; +} + +static int create_exclusion_list_and_validate(const char *event_name, + const char *exclusions_arg, + char ***exclusion_list) +{ + int ret = 0; + char **exclusions = NULL; + + /* Split exclusions. */ + exclusions = strutils_split(exclusions_arg, ',', true); + if (!exclusions) { + goto error; + } + + if (validate_exclusion_list(event_name, (const char **) exclusions) != + 0) { + goto error; + } + *exclusion_list = exclusions; goto end; diff --git a/tests/regression/tools/trigger/test_add_trigger_cli b/tests/regression/tools/trigger/test_add_trigger_cli index de50c9f0f..cf45b4e96 100755 --- a/tests/regression/tools/trigger/test_add_trigger_cli +++ b/tests/regression/tools/trigger/test_add_trigger_cli @@ -23,7 +23,7 @@ TESTDIR="$CURDIR/../../.." # shellcheck source=../../../utils/utils.sh source "$TESTDIR/utils/utils.sh" -plan_tests 276 +plan_tests 285 FULL_LTTNG_BIN="${TESTDIR}/../src/bin/lttng/${LTTNG_BIN}" @@ -138,6 +138,14 @@ test_success "--log-level range any" \ --condition event-rule-matches --domain=user --log-level=.. \ --action notify +test_success "--exclude-name one" \ + --condition event-rule-matches --domain=user --name='bernard*' --exclude-name=bernard-lermite \ + --action notify + +test_success "--exclude-name two" \ + --condition event-rule-matches --domain=user --name='jean-*' --exclude-name jean-chretien -x jean-charest \ + --action notify + skip $ist_root "non-root user: skipping kprobe tests" 18 || { for type in kprobe kernel-probe; do test_success "--condition event-rule-matches probe by symbol" \ @@ -371,6 +379,11 @@ test_failure "--condition event-rule-matches: --type=syscall:nope" \ --condition event-rule-matches --domain=kernel --type=syscall:nope \ --name=open +test_failure "--exclude-name with non-glob name" \ + "Error: Event jean: Exclusions can only be used with a globbing pattern" \ + --condition event-rule-matches --domain=user --name='jean' --exclude-name jean-chretien \ + --action notify + test_failure "--condition event-rule-matches --capture: missing argument (end of arg list)" \ 'Error: While parsing argument #2 (`--capture`): Missing required argument for option `--capture`' \ --action notify \ diff --git a/tests/regression/tools/trigger/test_list_triggers_cli b/tests/regression/tools/trigger/test_list_triggers_cli index 10e143e95..7abae223c 100755 --- a/tests/regression/tools/trigger/test_list_triggers_cli +++ b/tests/regression/tools/trigger/test_list_triggers_cli @@ -94,7 +94,7 @@ test_event_rule_matches_tracepoint () lttng_add_trigger_ok "C" --condition event-rule-matches --domain=user --action notify lttng_add_trigger_ok "A" --condition event-rule-matches --name=aaa --domain=user --filter 'p == 2' --action notify - lttng_add_trigger_ok "D" --condition event-rule-matches --name='hello*' --domain=user -x 'hello2,hello3,hello4' --action notify + lttng_add_trigger_ok "D" --condition event-rule-matches --name='hello*' --domain=user -x hello2 --exclude-name hello3 -x hello4 --action notify lttng_add_trigger_ok "B" --condition event-rule-matches --domain=user --name=gerboise --log-level INFO.. --action notify lttng_add_trigger_ok "E" --condition event-rule-matches --domain=user --name=lemming --log-level WARNING --action notify lttng_add_trigger_ok "J" --condition event-rule-matches --domain=user --name=lemming --log-level .. --action notify -- 2.34.1