Re: [PATCH] multipathd: fix inverted signal blocking logic

[Bart Van Assche <Bart.VanAssche@wdc.com>]

On Fri, 2018-03-02 at 22:18 +0100, Martin Wilck wrote:
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 61739ac6ea59..85ee9b713d75 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -2270,10 +2270,13 @@ signal_init(void)
>  {
>  	sigset_t set;
>  
> -	sigemptyset(&set);
> -	sigaddset(&set, SIGUSR2);
> +	/* block all signals */
> +	sigfillset(&set);
> +	/* SIGPIPE occurs if logging fails */
> +	sigdelset(&set, SIGPIPE);
>  	pthread_sigmask(SIG_SETMASK, &set, NULL);

The modified code was introduced by commit 90dd424afa65 ("multipathd:
fix SIGUSR2 handling"). This change looks fine to me.

> diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c
> index 98ac25a68c43..a2ca36ba1653 100644
> --- a/multipathd/uxlsnr.c
> +++ b/multipathd/uxlsnr.c
> @@ -170,11 +170,11 @@ void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, void * trigger_data)
>  		condlog(0, "uxsock: failed to allocate poll fds");
>  		return NULL;
>  	}
> -	sigemptyset(&mask);
> -	sigaddset(&mask, SIGINT);
> -	sigaddset(&mask, SIGTERM);
> -	sigaddset(&mask, SIGHUP);
> -	sigaddset(&mask, SIGUSR1);
> +	sigfillset(&mask);
> +	sigdelset(&mask, SIGINT);
> +	sigdelset(&mask, SIGTERM);
> +	sigdelset(&mask, SIGHUP);
> +	sigdelset(&mask, SIGUSR1);
>  	while (1) {
>  		struct client *c, *tmp;
>  		int i, poll_count, num_clients;

This change looks more complicated to me than necessary. Have you considered
to pass an empty signal set as the fourth ppoll() argument?

Thanks,

Bart.