manager: fix error handling after failure to set up child

exec_child() is supposed to set *exit_status when returning failure.
Unfortunately, we didn't do that in two cases. The result would be:
- a bogus error message "Failed at step SUCCESS spawning foo: …",
- a bogus success exit status.

Bugs introduced in 390902012c5177b6b01bc634b2e9c704073d9e7d and
ad21e542b20f0fb292d1958d3a759bf3403522c2.

The code is reworked to add some asserts and not set exit_status in the caller
so that it's clearer (also to the compiler) that it needs to be set.

diff --git a/src/core/execute.c b/src/core/execute.c
index a81a7d57d484cfd8e3ac451363a42c6ad2a687bd..4b6e927e66e0ea2afdaa423687fc409072c6b666 100644 (file)
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -4958,6 +4958,7 @@ static int exec_child(
                                 *exit_status = EXIT_SUCCESS;
                                 return 0;
                         }
+
                         *exit_status = EXIT_CONFIRM;
                         return log_unit_error_errno(unit, SYNTHETIC_ERRNO(ECANCELED),
                                                     "Execution cancelled by the user");
@@ -5128,14 +5129,18 @@ static int exec_child(
                 r = set_coredump_filter(context->coredump_filter);
                 if (ERRNO_IS_NEG_PRIVILEGE(r))
                         log_unit_debug_errno(unit, r, "Failed to adjust coredump_filter, ignoring: %m");
-                else if (r < 0)
+                else if (r < 0) {
+                        *exit_status = EXIT_LIMITS;
                         return log_unit_error_errno(unit, r, "Failed to adjust coredump_filter: %m");
+                }
         }
 
         if (context->nice_set) {
                 r = setpriority_closest(context->nice);
-                if (r < 0)
+                if (r < 0) {
+                        *exit_status = EXIT_NICE;
                         return log_unit_error_errno(unit, r, "Failed to set up process scheduling priority (nice level): %m");
+                }
         }
 
         if (context->cpu_sched_set) {
@@ -5578,11 +5583,11 @@ static int exec_child(
                                               LOG_UNIT_MESSAGE(unit, "Executable %s missing, skipping: %m",
                                                                command->path),
                                               "EXECUTABLE=%s", command->path);
+                        *exit_status = EXIT_SUCCESS;
                         return 0;
                 }
 
                 *exit_status = EXIT_EXEC;
-
                 return log_unit_struct_errno(unit, LOG_INFO, r,
                                              "MESSAGE_ID=" SD_MESSAGE_SPAWN_FAILED_STR,
                                              LOG_UNIT_INVOCATION_ID(unit),
@@ -6044,7 +6049,7 @@ int exec_spawn(Unit *unit,
                 return log_unit_error_errno(unit, errno, "Failed to fork: %m");
 
         if (pid == 0) {
-                int exit_status = EXIT_SUCCESS;
+                int exit_status;
 
                 r = exec_child(unit,
                                command,
@@ -6062,9 +6067,8 @@ int exec_spawn(Unit *unit,
                                &exit_status);
 
                 if (r < 0) {
-                        const char *status =
-                                exit_status_to_string(exit_status,
-                                                      EXIT_STATUS_LIBC | EXIT_STATUS_SYSTEMD);
+                        const char *status = ASSERT_PTR(
+                                        exit_status_to_string(exit_status, EXIT_STATUS_LIBC | EXIT_STATUS_SYSTEMD));
 
                         log_unit_struct_errno(unit, LOG_ERR, r,
                                               "MESSAGE_ID=" SD_MESSAGE_SPAWN_FAILED_STR,
@@ -6072,7 +6076,8 @@ int exec_spawn(Unit *unit,
                                               LOG_UNIT_MESSAGE(unit, "Failed at step %s spawning %s: %m",
                                                                status, command->path),
                                               "EXECUTABLE=%s", command->path);
-                }
+                } else
+                        assert(exit_status == EXIT_SUCCESS);
 
                 _exit(exit_status);
         }