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

[Pedro Alves <palves@redhat.com>]

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