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 afb4d93d1fe9bc30e14f56c6aee90a8c1181330e..f503c35617b7633e8d32f57b33e2dc42a57a1c5b 100644 (file)
--- 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 b4563b333c26a443440703bb11629b667ea310cc..f6ed6ba8bef1f9e4725c500311ea5af75c604348 100755 (executable)
--- 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