lttng: fix argument numbers in add-trigger error messages

Argument numbers in add-trigger argument parsing error messages are
wrong.  For example:

    $ ./src/bin/lttng/lttng add-trigger --condition event-rule-matches --foo
    Error: While parsing argument #1 (`--foo`): Unknown option `--foo`

This is due to the fact that multiple separate argpar iterators are
created to parse an add-trigger command line.  Iterators are created at
the top-level, to parse the top-level arguments.  Iterators are also
created when parsing a condition or an action, to parse the arguments
specific to that condition or action.  As a result, iterators are passed
a subset of the full command line, and the argument indices in the error
messages are off.

Fix that by passing around an "argc offset", which represents by how
much what's being parsed is offset from the full command-line.  Use that
to adjust the error messages to give indices that make sense to the
user:

    $ ./src/bin/lttng/lttng add-trigger --condition event-rule-matches --foo
    Error: While parsing argument #4 (`--foo`): Unknown option `--foo`

Change-Id: I1d312593d338641d0ec10abe515b640771a1f160
Signed-off-by: Simon Marchi <[email protected]>
Signed-off-by: Jérémie Galarneau <[email protected]>

Author: simark

src/bin/lttng/commands/add_trigger.cpp

@@ -649,7 +649,8 @@ struct parse_event_rule_res {
 };
 
 static
-struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv)
+struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv,
+		int argc_offset)
 {
 	enum lttng_event_rule_type event_rule_type =
 			LTTNG_EVENT_RULE_TYPE_UNKNOWN;
@@ -695,8 +696,8 @@ struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv)
 	while (true) {
 		enum parse_next_item_status status;
 
-		status = parse_next_item(argpar_iter, &argpar_item, *argv,
-			false, NULL);
+		status = parse_next_item(argpar_iter, &argpar_item,
+			argc_offset, *argv, false, NULL);
 		if (status == PARSE_NEXT_ITEM_STATUS_ERROR) {
 			goto error;
 		} else if (status == PARSE_NEXT_ITEM_STATUS_END) {
@@ -1347,13 +1348,14 @@ struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv)
 }
 
 static
