Re: [PATCH 2/2] [GDBserver] Block and unblock SIGIO

Pedro Alves
2016-01-26T12:01:11Z

On 01/26/2016 09:58 AM, Yao Qi wrote:

> -static void
> -unblock_async_io (void)
> -{
> -#ifndef USE_WIN32API
> - sigset_t sigio_set;
> -
> - sigemptyset (&sigio_set);
> - sigaddset (&sigio_set, SIGIO);
> - sigprocmask (SIG_UNBLOCK, &sigio_set, NULL);
> -#endif
> -}
> +/* Asynchronous I/O support. SIGIO must be unblocked when waiting,
> + in order to accept Control-C from the client, and must be blocked
> + when talking to the client. */
>
> #ifdef __QNX__
> static void
> @@ -820,12 +813,19 @@ static int async_io_enabled;
> void
> enable_async_io (void)
> {
> +#ifndef USE_WIN32API
> + sigset_t sigio_set;
> +#endif
> +
> if (async_io_enabled)
> return;
>
> #ifndef USE_WIN32API
> - signal (SIGIO, input_interrupt);
> + sigemptyset (&sigio_set);
> + sigaddset (&sigio_set, SIGIO);
> + sigprocmask (SIG_UNBLOCK, &sigio_set, NULL);
> #endif
> +
> async_io_enabled = 1;
> #ifdef __QNX__
> nto_comctrl (1);
> @@ -836,12 +836,19 @@ enable_async_io (void)
> void
> disable_async_io (void)
> {
> +#ifndef USE_WIN32API
> + sigset_t sigio_set;
> +#endif
> +
> if (!async_io_enabled)
> return;
>
> #ifndef USE_WIN32API
> - signal (SIGIO, SIG_IGN);
> + sigemptyset (&sigio_set);
> + sigaddset (&sigio_set, SIGIO);
> + sigprocmask (SIG_BLOCK, &sigio_set, NULL);
> #endif
> +

I'd suggest factoring out this duplicate sigprocmask handling,
like, say, rename unblock_async_io and add a parameter:

/* Block or unblock SIGIO. */

static void
block_unblock_async_io (int block)
{
#ifndef USE_WIN32API
sigset_t sigio_set;

sigemptyset (&sigio_set);
sigaddset (&sigio_set, SIGIO);
sigprocmask (block ? SIG_BLOCK : SIG_UNBLOCK, &sigio_set, NULL);
#endif
}

> async_io_enabled = 0;
> #ifdef __QNX__
> nto_comctrl (0);
> @@ -852,12 +859,14 @@ disable_async_io (void)
> void
> initialize_async_io (void)
> {
> - /* Make sure that async I/O starts disabled. */
> + /* Install the signal handler. */
> +#ifndef USE_WIN32API
> + signal (SIGIO, input_interrupt);
> +#endif
> +
> + /* Make sure that async I/O starts blocked. */
> async_io_enabled = 1;
> disable_async_io ();

I think it's safer practice to block the signal before
installing the handler.

Otherwise LGTM.

> -
> - /* Make sure the signal is unblocked. */
> - unblock_async_io ();
> }
>
> /* Internal buffer used by readchar.
>

Thanks,
Pedro Alves