Merge pull request #4337 from poettering/exit-code

Fix for #4275 and more

13 files changed, 64 insertions, 60 deletions

diff --git a/src/basic/exit-status.c b/src/basic/exit-status.c
index d488cfc59f..59557f8afe 100644
--- a/src/basic/exit-status.c
+++ b/src/basic/exit-status.c
@@ -24,12 +24,12 @@
 #include "macro.h"
 #include "set.h"
 
-const char* exit_status_to_string(ExitStatus status, ExitStatusLevel level) {
+const char* exit_status_to_string(int status, ExitStatusLevel level) {
 
         /* We cast to int here, so that -Wenum doesn't complain that
          * EXIT_SUCCESS/EXIT_FAILURE aren't in the enum */
 
-        switch ((int) status) {
+        switch (status) {
 
         case EXIT_SUCCESS:
                 return "SUCCESS";
@@ -39,7 +39,7 @@ const char* exit_status_to_string(ExitStatus status, ExitStatusLevel level) {
         }
 
         if (IN_SET(level, EXIT_STATUS_SYSTEMD, EXIT_STATUS_LSB)) {
-                switch ((int) status) {
+                switch (status) {
 
                 case EXIT_CHDIR:
                         return "CHDIR";
@@ -140,19 +140,19 @@ const char* exit_status_to_string(ExitStatus status, ExitStatusLevel level) {
                 case EXIT_RUNTIME_DIRECTORY:
                         return "RUNTIME_DIRECTORY";
 
-                case EXIT_CHOWN:
-                        return "CHOWN";
-
                 case EXIT_MAKE_STARTER:
                         return "MAKE_STARTER";
 
+                case EXIT_CHOWN:
+                        return "CHOWN";
+
                 case EXIT_SMACK_PROCESS_LABEL:
                         return "SMACK_PROCESS_LABEL";
                 }
         }
 
         if (level == EXIT_STATUS_LSB) {
-                switch ((int) status) {
+                switch (status) {
 
                 case EXIT_INVALIDARGUMENT:
                         return "INVALIDARGUMENT";
@@ -177,34 +177,23 @@ const char* exit_status_to_string(ExitStatus status, ExitStatusLevel level) {
         return NULL;
 }
 
-
-bool is_clean_exit(int code, int status, ExitStatusSet *success_status) {
+bool is_clean_exit(int code, int status, ExitClean clean, ExitStatusSet *success_status) {
 
         if (code == CLD_EXITED)
                 return status == 0 ||
                        (success_status &&
                        set_contains(success_status->status, INT_TO_PTR(status)));
 
-        /* If a daemon does not implement handlers for some of the
-         * signals that's not considered an unclean shutdown */
+        /* If a daemon does not implement handlers for some of the signals that's not considered an unclean shutdown */
         if (code == CLD_KILLED)
-                return IN_SET(status, SIGHUP, SIGINT, SIGTERM, SIGPIPE) ||
+                return
+                        (clean == EXIT_CLEAN_DAEMON && IN_SET(status, SIGHUP, SIGINT, SIGTERM, SIGPIPE)) ||
                         (success_status &&
                          set_contains(success_status->signal, INT_TO_PTR(status)));
 
         return false;
 }
 
-bool is_clean_exit_lsb(int code, int status, ExitStatusSet *success_status) {
-
-        if (is_clean_exit(code, status, success_status))
-                return true;
-
-        return
-                code == CLD_EXITED &&
-                IN_SET(status, EXIT_NOTINSTALLED, EXIT_NOTCONFIGURED);
-}
-
 void exit_status_set_free(ExitStatusSet *x) {
         assert(x);
 
diff --git a/src/basic/exit-status.h b/src/basic/exit-status.h
index 2309f68815..0cfdfd7891 100644
--- a/src/basic/exit-status.h
+++ b/src/basic/exit-status.h
@@ -31,7 +31,7 @@
  * https://refspecs.linuxbase.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/iniscrptact.html
  */
 
-typedef enum ExitStatus {
+enum {
         /* EXIT_SUCCESS defined by libc */
         /* EXIT_FAILURE defined by libc */
         EXIT_INVALIDARGUMENT = 2,
@@ -82,7 +82,7 @@ typedef enum ExitStatus {
         EXIT_MAKE_STARTER,
         EXIT_CHOWN,
         EXIT_SMACK_PROCESS_LABEL,
-} ExitStatus;
+};
 
 typedef enum ExitStatusLevel {
         EXIT_STATUS_MINIMAL,   /* only cover libc EXIT_STATUS/EXIT_FAILURE */
@@ -96,10 +96,14 @@ typedef struct ExitStatusSet {
         Set *signal;
 } ExitStatusSet;
 
-const char* exit_status_to_string(ExitStatus status, ExitStatusLevel level) _const_;
+const char* exit_status_to_string(int status, ExitStatusLevel level) _const_;
 
-bool is_clean_exit(int code, int status, ExitStatusSet *success_status);
-bool is_clean_exit_lsb(int code, int status, ExitStatusSet *success_status);
+typedef enum ExitClean {
+        EXIT_CLEAN_DAEMON,
+        EXIT_CLEAN_COMMAND,
+} ExitClean;
+
+bool is_clean_exit(int code, int status, ExitClean clean, ExitStatusSet *success_status);
 
 void exit_status_set_free(ExitStatusSet *x);
 bool exit_status_set_is_empty(ExitStatusSet *x);
diff --git a/src/core/busname.c b/src/core/busname.c
index 7952cd31aa..a69e3831f6 100644
--- a/src/core/busname.c
+++ b/src/core/busname.c
@@ -868,7 +868,7 @@ static void busname_sigchld_event(Unit *u, pid_t pid, int code, int status) {
 
         n->control_pid = 0;
 
-        if (is_clean_exit(code, status, NULL))
+        if (is_clean_exit(code, status, EXIT_CLEAN_COMMAND, NULL))
                 f = BUSNAME_SUCCESS;
         else if (code == CLD_EXITED)
                 f = BUSNAME_FAILURE_EXIT_CODE;
diff --git a/src/core/main.c b/src/core/main.c
index 9985510161..4b82a57b3c 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -203,7 +203,7 @@ noreturn static void crash(int sig) {
                                               pid, sigchld_code_to_string(status.si_code),
                                               status.si_status,
                                               strna(status.si_code == CLD_EXITED
-                                                    ? exit_status_to_string(status.si_status, EXIT_STATUS_FULL)
+                                                    ? exit_status_to_string(status.si_status, EXIT_STATUS_MINIMAL)
                                                     : signal_to_string(status.si_status)));
                         else
                                 log_emergency("Caught <%s>, dumped core as pid "PID_FMT".", signal_to_string(sig), pid);
diff --git a/src/core/mount.c b/src/core/mount.c
index 04025b83b9..f5e67b1d78 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -830,7 +830,7 @@ static void mount_enter_signal(Mount *m, MountState state, MountResult f) {
 fail:
         log_unit_warning_errno(UNIT(m), r, "Failed to kill processes: %m");
 
-        if (state == MOUNT_REMOUNTING_SIGTERM || state == MOUNT_REMOUNTING_SIGKILL)
+        if (IN_SET(state, MOUNT_REMOUNTING_SIGTERM, MOUNT_REMOUNTING_SIGKILL))
                 mount_enter_mounted(m, MOUNT_FAILURE_RESOURCES);
         else
                 mount_enter_dead(m, MOUNT_FAILURE_RESOURCES);
@@ -986,18 +986,19 @@ static int mount_start(Unit *u) {
 
         /* We cannot fulfill this request right now, try again later
          * please! */
-        if (m->state == MOUNT_UNMOUNTING ||
-            m->state == MOUNT_UNMOUNTING_SIGTERM ||
-            m->state == MOUNT_UNMOUNTING_SIGKILL ||
-            m->state == MOUNT_MOUNTING_SIGTERM ||
-            m->state == MOUNT_MOUNTING_SIGKILL)
+        if (IN_SET(m->state,
+                   MOUNT_UNMOUNTING,
+                   MOUNT_UNMOUNTING_SIGTERM,
+                   MOUNT_UNMOUNTING_SIGKILL,
+                   MOUNT_MOUNTING_SIGTERM,
+                   MOUNT_MOUNTING_SIGKILL))
                 return -EAGAIN;
 
         /* Already on it! */
         if (m->state == MOUNT_MOUNTING)
                 return 0;
 
-        assert(m->state == MOUNT_DEAD || m->state == MOUNT_FAILED);
+        assert(IN_SET(m->state, MOUNT_DEAD, MOUNT_FAILED));
 
         r = unit_start_limit_test(u);
         if (r < 0) {
@@ -1019,19 +1020,21 @@ static int mount_stop(Unit *u) {
         assert(m);
 
         /* Already on it */
-        if (m->state == MOUNT_UNMOUNTING ||
-            m->state == MOUNT_UNMOUNTING_SIGKILL ||
-            m->state == MOUNT_UNMOUNTING_SIGTERM ||
-            m->state == MOUNT_MOUNTING_SIGTERM ||
-            m->state == MOUNT_MOUNTING_SIGKILL)
+        if (IN_SET(m->state,
+                   MOUNT_UNMOUNTING,
+                   MOUNT_UNMOUNTING_SIGKILL,
+                   MOUNT_UNMOUNTING_SIGTERM,
+                   MOUNT_MOUNTING_SIGTERM,
+                   MOUNT_MOUNTING_SIGKILL))
                 return 0;
 
-        assert(m->state == MOUNT_MOUNTING ||
-               m->state == MOUNT_MOUNTING_DONE ||
-               m->state == MOUNT_MOUNTED ||
-               m->state == MOUNT_REMOUNTING ||
-               m->state == MOUNT_REMOUNTING_SIGTERM ||
-               m->state == MOUNT_REMOUNTING_SIGKILL);
+        assert(IN_SET(m->state,
+                      MOUNT_MOUNTING,
+                      MOUNT_MOUNTING_DONE,
+                      MOUNT_MOUNTED,
+                      MOUNT_REMOUNTING,
+                      MOUNT_REMOUNTING_SIGTERM,
+                      MOUNT_REMOUNTING_SIGKILL));
 
         mount_enter_unmounting(m);
         return 1;
@@ -1159,7 +1162,7 @@ static void mount_sigchld_event(Unit *u, pid_t pid, int code, int status) {
 
         m->control_pid = 0;
 
-        if (is_clean_exit(code, status, NULL))
+        if (is_clean_exit(code, status, EXIT_CLEAN_COMMAND, NULL))
                 f = MOUNT_SUCCESS;
         else if (code == CLD_EXITED)
                 f = MOUNT_FAILURE_EXIT_CODE;
@@ -1197,9 +1200,10 @@ static void mount_sigchld_event(Unit *u, pid_t pid, int code, int status) {
         case MOUNT_MOUNTING_SIGKILL:
         case MOUNT_MOUNTING_SIGTERM:
 
-                if (f == MOUNT_SUCCESS)
-                        mount_enter_mounted(m, f);
-                else if (m->from_proc_self_mountinfo)
+                if (f == MOUNT_SUCCESS || m->from_proc_self_mountinfo)
+                        /* If /bin/mount returned success, or if we see the mount point in /proc/self/mountinfo we are
+                         * happy. If we see the first condition first, we should see the the second condition
+                         * immediately after – or /bin/mount lies to us and is broken. */
                         mount_enter_mounted(m, f);
                 else
                         mount_enter_dead(m, f);
diff --git a/src/core/service.c b/src/core/service.c
index 99a70395fc..98edc437a2 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -2600,8 +2600,7 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) {
         assert(s);
         assert(pid >= 0);
 
-        if (UNIT(s)->fragment_path ? is_clean_exit(code, status, &s->success_status) :
-                                     is_clean_exit_lsb(code, status, &s->success_status))
+        if (is_clean_exit(code, status, s->type == SERVICE_ONESHOT ? EXIT_CLEAN_COMMAND : EXIT_CLEAN_DAEMON, &s->success_status))
                 f = SERVICE_SUCCESS;
         else if (code == CLD_EXITED)
                 f = SERVICE_FAILURE_EXIT_CODE;
diff --git a/src/core/socket.c b/src/core/socket.c
index b9032fa5c9..1b4a1b3dc3 100644
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -2743,7 +2743,7 @@ static void socket_sigchld_event(Unit *u, pid_t pid, int code, int status) {
 
         s->control_pid = 0;
 
-        if (is_clean_exit(code, status, NULL))
+        if (is_clean_exit(code, status, EXIT_CLEAN_COMMAND, NULL))
                 f = SOCKET_SUCCESS;
         else if (code == CLD_EXITED)
                 f = SOCKET_FAILURE_EXIT_CODE;
diff --git a/src/core/swap.c b/src/core/swap.c
index fb222b6858..fee9e7b0e6 100644
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -988,7 +988,7 @@ static void swap_sigchld_event(Unit *u, pid_t pid, int code, int status) {
 
         s->control_pid = 0;
 
-        if (is_clean_exit(code, status, NULL))
+        if (is_clean_exit(code, status, EXIT_CLEAN_COMMAND, NULL))
                 f = SWAP_SUCCESS;
         else if (code == CLD_EXITED)
                 f = SWAP_FAILURE_EXIT_CODE;
diff --git a/src/remount-fs/remount-fs.c b/src/remount-fs/remount-fs.c
index 6468d1eecd..c3bdcaf1da 100644
--- a/src/remount-fs/remount-fs.c
+++ b/src/remount-fs/remount-fs.c
@@ -137,7 +137,7 @@ int main(int argc, char *argv[]) {
 
                 s = hashmap_remove(pids, PID_TO_PTR(si.si_pid));
                 if (s) {
-                        if (!is_clean_exit(si.si_code, si.si_status, NULL)) {
+                        if (!is_clean_exit(si.si_code, si.si_status, EXIT_CLEAN_COMMAND, NULL)) {
                                 if (si.si_code == CLD_EXITED)
                                         log_error(MOUNT_PATH " for %s exited with exit status %i.", s, si.si_status);
                                 else
diff --git a/src/shared/install.c b/src/shared/install.c
index 5f12fb447f..60a6d1312d 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -1020,7 +1020,7 @@ static int config_parse_alias(
         type = unit_name_to_type(name);
         if (!unit_type_may_alias(type))
                 return log_syntax(unit, LOG_WARNING, filename, line, 0,
-                                  "Aliases are not allowed for %s units, ignoring.",
+                                  "Alias= is not allowed for %s units, ignoring.",
                                   unit_type_to_string(type));
 
         return config_parse_strv(unit, filename, line, section, section_line,
@@ -1098,7 +1098,7 @@ static int config_parse_default_instance(
                 return 0;
         if (!unit_name_is_valid(name, UNIT_NAME_TEMPLATE))
                 return log_syntax(unit, LOG_WARNING, filename, line, 0,
-                                  "DefaultInstance only makes sense for template units, ignoring.");
+                                  "DefaultInstance= only makes sense for template units, ignoring.");
 
         r = install_full_printf(i, rvalue, &printed);
         if (r < 0)
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index bb6002e8ef..18a8a4f248 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -3936,7 +3936,7 @@ static void print_status_info(
                 argv = strv_join(p->argv, " ");
                 printf("  Process: "PID_FMT" %s=%s ", p->pid, p->name, strna(argv));
 
-                good = is_clean_exit_lsb(p->code, p->status, NULL);
+                good = is_clean_exit(p->code, p->status, EXIT_CLEAN_DAEMON, NULL);
                 if (!good) {
                         on = ansi_highlight_red();
                         off = ansi_normal();
diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c
index 39821687b9..c2c80175a2 100644
--- a/src/sysv-generator/sysv-generator.c
+++ b/src/sysv-generator/sysv-generator.c
@@ -25,6 +25,7 @@
 
 #include "alloc-util.h"
 #include "dirent-util.h"
+#include "exit-status.h"
 #include "fd-util.h"
 #include "fileio.h"
 #include "hashmap.h"
@@ -199,6 +200,13 @@ static int generate_unit_file(SysvStub *s) {
         if (s->pid_file)
                 fprintf(f, "PIDFile=%s\n", s->pid_file);
 
+        /* Consider two special LSB exit codes a clean exit */
+        if (s->has_lsb)
+                fprintf(f,
+                        "SuccessExitStatus=%i %i\n",
+                        EXIT_NOTINSTALLED,
+                        EXIT_NOTCONFIGURED);
+
         fprintf(f,
                 "ExecStart=%s start\n"
                 "ExecStop=%s stop\n",
diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c
index 8851af449d..b45490be1a 100644
--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
@@ -827,7 +827,7 @@ static int ask_on_consoles(int argc, char *argv[]) {
                 break;
         }
 
-        if (!is_clean_exit(status.si_code, status.si_status, NULL))
+        if (!is_clean_exit(status.si_code, status.si_status, EXIT_CLEAN_DAEMON, NULL))
                 log_error("Password agent failed with: %d", status.si_status);
 
         terminate_agents(pids);