[PATCH] Attach discard_pending_stop_replies to observer inferior_exit

[PATCH] Attach discard_pending_stop_replies to observer inferior_exit

[Yao Qi]

Hi,
discard_pending_stop_replies is called in three places, remote_detach_1,
extended_remote_mourn_1 (/w param pid) and remote_close (/w param -1).
They will finally call observer_notify_inferior_exit in some way,

remote_detach_1 >remote_mourn_1 >generic_mourn_inferior >exit_inferior >observer_notify_inferior_exit
        extended_remote_mourn_1 >generic_mourn_inferior >exit_inferior >observer_notify_inferior_exit
remote_close >discard_all_inferiors >exit_inferior_silent >observer_notify_inferior_exit

so this inspires me that we can attach discard_pending_stop_replies
to inferior_exit observer, and don't have to worry about the case that
PID is -1.  This is what this patch does.  Regression tested on
x86_64-linux with native and gdbserver.  Is it OK?

gdb:

2012-12-07  Yao Qi  <yao@codesourcery.com>

	* remote.c (discard_pending_stop_replies): Remove declaration.
	(remote_close): Remove call discard_pending_stop_replies.
	(remote_detach_1, extended_remote_mourn_1): Likewise.
	(discard_pending_stop_replies): Change parameter from PID to
	INF.
	(_initialize_remote): Attach discard_pending_stop_replies to
	inferior_exit observer.
---
 gdb/remote.c |   22 +++++++---------------
 1 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 069f294..6d2f431 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -229,7 +229,6 @@ static void do_stop_reply_xfree (void *arg);
 static void remote_parse_stop_reply (char *buf, struct stop_reply *);
 static void push_stop_reply (struct stop_reply *);
 static void remote_get_pending_stop_replies (void);
-static void discard_pending_stop_replies (int pid);
 static int peek_stop_reply (ptid_t ptid);
 
 static void remote_async_inferior_event_handler (gdb_client_data);
@@ -3022,9 +3021,6 @@ remote_close (int quitting)
   inferior_ptid = null_ptid;
   discard_all_inferiors ();
 
-  /* We're no longer interested in any of these events.  */
-  discard_pending_stop_replies (-1);
-
   if (remote_async_inferior_event_token)
     delete_async_event_handler (&remote_async_inferior_event_token);
   if (remote_async_get_pending_events_token)
@@ -4351,7 +4347,6 @@ remote_detach_1 (char *args, int from_tty, int extended)
   if (from_tty && !extended)
     puts_filtered (_("Ending remote debugging.\n"));
 
-  discard_pending_stop_replies (pid);
   target_mourn_inferior ();
 }
 
@@ -5159,18 +5154,16 @@ stop_reply_xfree (struct stop_reply *r)
     }
 }
 
-/* Discard all pending stop replies of inferior PID.  If PID is -1,
-   discard everything.  */
+/* Discard all pending stop replies of inferior INF.  */
 
 static void
