From 65e3fd83c98a44111c1b05a6e0d492dcab58853b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 10 Oct 2016 20:08:41 +0200 Subject: exit-status: remove ExitStatus typedef Do not make up our own type for ExitStatus, but use the type used by POSIX for this, which is "int". In particular as we never used that type outside of the definition of exit_status_to_string() where we internally cast the paramter to (int) every single time we used it. Hence, let's simplify things, drop the type and use the kernel type directly. --- src/basic/exit-status.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'src/basic/exit-status.c') diff --git a/src/basic/exit-status.c b/src/basic/exit-status.c index d488cfc59f..d2a8d3b418 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"; @@ -152,7 +152,7 @@ const char* exit_status_to_string(ExitStatus status, ExitStatusLevel level) { } if (level == EXIT_STATUS_LSB) { - switch ((int) status) { + switch (status) { case EXIT_INVALIDARGUMENT: return "INVALIDARGUMENT"; @@ -177,7 +177,6 @@ const char* exit_status_to_string(ExitStatus status, ExitStatusLevel level) { return NULL; } - bool is_clean_exit(int code, int status, ExitStatusSet *success_status) { if (code == CLD_EXITED) -- cgit v1.2.3-54-g00ecf From 56ecbcc04868e749ae320b7ed9c3fd90aff2951f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 10 Oct 2016 20:11:21 +0200 Subject: exit-status: reorder the exit status switch table Let's make sure it's in the same order as the actual enum defining the exit statuses. --- src/basic/exit-status.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/basic/exit-status.c') diff --git a/src/basic/exit-status.c b/src/basic/exit-status.c index d2a8d3b418..81c2c6675c 100644 --- a/src/basic/exit-status.c +++ b/src/basic/exit-status.c @@ -140,12 +140,12 @@ const char* exit_status_to_string(int 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"; } -- cgit v1.2.3-54-g00ecf From 41e2036eb83204df95a1c3e829bcfd78ee17aaa3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 10 Oct 2016 21:48:08 +0200 Subject: exit-status: kill is_clean_exit_lsb(), move logic to sysv-generator Let's get rid of is_clean_exit_lsb(), let's move the logic for the special handling of the two LSB exit codes into the sysv-generator by writing out appropriate SuccessExitStatus= lines if the LSB header exists. This is not only semantically more correct, bug also fixes a bug as the code in service.c that chose between is_clean_exit_lsb() and is_clean_exit() based this check on whether a native unit files was available for the unit. However, that check was bogus since a long time, since the SysV generator was introduced and native SysV script support was removed from PID 1, as in that case a unit file always existed. --- src/basic/exit-status.c | 10 ---------- src/basic/exit-status.h | 1 - src/core/service.c | 3 +-- src/systemctl/systemctl.c | 2 +- src/sysv-generator/sysv-generator.c | 8 ++++++++ 5 files changed, 10 insertions(+), 14 deletions(-) (limited to 'src/basic/exit-status.c') diff --git a/src/basic/exit-status.c b/src/basic/exit-status.c index 81c2c6675c..96d4619da6 100644 --- a/src/basic/exit-status.c +++ b/src/basic/exit-status.c @@ -194,16 +194,6 @@ bool is_clean_exit(int code, int status, ExitStatusSet *success_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 46cc905865..b3baa50cf4 100644 --- a/src/basic/exit-status.h +++ b/src/basic/exit-status.h @@ -99,7 +99,6 @@ typedef struct ExitStatusSet { 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); void exit_status_set_free(ExitStatusSet *x); bool exit_status_set_is_empty(ExitStatusSet *x); diff --git a/src/core/service.c b/src/core/service.c index 99a70395fc..fc1d353356 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->success_status)) f = SERVICE_SUCCESS; else if (code == CLD_EXITED) f = SERVICE_FAILURE_EXIT_CODE; diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index bb6002e8ef..9c6a475a39 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, 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", -- cgit v1.2.3-54-g00ecf From 1f0958f640b87175cd547c1e69084cfe54a22e9d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 10 Oct 2016 22:07:30 +0200 Subject: core: when determining whether a process exit status is clean, consider whether it is a command or a daemon SIGTERM should be considered a clean exit code for daemons (i.e. long-running processes, as a daemon without SIGTERM handler may be shut down without issues via SIGTERM still) while it should not be considered a clean exit code for commands (i.e. short-running processes). Let's add two different clean checking modes for this, and use the right one at the appropriate places. Fixes: #4275 --- src/basic/exit-status.c | 8 ++++---- src/basic/exit-status.h | 7 ++++++- src/core/busname.c | 2 +- src/core/mount.c | 2 +- src/core/service.c | 2 +- src/core/socket.c | 2 +- src/core/swap.c | 2 +- src/remount-fs/remount-fs.c | 2 +- src/systemctl/systemctl.c | 2 +- src/tty-ask-password-agent/tty-ask-password-agent.c | 2 +- 10 files changed, 18 insertions(+), 13 deletions(-) (limited to 'src/basic/exit-status.c') diff --git a/src/basic/exit-status.c b/src/basic/exit-status.c index 96d4619da6..59557f8afe 100644 --- a/src/basic/exit-status.c +++ b/src/basic/exit-status.c @@ -177,17 +177,17 @@ const char* exit_status_to_string(int 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))); diff --git a/src/basic/exit-status.h b/src/basic/exit-status.h index b3baa50cf4..0cfdfd7891 100644 --- a/src/basic/exit-status.h +++ b/src/basic/exit-status.h @@ -98,7 +98,12 @@ typedef struct ExitStatusSet { const char* exit_status_to_string(int status, ExitStatusLevel level) _const_; -bool is_clean_exit(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/mount.c b/src/core/mount.c index 04025b83b9..ed542776ac 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1159,7 +1159,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; diff --git a/src/core/service.c b/src/core/service.c index fc1d353356..98edc437a2 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -2600,7 +2600,7 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) { assert(s); assert(pid >= 0); - if (is_clean_exit(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/systemctl/systemctl.c b/src/systemctl/systemctl.c index 9c6a475a39..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(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/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); -- cgit v1.2.3-54-g00ecf