]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
signal-util: make -1 termination of ignore_signals() argument list unnecessary
authorLennart Poettering <lennart@poettering.net>
Thu, 25 Feb 2021 07:56:57 +0000 (08:56 +0100)
committerZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Thu, 25 Feb 2021 10:32:28 +0000 (11:32 +0100)
Clean up ignore_signals() + default_signals() + sigaction_many() a bit:
make it unnecessary to explicitly terminate the signal list with -1.
Merge all three calls into a single function that is just called with
slightly different parameters. And eliminate an unnecessary extra
iteration in its inner for() loop.

No change in behaviour.

15 files changed:
src/basic/signal-util.c
src/basic/signal-util.h
src/core/execute.c
src/core/main.c
src/core/unit.c
src/coredump/coredumpctl.c
src/import/export.c
src/import/import.c
src/import/pull.c
src/login/inhibit.c
src/login/logind.c
src/nspawn/nspawn.c
src/shared/pager.c
src/systemctl/systemctl-switch-root.c
src/test/test-signal-util.c

index 331295c2c509d1e1c3720842e5929efc01e668c4..131fd3ba0047976b43a9efe7d2e24771594ba0a3 100644 (file)
@@ -45,11 +45,14 @@ int reset_signal_mask(void) {
         return 0;
 }
 