-struct lttng_condition *handle_condition_event(int *argc, const char ***argv)
+struct lttng_condition *handle_condition_event(int *argc, const char ***argv,
+		int argc_offset)
 {
 	struct parse_event_rule_res res;
 	struct lttng_condition *c;
 	size_t i;
 
-	res = parse_event_rule(argc, argv);
+	res = parse_event_rule(argc, argv, argc_offset);
 	if (!res.er) {
 		c = NULL;
 		goto error;
@@ -1403,7 +1405,8 @@ struct lttng_condition *handle_condition_event(int *argc, const char ***argv)
 
 struct condition_descr {
 	const char *name;
-	struct lttng_condition *(*handler) (int *argc, const char ***argv);
+	struct lttng_condition *(*handler) (int *argc, const char ***argv,
+		int argc_offset);
 };
 
 static const
@@ -1413,7 +1416,7 @@ struct condition_descr condition_descrs[] = {
 
 static
 struct lttng_condition *parse_condition(const char *condition_name, int *argc,
-		const char ***argv)
+		const char ***argv, int argc_offset)
 {
 	int i;
 	struct lttng_condition *cond;
@@ -1431,7 +1434,7 @@ struct lttng_condition *parse_condition(const char *condition_name, int *argc,
 		goto error;
 	}
 
-	cond = descr->handler(argc, argv);
+	cond = descr->handler(argc, argv, argc_offset);
 	if (!cond) {
 		/* The handler has already printed an error message. */
 		goto error;
@@ -1524,7 +1527,8 @@ static const struct argpar_opt_descr notify_action_opt_descrs[] = {
 };
 
 static
-struct lttng_action *handle_action_notify(int *argc, const char ***argv)
+struct lttng_action *handle_action_notify(int *argc, const char ***argv,
+		int argc_offset)
 {
 	struct lttng_action *action = NULL;
 	struct argpar_iter *argpar_iter = NULL;
@@ -1540,8 +1544,9 @@ struct lttng_action *handle_action_notify(int *argc, const char ***argv)
 	while (true) {
 		enum parse_next_item_status status;
 
-		status = parse_next_item(argpar_iter, &argpar_item, *argv,
-			false, "While parsing `notify` action:");
+		status = parse_next_item(argpar_iter, &argpar_item,
+			argc_offset, *argv, false,
+			"While parsing `notify` action:");
 		if (status == PARSE_NEXT_ITEM_STATUS_ERROR) {
 			goto error;
 		} else if (status == PARSE_NEXT_ITEM_STATUS_END) {
@@ -1612,6 +1617,7 @@ struct lttng_action *handle_action_notify(int *argc, const char ***argv)
 
 static struct lttng_action *handle_action_simple_session_with_policy(int *argc,
 		const char ***argv,
+		int argc_offset,
 		struct lttng_action *(*create_action_cb)(void),
 		enum lttng_action_status (*set_session_name_cb)(
 				struct lttng_action *, const char *),
@@ -1644,8 +1650,9 @@ static struct lttng_action *handle_action_simple_session_with_policy(int *argc,
 	while (true) {
 		enum parse_next_item_status status;
 
-		status = parse_next_item(argpar_iter, &argpar_item, *argv,
-			false, "While parsing `%s` action:", action_name);
+		status = parse_next_item(argpar_iter, &argpar_item, argc_offset,
+			*argv, false,
+			"While parsing `%s` action:", action_name);
 		if (status == PARSE_NEXT_ITEM_STATUS_ERROR) {
 			goto error;
 		} else if (status == PARSE_NEXT_ITEM_STATUS_END) {
@@ -1730,29 +1737,32 @@ static struct lttng_action *handle_action_simple_session_with_policy(int *argc,
 
 static
 struct lttng_action *handle_action_start_session(int *argc,
-		const char ***argv)
+		const char ***argv, int argc_offset)
 {
 	return handle_action_simple_session_with_policy(argc, argv,
+			argc_offset,
 			lttng_action_start_session_create,
 			lttng_action_start_session_set_session_name,
 			lttng_action_start_session_set_rate_policy, "start");
 }
 
 static
 struct lttng_action *handle_action_stop_session(int *argc,
-		const char ***argv)
+		const char ***argv, int argc_offset)
 {
 	return handle_action_simple_session_with_policy(argc, argv,
+			argc_offset,
 			lttng_action_stop_session_create,
 			lttng_action_stop_session_set_session_name,
 			lttng_action_stop_session_set_rate_policy, "stop");
 }
 
 static
 struct lttng_action *handle_action_rotate_session(int *argc,
-		const char ***argv)
+		const char ***argv, int argc_offset)
 {
 	return handle_action_simple_session_with_policy(argc, argv,
+		argc_offset,
 		lttng_action_rotate_session_create,
 		lttng_action_rotate_session_set_session_name,
 		lttng_action_rotate_session_set_rate_policy,
@@ -1772,7 +1782,7 @@ static const struct argpar_opt_descr snapshot_action_opt_descrs[] = {
 
 static
 struct lttng_action *handle_action_snapshot_session(int *argc,
-		const char ***argv)
+		const char ***argv, int argc_offset)
 {
 	struct lttng_action *action = NULL;
 	struct argpar_iter *argpar_iter = NULL;
@@ -1800,8 +1810,8 @@ struct lttng_action *handle_action_snapshot_session(int *argc,
 	while (true) {
 		enum parse_next_item_status status;
 
-		status = parse_next_item(argpar_iter, &argpar_item, *argv,
-			false, "While parsing `snapshot` action:");
+		status = parse_next_item(argpar_iter, &argpar_item, argc_offset,
+			*argv, false, "While parsing `snapshot` action:");
 		if (status == PARSE_NEXT_ITEM_STATUS_ERROR) {
 			goto error;
 		} else if (status == PARSE_NEXT_ITEM_STATUS_END) {
@@ -2072,7 +2082,8 @@ struct lttng_action *handle_action_snapshot_session(int *argc,
 
 struct action_descr {
 	const char *name;
-	struct lttng_action *(*handler) (int *argc, const char ***argv);
+	struct lttng_action *(*handler) (int *argc, const char ***argv,
+		int argc_offset);
 };
 
 static const
@@ -2085,7 +2096,8 @@ struct action_descr action_descrs[] = {
 };
 
 static
-struct lttng_action *parse_action(const char *action_name, int *argc, const char ***argv)
+struct lttng_action *parse_action(const char *action_name, int *argc,
+		const char ***argv, int argc_offset)
 {
 	int i;
 	struct lttng_action *action;
@@ -2103,7 +2115,7 @@ struct lttng_action *parse_action(const char *action_name, int *argc, const char
 		goto error;
 	}
 
-	action = descr->handler(argc, argv);
+	action = descr->handler(argc, argv, argc_offset);
 	if (!action) {
 		/* The handler has already printed an error message. */
 		goto error;
@@ -2194,8 +2206,8 @@ int cmd_add_trigger(int argc, const char **argv)
 			goto error;
 		}
 
-		status = parse_next_item(argpar_iter, &argpar_item, my_argv,
-			true, NULL);
+		status = parse_next_item(argpar_iter, &argpar_item,
+			argc - my_argc, my_argv, true, NULL);
 		if (status == PARSE_NEXT_ITEM_STATUS_ERROR) {
 			goto error;
 		} else if (status == PARSE_NEXT_ITEM_STATUS_END) {
@@ -2234,7 +2246,8 @@ int cmd_add_trigger(int argc, const char **argv)
 				goto error;
 			}
 
-			condition = parse_condition(arg, &my_argc, &my_argv);
+			condition = parse_condition(arg, &my_argc, &my_argv,
+				argc - my_argc);
 			if (!condition) {
 				/*
 				 * An error message was already printed by
@@ -2247,7 +2260,8 @@ int cmd_add_trigger(int argc, const char **argv)
 		}
 		case OPT_ACTION:
 		{
-			action = parse_action(arg, &my_argc, &my_argv);
+			action = parse_action(arg, &my_argc, &my_argv,
+				argc - my_argc);
 			if (!action) {
 				/*
 				 * An error message was already printed by

src/bin/lttng/commands/list_triggers.cpp

@@ -1335,7 +1335,7 @@ int cmd_list_triggers(int argc, const char **argv)
 	while (true) {
 		enum parse_next_item_status status;
 
-		status = parse_next_item(argpar_iter, &argpar_item, argv,
+		status = parse_next_item(argpar_iter, &argpar_item, 1, argv,
 			true, NULL);
 		if (status == PARSE_NEXT_ITEM_STATUS_ERROR) {
 			goto error;

src/bin/lttng/commands/remove_trigger.cpp

@@ -111,7 +111,7 @@ int cmd_remove_trigger(int argc, const char **argv)
 	while (true) {
 		enum parse_next_item_status status;
 
-		status = parse_next_item(argpar_iter, &argpar_item, argv,
+		status = parse_next_item(argpar_iter, &argpar_item, 1, argv,
 			true, NULL);
 		if (status == PARSE_NEXT_ITEM_STATUS_ERROR) {
 			goto error;

src/common/argpar-utils/argpar-utils.c

@@ -23,10 +23,12 @@
  * `context_fmt`, if non-NULL, is formatted using `args` and prepended to the
  * error message.
  *
+ * Add `argc_offset` the the argument index mentioned in the error message.
+ *
  * The returned string must be freed by the caller.
  */
-static ATTR_FORMAT_PRINTF(3, 0)
-char *format_arg_error_v(const struct argpar_error *error,
+static ATTR_FORMAT_PRINTF(4, 0)
+char *format_arg_error_v(const struct argpar_error *error, int argc_offset,
 		const char **argv, const char *context_fmt, va_list args)
 {
 	char *str = NULL;
@@ -59,7 +61,7 @@ char *format_arg_error_v(const struct argpar_error *error,
 
 		ret = strutils_appendf(&str,
 			WHILE_PARSING_ARG_N_ARG_FMT "Missing required argument for option `%s`",
-			orig_index + 1, arg, arg);
+			orig_index + 1 + argc_offset, argv[orig_index], arg);
 		if (ret < 0) {
 			goto end;
 		}
@@ -77,11 +79,11 @@ char *format_arg_error_v(const struct argpar_error *error,
 		if (is_short) {
 			ret = strutils_appendf(&str,
 				WHILE_PARSING_ARG_N_ARG_FMT "Unexpected argument for option `-%c`",
-				orig_index + 1, arg, descr->short_name);
+				orig_index + 1 + argc_offset, arg, descr->short_name);
 		} else {
 			ret = strutils_appendf(&str,
 				WHILE_PARSING_ARG_N_ARG_FMT "Unexpected argument for option `--%s`",
-				orig_index + 1, arg, descr->long_name);
+				orig_index + 1 + argc_offset, arg, descr->long_name);
 		}
 
 		if (ret < 0) {
@@ -97,7 +99,7 @@ char *format_arg_error_v(const struct argpar_error *error,
 
 		ret = strutils_appendf(&str,
 			WHILE_PARSING_ARG_N_ARG_FMT "Unknown option `%s`",
-			orig_index + 1, argv[orig_index], unknown_opt);
+			orig_index + 1 + argc_offset, argv[orig_index], unknown_opt);
 
 		if (ret < 0) {
 			goto end;
@@ -118,8 +120,9 @@ char *format_arg_error_v(const struct argpar_error *error,
 }
 
 enum parse_next_item_status parse_next_item(struct argpar_iter *iter,
-		const struct argpar_item **item, const char **argv,
-		bool unknown_opt_is_error, const char *context_fmt, ...)
+		const struct argpar_item **item, int argc_offset,
+		const char **argv, bool unknown_opt_is_error,
+		const char *context_fmt, ...)
 {
 	enum argpar_iter_next_status status;
 	const struct argpar_error *error = NULL;
@@ -145,7 +148,8 @@ enum parse_next_item_status parse_next_item(struct argpar_iter *iter,
 		}
 
 		va_start(args, context_fmt);
-		err_str = format_arg_error_v(error, argv, context_fmt, args);
+		err_str = format_arg_error_v(error, argc_offset, argv,
+			context_fmt, args);
 		va_end(args);
 
 		if (err_str) {

src/common/argpar-utils/argpar-utils.h

@@ -39,14 +39,16 @@ enum parse_next_item_status
  * On error, print a descriptive error message and return
  * PARSE_NEXT_ITEM_STATUS_ERROR.  If `context_fmt` is non-NULL, it is formatted
  * using the following arguments and prepended to the error message.
+ * Add `argc_offset` to the argument index mentioned in the error message.
  *
  * If `unknown_opt_is_error` is true, an unknown option is considered an error.
  * Otherwise, it is considered as the end of the argument list.
  */
-ATTR_FORMAT_PRINTF(5, 6)
+ATTR_FORMAT_PRINTF(6, 7)
 enum parse_next_item_status parse_next_item(struct argpar_iter *iter,
-		const struct argpar_item **item, const char **argv,
-		bool unknown_opt_is_error, const char *context_fmt, ...);
+		const struct argpar_item **item, int argc_offset,
+		const char **argv, bool unknown_opt_is_error,
+		const char *context_fmt, ...);
 
 #ifdef __cplusplus
 }

tests/regression/tools/trigger/test_add_trigger_cli

@@ -409,7 +409,7 @@ test_success "--action snapshot-session with ctrl/data URIs" "notify-15"\
 test_failure "no args" "Error: Missing --condition."
 
 test_failure "unknown option" \
-	"Error: While parsing argument #1 (\`--hello\`): Unknown option \`--hello\`" \
+	"Error: While parsing argument #2 (\`--hello\`): Unknown option \`--hello\`" \
 	--hello
 
 test_failure "missing --action" \
@@ -423,7 +423,7 @@ test_failure "two --condition" \
 	--action notify
 
 test_failure "missing argument to --name" \
-	"Error: While parsing argument #1 (\`--name\`): Missing required argument for option \`--name\`" \
+	"Error: While parsing argument #2 (\`--name\`): Missing required argument for option \`--name\`" \
 	--name
 
 for cmd in rate-policy=once-after rate-policy=every; do
@@ -450,7 +450,7 @@ test_failure "invalid argument to --rate-policy: unknown policy type" \
 
 # `--condition` failures
 test_failure "missing args after --condition" \
-	"Error: While parsing argument #1 (\`--condition\`): Missing required argument for option \`--condition\`" \
+	"Error: While parsing argument #2 (\`--condition\`): Missing required argument for option \`--condition\`" \
 	--condition
 test_failure "unknown --condition" \
 	"Error: Unknown condition name 'zoofest'" \
@@ -502,7 +502,7 @@ test_failure "--exclude-name with non-glob name" \
 	--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`' \
+	'Error: While parsing argument #7 (`--capture`): Missing required argument for option `--capture`' \
 	--action notify \
 	--condition event-rule-matches --type=user --capture
 
@@ -548,7 +548,7 @@ test_failure "--condition event-rule-matches --capture: missing colon in app-spe
 
 # `--action` failures
 test_failure "missing args after --action" \
-	"Error: While parsing argument #1 (\`--action\`): Missing required argument for option \`--action\`" \
+	"Error: While parsing argument #5 (\`--action\`): Missing required argument for option \`--action\`" \
 	--condition event-rule-matches --type=user \
 	--action