-discard_pending_stop_replies (int pid)
+discard_pending_stop_replies (struct inferior *inf)
 {
   struct stop_reply *prev = NULL, *reply, *next;
 
   /* Discard the in-flight notification.  */
   if (pending_stop_reply != NULL
-      && (pid == -1
-	  || ptid_get_pid (pending_stop_reply->ptid) == pid))
+      && ptid_get_pid (pending_stop_reply->ptid) == inf->pid)
     {
       stop_reply_xfree (pending_stop_reply);
       pending_stop_reply = NULL;
@@ -5181,8 +5174,7 @@ discard_pending_stop_replies (int pid)
   for (reply = stop_reply_queue; reply; reply = next)
     {
       next = reply->next;
-      if (pid == -1
-	  || ptid_get_pid (reply->ptid) == pid)
+      if (ptid_get_pid (reply->ptid) == inf->pid)
 	{
 	  if (reply == stop_reply_queue)
 	    stop_reply_queue = reply->next;
@@ -7656,9 +7648,6 @@ extended_remote_mourn_1 (struct target_ops *target)
      connected.  */
   rs->waiting_for_stop_reply = 0;
 
-  /* We're no longer interested in these events.  */
-  discard_pending_stop_replies (ptid_get_pid (inferior_ptid));
-
   /* If the current general thread belonged to the process we just
      detached from or has exited, the remote side current general
      thread becomes undefined.  Considering a case like this:
@@ -11335,6 +11324,9 @@ _initialize_remote (void)
 
   /* Hook into new objfile notification.  */
   observer_attach_new_objfile (remote_new_objfile);
+  /* We're no longer interested in notification events of an inferior
+     when it exits.  */
+  observer_attach_inferior_exit (discard_pending_stop_replies);
 
   /* Set up signal handlers.  */
   sigint_remote_token =
-- 
1.7.7.6

[ping]: [PATCH] Attach discard_pending_stop_replies to observer inferior_exit

[Yao Qi]

On 12/07/2012 10:50 AM, Yao Qi wrote:
> Hi,
> discard_pending_stop_replies is called in three places, remote_detach_1,
> extended_remote_mourn_1 (/w param pid) and remote_close (/w param -1).
> They will finally call observer_notify_inferior_exit in some way,
>
> remote_detach_1 >remote_mourn_1 >generic_mourn_inferior >exit_inferior >observer_notify_inferior_exit
>          extended_remote_mourn_1 >generic_mourn_inferior >exit_inferior >observer_notify_inferior_exit
> remote_close >discard_all_inferiors >exit_inferior_silent >observer_notify_inferior_exit
>
> so this inspires me that we can attach discard_pending_stop_replies
> to inferior_exit observer, and don't have to worry about the case that
> PID is -1.  This is what this patch does.  Regression tested on
> x86_64-linux with native and gdbserver.  Is it OK?
>
> gdb:
>
> 2012-12-07  Yao Qi  <yao@codesourcery.com>
>
> 	* remote.c (discard_pending_stop_replies): Remove declaration.
> 	(remote_close): Remove call discard_pending_stop_replies.
> 	(remote_detach_1, extended_remote_mourn_1): Likewise.
> 	(discard_pending_stop_replies): Change parameter from PID to
> 	INF.
> 	(_initialize_remote): Attach discard_pending_stop_replies to
> 	inferior_exit observer.

Ping.  http://sourceware.org/ml/gdb-patches/2012-12/msg00126.html

This patch is kind of a cleanup/preparatory patch for 'async remote 
notification' patch series.  This patch uses 'inferior_exit' observer to 
discard pending stop replies, which is better than the current approach 
(call discard_pending_stop_replies in different places).

-- 
Yao (齐尧)

Re: [PATCH] Attach discard_pending_stop_replies to observer inferior_exit

[Pedro Alves]

On 12/07/2012 02:50 AM, Yao Qi wrote:
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -229,7 +229,6 @@ static void do_stop_reply_xfree (void *arg);
>  static void remote_parse_stop_reply (char *buf, struct stop_reply *);
>  static void push_stop_reply (struct stop_reply *);
>  static void remote_get_pending_stop_replies (void);
> -static void discard_pending_stop_replies (int pid);
>  static int peek_stop_reply (ptid_t ptid);
>  
>  static void remote_async_inferior_event_handler (gdb_client_data);
> @@ -3022,9 +3021,6 @@ remote_close (int quitting)
>    inferior_ptid = null_ptid;
>    discard_all_inferiors ();
>  
> -  /* We're no longer interested in any of these events.  */
> -  discard_pending_stop_replies (-1);

GDB can be learning about new processes it didn't know about yet
from some yet-unprocessed stop reply.  So if we only clear stop replies
specific to each inferior, we'd leave those behind.  But, we're closing
the remote target here.  We really want to discard everything.

-- 
Pedro Alves

Re: [PATCH] Attach discard_pending_stop_replies to observer inferior_exit

[Yao Qi]

On 12/14/2012 05:53 PM, Pedro Alves wrote:
> GDB can be learning about new processes it didn't know about yet
> from some yet-unprocessed stop reply.  So if we only clear stop replies
> specific to each inferior, we'd leave those behind.  But, we're closing
> the remote target here.  We really want to discard everything.

I see, then I change 'discard_pending_stop_replies' a little that INF
is NULL means discard everything.  This patch is tested on x86_64-linux
with native and gdbserver.  OK?

-- 
Yao (齐尧)

gdb:

2012-12-14  Yao Qi  <yao@codesourcery.com>

	* remote.c (discard_pending_stop_replies): Update declaration.
	(remote_detach_1, extended_remote_mourn_1): Likewise.
	(discard_pending_stop_replies): Change parameter from PID to
	INF.
	(remote_close): Update caller.
	(_initialize_remote): Attach discard_pending_stop_replies to
	inferior_exit observer.
---
 gdb/remote.c |   28 +++++++++++++++-------------
 1 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 069f294..673938d 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -229,7 +229,7 @@ static void do_stop_reply_xfree (void *arg);
 static void remote_parse_stop_reply (char *buf, struct stop_reply *);
 static void push_stop_reply (struct stop_reply *);
 static void remote_get_pending_stop_replies (void);
-static void discard_pending_stop_replies (int pid);
+static void discard_pending_stop_replies (struct inferior *);
 static int peek_stop_reply (ptid_t ptid);
 
 static void remote_async_inferior_event_handler (gdb_client_data);
@@ -3022,8 +3022,11 @@ remote_close (int quitting)
   inferior_ptid = null_ptid;
   discard_all_inferiors ();
 
-  /* We're no longer interested in any of these events.  */
-  discard_pending_stop_replies (-1);
+  /* Stop replies may from inferiors which are still unknown to GDB.
+     We are closing the remote target, so we should discard
+     everything, including the stop replies from GDB-unknown
+     inferiors.  */
+  discard_pending_stop_replies (NULL);
 
   if (remote_async_inferior_event_token)
     delete_async_event_handler (&remote_async_inferior_event_token);
@@ -4351,7 +4354,6 @@ remote_detach_1 (char *args, int from_tty, int extended)
   if (from_tty && !extended)
     puts_filtered (_("Ending remote debugging.\n"));
 
-  discard_pending_stop_replies (pid);
   target_mourn_inferior ();
 }
 
@@ -5159,18 +5161,18 @@ stop_reply_xfree (struct stop_reply *r)
     }
 }
 
