[RFC] Target-defined breakpoints [3/9] notification async

[RFC] Target-defined breakpoints [3/9] notification async

[Hui Zhu]

[-- Attachment #1: Type: text/plain, Size: 1006 bytes --]

This patch is the extend for the notification function.
Current notification function cannot handle the packet when GDB doesn't want to send or receive packet from the remote target.
This patch do the extend make GDB can handle the notification even if it doesn't send or receive packet.

Thanks,
Hui

2012-10-24  Hui Zhu  <hui_zhu@mentor.com>

	* remote.c (async_client_callback): Move to the top.
	(async_client_context): Ditto.
	(remote_async_serial_handler): Ditto.
	(PACKET_NotificationAsync): New enum.
	(remote_pr): Add PACKET_NotificationAsync.
	(remote_open_1): Call serial_async if need.
	(readchar_buffer_ch): New variable.
	(readchar): If need, return the value of readchar_buffer_ch.
	(readchar_buffer_put): New function.
	(remote_is_async_p): Add check for PACKET_ReportAsync.
	(remote_is_notificationasync): New function.
	(remote_async_serial_handler): Add code to handle notification.
	(remote_async): Add check for PACKET_ReportAsync.
	(_initialize_remote): Add PACKET_NotificationAsync.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: notification_async.txt --]
[-- Type: text/plain; charset="us-ascii"; name="notification_async.txt", Size: 6709 bytes --]

---

---
 remote.c |  108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 98 insertions(+), 10 deletions(-)

--- a/remote.c
+++ b/remote.c
@@ -224,6 +224,11 @@ static void remote_query_supported (void
 
 static void remote_check_symbols (struct objfile *objfile);
 
+static void (*async_client_callback) (enum inferior_event_type event_type,
+				      void *context);
+static void *async_client_context;
+static serial_event_ftype remote_async_serial_handler;
+
 void _initialize_remote (void);
 
 struct stop_reply;
@@ -1297,6 +1302,7 @@ enum {
   PACKET_QDisableRandomization,
   PACKET_QAgent,
   PACKET_TargetDefinedBreakpoints,
+  PACKET_NotificationAsync,
   PACKET_MAX
 };
 
@@ -3999,6 +4005,8 @@ static struct protocol_feature remote_pr
     remote_string_tracing_feature, -1 },
   { "TargetDefinedBreakpoints", PACKET_DISABLE, remote_supported_packet,
     PACKET_TargetDefinedBreakpoints },
+  { "NotificationAsync", PACKET_DISABLE, remote_supported_packet,
+    PACKET_NotificationAsync },
 };
 
 static char *remote_support_xml;
@@ -4356,6 +4364,10 @@ remote_open_1 (char *name, int from_tty,
 
   if (target_async_permitted)
     wait_forever_enabled_p = 1;
+
+  if (remote_protocol_packets[PACKET_NotificationAsync].support
+	  != PACKET_DISABLE)
+    serial_async (remote_desc, remote_async_serial_handler, NULL);
 }
 
 /* This takes a program previously attached to and detaches it.  After
@@ -6981,11 +6993,20 @@ remote_files_info (struct target_ops *ig
 
 /* Read a single character from the remote end.  */
 
