[2/2,GDBserver] Block and unblock SIGIO

Message ID 86d1soqwd4.fsf@gmail.com
State New, archived
Headers

Commit Message

Yao Qi Jan. 26, 2016, 1:54 p.m. UTC
  Pedro Alves <palves@redhat.com> writes:

> 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
> }

Done.

>
>>    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.

OK, and fixed.

>
> Otherwise LGTM.

Patch below is pushed in.
  

Patch

diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 4aa7350..0ba902e 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,5 +1,16 @@ 
 2016-01-26  Yao Qi  <yao.qi@linaro.org>
 
+	* remote-utils.c (remote_close) [!USE_WIN32API]: Ignore SIGIO.
+	(unblock_async_io): Rename to ...
+	(block_unblock_async_io): ... it.  New function.
+	(enable_async_io): Don't install SIGIO handler.  Unblock it
+	instead.
+	(disable_async_io): Don't ignore SIGIO.  Block it instead.
+	(initialize_async_io): Install SIGIO handler.  Don't call
+	unblock_async_io.
+
+2016-01-26  Yao Qi  <yao.qi@linaro.org>
+
 	* remote-utils.c (getpkt): If the buffer isn't empty, and the
 	first character is '\003', call *the_target->request_interrupt.
 
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index ccc99f1..e751473 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -402,6 +402,11 @@  remote_close (void)
 {
   delete_file_handler (remote_desc);
 
+#ifndef USE_WIN32API
+  /* Remove SIGIO handler.  */
+  signal (SIGIO, SIG_IGN);
+#endif
+
 #ifdef USE_WIN32API
   closesocket (remote_desc);
 #else
@@ -775,19 +780,19 @@  check_remote_input_interrupt_request (void)
   input_interrupt (0);
 }
 
-/* Asynchronous I/O support.  SIGIO must be enabled when waiting, in order to
-   accept Control-C from the client, and must be disabled when talking to
-   the client.  */
+/* 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.  */
 
 static void
-unblock_async_io (void)
+block_unblock_async_io (int block)
 {
 #ifndef USE_WIN32API
   sigset_t sigio_set;
 
   sigemptyset (&sigio_set);
   sigaddset (&sigio_set, SIGIO);
-  sigprocmask (SIG_UNBLOCK, &sigio_set, NULL);
+  sigprocmask (block ? SIG_BLOCK : SIG_UNBLOCK, &sigio_set, NULL);
 #endif
 }
 
@@ -823,9 +828,8 @@  enable_async_io (void)
   if (async_io_enabled)
     return;
 
-#ifndef USE_WIN32API
-  signal (SIGIO, input_interrupt);
-#endif
+  block_unblock_async_io (0);
+
   async_io_enabled = 1;
 #ifdef __QNX__
   nto_comctrl (1);
@@ -839,9 +843,8 @@  disable_async_io (void)
   if (!async_io_enabled)
     return;
 
-#ifndef USE_WIN32API
-  signal (SIGIO, SIG_IGN);
-#endif
+  block_unblock_async_io (1);
+
   async_io_enabled = 0;
 #ifdef __QNX__
   nto_comctrl (0);
@@ -852,12 +855,14 @@  disable_async_io (void)
 void
 initialize_async_io (void)
 {
-  /* Make sure that async I/O starts disabled.  */
+  /* Make sure that async I/O starts blocked.  */
   async_io_enabled = 1;
   disable_async_io ();
 
-  /* Make sure the signal is unblocked.  */
-  unblock_async_io ();
+  /* Install the signal handler.  */
+#ifndef USE_WIN32API
+  signal (SIGIO, input_interrupt);
+#endif
 }
 
 /* Internal buffer used by readchar.