-/* Discard all pending stop replies of inferior PID.  If PID is -1,
+/* Discard all pending stop replies of inferior INF.  If INF is NULL,
    discard everything.  */
 
 static void
-discard_pending_stop_replies (int pid)
+discard_pending_stop_replies (struct inferior *inf)
 {
   struct stop_reply *prev = NULL, *reply, *next;
 
   /* Discard the in-flight notification.  */
   if (pending_stop_reply != NULL
-      && (pid == -1
-	  || ptid_get_pid (pending_stop_reply->ptid) == pid))
+      && (inf == NULL
+	  || ptid_get_pid (pending_stop_reply->ptid) == inf->pid))
     {
       stop_reply_xfree (pending_stop_reply);
       pending_stop_reply = NULL;
@@ -5181,8 +5183,8 @@ discard_pending_stop_replies (int pid)
   for (reply = stop_reply_queue; reply; reply = next)
     {
       next = reply->next;
-      if (pid == -1
-	  || ptid_get_pid (reply->ptid) == pid)
+      if (inf == NULL
+	  || ptid_get_pid (reply->ptid) == inf->pid)
 	{
 	  if (reply == stop_reply_queue)
 	    stop_reply_queue = reply->next;
@@ -7656,9 +7658,6 @@ extended_remote_mourn_1 (struct target_ops *target)
      connected.  */
   rs->waiting_for_stop_reply = 0;
 
-  /* We're no longer interested in these events.  */
-  discard_pending_stop_replies (ptid_get_pid (inferior_ptid));
-
   /* If the current general thread belonged to the process we just
      detached from or has exited, the remote side current general
      thread becomes undefined.  Considering a case like this:
@@ -11335,6 +11334,9 @@ _initialize_remote (void)
 
   /* Hook into new objfile notification.  */
   observer_attach_new_objfile (remote_new_objfile);
+  /* We're no longer interested in notification events of an inferior
+     when it exits.  */
+  observer_attach_inferior_exit (discard_pending_stop_replies);
 
   /* Set up signal handlers.  */
   sigint_remote_token =
-- 
1.7.7.6

Re: [PATCH] Attach discard_pending_stop_replies to observer inferior_exit

[Pedro Alves]

On 12/14/2012 01:29 PM, Yao Qi wrote:
> On 12/14/2012 05:53 PM, Pedro Alves wrote:
>> GDB can be learning about new processes it didn't know about yet
>> from some yet-unprocessed stop reply.  So if we only clear stop replies
>> specific to each inferior, we'd leave those behind.  But, we're closing
>> the remote target here.  We really want to discard everything.
> 
> I see, then I change 'discard_pending_stop_replies' a little that INF
> is NULL means discard everything.  This patch is tested on x86_64-linux
> with native and gdbserver.  OK?

OK.

Thanks,
-- 
Pedro Alves