"Carlo Marcelo Arenas Belón via GitGitGadget"
<gitgitgadget@xxxxxxxxx> writes:
> From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= <carenas@xxxxxxxxx>
>
> In a future change, the flags used for processing SIGCHLD will need to
> be updated, which is only possible by using sigaction().
>
> Factor out the call to set the signal handler and use sigaction instead
> of signal for the systems that allow that, which has the added benefit
> of using BSD semantics reliably and therefore not needing the rearming
> call.
>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@xxxxxxxxx>
> ---
> daemon.c | 35 +++++++++++++++++++++++++++++------
> 1 file changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/daemon.c b/daemon.c
> index d1be61fd5789..8133bd902157 100644
> --- a/daemon.c
> +++ b/daemon.c
> @@ -912,14 +912,19 @@ static void handle(int incoming, struct sockaddr *addr, socklen_t addrlen)
> add_child(&cld, addr, addrlen);
> }
>
> -static void child_handler(int signo UNUSED)
> +static void child_handler(int signo MAYBE_UNUSED)
> {
> /*
> - * Otherwise empty handler because systemcalls will get interrupted
> - * upon signal receipt
> - * SysV needs the handler to be rearmed
> + * Otherwise empty handler because systemcalls should get interrupted
> + * upon signal receipt.
> */
> - signal(SIGCHLD, child_handler);
> +#ifdef USE_NON_POSIX_SIGNAL
> + /*
> + * SysV needs the handler to be rearmed, but this is known
> + * to trigger infinite recursion crashes at least in AIX.
> + */
> + signal(signo, child_handler);
> +#endif
> }
>
> static int set_reuse_addr(int sockfd)
> @@ -1118,8 +1123,26 @@ static void socksetup(struct string_list *listen_addr, int listen_port, struct s
> }
> }
>
> +#ifndef USE_NON_POSIX_SIGNAL
Does this #ifndef block with #else lack its #endif probably before
the service_loop() is defined ...
> +static void set_signal_handler(struct sigaction *psa)
> +{
> + sigemptyset(&psa->sa_mask);
> + psa->sa_flags = SA_NOCLDSTOP | SA_RESTART;
> + psa->sa_handler = child_handler;
> + sigaction(SIGCHLD, psa, NULL);
> +}
> +
> +#else
> +
> +static void set_signal_handler(struct sigaction *psa UNUSED)
> +{
> + signal(SIGCHLD, child_handler);
> +}
... around here?
I am wondering if it would make it even cleaner to also define
rearm_signal_handler() in this "if sigaction() can be used, do this,
otherwise do that" block, and move the whole thing a bit earlier in
this file. That way, the primary code paths do not have to see much
of the #ifdef conditionals. With IPV6 related #ifdef noise already
contaminating the file, it may not be a huge deal, but these crufts
tend to build up unless we tightly control them with discipline.
> static int service_loop(struct socketlist *socklist)
> {
> + struct sigaction sa;
> struct pollfd *pfd;
>
> CALLOC_ARRAY(pfd, socklist->nr);
> @@ -1129,7 +1152,7 @@ static int service_loop(struct socketlist *socklist)
> pfd[i].events = POLLIN;
> }
>
> - signal(SIGCHLD, child_handler);
> + set_signal_handler(&sa);
>
> for (;;) {
> check_dead_children();
