diff --git a/src/basic/fd-util.h b/src/basic/fd-util.h index dd5207bd88..f5cfcb4ede 100644 --- a/src/basic/fd-util.h +++ b/src/basic/fd-util.h @@ -91,9 +91,10 @@ static inline int make_null_stdio(void) { /* Like TAKE_PTR() but for file descriptors, resetting them to -1 */ #define TAKE_FD(fd) \ ({ \ - int _fd_ = (fd); \ - (fd) = -1; \ - _fd_; \ + int *_fd_ = &(fd); \ + int _ret_ = *_fd_; \ + *_fd_ = -1; \ + _ret_; \ }) /* Like free_and_replace(), but for file descriptors */ diff --git a/src/basic/process-util.c b/src/basic/process-util.c index 06493f7799..82bbda895f 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -857,8 +857,8 @@ int wait_for_terminate_with_timeout(pid_t pid, usec_t timeout) { void sigkill_wait(pid_t pid) { assert(pid > 1); - if (kill(pid, SIGKILL) >= 0) - (void) wait_for_terminate(pid, NULL); + (void) kill(pid, SIGKILL); + (void) wait_for_terminate(pid, NULL); } void sigkill_waitp(pid_t *pid) { @@ -875,8 +875,8 @@ void sigkill_waitp(pid_t *pid) { void sigterm_wait(pid_t pid) { assert(pid > 1); - if (kill_and_sigcont(pid, SIGTERM) >= 0) - (void) wait_for_terminate(pid, NULL); + (void) kill_and_sigcont(pid, SIGTERM); + (void) wait_for_terminate(pid, NULL); } int kill_and_sigcont(pid_t pid, int sig) { diff --git a/src/basic/process-util.h b/src/basic/process-util.h index c7622c98df..25dea35ce6 100644 --- a/src/basic/process-util.h +++ b/src/basic/process-util.h @@ -192,8 +192,9 @@ assert_cc(TASKS_MAX <= (unsigned long) PID_T_MAX); /* Like TAKE_PTR() but for child PIDs, resetting them to 0 */ #define TAKE_PID(pid) \ ({ \ - pid_t _pid_ = (pid); \ - (pid) = 0; \ + pid_t *_ppid_ = &(pid); \ + pid_t _pid_ = *_ppid_; \ + *_ppid_ = 0; \ _pid_; \ }) diff --git a/src/core/execute.c b/src/core/execute.c index bf4a66e3bd..d8bbc694b4 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -2248,8 +2248,7 @@ static int setup_private_users(uid_t ouid, gid_t ogid, uid_t uid, gid_t gid) { if (n != 0) /* on success we should have read 0 bytes */ return -EIO; - r = wait_for_terminate_and_check("(sd-userns)", pid, 0); - pid = 0; + r = wait_for_terminate_and_check("(sd-userns)", TAKE_PID(pid), 0); if (r < 0) return r; if (r != EXIT_SUCCESS) /* If something strange happened with the child, let's consider this fatal, too */ diff --git a/src/core/mount.c b/src/core/mount.c index c4d7f981c6..321c7986b3 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -236,8 +236,7 @@ static void mount_unwatch_control_pid(Mount *m) { if (m->control_pid <= 0) return; - unit_unwatch_pid(UNIT(m), m->control_pid); - m->control_pid = 0; + unit_unwatch_pid(UNIT(m), TAKE_PID(m->control_pid)); } static void mount_parameters_done(MountParameters *p) { diff --git a/src/core/service.c b/src/core/service.c index 16d9456719..4b99311e5a 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -130,8 +130,7 @@ static void service_unwatch_control_pid(Service *s) { if (s->control_pid <= 0) return; - unit_unwatch_pid(UNIT(s), s->control_pid); - s->control_pid = 0; + unit_unwatch_pid(UNIT(s), TAKE_PID(s->control_pid)); } static void service_unwatch_main_pid(Service *s) { @@ -140,8 +139,7 @@ static void service_unwatch_main_pid(Service *s) { if (s->main_pid <= 0) return; - unit_unwatch_pid(UNIT(s), s->main_pid); - s->main_pid = 0; + unit_unwatch_pid(UNIT(s), TAKE_PID(s->main_pid)); } static void service_unwatch_pid_file(Service *s) { diff --git a/src/core/socket.c b/src/core/socket.c index 81178e3de2..ba3df32997 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -109,8 +109,7 @@ static void socket_unwatch_control_pid(Socket *s) { if (s->control_pid <= 0) return; - unit_unwatch_pid(UNIT(s), s->control_pid); - s->control_pid = 0; + unit_unwatch_pid(UNIT(s), TAKE_PID(s->control_pid)); } static void socket_cleanup_fd_list(SocketPort *p) { diff --git a/src/core/swap.c b/src/core/swap.c index 29c63118ac..de72ac9232 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -155,8 +155,7 @@ static void swap_unwatch_control_pid(Swap *s) { if (s->control_pid <= 0) return; - unit_unwatch_pid(UNIT(s), s->control_pid); - s->control_pid = 0; + unit_unwatch_pid(UNIT(s), TAKE_PID(s->control_pid)); } static void swap_done(Unit *u) { diff --git a/src/fundamental/macro-fundamental.h b/src/fundamental/macro-fundamental.h index 36d759ea1d..ed527adb77 100644 --- a/src/fundamental/macro-fundamental.h +++ b/src/fundamental/macro-fundamental.h @@ -257,8 +257,9 @@ * resets it to NULL. See: https://doc.rust-lang.org/std/option/enum.Option.html#method.take */ #define TAKE_PTR(ptr) \ ({ \ - typeof(ptr) _ptr_ = (ptr); \ - (ptr) = NULL; \ + typeof(ptr) *_pptr_ = &(ptr); \ + typeof(ptr) _ptr_ = *_pptr_; \ + *_pptr_ = NULL; \ _ptr_; \ }) diff --git a/src/import/export-tar.c b/src/import/export-tar.c index b734c3004a..78f0a7ca75 100644 --- a/src/import/export-tar.c +++ b/src/import/export-tar.c @@ -56,10 +56,8 @@ TarExport *tar_export_unref(TarExport *e) { sd_event_source_unref(e->output_event_source); - if (e->tar_pid > 1) { - (void) kill_and_sigcont(e->tar_pid, SIGKILL); - (void) wait_for_terminate(e->tar_pid, NULL); - } + if (e->tar_pid > 1) + sigkill_wait(e->tar_pid); if (e->temp_path) { (void) btrfs_subvol_remove(e->temp_path, BTRFS_REMOVE_QUOTA); @@ -147,8 +145,7 @@ static int tar_export_finish(TarExport *e) { assert(e->tar_fd >= 0); if (e->tar_pid > 0) { - r = wait_for_terminate_and_check("tar", e->tar_pid, WAIT_LOG); - e->tar_pid = 0; + r = wait_for_terminate_and_check("tar", TAKE_PID(e->tar_pid), WAIT_LOG); if (r < 0) return r; if (r != EXIT_SUCCESS) diff --git a/src/import/import-tar.c b/src/import/import-tar.c index bb67862d62..35db30de78 100644 --- a/src/import/import-tar.c +++ b/src/import/import-tar.c @@ -69,10 +69,8 @@ TarImport* tar_import_unref(TarImport *i) { sd_event_source_unref(i->input_event_source); - if (i->tar_pid > 1) { - (void) kill_and_sigcont(i->tar_pid, SIGKILL); - (void) wait_for_terminate(i->tar_pid, NULL); - } + if (i->tar_pid > 1) + sigkill_wait(i->tar_pid); rm_rf_subvolume_and_free(i->temp_path); @@ -167,8 +165,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_check("tar", i->tar_pid, WAIT_LOG); - i->tar_pid = 0; + r = wait_for_terminate_and_check("tar", TAKE_PID(i->tar_pid), WAIT_LOG); if (r < 0) return r; if (r != EXIT_SUCCESS) diff --git a/src/import/importd.c b/src/import/importd.c index 86181628d9..3e2d8427cb 100644 --- a/src/import/importd.c +++ b/src/import/importd.c @@ -126,10 +126,8 @@ static Transfer *transfer_unref(Transfer *t) { free(t->format); free(t->object_path); - if (t->pid > 0) { - (void) kill_and_sigcont(t->pid, SIGKILL); - (void) wait_for_terminate(t->pid, NULL); - } + if (t->pid > 1) + sigkill_wait(t->pid); safe_close(t->log_fd); safe_close(t->stdin_fd); diff --git a/src/import/pull-common.c b/src/import/pull-common.c index 39067933b5..adb366222d 100644 --- a/src/import/pull-common.c +++ b/src/import/pull-common.c @@ -482,8 +482,7 @@ static int verify_gpg( gpg_pipe[1] = safe_close(gpg_pipe[1]); - r = wait_for_terminate_and_check("gpg", pid, WAIT_LOG_ABNORMAL); - pid = 0; + r = wait_for_terminate_and_check("gpg", TAKE_PID(pid), WAIT_LOG_ABNORMAL); if (r < 0) goto finish; if (r != EXIT_SUCCESS) diff --git a/src/import/pull-tar.c b/src/import/pull-tar.c index 06d336bca9..67e06a1445 100644 --- a/src/import/pull-tar.c +++ b/src/import/pull-tar.c @@ -71,10 +71,8 @@ TarPull* tar_pull_unref(TarPull *i) { if (!i) return NULL; - if (i->tar_pid > 1) { - (void) kill_and_sigcont(i->tar_pid, SIGKILL); - (void) wait_for_terminate(i->tar_pid, NULL); - } + if (i->tar_pid > 1) + sigkill_wait(i->tar_pid); pull_job_unref(i->tar_job); pull_job_unref(i->checksum_job); @@ -369,8 +367,7 @@ static void tar_pull_job_on_finished(PullJob *j) { pull_job_close_disk_fd(i->settings_job); if (i->tar_pid > 0) { - r = wait_for_terminate_and_check("tar", i->tar_pid, WAIT_LOG); - i->tar_pid = 0; + r = wait_for_terminate_and_check("tar", TAKE_PID(i->tar_pid), WAIT_LOG); if (r < 0) goto finish; if (r != EXIT_SUCCESS) { diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 96d5b9605e..9ebf7b03c0 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -1082,10 +1082,10 @@ static int bus_parse_next_address(sd_bus *b) { } static void bus_kill_exec(sd_bus *bus) { - if (pid_is_valid(bus->busexec_pid) > 0) { - sigterm_wait(bus->busexec_pid); - bus->busexec_pid = 0; - } + if (!pid_is_valid(bus->busexec_pid)) + return; + + sigterm_wait(TAKE_PID(bus->busexec_pid)); } static int bus_start_address(sd_bus *b) { diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 8c0bc99d72..09c79b22e7 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -5248,8 +5248,7 @@ static int run_container( } } - r = wait_for_container(*pid, &container_status); - *pid = 0; + r = wait_for_container(TAKE_PID(*pid), &container_status); /* Tell machined that we are gone. */ if (bus) diff --git a/src/shared/pager.c b/src/shared/pager.c index 90c347f8c0..5418065333 100644 --- a/src/shared/pager.c +++ b/src/shared/pager.c @@ -286,7 +286,7 @@ void pager_close(void) { stdout_redirected = stderr_redirected = false; (void) kill(pager_pid, SIGCONT); - (void) wait_for_terminate(pager_pid, NULL); + (void) wait_for_terminate(TAKE_PID(pager_pid), NULL); pager_pid = 0; } diff --git a/src/shared/spawn-ask-password-agent.c b/src/shared/spawn-ask-password-agent.c index 38fab21203..902d7543a5 100644 --- a/src/shared/spawn-ask-password-agent.c +++ b/src/shared/spawn-ask-password-agent.c @@ -43,9 +43,7 @@ void ask_password_agent_close(void) { return; /* Inform agent that we are done */ - (void) kill_and_sigcont(agent_pid, SIGTERM); - (void) wait_for_terminate(agent_pid, NULL); - agent_pid = 0; + sigterm_wait(TAKE_PID(agent_pid)); } int ask_password_agent_open_if_enabled(BusTransport transport, bool ask_password) { diff --git a/src/shared/spawn-polkit-agent.c b/src/shared/spawn-polkit-agent.c index cd0b4601da..7fa7508178 100644 --- a/src/shared/spawn-polkit-agent.c +++ b/src/shared/spawn-polkit-agent.c @@ -69,9 +69,7 @@ void polkit_agent_close(void) { return; /* Inform agent that we are done */ - (void) kill_and_sigcont(agent_pid, SIGTERM); - (void) wait_for_terminate(agent_pid, NULL); - agent_pid = 0; + sigterm_wait(TAKE_PID(agent_pid)); } #else diff --git a/src/test/test-fd-util.c b/src/test/test-fd-util.c index 0aa229fbc9..3638103bf1 100644 --- a/src/test/test-fd-util.c +++ b/src/test/test-fd-util.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #include +#include #include #include "alloc-util.h" @@ -486,6 +487,47 @@ static void test_fd_reopen(void) { fd1 = -1; } +static void test_take_fd(void) { + _cleanup_close_ int fd1 = -1, fd2 = -1; + int array[2] = { -1, -1 }, i = 0; + + log_info("/* %s */", __func__); + + assert_se(fd1 == -1); + assert_se(fd2 == -1); + + fd1 = eventfd(0, EFD_CLOEXEC); + assert_se(fd1 >= 0); + + fd2 = TAKE_FD(fd1); + assert_se(fd1 == -1); + assert_se(fd2 >= 0); + + assert_se(array[0] == -1); + assert_se(array[1] == -1); + + array[0] = TAKE_FD(fd2); + assert_se(fd1 == -1); + assert_se(fd2 == -1); + assert_se(array[0] >= 0); + assert_se(array[1] == -1); + + array[1] = TAKE_FD(array[i]); + assert_se(array[0] == -1); + assert_se(array[1] >= 0); + + i = 1 - i; + array[0] = TAKE_FD(*(array + i)); + assert_se(array[0] >= 0); + assert_se(array[1] == -1); + + i = 1 - i; + fd1 = TAKE_FD(array[i]); + assert_se(fd1 >= 0); + assert_se(array[0] == -1); + assert_se(array[1] == -1); +} + int main(int argc, char *argv[]) { test_setup_logging(LOG_DEBUG); @@ -500,6 +542,7 @@ int main(int argc, char *argv[]) { test_close_all_fds(); test_format_proc_fd_path(); test_fd_reopen(); + test_take_fd(); return 0; }