From 913c19161a8235f8e89fe31132196ad5af33c269 Mon Sep 17 00:00:00 2001
From: Alan Jenkins <alan.christopher.jenkins@gmail.com>
Date: Fri, 18 Aug 2017 13:18:09 +0100
Subject: [PATCH] systemctl: improve readability of start_unit()

start_unit() is a little tangled.  There's an easy part we can untangle,
then readers can concentrate on the more necessary complexity.

* Derive (method, action, mode) more clearly, as disjoint cases based on
  the command.  Don't rely on action_table[_ACTION_INVALID].target being
  implicitly initialized to NULL.

  verb_to_method() is now only used on one case, but not because I strongly
  object to the implicit "StartUnit" cases.  It's more a syntax problem.
  I think the old code takes me longer to understand, because the call
  comes just above a similar-looking call to verb_to_action(), but the
  results of the two functions are used in different ways.  It also helps
  that the new code ends up having a more regular form, for the 4 different
  cases.

  These changes cost 6 extra lines.

* Add an assertion to confirm that we do not pass mode=NULL.
---
 src/systemctl/systemctl.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 9b0fe1baa21..023bd40be39 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -145,7 +145,6 @@ static char *arg_root = NULL;
 static usec_t arg_when = 0;
 static char *argv_cmdline = NULL;
 static enum action {
-        _ACTION_INVALID,
         ACTION_SYSTEMCTL,
         ACTION_HALT,
         ACTION_POWEROFF,
@@ -166,7 +165,8 @@ static enum action {
         ACTION_REEXEC,
         ACTION_RUNLEVEL,
         ACTION_CANCEL_SHUTDOWN,
-        _ACTION_MAX
+        _ACTION_MAX,
+        _ACTION_INVALID = -1
 } arg_action = ACTION_SYSTEMCTL;
 static BusTransport arg_transport = BUS_TRANSPORT_LOCAL;
 static const char *arg_host = NULL;
@@ -3052,7 +3052,7 @@ static const struct {
 static enum action verb_to_action(const char *verb) {
         enum action i;
 
-        for (i = _ACTION_INVALID; i < _ACTION_MAX; i++)
+        for (i = 0; i < _ACTION_MAX; i++)
                 if (streq_ptr(action_table[i].verb, verb))
                         return i;
 
@@ -3085,22 +3085,30 @@ static int start_unit(int argc, char *argv[], void *userdata) {
         if (arg_action == ACTION_SYSTEMCTL) {
                 enum action action;
 
-                method = verb_to_method(argv[0]);
                 action = verb_to_action(argv[0]);
 
-                if (streq(argv[0], "isolate")) {
-                        mode = "isolate";
-                        suffix = ".target";
-                } else
-                        mode = action_table[action].mode ?: arg_job_mode;
+                if (action != _ACTION_INVALID) {
+                        method = "StartUnit";
+                        mode = action_table[action].mode;
+                        one_name = action_table[action].target;
+                } else {
+                        if (streq(argv[0], "isolate")) {
+                                method = "StartUnit";
+                                mode = "isolate";
 
-                one_name = action_table[action].target;
+                                suffix = ".target";
+                        } else {
+                                method = verb_to_method(argv[0]);
+                                mode = arg_job_mode;
+                        }
+                        one_name = NULL;
+                }
         } else {
                 assert(arg_action < ELEMENTSOF(action_table));
                 assert(action_table[arg_action].target);
+                assert(action_table[arg_action].mode);
 
                 method = "StartUnit";
-
                 mode = action_table[arg_action].mode;
                 one_name = action_table[arg_action].target;
         }
-- 
2.47.3