core/main: fix setting of arguments for shutdown

Fixup for d2ebd50d7f9740dcf30e84efc75610af173967d2
and 6920049fad4fa39db5fec712f82f7f75b98fd4b9:
- 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.)

diff --git a/src/core/main.c b/src/core/main.c
index e35f9574fcc4bc7a42e6277fdda6393fa3b4c495..3d7b2a5f649d6c79e41fbb34621e321ab56096b8 100644 (file)
--- a/src/core/main.c
+++ b/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);