signal-util: make -1 termination of ignore_signals() argument list unnecessary

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.
This commit is contained in:
Lennart Poettering
2021-02-25 11:32:28 +01:00
committed by Zbigniew Jędrzejewski-Szmek
parent 155d626bc6
commit 9c274488a9
15 changed files with 54 additions and 78 deletions
+6 -46
View 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;
+22 -3
View 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, ...);
+6 -8
View 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) {
+3 -4
View 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);
+2 -2
View 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);
+2 -2
View 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);
+1 -1
View 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);
}
+1 -1
View 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);
}
+1 -1
View 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);
}
+1 -1
View 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, " ");
+1 -1
View 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);
+2 -2
View 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) {
+1 -1
View 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;
}
+2 -2
View 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));
}
+3 -3
View 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[]) {