Re: [hackers] [surf][PATCH] Use sigaction instead of signal for installing signal handlers

From: Hiltjo Posthuma <hiltjo_AT_codemadness.org>
Date: Thu, 22 Oct 2020 11:12:20 +0200

On Thu, Oct 22, 2020 at 12:16:05AM -0500, Nihal Jere wrote:
> sigaction provides a more portable way of installing signals
> ---
> surf.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/surf.c b/surf.c
> index 2b54e3c..65a5e66 100644
> --- a/surf.c
> +++ b/surf.c
> _AT_@ -316,9 +316,20 @@ setup(void)
> GdkDisplay *gdpy;
> int i, j;
>
> + struct sigaction sachld;
> + struct sigaction sahup;
> +
> + memset(&sachld, 0, sizeof(struct sigaction));
> + memset(&sahup, 0, sizeof(struct sigaction));
> +
> + sachld.sa_handler = sigchld;
> + sahup.sa_handler = sighup;
> +
> /* clean up any zombies immediately */
> - sigchld(0);
> - if (signal(SIGHUP, sighup) == SIG_ERR)
> + if (sigaction(SIGCHLD, &sachld, NULL) == -1)
> + die("Can't install SIGCHLD handler");
> +
> + if (sigaction(SIGHUP, &sahup, NULL) == -1)
> die("Can't install SIGHUP handler");
>
> if (!(dpy = XOpenDisplay(NULL)))
> _AT_@ -402,8 +413,6 @@ setup(void)
> void
> sigchld(int unused)
> {
> - if (signal(SIGCHLD, sigchld) == SIG_ERR)
> - die("Can't install SIGCHLD handler");
> while (waitpid(-1, NULL, WNOHANG) > 0)
> ;
> }
> --
> 2.29.0
>
>

Hi Nihal,

Theres no need to use two separate sigaction structs.
Just change sa_handler after calling sigaction().

I would also prefer to add an explicity call:

        sigemptyset(&sa.sa_mask);

And possibly BSD signal semantics are preferred:

        sa.sa_flags = SA_RESTART;

Side-note:
I'm not sure how signal-safe the current code is though. It seems to call a
webkit function in a signal handler...

-- 
Kind regards,
Hiltjo
Received on Thu Oct 22 2020 - 11:12:20 CEST

This archive was generated by hypermail 2.3.0 : Thu Oct 22 2020 - 11:12:35 CEST