[binutils-gdb] gdb/remote-fileio.c: Eliminate custom SIGINT signal handler

[Pedro Alves <palves at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=bb7c96deb1a14ef7e8b51d5339a65a8064515c78

commit bb7c96deb1a14ef7e8b51d5339a65a8064515c78
Author: Pedro Alves <palves@redhat.com>
Date:   Wed Jun 1 16:34:49 2016 +0100

    gdb/remote-fileio.c: Eliminate custom SIGINT signal handler
    
    ... and fix Ctrl-C races.
    
    The current remote-fileio.c SIGINT/EINTR code can lose Ctrl-C --
    there's a period where SIG_IGN is installed as signal handler, for
    example.
    
    Since:
    
     - remote.c no longer installs a custom SIGINT handler;
    
     - The current remote-fileio.c SIGINT handler is basically the same as
       the default SIGINT handler (event-top.c:handle_sigint), in
       principle, except that instead of setting the quit flag, it sets a
       separate flag.
    
    I think we should be able to completely remove the remote-fileio.c
    SIGINT handler, and centralize on the quit flag, thus fixing the
    Ctrl-C race.
    
    gdb/ChangeLog:
    yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
    
    	* remote-fileio.c (remote_fio_ctrl_c_flag, remote_fio_sa)
    	(remote_fio_osa)
    	(remote_fio_ofunc, remote_fileio_sig_init, remote_fileio_sig_set)
    	(remote_fileio_sig_exit, remote_fileio_ctrl_c_signal_handler):
    	Delete.
    	(remote_fileio_o_quit_handler): New global.
    	(remote_fileio_quit_handler): New function.
    	(remote_fileio_reply): Check the quit flag instead of the custom
    	'remote_fio_ctrl_c_flag' flag.  Restore the quit handler instead
    	of changing the SIGINT handler.
    	(do_remote_fileio_request): Override the quit handler instead of
    	changing the SIGINT handler.

Diff:
---
 gdb/ChangeLog       | 15 ++++++++++
 gdb/remote-fileio.c | 80 ++++++++++++++---------------------------------------
 2 files changed, 35 insertions(+), 60 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9d08c67..18261a7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,18 @@
+2016-06-01  Pedro Alves  <palves@redhat.com>
+
+	* remote-fileio.c (remote_fio_ctrl_c_flag, remote_fio_sa)
+	(remote_fio_osa)
+	(remote_fio_ofunc, remote_fileio_sig_init, remote_fileio_sig_set)
+	(remote_fileio_sig_exit, remote_fileio_ctrl_c_signal_handler):
+	Delete.
+	(remote_fileio_o_quit_handler): New global.
+	(remote_fileio_quit_handler): New function.
+	(remote_fileio_reply): Check the quit flag instead of the custom
+	'remote_fio_ctrl_c_flag' flag.  Restore the quit handler instead
+	of changing the SIGINT handler.
+	(do_remote_fileio_request): Override the quit handler instead of
+	changing the SIGINT handler.
+
 2016-06-01  Nick Clifton  <nickc@redhat.com>
 
 	* common/common-utils.c (xmalloc_failed): New function.  Provided
diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c
index 29c5ca3..93121aa 100644
--- a/gdb/remote-fileio.c
+++ b/gdb/remote-fileio.c
@@ -298,66 +298,25 @@ remote_fileio_to_fio_timeval (struct timeval *tv, struct fio_timeval *ftv)
   remote_fileio_to_fio_long (tv->tv_usec, ftv->ftv_usec);
 }
 
-static int remote_fio_ctrl_c_flag = 0;
+/* The quit handler originally installed.  */
+static quit_handler_ftype *remote_fileio_o_quit_handler;
 
-#if defined (HAVE_SIGACTION) && defined (SA_RESTART)
-static struct sigaction remote_fio_sa;
-static struct sigaction remote_fio_osa;
-#else
-static void (*remote_fio_ofunc)(int);
-#endif
+/* What to do on a QUIT call while handling a file I/O request.  We
+   throw a quit exception, which is caught by remote_fileio_request
+   and translated to an EINTR reply back to the target.  */
 
 static void
