From: Francis Deslauriers <francis.deslauriers@efficios.com>
Date: Tue, 7 Jan 2020 20:34:07 +0000 (-0500)
Subject: Fix: track-untrack.c: regression of `--all --pid` option ordering
X-Git-Url: http://git.efficios.com/?p=lttng-tools.git;a=commitdiff_plain;h=adce7589954ef2c8236be5145e664b4383474d43

Fix: track-untrack.c: regression of `--all --pid` option ordering

Background
==========
The `tests/regression/kernel/test_callstack` test uses the PID tracker
to restrict what events end up in the trace. It does that by using the
following lttng command:
  lttng untrack -s $SESSION_NAME --all --pid -k

Issue
=====
A regression was introduced along with the UID tracker patch series that
makes the above command fail silently and return status 1. The bug is in
the argument parsing function that assumes that the tracker option
(`--pid` in this case) would appear before the `--all` option.

Where the above command fails, the one below works as expected:
  lttng untrack -s $SESSION_NAME --pid --all -k

Both ordering are valid and should be accepted.

Fix
===
When parsing the arguments, first record the fact that we encountered the
`--all` flag, and later check to what tracker it applies too.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I85f7a2fa8d6f67056aeff8edf3d12508f5ae0879
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
---

diff --git a/src/bin/lttng/commands/track-untrack.c b/src/bin/lttng/commands/track-untrack.c
index afb4d93d1..f503c3561 100644
--- a/src/bin/lttng/commands/track-untrack.c
+++ b/src/bin/lttng/commands/track-untrack.c
@@ -19,6 +19,7 @@
 #define _LGPL_SOURCE
 #include <ctype.h>
 #include <popt.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -591,6 +592,7 @@ int cmd_track_untrack(enum cmd_type cmd_type, const char *cmd_str,
 		int argc, const char **argv, const char *help_msg)
 {
 	int opt, ret = 0, success = 1;
+	bool opt_all_present = false;
 	enum cmd_error_code command_ret = CMD_SUCCESS;
 	static poptContext pc;
 	char *session_name = NULL;
@@ -670,29 +672,7 @@ int cmd_track_untrack(enum cmd_type cmd_type, const char *cmd_str,
 			type_state = STATE_VGID;
 			break;
 		case OPT_ALL:
-			switch (type_state) {
-			case STATE_PID:
-				opt_pid.all = 1;
-				break;
-			case STATE_VPID:
-				opt_vpid.all = 1;
-				break;
-			case STATE_UID:
-				opt_uid.all = 1;
-				break;
-			case STATE_VUID:
-				opt_vuid.all = 1;
-				break;
-			case STATE_GID:
-				opt_gid.all = 1;
-				break;
-			case STATE_VGID:
-				opt_vgid.all = 1;
-				break;
-			default:
-				command_ret = CMD_ERROR;
-				goto end;
-			}
+			opt_all_present = true;
 			break;
 		default:
 			command_ret = CMD_UNDEFINED;
@@ -706,6 +686,36 @@ int cmd_track_untrack(enum cmd_type cmd_type, const char *cmd_str,
 		goto end;
 	}
 
+	/*
+	 * If the `--all` option is present set the appropriate tracker's `all`
+	 * field.
+	 */
+	if (opt_all_present) {
+		switch (type_state) {
+		case STATE_PID:
+			opt_pid.all = 1;
+			break;
+		case STATE_VPID:
+			opt_vpid.all = 1;
+			break;
+		case STATE_UID:
+			opt_uid.all = 1;
+			break;
+		case STATE_VUID:
+			opt_vuid.all = 1;
+			break;
+		case STATE_GID:
+			opt_gid.all = 1;
+			break;
+		case STATE_VGID:
+			opt_vgid.all = 1;
+			break;
+		default:
+			command_ret = CMD_ERROR;
+			goto end;
+		}
+	}
+
 	if (!opt_session_name) {
 		session_name = get_session_name();
 		if (session_name == NULL) {
diff --git a/tests/regression/tools/tracker/test_event_tracker b/tests/regression/tools/tracker/test_event_tracker
index b4563b333..f6ed6ba8b 100755
--- a/tests/regression/tools/tracker/test_event_tracker
+++ b/tests/regression/tools/tracker/test_event_tracker
@@ -28,7 +28,7 @@ TESTAPP_KERNEL_BIN="$TESTAPP_PATH/$TESTAPP_KERNEL_NAME/$TESTAPP_KERNEL_NAME"
 SESSION_NAME="tracker"
 NR_ITER=100
 NUM_GLOBAL_TESTS=2
-NUM_UST_TESTS=265
+NUM_UST_TESTS=283
 NUM_KERNEL_TESTS=462
 NUM_TESTS=$((NUM_UST_TESTS+NUM_KERNEL_TESTS+NUM_GLOBAL_TESTS))
 
@@ -390,6 +390,10 @@ fi
 
 EVENT_NAME="tp:tptest"
 
+# Both ordering of tracker type and `--all` are valid.
+test_event_track_untrack ust 0 "${EVENT_NAME}" "--vgid --all"
+test_event_track_untrack ust 0 "${EVENT_NAME}" "--all --vgid"
+
 #vuid, vgid
 
 # non-matching