-static int sigaction_many_ap(const struct sigaction *sa, int sig, va_list ap) {
-        int r = 0;
+int sigaction_many_internal(const struct sigaction *sa, ...) {
+        int sig, r = 0;
+        va_list ap;
+
+        va_start(ap, sa);
 
         /* negative signal ends the list. 0 signal is skipped. */
-        for (; sig >= 0; sig = va_arg(ap, int)) {
+        while ((sig = va_arg(ap, int)) >= 0) {
 
                 if (sig == 0)
                         continue;
@@ -60,49 +63,6 @@ static int sigaction_many_ap(const struct sigaction *sa, int sig, va_list ap) {
                 }
         }
 
-        return r;
-}
-
-int sigaction_many(const struct sigaction *sa, ...) {
-        va_list ap;
-        int r;
-
-        va_start(ap, sa);
-        r = sigaction_many_ap(sa, 0, ap);
-        va_end(ap);
-
-        return r;
-}
-
-int ignore_signals(int sig, ...) {
-
-        static const struct sigaction sa = {
-                .sa_handler = SIG_IGN,
-                .sa_flags = SA_RESTART,
-        };
-
-        va_list ap;
-        int r;
-
-        va_start(ap, sig);
-        r = sigaction_many_ap(&sa, sig, ap);
-        va_end(ap);
-
-        return r;
-}
-
-int default_signals(int sig, ...) {
-
-        static const struct sigaction sa = {
-                .sa_handler = SIG_DFL,
-                .sa_flags = SA_RESTART,
-        };
-
-        va_list ap;
-        int r;
-
-        va_start(ap, sig);
-        r = sigaction_many_ap(&sa, sig, ap);
         va_end(ap);
 
         return r;
index bdd39d429ddaef75eb92043291a12d368bf6e632..37271d7a68e4eb4888870d36ab7f0cf6f57ef041 100644 (file)
@@ -8,9 +8,28 @@
 int reset_all_signal_handlers(void);
 int reset_signal_mask(void);
 
-int ignore_signals(int sig, ...);
-int default_signals(int sig, ...);
-int sigaction_many(const struct sigaction *sa, ...);
+int sigaction_many_internal(const struct sigaction *sa, ...);
+
+#define ignore_signals(...)                                             \
+        sigaction_many_internal(                                        \
+                        &(const struct sigaction) {                     \
+                                .sa_handler = SIG_IGN,                  \
+                                .sa_flags = SA_RESTART                  \
+                        },                                              \
+                        __VA_ARGS__,                                    \
+                        -1)
+
+#define default_signals(...)                                            \
+        sigaction_many_internal(                                        \
+                        &(const struct sigaction) {                     \
+                                .sa_handler = SIG_DFL,                  \
+                                .sa_flags = SA_RESTART                  \
+                        },                                              \
+                        __VA_ARGS__,                                    \
+                        -1)
+
+#define sigaction_many(sa, ...)                                         \
+        sigaction_many_internal(sa, __VA_ARGS__, -1)
 
 int sigset_add_many(sigset_t *ss, ...);
 int sigprocmask_many(int how, sigset_t *old, ...);
index 69f7a98c7bdbe41cbed10bbaad10382231d5d611..de51eba11c0798edad880309e2dd354c54f8f89f 100644 (file)
@@ -1268,7 +1268,7 @@ static int setup_pam(
                 if (setresuid(uid, uid, uid) < 0)
                         log_warning_errno(errno, "Failed to setresuid() in sd-pam: %m");
 
-                (void) ignore_signals(SIGPIPE, -1);
+                (void) ignore_signals(SIGPIPE);
 
                 /* Wait until our parent died. This will only work if
                  * the above setresuid() succeeds, otherwise the kernel
@@ -3733,16 +3733,14 @@ static int exec_child(
 
         rename_process_from_path(command->path);
 
-        /* We reset exactly these signals, since they are the
-         * only ones we set to SIG_IGN in the main daemon. All
-         * others we leave untouched because we set them to
-         * SIG_DFL or a valid handler initially, both of which
-         * will be demoted to SIG_DFL. */
+        /* We reset exactly these signals, since they are the only ones we set to SIG_IGN in the main
+         * daemon. All others we leave untouched because we set them to SIG_DFL or a valid handler initially,
+         * both of which will be demoted to SIG_DFL. */
         (void) default_signals(SIGNALS_CRASH_HANDLER,
-                               SIGNALS_IGNORE, -1);
+                               SIGNALS_IGNORE);
 
         if (context->ignore_sigpipe)
-                (void) ignore_signals(SIGPIPE, -1);
+                (void) ignore_signals(SIGPIPE);
 
         r = reset_signal_mask();
         if (r < 0) {
index e41e73c581d4276545acbec556794a45e9838957..af5a1f426d1fb6cb63103b193d3a3a7e58386254 100644 (file)
@@ -323,9 +323,8 @@ static void install_crash_handler(void) {
         };
         int r;
 
-        /* We ignore the return value here, since, we don't mind if we
-         * cannot set up a crash handler */
-        r = sigaction_many(&sa, SIGNALS_CRASH_HANDLER, -1);
+        /* We ignore the return value here, since, we don't mind if we cannot set up a crash handler */
+        r = sigaction_many(&sa, SIGNALS_CRASH_HANDLER);
         if (r < 0)
                 log_debug_errno(r, "I had trouble setting up the crash handler, ignoring: %m");
 }
@@ -2756,7 +2755,7 @@ int main(int argc, char *argv[]) {
 
         /* Reset all signal handlers. */
         (void) reset_all_signal_handlers();
-        (void) ignore_signals(SIGNALS_IGNORE, -1);
+        (void) ignore_signals(SIGNALS_IGNORE);
 
         (void) parse_configuration(&saved_rlimit_nofile, &saved_rlimit_memlock);
 
index e9c22375dd184ba2b8e568fc0d478b97ed5ff6ce..2c5dc54379a77e0e86f2816ba79c58a91ee6e6d3 100644 (file)
@@ -4756,8 +4756,8 @@ int unit_fork_helper_process(Unit *u, const char *name, pid_t *ret) {
         if (r != 0)
                 return r;
 
-        (void) default_signals(SIGNALS_CRASH_HANDLER, SIGNALS_IGNORE, -1);
-        (void) ignore_signals(SIGPIPE, -1);
+        (void) default_signals(SIGNALS_CRASH_HANDLER, SIGNALS_IGNORE);
+        (void) ignore_signals(SIGPIPE);
 
         (void) prctl(PR_SET_PDEATHSIG, SIGTERM);
 
index 20cd90bd568ab140085242c89af456a8365dccd5..5364169c4ff904a8036057b46b404995dabd6d65 100644 (file)
@@ -1110,7 +1110,7 @@ static int run_debug(int argc, char **argv, void *userdata) {
                 return log_oom();
 
         /* Don't interfere with gdb and its handling of SIGINT. */
-        (void) ignore_signals(SIGINT, -1);
+        (void) ignore_signals(SIGINT);
 
         fork_name = strjoina("(", debugger_call[0], ")");
 
@@ -1127,7 +1127,7 @@ static int run_debug(int argc, char **argv, void *userdata) {
         r = wait_for_terminate_and_check(debugger_call[0], pid, WAIT_LOG_ABNORMAL);
 
 finish:
-        (void) default_signals(SIGINT, -1);
+        (void) default_signals(SIGINT);
 
         if (unlink_path) {
                 log_debug("Removed temporary file %s", path);
index 5faf4ccc06b0516577f752c776369b59c22e72e5..a4f3f6c38e073402ab28dfe622d0cbccc347cbd2 100644 (file)
@@ -289,7 +289,7 @@ static int run(int argc, char *argv[]) {
         if (r <= 0)
                 return r;
 
-        (void) ignore_signals(SIGPIPE, -1);
+        (void) ignore_signals(SIGPIPE);
 
         return export_main(argc, argv);
 }
index 661fa2888c58b41f5f1b39b9cf29cbb49b15b4b0..fe4c03a4da52f7972e1cb261d30b6f9bc4b9a0be 100644 (file)
@@ -306,7 +306,7 @@ static int run(int argc, char *argv[]) {
         if (r <= 0)
                 return 0;
 
-        (void) ignore_signals(SIGPIPE, -1);
+        (void) ignore_signals(SIGPIPE);
 
         return import_main(argc, argv);
 }
index dc0bf20201daec125f2b57f49b80ce2ee204138e..d24c71b00ecf530d4173306ab91edd1c6797b34c 100644 (file)
@@ -349,7 +349,7 @@ static int run(int argc, char *argv[]) {
         if (r <= 0)
                 return r;
 
-        (void) ignore_signals(SIGPIPE, -1);
+        (void) ignore_signals(SIGPIPE);
 
         return pull_main(argc, argv);
 }
index 20685c18a30643a9c3a824c56ee498289d87f62d..98bc21eb2dfc950f314411bb120c4d7143e0c0fa 100644 (file)
@@ -295,7 +295,7 @@ static int run(int argc, char *argv[]) {
                 pid_t pid;
 
                 /* Ignore SIGINT and allow the forked process to receive it */
-                (void) ignore_signals(SIGINT, -1);
+                (void) ignore_signals(SIGINT);
 
                 if (!arg_who) {
                         w = strv_join(argv + optind, " ");
index dac0bd2728de852628e1c38a8b7ab5535c7d556c..56235230cf7d5d34083feb460b9ceead76c50632 100644 (file)
@@ -788,7 +788,7 @@ static int manager_connect_console(Manager *m) {
                                        "Not enough real-time signals available: %u-%u",
                                        SIGRTMIN, SIGRTMAX);
 
-        assert_se(ignore_signals(SIGRTMIN + 1, -1) >= 0);
+        assert_se(ignore_signals(SIGRTMIN + 1) >= 0);
         assert_se(sigprocmask_many(SIG_BLOCK, NULL, SIGRTMIN, -1) >= 0);
 
         r = sd_event_add_signal(m->event, NULL, SIGRTMIN, manager_vt_switch, m);
index f38f8c2a87633f2b39bfc7be5892a4151d6fd74e..44f457a5b549e93572d2aa6e9de1d4ceef85e223 100644 (file)
@@ -4850,7 +4850,7 @@ static int run_container(
         assert_se(sigprocmask(SIG_BLOCK, &mask_chld, NULL) >= 0);
 
         /* Reset signal to default */
-        r = default_signals(SIGCHLD, -1);
+        r = default_signals(SIGCHLD);
         if (r < 0)
                 return log_error_errno(r, "Failed to reset SIGCHLD: %m");
 
@@ -5219,7 +5219,7 @@ static int run(int argc, char *argv[]) {
         /* Ignore SIGPIPE here, because we use splice() on the ptyfwd stuff and that will generate SIGPIPE if
          * the result is closed. Note that the container payload child will reset signal mask+handler anyway,
          * so just turning this off here means we only turn it off in nspawn itself, not any children. */
-        (void) ignore_signals(SIGPIPE, -1);
+        (void) ignore_signals(SIGPIPE);
 
         n_fd_passed = sd_listen_fds(false);
         if (n_fd_passed > 0) {
index f689d9f28f0f6f287641629ac361eecab1ffa6fb..d140c371c2474df4e4c33c2567158a392d86ab9a 100644 (file)
@@ -262,7 +262,7 @@ int pager_open(PagerFlags flags) {
         if (r < 0)
                 return r;
         if (r > 0)
-                (void) ignore_signals(SIGINT, -1);
+                (void) ignore_signals(SIGINT);
 
         return 1;
 }
index 9ed40e6ec3aa44d908a68e2de3fc67754177fe26..b8012679748230018e872d32fae8666409f1afc9 100644 (file)
@@ -60,7 +60,7 @@ int switch_root(int argc, char *argv[], void *userdata) {
 
         /* If we are slow to exit after the root switch, the new systemd instance will send us a signal to
          * terminate. Just ignore it and exit normally.  This way the unit does not end up as failed. */
-        r = ignore_signals(SIGTERM, -1);
+        r = ignore_signals(SIGTERM);
         if (r < 0)
                 log_warning_errno(r, "Failed to change disposition of SIGTERM to ignore: %m");
 
@@ -68,7 +68,7 @@ int switch_root(int argc, char *argv[], void *userdata) {
 
         r = bus_call_method(bus, bus_systemd_mgr, "SwitchRoot", &error, NULL, "ss", root, init);
         if (r < 0) {
-                (void) default_signals(SIGTERM, -1);
+                (void) default_signals(SIGTERM);
 
                 return log_error_errno(r, "Failed to switch root: %s", bus_error_message(&error, r));
         }
index e5096a8c00c4629b352c63a7f044c5a81f10c942..51e604f72367ec450eac8dc3aa0b441f8ca8c49c 100644 (file)
@@ -130,14 +130,14 @@ static void test_block_signals(void) {
 }
 
 static void test_ignore_signals(void) {
-        assert_se(ignore_signals(SIGINT, -1) >= 0);
+        assert_se(ignore_signals(SIGINT) >= 0);
         assert_se(kill(getpid_cached(), SIGINT) >= 0);
-        assert_se(ignore_signals(SIGUSR1, SIGUSR2, SIGTERM, SIGPIPE, -1) >= 0);
+        assert_se(ignore_signals(SIGUSR1, SIGUSR2, SIGTERM, SIGPIPE) >= 0);
         assert_se(kill(getpid_cached(), SIGUSR1) >= 0);
         assert_se(kill(getpid_cached(), SIGUSR2) >= 0);
         assert_se(kill(getpid_cached(), SIGTERM) >= 0);
         assert_se(kill(getpid_cached(), SIGPIPE) >= 0);
-        assert_se(default_signals(SIGINT, SIGUSR1, SIGUSR2, SIGTERM, SIGPIPE, -1) >= 0);
+        assert_se(default_signals(SIGINT, SIGUSR1, SIGUSR2, SIGTERM, SIGPIPE) >= 0);
 }
 
 int main(int argc, char *argv[]) {