From: Lennart Poettering <lennart@poettering.net>
Date: Wed, 27 Dec 2017 23:51:19 +0000 (+0100)
Subject: process-util: rework wait_for_terminate_and_warn() to take a flags parameter
X-Git-Tag: v237~146^2~21
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=7d4904fe7a14852c1469a55ec1dfdf747d829cb1;p=thirdparty%2Fsystemd.git

process-util: rework wait_for_terminate_and_warn() to take a flags parameter

This renames wait_for_terminate_and_warn() to
wait_for_terminate_and_check(), and adds a flags parameter, that
controls how much to log: there's one flag that means we log about
abnormal stuff, and another one that controls whether we log about
non-zero exit codes. Finally, there's a shortcut flag value for logging
in both cases, as that's what we usually use.

All callers are accordingly updated. At three occasions duplicate logging
is removed, i.e. where the old function was called but logged in the
caller, too.
---

diff --git a/src/basic/exec-util.c b/src/basic/exec-util.c
index 45a699e4269..762537afc93 100644
--- a/src/basic/exec-util.c
+++ b/src/basic/exec-util.c
@@ -147,7 +147,7 @@ static int do_execute(
                                 return log_oom();
                         t = NULL;
                 } else {
-                        r = wait_for_terminate_and_warn(t, pid, true);
+                        r = wait_for_terminate_and_check(t, pid, WAIT_LOG);
                         if (r < 0)
                                 continue;
 
@@ -177,7 +177,7 @@ static int do_execute(
                 t = hashmap_remove(pids, PID_TO_PTR(pid));
                 assert(t);
 
-                wait_for_terminate_and_warn(t, pid, true);
+                (void) wait_for_terminate_and_check(t, pid, WAIT_LOG);
         }
 
         return 0;
@@ -224,14 +224,11 @@ int execute_directories(
                 _exit(r < 0 ? EXIT_FAILURE : EXIT_SUCCESS);
         }
 
-        r = wait_for_terminate_and_warn(name, executor_pid, true);
+        r = wait_for_terminate_and_check(name, executor_pid, WAIT_LOG);
         if (r < 0)
-                return log_error_errno(r, "Execution failed: %m");
-        if (r > 0) {
-                /* non-zero return code from child */
-                log_error("Forker process failed.");
+                return r;
+        if (r > 0) /* non-zero return code from child */
                 return -EREMOTEIO;
-        }
 
         if (!callbacks)
                 return 0;
diff --git a/src/basic/process-util.c b/src/basic/process-util.c
index c155a35a3eb..9ca187a9544 100644
--- a/src/basic/process-util.c
+++ b/src/basic/process-util.c
@@ -687,32 +687,43 @@ int wait_for_terminate(pid_t pid, siginfo_t *status) {
  * A warning is emitted if the process terminates abnormally,
  * and also if it returns non-zero unless check_exit_code is true.
  */
-int wait_for_terminate_and_warn(const char *name, pid_t pid, bool check_exit_code) {
-        int r;
+int wait_for_terminate_and_check(const char *name, pid_t pid, WaitFlags flags) {
+        _cleanup_free_ char *buffer = NULL;
         siginfo_t status;
+        int r, prio;
 
-        assert(name);
         assert(pid > 1);
 
+        if (!name) {
+                r = get_process_comm(pid, &buffer);
+                if (r < 0)
+                        log_debug_errno(r, "Failed to acquire process name of " PID_FMT ", ignoring: %m", pid);
+                else
+                        name = buffer;
+        }
+
+        prio = flags & WAIT_LOG_ABNORMAL ? LOG_ERR : LOG_DEBUG;
+
         r = wait_for_terminate(pid, &status);
         if (r < 0)
-                return log_warning_errno(r, "Failed to wait for %s: %m", name);
+                return log_full_errno(prio, r, "Failed to wait for %s: %m", strna(name));
 
         if (status.si_code == CLD_EXITED) {
-                if (status.si_status != 0)
-                        log_full(check_exit_code ? LOG_WARNING : LOG_DEBUG,
-                                 "%s failed with error code %i.", name, status.si_status);
+                if (status.si_status != EXIT_SUCCESS)
+                        log_full(flags & WAIT_LOG_NON_ZERO_EXIT_STATUS ? LOG_ERR : LOG_DEBUG,
+                                 "%s failed with exit status %i.", strna(name), status.si_status);
                 else
                         log_debug("%s succeeded.", name);
 
                 return status.si_status;
+
         } else if (IN_SET(status.si_code, CLD_KILLED, CLD_DUMPED)) {
 
-                log_warning("%s terminated by signal %s.", name, signal_to_string(status.si_status));
+                log_full(prio, "%s terminated by signal %s.", strna(name), signal_to_string(status.si_status));
                 return -EPROTO;
         }
 
-        log_warning("%s failed due to unknown reason.", name);
+        log_full(prio, "%s failed due to unknown reason.", strna(name));
         return -EPROTO;
 }
 
diff --git a/src/basic/process-util.h b/src/basic/process-util.h
index ef3269d94e4..1afb8602641 100644
--- a/src/basic/process-util.h
+++ b/src/basic/process-util.h
@@ -61,7 +61,16 @@ int get_process_environ(pid_t pid, char **environ);
 int get_process_ppid(pid_t pid, pid_t *ppid);
 
 int wait_for_terminate(pid_t pid, siginfo_t *status);
-int wait_for_terminate_and_warn(const char *name, pid_t pid, bool check_exit_code);
+
+typedef enum WaitFlags {
+        WAIT_LOG_ABNORMAL             = 1U << 0,
+        WAIT_LOG_NON_ZERO_EXIT_STATUS = 1U << 1,
+
+        /* A shortcut for requesting the most complete logging */
+        WAIT_LOG = WAIT_LOG_ABNORMAL|WAIT_LOG_NON_ZERO_EXIT_STATUS,
+} WaitFlags;
+
+int wait_for_terminate_and_check(const char *name, pid_t pid, WaitFlags flags);
 int wait_for_terminate_with_timeout(pid_t pid, usec_t timeout);
 
 void sigkill_wait(pid_t pid);
diff --git a/src/core/shutdown.c b/src/core/shutdown.c
index 440cb4fba03..1be0f8b603f 100644
--- a/src/core/shutdown.c
+++ b/src/core/shutdown.c
@@ -500,7 +500,7 @@ int main(int argc, char *argv[]) {
                                 _exit(EXIT_FAILURE);
                         }
 
-                        (void) wait_for_terminate_and_warn("kexec", pid, true);
+                        (void) wait_for_terminate_and_check("kexec", pid, WAIT_LOG);
                 }
 
                 cmd = RB_AUTOBOOT;
diff --git a/src/core/socket.c b/src/core/socket.c
index 8e796e9e2c6..cc4c3c2a9b5 100644
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -1554,7 +1554,7 @@ static int socket_address_listen_in_cgroup(
         fd = receive_one_fd(pair[0], 0);
 
         /* We synchronously wait for the helper, as it shouldn't be slow */
-        r = wait_for_terminate_and_warn("listen-cgroup-helper", pid, false);
+        r = wait_for_terminate_and_check("listen-cgroup-helper", pid, WAIT_LOG_ABNORMAL);
         if (r < 0) {
                 safe_close(fd);
                 return r;
@@ -2898,7 +2898,7 @@ static int socket_accept_in_cgroup(Socket *s, SocketPort *p, int fd) {
         cfd = receive_one_fd(pair[0], 0);
 
         /* We synchronously wait for the helper, as it shouldn't be slow */
-        r = wait_for_terminate_and_warn("accept-cgroup-helper", pid, false);
+        r = wait_for_terminate_and_check("accept-cgroup-helper", pid, WAIT_LOG_ABNORMAL);
         if (r < 0) {
                 safe_close(cfd);
                 return r;
diff --git a/src/delta/delta.c b/src/delta/delta.c
index 779dec6cb76..d94ae027543 100644
--- a/src/delta/delta.c
+++ b/src/delta/delta.c
@@ -195,7 +195,7 @@ static int found_override(const char *top, const char *bottom) {
                 _exit(EXIT_FAILURE);
         }
 
-        wait_for_terminate_and_warn("diff", pid, false);
+        (void) wait_for_terminate_and_check("diff", pid, WAIT_LOG_ABNORMAL);
         putchar('\n');
 
         return r;
diff --git a/src/import/import-tar.c b/src/import/import-tar.c
index c5014499ac5..09c7654adca 100644
--- a/src/import/import-tar.c
+++ b/src/import/import-tar.c
@@ -190,7 +190,7 @@ static int tar_import_finish(TarImport *i) {
         i->tar_fd = safe_close(i->tar_fd);
 
         if (i->tar_pid > 0) {
-                r = wait_for_terminate_and_warn("tar", i->tar_pid, true);
+                r = wait_for_terminate_and_check("tar", i->tar_pid, WAIT_LOG);
                 i->tar_pid = 0;
                 if (r < 0)
                         return r;
diff --git a/src/import/pull-common.c b/src/import/pull-common.c
index 593847f883d..e290b940202 100644
--- a/src/import/pull-common.c
+++ b/src/import/pull-common.c
@@ -542,7 +542,7 @@ int pull_verify(PullJob *main_job,
 
         gpg_pipe[1] = safe_close(gpg_pipe[1]);
 
-        r = wait_for_terminate_and_warn("gpg", pid, true);
+        r = wait_for_terminate_and_check("gpg", pid, WAIT_LOG_ABNORMAL);
         pid = 0;
         if (r < 0)
                 goto finish;
diff --git a/src/import/pull-tar.c b/src/import/pull-tar.c
index ed915112455..a6fb0d49534 100644
--- a/src/import/pull-tar.c
+++ b/src/import/pull-tar.c
@@ -333,7 +333,7 @@ static void tar_pull_job_on_finished(PullJob *j) {
                 goto finish;
 
         if (i->tar_pid > 0) {
-                r = wait_for_terminate_and_warn("tar", i->tar_pid, true);
+                r = wait_for_terminate_and_check("tar", i->tar_pid, WAIT_LOG);
                 i->tar_pid = 0;
                 if (r < 0)
                         goto finish;
diff --git a/src/login/inhibit.c b/src/login/inhibit.c
index a197464b76c..22657f9eda0 100644
--- a/src/login/inhibit.c
+++ b/src/login/inhibit.c
@@ -276,7 +276,7 @@ int main(int argc, char *argv[]) {
                         _exit(EXIT_FAILURE);
                 }
 
-                r = wait_for_terminate_and_warn(argv[optind], pid, true);
+                r = wait_for_terminate_and_check(argv[optind], pid, WAIT_LOG);
                 return r < 0 ? EXIT_FAILURE : r;
         }
 
diff --git a/src/nspawn/nspawn-setuid.c b/src/nspawn/nspawn-setuid.c
index b1d03d2d93d..cb1be273f4e 100644
--- a/src/nspawn/nspawn-setuid.c
+++ b/src/nspawn/nspawn-setuid.c
@@ -133,7 +133,7 @@ int change_uid_gid(const char *user, char **_home) {
 
         truncate_nl(line);
 
-        wait_for_terminate_and_warn("getent passwd", pid, true);
+        (void) wait_for_terminate_and_check("getent passwd", pid, WAIT_LOG);
 
         x = strchr(line, ':');
         if (!x) {
@@ -216,7 +216,7 @@ int change_uid_gid(const char *user, char **_home) {
 
         truncate_nl(line);
 
-        wait_for_terminate_and_warn("getent initgroups", pid, true);
+        (void) wait_for_terminate_and_check("getent initgroups", pid, WAIT_LOG);
 
         /* Skip over the username and subsequent separator whitespace */
         x = line;
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index f646971c30c..6a66c56ff1a 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -3516,7 +3516,7 @@ static int run(int master,
         }
 
         /* Wait for the outer child. */
-        r = wait_for_terminate_and_warn("namespace helper", *pid, NULL);
+        r = wait_for_terminate_and_check("namespace helper", *pid, WAIT_LOG_ABNORMAL);
         if (r != 0)
                 return r < 0 ? r : -EIO;
 
diff --git a/src/partition/makefs.c b/src/partition/makefs.c
index df932fd35af..2ed31ac7abe 100644
--- a/src/partition/makefs.c
+++ b/src/partition/makefs.c
@@ -55,7 +55,7 @@ static int makefs(const char *type, const char *device) {
                 _exit(EXIT_FAILURE);
         }
 
-        return wait_for_terminate_and_warn(mkfs, pid, true);
+        return wait_for_terminate_and_check(mkfs, pid, WAIT_LOG);
 }
 
 int main(int argc, char *argv[]) {
diff --git a/src/quotacheck/quotacheck.c b/src/quotacheck/quotacheck.c
index 2773e4b5817..c40c7d5d079 100644
--- a/src/quotacheck/quotacheck.c
+++ b/src/quotacheck/quotacheck.c
@@ -117,7 +117,7 @@ int main(int argc, char *argv[]) {
                 _exit(EXIT_FAILURE); /* Operational error */
         }
 
-        r = wait_for_terminate_and_warn("quotacheck", pid, true);
+        r = wait_for_terminate_and_check("quotacheck", pid, WAIT_LOG);
 
 finish:
         return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS;
diff --git a/src/sulogin-shell/sulogin-shell.c b/src/sulogin-shell/sulogin-shell.c
index e8839eae18e..33dc07c5bde 100644
--- a/src/sulogin-shell/sulogin-shell.c
+++ b/src/sulogin-shell/sulogin-shell.c
@@ -93,7 +93,7 @@ static int fork_wait(const char* const cmdline[]) {
                 _exit(EXIT_FAILURE); /* Operational error */
         }
 
-        return wait_for_terminate_and_warn(cmdline[0], pid, false);
+        return wait_for_terminate_and_check(cmdline[0], pid, WAIT_LOG_ABNORMAL);
 }
 
 static void print_mode(const char* mode) {
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index f97998836a2..fc921d1dd7d 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -3556,7 +3556,7 @@ static int load_kexec_kernel(void) {
                 _exit(EXIT_FAILURE);
         }
 
-        return wait_for_terminate_and_warn("kexec", pid, true);
+        return wait_for_terminate_and_check("kexec", pid, WAIT_LOG);
 }
 
 static int set_exit_code(uint8_t code) {
@@ -7057,9 +7057,9 @@ static int run_editor(char **paths) {
                 _exit(EXIT_FAILURE);
         }
 
-        r = wait_for_terminate_and_warn("editor", pid, true);
+        r = wait_for_terminate_and_check("editor", pid, WAIT_LOG);
         if (r < 0)
-                return log_error_errno(r, "Failed to wait for child: %m");
+                return r;
 
         return 0;
 }
diff --git a/src/test/test-seccomp.c b/src/test/test-seccomp.c
index 36b49ebc71a..aed307077e7 100644
--- a/src/test/test-seccomp.c
+++ b/src/test/test-seccomp.c
@@ -141,7 +141,7 @@ static void test_filter_sets(void) {
                         _exit(EXIT_SUCCESS);
                 }
 
-                assert_se(wait_for_terminate_and_warn(syscall_filter_sets[i].name, pid, true) == EXIT_SUCCESS);
+                assert_se(wait_for_terminate_and_check(syscall_filter_sets[i].name, pid, WAIT_LOG) == EXIT_SUCCESS);
         }
 }
 
@@ -227,7 +227,7 @@ static void test_restrict_namespace(void) {
                 _exit(EXIT_SUCCESS);
         }
 
-        assert_se(wait_for_terminate_and_warn("nsseccomp", pid, true) == EXIT_SUCCESS);
+        assert_se(wait_for_terminate_and_check("nsseccomp", pid, WAIT_LOG) == EXIT_SUCCESS);
 }
 
 static void test_protect_sysctl(void) {
@@ -260,7 +260,7 @@ static void test_protect_sysctl(void) {
                 _exit(EXIT_SUCCESS);
         }
 
-        assert_se(wait_for_terminate_and_warn("sysctlseccomp", pid, true) == EXIT_SUCCESS);
+        assert_se(wait_for_terminate_and_check("sysctlseccomp", pid, WAIT_LOG) == EXIT_SUCCESS);
 }
 
 static void test_restrict_address_families(void) {
@@ -343,7 +343,7 @@ static void test_restrict_address_families(void) {
                 _exit(EXIT_SUCCESS);
         }
 
-        assert_se(wait_for_terminate_and_warn("socketseccomp", pid, true) == EXIT_SUCCESS);
+        assert_se(wait_for_terminate_and_check("socketseccomp", pid, WAIT_LOG) == EXIT_SUCCESS);
 }
 
 static void test_restrict_realtime(void) {
@@ -381,7 +381,7 @@ static void test_restrict_realtime(void) {
                 _exit(EXIT_SUCCESS);
         }
 
-        assert_se(wait_for_terminate_and_warn("realtimeseccomp", pid, true) == EXIT_SUCCESS);
+        assert_se(wait_for_terminate_and_check("realtimeseccomp", pid, WAIT_LOG) == EXIT_SUCCESS);
 }
 
 static void test_memory_deny_write_execute_mmap(void) {
@@ -424,7 +424,7 @@ static void test_memory_deny_write_execute_mmap(void) {
                 _exit(EXIT_SUCCESS);
         }
 
-        assert_se(wait_for_terminate_and_warn("memoryseccomp-mmap", pid, true) == EXIT_SUCCESS);
+        assert_se(wait_for_terminate_and_check("memoryseccomp-mmap", pid, WAIT_LOG) == EXIT_SUCCESS);
 }
 
 static void test_memory_deny_write_execute_shmat(void) {
@@ -471,7 +471,7 @@ static void test_memory_deny_write_execute_shmat(void) {
                 _exit(EXIT_SUCCESS);
         }
 
-        assert_se(wait_for_terminate_and_warn("memoryseccomp-shmat", pid, true) == EXIT_SUCCESS);
+        assert_se(wait_for_terminate_and_check("memoryseccomp-shmat", pid, WAIT_LOG) == EXIT_SUCCESS);
 }
 
 static void test_restrict_archs(void) {
@@ -505,7 +505,7 @@ static void test_restrict_archs(void) {
                 _exit(EXIT_SUCCESS);
         }
 
-        assert_se(wait_for_terminate_and_warn("archseccomp", pid, true) == EXIT_SUCCESS);
+        assert_se(wait_for_terminate_and_check("archseccomp", pid, WAIT_LOG) == EXIT_SUCCESS);
 }
 
 static void test_load_syscall_filter_set_raw(void) {
@@ -596,7 +596,7 @@ static void test_load_syscall_filter_set_raw(void) {
                 _exit(EXIT_SUCCESS);
         }
 
-        assert_se(wait_for_terminate_and_warn("syscallrawseccomp", pid, true) == EXIT_SUCCESS);
+        assert_se(wait_for_terminate_and_check("syscallrawseccomp", pid, WAIT_LOG) == EXIT_SUCCESS);
 }
 
 static void test_lock_personality(void) {
@@ -643,7 +643,7 @@ static void test_lock_personality(void) {
                 _exit(EXIT_SUCCESS);
         }
 
-        assert_se(wait_for_terminate_and_warn("lockpersonalityseccomp", pid, true) == EXIT_SUCCESS);
+        assert_se(wait_for_terminate_and_check("lockpersonalityseccomp", pid, WAIT_LOG) == EXIT_SUCCESS);
 }
 
 static void test_filter_sets_ordered(void) {
diff --git a/src/vconsole/vconsole-setup.c b/src/vconsole/vconsole-setup.c
index 7685f1d7146..2e0e09d8434 100644
--- a/src/vconsole/vconsole-setup.c
+++ b/src/vconsole/vconsole-setup.c
@@ -161,7 +161,7 @@ static int keyboard_load_and_wait(const char *vc, const char *map, const char *m
                 _exit(EXIT_FAILURE);
         }
 
-        return wait_for_terminate_and_warn(KBD_LOADKEYS, pid, true);
+        return wait_for_terminate_and_check(KBD_LOADKEYS, pid, WAIT_LOG);
 }
 
 static int font_load_and_wait(const char *vc, const char *font, const char *map, const char *unimap) {
@@ -201,7 +201,7 @@ static int font_load_and_wait(const char *vc, const char *font, const char *map,
                 _exit(EXIT_FAILURE);
         }
 
-        return wait_for_terminate_and_warn(KBD_SETFONT, pid, true);
+        return wait_for_terminate_and_check(KBD_SETFONT, pid, WAIT_LOG);
 }
 
 /*