lttng add-trigger: replace --exclude-names with --exclude-name

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 <[email protected]> Signed-off-by: Jérémie Galarneau <[email protected]>
lttng · Apr 23, 2021 · b03a81f · b03a81f
1 parent a5a2128
commit b03a81f
Show file tree

Hide file tree

Showing 4 changed files with 98 additions and 46 deletions.
diff --git 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 @@ static bool parse_syscall_emission_site_from_type(const char *str,
 
 /* 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 @@ struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv)
 	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
@@ -327,18 +327,14 @@ int check_exclusion_subsets(const char *event_name, const char *exclusion)
  * 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
@@ -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
@@ -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