-remote_fileio_sig_init (void)
+remote_fileio_quit_handler (void)
 {
-#if defined (HAVE_SIGACTION) && defined (SA_RESTART)
-  remote_fio_sa.sa_handler = SIG_IGN;
-  sigemptyset (&remote_fio_sa.sa_mask);
-  remote_fio_sa.sa_flags = 0;
-  sigaction (SIGINT, &remote_fio_sa, &remote_fio_osa);
-#else
-  remote_fio_ofunc = signal (SIGINT, SIG_IGN);
-#endif
-}
-
-static void
-remote_fileio_sig_set (void (*sigint_func)(int))
-{
-#if defined (HAVE_SIGACTION) && defined (SA_RESTART)
-  remote_fio_sa.sa_handler = sigint_func;
-  sigemptyset (&remote_fio_sa.sa_mask);
-  remote_fio_sa.sa_flags = 0;
-  sigaction (SIGINT, &remote_fio_sa, NULL);
-#else
-  signal (SIGINT, sigint_func);
-#endif
-}
-
-static void
-remote_fileio_sig_exit (void)
-{
-#if defined (HAVE_SIGACTION) && defined (SA_RESTART)
-  sigaction (SIGINT, &remote_fio_osa, NULL);
-#else
-  signal (SIGINT, remote_fio_ofunc);
-#endif
-}
-
-static void
-remote_fileio_ctrl_c_signal_handler (int signo)
-{
-  remote_fileio_sig_set (remote_fileio_ctrl_c_signal_handler);
-  remote_fio_ctrl_c_flag = 1;
-  /* Wake up interruptible_select.  */
-  quit_serial_event_set ();
+  quit ();
 }
 
 static void
 remote_fileio_reply (int retcode, int error)
 {
   char buf[32];
+  int ctrl_c = check_quit_flag ();
 
-  remote_fileio_sig_set (SIG_IGN);
   strcpy (buf, "F");
   if (retcode < 0)
     {
@@ -365,9 +324,9 @@ remote_fileio_reply (int retcode, int error)
       retcode = -retcode;
     }
   sprintf (buf + strlen (buf), "%x", retcode);
-  if (error || remote_fio_ctrl_c_flag)
+  if (error || ctrl_c)
     {
-      if (error && remote_fio_ctrl_c_flag)
+      if (error && ctrl_c)
         error = FILEIO_EINTR;
       if (error < 0)
         {
@@ -375,11 +334,10 @@ remote_fileio_reply (int retcode, int error)
 	  error = -error;
 	}
       sprintf (buf + strlen (buf), ",%x", error);
-      if (remote_fio_ctrl_c_flag)
+      if (ctrl_c)
         strcat (buf, ",C");
     }
-  remote_fio_ctrl_c_flag = 0;
-  remote_fileio_sig_set (remote_fileio_ctrl_c_signal_handler);
+  quit_handler = remote_fileio_o_quit_handler;
   putpkt (buf);
 }
 
@@ -1166,7 +1124,7 @@ do_remote_fileio_request (struct ui_out *uiout, void *buf_arg)
   char *c;
   int idx;
 
-  remote_fileio_sig_set (remote_fileio_ctrl_c_signal_handler);
+  quit_handler = remote_fileio_quit_handler;
 
   c = strchr (++buf, ',');
   if (c)
@@ -1213,20 +1171,22 @@ remote_fileio_request (char *buf, int ctrlc_pending_p)
 {
   int ex;
 
-  remote_fileio_sig_init ();
+  /* Save the previous quit handler, so we can restore it.  No need
+     for a cleanup since we catch all exceptions below.  Note that the
+     quit handler is also restored by remote_fileio_reply just before
+     pushing a packet.  */
+  remote_fileio_o_quit_handler = quit_handler;
 
   if (ctrlc_pending_p)
     {
       /* If the target hasn't responded to the Ctrl-C sent
 	 asynchronously earlier, take this opportunity to send the
 	 Ctrl-C synchronously.  */
-      remote_fio_ctrl_c_flag = 1;
+      set_quit_flag ();
       remote_fileio_reply (-1, FILEIO_EINTR);
     }
   else
     {
-      remote_fio_ctrl_c_flag = 0;
-
       ex = catch_exceptions (current_uiout,
 			     do_remote_fileio_request, (void *)buf,
 			     RETURN_MASK_ALL);
@@ -1243,7 +1203,7 @@ remote_fileio_request (char *buf, int ctrlc_pending_p)
 	}
     }
 
-  remote_fileio_sig_exit ();
+  quit_handler = remote_fileio_o_quit_handler;
 }