+static int readchar_buffer_ch = -1;
+
 static int
 readchar (int timeout)
 {
   int ch;
 
+  if (readchar_buffer_ch != -1)
+    {
+      ch = readchar_buffer_ch;
+      readchar_buffer_ch = -1;
+      return ch;
+    }
+
   ch = serial_readchar (remote_desc, timeout);
 
   if (ch >= 0)
@@ -6994,8 +7015,12 @@ readchar (int timeout)
   switch ((enum serial_rc) ch)
     {
     case SERIAL_EOF:
-      pop_target ();
-      error (_("Remote connection closed"));
+      if (timeout)
+        {
+	  pop_target ();
+	  error (_("Remote connection closed"));
+	}
+      break;
       /* no return */
     case SERIAL_ERROR:
       pop_target ();
@@ -7008,6 +7033,17 @@ readchar (int timeout)
   return ch;
 }
 
+/* When the function that call the readchar got a char is not for it
+   and other function need this char，call this function to put
+   this char back.  Then when other function call readchar, it will
+   got this char first. */
+
+static void
+readchar_buffer_put (int ch)
+{
+  readchar_buffer_ch = ch;
+}
+
 /* Send the command in *BUF to the remote machine, and read the reply
    into *BUF.  Report an error if we get an error reply.  Resize
    *BUF using xrealloc if necessary to hold the result, and update
@@ -7509,7 +7545,11 @@ getpkt_or_notif_sane_1 (char **buf, long
 	     show up within remote_timeout intervals.  */
 	  do
 	    c = readchar (timeout);
-	  while (c != SERIAL_TIMEOUT && c != '$' && c != '%');
+	  while (c != SERIAL_TIMEOUT && c != SERIAL_EOF && c != '$'
+		 && c != '%');
+
+	  if (c == SERIAL_EOF)
+	    return -1;
 
 	  if (c == SERIAL_TIMEOUT)
 	    {
@@ -11213,25 +11253,72 @@ remote_is_async_p (void)
     /* We only enable async when the user specifically asks for it.  */
     return 0;
 
+  if (remote_protocol_packets[PACKET_NotificationAsync].support
+	  != PACKET_DISABLE)
+    return async_client_callback != NULL;
+
   /* We're async whenever the serial device is.  */
   return serial_is_async_p (remote_desc);
 }
 
+static int
+remote_is_notificationasync (void)
+{
+  int c = serial_readchar (remote_desc, 0);
+
+  if (c == '%')
+    return 1;
+
+  if (c >= 0)
+    readchar_buffer_put (c);
+  return 0;
+}
+
 /* Pass the SERIAL event on and up to the client.  One day this code
    will be able to delay notifying the client of an event until the
    point where an entire packet has been received.  */
 
-static void (*async_client_callback) (enum inferior_event_type event_type,
-				      void *context);
-static void *async_client_context;
-static serial_event_ftype remote_async_serial_handler;
-
 static void
 remote_async_serial_handler (struct serial *scb, void *context)
 {
-  /* Don't propogate error information up to the client.  Instead let
-     the client find out about the error by querying the target.  */
-  async_client_callback (INF_REG_EVENT, async_client_context);
+  /* Check if this is ReportAsync.  */
+  if (remote_is_notificationasync ())
+    {
+      int val;
+      struct remote_state *rs = get_remote_state ();
+
+      val = read_frame (&rs->buf, &rs->buf_size);
+      if (val >= 0)
+	{
+	  if (remote_debug)
+	    {
+	      struct cleanup *old_chain;
+	      char *str;
+
+	      str = escape_buffer (rs->buf, val);
+	      old_chain = make_cleanup (xfree, str);
+	      fprintf_unfiltered (gdb_stdlog, "  Notification received: %s\n",
+				  str);
+	      do_cleanups (old_chain);
+	    }
+	  handle_notification (rs->buf);
+	}
+      else
+	{
+	  if (remote_debug)
+	    {
+	      fprintf_unfiltered (gdb_stdlog, "putpkt: Junk: ");
+	      fprintf_unfiltered (gdb_stdlog, "%%%s", rs->buf);
+	    }
+	}
+    }
+  else
+    {
+      /* Don't propogate error information up to the client.  Instead let
+         the client find out about the error by querying the target.  */
+      if (async_client_callback)
+	async_client_callback (INF_REG_EVENT, async_client_context);
+    }
 }
 
 static void
@@ -11252,12 +11339,19 @@ remote_async (void (*callback) (enum inf
 {
   if (callback != NULL)
     {
-      serial_async (remote_desc, remote_async_serial_handler, NULL);
+      if (remote_protocol_packets[PACKET_NotificationAsync].support
+	      == PACKET_DISABLE)
+        serial_async (remote_desc, remote_async_serial_handler, NULL);
       async_client_callback = callback;
       async_client_context = context;
     }
   else
-    serial_async (remote_desc, NULL, NULL);
+    {
+      if (remote_protocol_packets[PACKET_NotificationAsync].support
+	      == PACKET_DISABLE)
+	serial_async (remote_desc, NULL, NULL);
+      async_client_callback = NULL;
+    }
 }
 
 static void
@@ -11775,6 +11869,9 @@ Show the maximum size of the address (in
   add_packet_config_cmd (&remote_protocol_packets[PACKET_TargetDefinedBreakpoints],
 			 "TargetDefinedBreakpoints", "target-defined-breakpoints", 0);
 
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_NotificationAsync],
+			 "NotificationAsync", "notification-async", 0);
+
   /* Keep the old ``set remote Z-packet ...'' working.  Each individual
      Z sub-packet has its own set and show commands, but users may
      have sets to this variable in their .gdbinit files (or in their

Re: [RFC] Target-defined breakpoints [3/9] notification async

[Yao Qi]

On 10/24/2012 11:03 PM, Hui Zhu wrote:
> This patch is the extend for the notification function.
> Current notification function cannot handle the packet when GDB doesn't
> want to send or receive packet from the remote target.
> This patch do the extend make GDB can handle the notification even if it
> doesn't send or receive packet.

Looks we have two set of 'async notification' stuff here :)  I am trying 
to set up an infrastructure of 'async notification' in gdb, so that 
other components can use it.  Patches are here,

   http://sourceware.org/ml/gdb-patches/2012-10/msg00404.html
   [RFC 0/7 V3]A general notification in GDB RSP

It is general, not specific to any notifications, and scalable.  We've 
probably have two internal cases of using it, 'trace status change' and 
'stub/probe status change', so "Target-defined breakpoint" can use it 
easily as well.

Do you mind if I suggest maintainers to review my 'async notification' 
patches first?  Once they go in, we can rebase your patch on top of it. 
  I am sure your patch will be shorter dramatically then.  WDYT?  Or if 
you find my 'async notification' doesn't meet your needs, let me know.

-- 
Yao

Re: [RFC] Target-defined breakpoints [3/9] notification async

[Hui Zhu]

On Thu, Oct 25, 2012 at 12:38 AM, Yao Qi <yao@codesourcery.com> wrote:
> On 10/24/2012 11:03 PM, Hui Zhu wrote:
>>
>> This patch is the extend for the notification function.
>> Current notification function cannot handle the packet when GDB doesn't
>> want to send or receive packet from the remote target.
>> This patch do the extend make GDB can handle the notification even if it
>> doesn't send or receive packet.
>
>
> Looks we have two set of 'async notification' stuff here :)  I am trying to
> set up an infrastructure of 'async notification' in gdb, so that other
> components can use it.  Patches are here,
>
>   http://sourceware.org/ml/gdb-patches/2012-10/msg00404.html
>   [RFC 0/7 V3]A general notification in GDB RSP
>
> It is general, not specific to any notifications, and scalable.  We've
> probably have two internal cases of using it, 'trace status change' and
> 'stub/probe status change', so "Target-defined breakpoint" can use it easily
> as well.
>
> Do you mind if I suggest maintainers to review my 'async notification'
> patches first?  Once they go in, we can rebase your patch on top of it.  I
> am sure your patch will be shorter dramatically then.  WDYT?  Or if you find
> my 'async notification' doesn't meet your needs, let me know.
>
> --
> Yao

I don't mind change my patch according to your change if you can check in.

PS.  I think target-defind xxx patches still have some work (format
and handler, please read the 0/9)  not done.   So please don't worry
about it.

Best,
Hui