core/main: fix setting of arguments for shutdown

Fixup for d2ebd50d7f
and 6920049fad:
- add a comment that the last arg must be NULL and adjust the assert.
- move initialization around so that fields are declared,
  initialized, and consumed in the same order.
- move declaration of pos adjacent do declaration of command_line.
  This makes it easy to see that it was not initialized correctly.
- initialize buffers before writing the pointer into the args array.
  This makes no difference for the compiler, but it just feels "wrong"
  to do it in opposite order.

Because pos was off, we would ignore args after the timeout, and also
overwrite the buffer if enough args were used.

I think this is case shows clearly that declaring all variables at the
top of the function, with some initialized and other not, is very
error-prone. The compiler has no issue with declaring variables whereever,
and we should take advantage of this to make it keep declaration,
initialization, and use close. (Within reason of course.)

src/core/main.c

@@ -1506,7 +1506,6 @@ static void redirect_telinit(int argc, char *argv[]) {
 }
 
 static int become_shutdown(int objective, int retval) {
-
         static const char* const table[_MANAGER_OBJECTIVE_MAX] = {
                 [MANAGER_EXIT]     = "exit",
                 [MANAGER_REBOOT]   = "reboot",
@@ -1516,29 +1515,31 @@ static int become_shutdown(int objective, int retval) {
         };
 
         char log_level[STRLEN("--log-level=") + DECIMAL_STR_MAX(int)],
-                exit_code[STRLEN("--exit-code=") + DECIMAL_STR_MAX(uint8_t)],
-                timeout[STRLEN("--timeout=") + DECIMAL_STR_MAX(usec_t) + STRLEN("us")];
-
-        const char* command_line[10] = {
-                SYSTEMD_SHUTDOWN_BINARY_PATH,
-                table[objective],
-                timeout,
-                log_level,
-        };
+             timeout[STRLEN("--timeout=") + DECIMAL_STR_MAX(usec_t) + STRLEN("us")],
+             exit_code[STRLEN("--exit-code=") + DECIMAL_STR_MAX(uint8_t)];
 
         _cleanup_strv_free_ char **env_block = NULL;
         usec_t watchdog_timer = 0;
-        size_t pos = 7;
         int r;
 
         assert(objective >= 0 && objective < _MANAGER_OBJECTIVE_MAX);
         assert(table[objective]);
-        assert(!command_line[pos]);
-        env_block = strv_copy(environ);
 
         xsprintf(log_level, "--log-level=%d", log_get_max_level());
         xsprintf(timeout, "--timeout=%" PRI_USEC "us", arg_default_timeout_stop_usec);
 
+        const char* command_line[10] = {
+                SYSTEMD_SHUTDOWN_BINARY_PATH,
+                table[objective],
+                log_level,
+                timeout,
+                /* Note that the last position is a terminator and must contain NULL. */
+        };
+        size_t pos = 4;
+
+        assert(command_line[pos-1]);
+        assert(!command_line[pos]);
+
         switch (log_get_target()) {
 
         case LOG_TARGET_KMSG:
@@ -1567,11 +1568,13 @@ static int become_shutdown(int objective, int retval) {
                 command_line[pos++] = "--log-time";
 
         if (objective == MANAGER_EXIT) {
-                command_line[pos++] = exit_code;
                 xsprintf(exit_code, "--exit-code=%d", retval);
+                command_line[pos++] = exit_code;
         }
 
-        assert(pos < ELEMENTSOF(command_line));
+        assert(pos < ELEMENTSOF(command_line) - 1);
+
+        /* The watchdog: */
 
         if (objective == MANAGER_REBOOT)
                 watchdog_timer = arg_reboot_watchdog;
@@ -1586,6 +1589,10 @@ static int become_shutdown(int objective, int retval) {
         r = watchdog_setup(watchdog_timer);
         watchdog_close(r < 0);
 
+        /* The environment block: */
+
+        env_block = strv_copy(environ);
+
         /* Tell the binary how often to ping, ignore failure */
         (void) strv_extendf(&env_block, "WATCHDOG_USEC="USEC_FMT, watchdog_timer);