This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 2/7] de-couple %Stop from notification: gdbserver
- From: Pedro Alves <palves at redhat dot com>
- To: Yao Qi <yao at codesourcery dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Thu, 29 Nov 2012 18:40:11 +0000
- Subject: Re: [PATCH 2/7] de-couple %Stop from notification: gdbserver
- References: <1350991620-12950-1-git-send-email-yao@codesourcery.com> <1350991620-12950-3-git-send-email-yao@codesourcery.com>
On 10/23/2012 12:26 PM, Yao Qi wrote:
> Hi,
> I give some explanations/descriptions in the comment in
> gdbserver/notif.c, so I don't write the same words down here.
>
> gdb/gdbserver:
>
> 2012-10-23 Yao Qi <yao@codesourcery.com>
>
> * Makefile.in (OBS): Add notif.o.
> (notif_h, queue_h): New.
> (notif.o): New rule.
>
> * linux-low.c: Include "notif.h".
> (linux_wait): Put notif_stop into queue.
> * notif.c, notif.h: New.
> * server.c: Include 'notif.h'.
> (struct vstop_notif): Move it to notif.h.
> (notif_reply_stop): New.
> (notif_stop): New variable.
> (push_event, notif_queue): Remove.
> (vstop_notif_match_pid, notif_queued_replies_discard): New.
> (notif_queued_replies_discard_all): New.
> (discard_queued_stop_replies, send_next_stop_reply): Remove.
> (handle_v_kill): Don't call discard_queued_stop_replies. Call
> notif_queued_replies_discard_all instead.
> (handle_v_stopped): Remove.
> (handle_v_requests): Don't call handle_v_stopped. Call
> handle_ack_notif instead.
> (queue_stop_reply_callback): Call notif_reply_enqueue instead
> of queue_stop_reply.
> (handle_status): Don't call discard_queued_stop_replies. Call
> notif_queued_replies_discard instead.
> (kill_inferior_callback): Likewise.
> (detach_or_kill_inferior_callback): Likewise.
> (main): Call initialize_notif.
> (process_serial_event): Likewise. Call
> notif_queued_replies_empty_p.
> (handle_target_event): Process queued replies.
> * server.h: Remove declaration of push_event.
> ---
> gdb/gdbserver/Makefile.in | 5 +-
> gdb/gdbserver/linux-low.c | 1 +
> gdb/gdbserver/notif.c | 237 +++++++++++++++++++++++++++++++++++++++++++++
> gdb/gdbserver/notif.h | 83 ++++++++++++++++
> gdb/gdbserver/server.c | 180 ++++++----------------------------
> gdb/gdbserver/server.h | 2 -
> 6 files changed, 356 insertions(+), 152 deletions(-)
> create mode 100644 gdb/gdbserver/notif.c
> create mode 100644 gdb/gdbserver/notif.h
>
> diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
> index 489cf35..a3283ef 100644
> --- a/gdb/gdbserver/Makefile.in
> +++ b/gdb/gdbserver/Makefile.in
> @@ -156,7 +156,7 @@ OBS = agent.o ax.o inferiors.o regcache.o remote-utils.o server.o signals.o targ
> utils.o version.o vec.o gdb_vecs.o \
> mem-break.o hostio.o event-loop.o tracepoint.o \
> xml-utils.o common-utils.o ptid.o buffer.o format.o \
> - dll.o \
> + dll.o notif.o \
> $(XML_BUILTIN) \
> $(DEPFILES) $(LIBOBJS)
> GDBREPLAY_OBS = gdbreplay.o version.o
> @@ -407,6 +407,7 @@ gdb_proc_service_h = $(srcdir)/gdb_proc_service.h
> regdat_sh = $(srcdir)/../regformats/regdat.sh
> regdef_h = $(srcdir)/../regformats/regdef.h
> regcache_h = $(srcdir)/regcache.h
> +queue_h = $(srcdir)/../common/queue.h
> signals_def = $(srcdir)/../../include/gdb/signals.def
> signals_h = $(srcdir)/../../include/gdb/signals.h $(signals_def)
> ptid_h = $(srcdir)/../common/ptid.h
> @@ -431,6 +432,7 @@ server_h = $(srcdir)/server.h $(regcache_h) $(srcdir)/target.h \
> $(libiberty_h) \
> $(srcdir)/../../include/ansidecl.h \
> $(generated_files)
> +notif_h = ${srcdir}/notif.h
>
> gdbthread_h = $(srcdir)/gdbthread.h $(target_h) $(srcdir)/server.h
> linux_low_h = $(srcdir)/linux-low.h $(gdbthread_h)
> @@ -488,6 +490,7 @@ proc-service.o: proc-service.c $(server_h) $(gdb_proc_service_h)
> regcache.o: regcache.c $(server_h) $(regdef_h) $(gdbthread_h)
> remote-utils.o: remote-utils.c terminal.h $(server_h) $(gdbthread_h)
> server.o: server.c $(server_h) $(agent_h) $(gdbthread_h)
> +notif.o: notif.c $(notif_h) $(queue_h)
> target.o: target.c $(server_h)
> thread-db.o: thread-db.c $(server_h) $(linux_low_h) $(gdb_proc_service_h) \
> $(gdb_thread_db_h) $(gdb_vecs_h)
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index a476031..7c82eab 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -20,6 +20,7 @@
> #include "linux-low.h"
> #include "linux-osdata.h"
> #include "agent.h"
> +#include "notif.h"
>
> #include <sys/wait.h>
> #include <stdio.h>
> diff --git a/gdb/gdbserver/notif.c b/gdb/gdbserver/notif.c
> new file mode 100644
> index 0000000..eede13e
> --- /dev/null
> +++ b/gdb/gdbserver/notif.c
> @@ -0,0 +1,237 @@
> +/* Notification to GDB.
> + Copyright (C) 1989, 1993-1995, 1997-2000, 2002-2012 Free Software
> + Foundation, Inc.
> +
> + This file is part of GDB.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +/* Async notifications to GDB. When the state of remote target is
> + changed or something interesting to GDB happened, async notifications
> + are used to tell GDB.
> +
> + Each type of notification is represented by an object 'struct notif',
> + in which there is a queue for replies to GDB represented by
> + 'struct notif_reply'. See more in the comments to field 'queue' of
> + 'struct notif'.
> +
> + Here is the workflow of sending replies and managing queue:
> + 1. At any time, when something interesting FOO happens, a object of
> + 'struct notif_reply' or its sub-class REPLY is created for FOO,
> + immediately or shortly (when the information about this reply is
> + complete, see more details below).
> + 2. Enque REPLY to the notification object corresponding to FOO,
> + and send corresponding notification packet to GDB if REPLY is the
> + first reply.
> + #1 and #2 are done by function 'notif_process'.
> +
> + 3. REPLY is not deque'ed until the ack of FOO from GDB arrives,
> + and continue to send replies to GDB if queue is not empty.
> + Before ack arrives, FOO happens, a new object of REPLY is created,
> + and enque REPLY silently.
> + # 3 is done by function 'handle_notif_ack'.
> +
> + The timing of creating REPLY for FOO varies on different types of
> + notifications, nowadays, there are two kinds:
> + 1. The information of notification FOO is ready when FOO happens.
> + Most of the notifications are of this kind.
> + 2. The information of notification FOO is not ready or complete
> + when FOO happens. Only %Stop notification is of this kind, because
> + GDBserver has to call 'mywait' to get the waitstatus which is needed.
I don't understand the implications of this distinction. This patch doesn't
appear to make it anywhere?
> +
> + When add a new type of notification, we should be clear on which
> + kind of this new notification belong to. */
> +
> +#include "notif.h"
> +
> +static struct notif *notif_packets[] =
> +{
> + ¬if_stop,
> + NULL,
> +};
> +
> +DEFINE_QUEUE_P (notif_reply_p);
> +
> +/* Push another reply, or if there are no more left, an OK. */
> +
> +void
> +notif_send_reply (struct notif *notif, char *own_buf)
> +{
> + if (!QUEUE_is_empty (notif_reply_p, notif->queue))
> + {
> + struct notif_reply *reply = QUEUE_peek (notif_reply_p, notif->queue);
> +
> + notif->reply (reply, own_buf);
> + }
> + else
> + write_ok (own_buf);
> +}
> +
> +/* Handle the ack in buffer OWN_BUF. Return 1 if the ack is handled,
> + and return 0 if the contents in OWN_BUF is not a ack. */
> +
> +int
> +handle_notif_ack (char *own_buf)
> +{
> + int i = 0;
> + struct notif *np = NULL;
> +
> + for (i = 0; notif_packets[i] != NULL; i++)
> + {
> + np = notif_packets[i];
> + if (strncmp (own_buf, np->name, strlen (np->name)) == 0)
> + break;
This should make sure an unrelated packet with the same prefix never
passes through my mistake.
> + }
> +
> + if (np == NULL)
> + return 0;
> +
> + /* If we're waiting for GDB to acknowledge a pending reply,
> + consider that done. */
> + if (!QUEUE_is_empty (notif_reply_p, np->queue))
> + {
> + struct notif_reply *head = QUEUE_deque (notif_reply_p, np->queue);
> +
> + if (remote_debug)
> + fprintf (stderr, "%s: acking %d\n", np->name,
> + QUEUE_length (notif_reply_p, np->queue));
> +
> + xfree (head);
> + }
> +
> + notif_send_reply (np, own_buf);
> +
> + return 1;
> +}
> +
> +void
> +notif_reply_enque (struct notif *notif, struct notif_reply *reply)
> +{
> + QUEUE_enque (notif_reply_p, notif->queue, reply);
> +
> + if (remote_debug)
> + fprintf (stderr, "pending replies: %s %d\n", notif->notif_name,
> + QUEUE_length (notif_reply_p, notif->queue));
> +
> +}
> +
> +/* Process one notification NP of a thread identified by PTID. */
> +
> +void
> +notif_process (struct notif *np, ptid_t ptid, void *data)
> +{
> + struct notif_reply *new_notif = NULL;
> + int is_first_event = 0;
No need to initialize is_first_event.
> +
> + switch (np->type)
> + {
> + case NOTIF_STOP:
> + {
> + struct vstop_notif *vstop_notif
> + = xmalloc (sizeof (struct vstop_notif));
> +
> + vstop_notif->status = *(struct target_waitstatus *) data;
> + new_notif = (struct notif_reply *) vstop_notif;
> + }
> + break;
> + default:
> + error ("Unknown notification type");
> + }
> +
> + new_notif->ptid = ptid;
> + is_first_event = QUEUE_is_empty (notif_reply_p, np->queue);
> +
> + /* Something interesting. Tell GDB about it. */
> + notif_reply_enque (np, new_notif);
> +
> + /* If this is the first stop reply in the queue, then inform GDB
> + about it, by sending a corresponding notification. */
> + if (is_first_event)
> + {
> + char buf[PBUFSIZ];
> + char *p = buf;
> +
> + xsnprintf (p, PBUFSIZ, "%s:", np->notif_name);
> + p += strlen (p);
> +
> + np->reply (new_notif, p);
> + putpkt_notif (buf);
> + }
> +}
> +
> +/* Return true if the queues of all notifications are empty. If the
> + queue of one of notification is not empty, return false. */
> +
> +int
> +notif_queued_replies_empty_p (void)
> +{
> + int i;
> +
> + for (i = 0; notif_packets[i] != NULL; i++)
> + if (!QUEUE_is_empty (notif_reply_p, notif_packets[i]->queue))
> + return 0;
> +
> + return 1;
> +}
> +
> +static int
> +always_remove_on_match_pid (QUEUE (notif_reply_p) *q,
> + QUEUE_ITER (notif_reply_p) *iter,
> + struct notif_reply *reply,
> + void *data)
> +{
> + int *pid = data;
> +
> + if (*pid == -1 || ptid_get_pid (reply->ptid) == *pid)
> + {
> + if (q->free_func != NULL)
> + q->free_func (reply);
> +
> + QUEUE_remove_elem (notif_reply_p, q, iter);
> + }
> +
> + return 1;
> +}
> +
> +/* Get rid of the currently pending replies of NOTIF for PID. If PID is
> + -1, then apply to all processes. */
> +
> +void
> +notif_queued_replies_discard (int pid, struct notif *notif)
> +{
> + QUEUE_iterate (notif_reply_p, notif->queue,
> + always_remove_on_match_pid, &pid);
> +}
> +
> +/* Get rid of pending replies of all notifications. */
> +
> +void
> +notif_queued_replies_discard_all (int pid)
> +{
> + int i;
> +
> + for (i = 0; notif_packets[i] != NULL; i++)
> + notif_queued_replies_discard (pid, notif_packets[i]);
> +}
> +
> +void
> +initialize_notif (void)
> +{
> + int i = 0;
> +
> + for (i = 0; notif_packets[i] != NULL; i++)
> + notif_packets[i]->queue
> + = QUEUE_alloc (notif_reply_p,
> + (void (*)(struct notif_reply *)) xfree);
Please don't cast xfree like that. See comment on make_cleanup_ftype.
It's the same issue.
> +}
> diff --git a/gdb/gdbserver/notif.h b/gdb/gdbserver/notif.h
> new file mode 100644
> index 0000000..bf45910
> --- /dev/null
> +++ b/gdb/gdbserver/notif.h
> @@ -0,0 +1,83 @@
> +/* Notification to GDB.
> + Copyright (C) 1989, 1993-1995, 1997-2000, 2002-2012 Free Software
> + Foundation, Inc.
> +
> + This file is part of GDB.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +#include "ptid.h"
> +#include "server.h"
> +#include "target.h"
> +#include "queue.h"
> +
> +/* Structure holding information relative to a single reply. We
> + keep a queue of these to push to GDB. */
> +
> +typedef struct notif_reply
> +{
> + /* Thread or process that got the event. */
> + ptid_t ptid;
Seems very odd to me that this is here, and instead of on
the subclass. The fact that stop replies are associated
with a ptid seems to me to be a detail of stop replies.
I think it's better to not store knowlege of specific notification
types in notif.c, but instead make the caller/client hold the struct
notif instances. Then if discarding stop replies for a given ptid
makes sense, that is handled by the client. It may not make sense
for some other notification.
> +} *notif_reply_p;
> +
> +/* A sub-class of 'struct ack_notif' for stop reply. */
> +
> +struct vstop_notif
> +{
> + struct notif_reply base;
> +
> + /* Event info. */
> + struct target_waitstatus status;
> +};
> +
> +enum notif_type { NOTIF_STOP };
> +
> +/* A type notification to GDB. An object of 'struct notif' represents
> + a type of notification. */
> +
> +typedef struct notif
> +{
> + /* The name of ack packet, for example, 'vStopped'. */
> + const char *name;
Please rename this to something more descriptive.
When I read "notif->name" I get confused on what "name" that might be.
> +
> + /* The notification packet, for example, '%Stop'. Note that '%' is
> + not in 'notif_name'. */
> + const char *notif_name;
> +
> + const enum notif_type type;
> +
> + /* A queue of replies to GDB. A new notif_reply can be enque'ed
> + into QUEUE at any appropriate time, and the notif_reply is deque'ed
> + only when the ack from GDB arrives. */
> + QUEUE (notif_reply_p) *queue;
> +
> + /* Reply to GDB. */
> + void (*reply) (struct notif_reply *reply, char *own_buf);
> +} *notif_p;
> +
> +extern struct notif notif_stop;
> +
> +int handle_notif_ack (char *own_buf);
> +void notif_send_reply (struct notif *notif, char *own_buf);
> +
> +int notif_queued_replies_empty_p (void);
> +void notif_queued_replies_discard (int pid, struct notif *notif);
> +void notif_queued_replies_discard_all (int pid);
> +
> +void notif_process (struct notif *np, ptid_t ptid, void *data);
> +void notif_reply_enque (struct notif *notif, struct notif_reply *reply);
> +
> +DECLARE_QUEUE_P (notif_reply_p);
> +
> +void initialize_notif (void);
> diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
> index 61a7313..43fafa6 100644
> --- a/gdb/gdbserver/server.c
> +++ b/gdb/gdbserver/server.c
> @@ -20,6 +20,7 @@
> #include "server.h"
> #include "gdbthread.h"
> #include "agent.h"
> +#include "notif.h"
>
> #if HAVE_UNISTD_H
> #include <unistd.h>
> @@ -117,122 +118,18 @@ static ptid_t last_ptid;
> static char *own_buf;
> static unsigned char *mem_buf;
>
> -/* Structure holding information relative to a single stop reply. We
> - keep a queue of these (really a singly-linked list) to push to GDB
> - in non-stop mode. */
> -struct vstop_notif
> -{
> - /* Pointer to next in list. */
> - struct vstop_notif *next;
> -
> - /* Thread or process that got the event. */
> - ptid_t ptid;
> -
> - /* Event info. */
> - struct target_waitstatus status;
> -};
> -
> -/* The pending stop replies list head. */
> -static struct vstop_notif *notif_queue = NULL;
> -
> -/* Put a stop reply to the stop reply queue. */
> -
> static void
> -queue_stop_reply (ptid_t ptid, struct target_waitstatus *status)
> -{
> - struct vstop_notif *new_notif;
> -
> - new_notif = xmalloc (sizeof (*new_notif));
> - new_notif->next = NULL;
> - new_notif->ptid = ptid;
> - new_notif->status = *status;
> -
> - if (notif_queue)
> - {
> - struct vstop_notif *tail;
> - for (tail = notif_queue;
> - tail && tail->next;
> - tail = tail->next)
> - ;
> - tail->next = new_notif;
> - }
> - else
> - notif_queue = new_notif;
> -
> - if (remote_debug)
> - {
> - int i = 0;
> - struct vstop_notif *n;
> -
> - for (n = notif_queue; n; n = n->next)
> - i++;
> -
> - fprintf (stderr, "pending stop replies: %d\n", i);
> - }
> -}
> -
> -/* Place an event in the stop reply queue, and push a notification if
> - we aren't sending one yet. */
> -
> -void
> -push_event (ptid_t ptid, struct target_waitstatus *status)
> +notif_reply_stop (struct notif_reply *reply, char *own_buf)
For subclassing-style callbacks, I'd rather follow a
namespace-style naming style:
struct vstop_notif + reply callback -> vstop_notif_reply.
> {
> - gdb_assert (status->kind != TARGET_WAITKIND_IGNORE);
> + struct vstop_notif *vstop = (struct vstop_notif *) reply;
>
> - queue_stop_reply (ptid, status);
> -
> - /* If this is the first stop reply in the queue, then inform GDB
> - about it, by sending a Stop notification. */
> - if (notif_queue->next == NULL)
> - {
> - char *p = own_buf;
> - strcpy (p, "Stop:");
> - p += strlen (p);
> - prepare_resume_reply (p,
> - notif_queue->ptid, ¬if_queue->status);
> - putpkt_notif (own_buf);
> - }
> + prepare_resume_reply (own_buf, reply->ptid, &vstop->status);
> }
>
> -/* Get rid of the currently pending stop replies for PID. If PID is
> - -1, then apply to all processes. */
> -
> -static void
> -discard_queued_stop_replies (int pid)
> +struct notif notif_stop =
> {
> - struct vstop_notif *prev = NULL, *reply, *next;
> -
> - for (reply = notif_queue; reply; reply = next)
> - {
> - next = reply->next;
> -
> - if (pid == -1
> - || ptid_get_pid (reply->ptid) == pid)
> - {
> - if (reply == notif_queue)
> - notif_queue = next;
> - else
> - prev->next = reply->next;
> -
> - free (reply);
> - }
> - else
> - prev = reply;
> - }
> -}
> -
> -/* If there are more stop replies to push, push one now. */
> -
> -static void
> -send_next_stop_reply (char *own_buf)
> -{
> - if (notif_queue)
> - prepare_resume_reply (own_buf,
> - notif_queue->ptid,
> - ¬if_queue->status);
> - else
> - write_ok (own_buf);
> -}
> + "vStopped", "Stop", NOTIF_STOP, NULL, notif_reply_stop,
> +};
>
> static int
> target_running (void)
> @@ -2160,7 +2057,8 @@ handle_v_kill (char *own_buf)
> last_status.kind = TARGET_WAITKIND_SIGNALLED;
> last_status.value.sig = GDB_SIGNAL_KILL;
> last_ptid = pid_to_ptid (pid);
> - discard_queued_stop_replies (pid);
> +
> + notif_queued_replies_discard_all (pid);
> write_ok (own_buf);
> return 1;
> }
> @@ -2171,29 +2069,6 @@ handle_v_kill (char *own_buf)
> }
> }
>
> -/* Handle a 'vStopped' packet. */
> -static void
> -handle_v_stopped (char *own_buf)
> -{
> - /* If we're waiting for GDB to acknowledge a pending stop reply,
> - consider that done. */
> - if (notif_queue)
> - {
> - struct vstop_notif *head;
> -
> - if (remote_debug)
> - fprintf (stderr, "vStopped: acking %s\n",
> - target_pid_to_str (notif_queue->ptid));
> -
> - head = notif_queue;
> - notif_queue = notif_queue->next;
> - free (head);
> - }
> -
> - /* Push another stop reply, or if there are no more left, an OK. */
> - send_next_stop_reply (own_buf);
> -}
> -
> /* Handle all of the extended 'v' packets. */
> void
> handle_v_requests (char *own_buf, int packet_len, int *new_packet_len)
> @@ -2254,11 +2129,8 @@ handle_v_requests (char *own_buf, int packet_len, int *new_packet_len)
> return;
> }
>
> - if (strncmp (own_buf, "vStopped", 8) == 0)
> - {
> - handle_v_stopped (own_buf);
> - return;
> - }
> + if (handle_notif_ack (own_buf))
> + return;
>
> /* Otherwise we didn't know what packet it was. Say we didn't
> understand it. */
> @@ -2340,14 +2212,20 @@ queue_stop_reply_callback (struct inferior_list_entry *entry, void *arg)
> manage the thread's last_status field. */
> if (the_target->thread_stopped == NULL)
> {
> + struct vstop_notif *new_notif = xmalloc (sizeof (*new_notif));
> +
> + new_notif->base.ptid = entry->id;
> + new_notif->status = thread->last_status;
> /* Pass the last stop reply back to GDB, but don't notify
> yet. */
> - queue_stop_reply (entry->id, &thread->last_status);
> + notif_reply_enque (¬if_stop, (struct notif_reply *) new_notif);
> }
> else
> {
> if (thread_stopped (thread))
> {
> + struct vstop_notif *new_notif = xmalloc (sizeof (*new_notif));
> +
> if (debug_threads)
> fprintf (stderr,
> "Reporting thread %s as already stopped with %s\n",
> @@ -2356,9 +2234,11 @@ queue_stop_reply_callback (struct inferior_list_entry *entry, void *arg)
>
> gdb_assert (thread->last_status.kind != TARGET_WAITKIND_IGNORE);
>
> + new_notif->base.ptid = entry->id;
> + new_notif->status = thread->last_status;
> /* Pass the last stop reply back to GDB, but don't notify
> yet. */
> - queue_stop_reply (entry->id, &thread->last_status);
> + notif_reply_enque (¬if_stop, (struct notif_reply *) new_notif);
Duplication alert. I'd rather keep the existing queue_stop_reply function,
and make _that_ do the new implementation details:
struct vstop_notif *new_notif = xmalloc (sizeof (*new_notif));
new_notif->base.ptid = entry->id;
new_notif->status = thread->last_status;
notif_reply_enque (¬if_stop, (struct notif_reply *) new_notif);
> }
> }
>
> @@ -2417,13 +2297,13 @@ handle_status (char *own_buf)
>
> if (non_stop)
> {
> - discard_queued_stop_replies (-1);
> + notif_queued_replies_discard (-1, ¬if_stop);
> find_inferior (&all_threads, queue_stop_reply_callback, NULL);
>
> /* The first is sent immediatly. OK is sent if there is no
> stopped thread, which is the same handling of the vStopped
> packet (by design). */
> - send_next_stop_reply (own_buf);
> + notif_send_reply (¬if_stop, own_buf);
> }
> else
> {
> @@ -2516,7 +2396,7 @@ kill_inferior_callback (struct inferior_list_entry *entry)
> int pid = ptid_get_pid (process->head.id);
>
> kill_inferior (pid);
> - discard_queued_stop_replies (pid);
> + notif_queued_replies_discard_all (pid);
> }
>
> /* Callback for for_each_inferior to detach or kill the inferior,
> @@ -2535,7 +2415,7 @@ detach_or_kill_inferior_callback (struct inferior_list_entry *entry)
> else
> kill_inferior (pid);
>
> - discard_queued_stop_replies (pid);
> + notif_queued_replies_discard_all (pid);
> }
>
> /* for_each_inferior callback for detach_or_kill_for_exit to print
> @@ -2792,6 +2672,8 @@ main (int argc, char *argv[])
> last_ptid = minus_one_ptid;
> }
>
> + initialize_notif ();
> +
> /* Don't report shared library events on the initial connection,
> even if some libraries are preloaded. Avoids the "stopped by
> shared library event" notice on gdb side. */
> @@ -3062,7 +2944,7 @@ process_serial_event (void)
> write_enn (own_buf);
> else
> {
> - discard_queued_stop_replies (pid);
> + notif_queued_replies_discard_all (pid);
> write_ok (own_buf);
>
> if (extended_protocol)
> @@ -3404,7 +3286,7 @@ process_serial_event (void)
> {
> /* In non-stop, defer exiting until GDB had a chance to query
> the whole vStopped list (until it gets an OK). */
> - if (!notif_queue)
> + if (notif_queued_replies_empty_p ())
> {
> fprintf (stderr, "GDBserver exiting\n");
> remote_close ();
> @@ -3502,8 +3384,8 @@ handle_target_event (int err, gdb_client_data client_data)
> }
> else
> {
> - /* Something interesting. Tell GDB about it. */
> - push_event (last_ptid, &last_status);
> + /* Process Stop notification. */
> + notif_process (¬if_stop, last_ptid, &last_status);
When I read this, "process" doesn't really help me understand what
the function does, push "push" did. We're not really processing any
existing notification, we're creating a new one. Could you rename this
to notif_push (possibly keep the existing function as wrapper)?
> }
> }
>
> diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
> index e93ad00..3bf72dc 100644
> --- a/gdb/gdbserver/server.h
> +++ b/gdb/gdbserver/server.h
> @@ -265,8 +265,6 @@ extern void start_event_loop (void);
> extern int handle_serial_event (int err, gdb_client_data client_data);
> extern int handle_target_event (int err, gdb_client_data client_data);
>
> -extern void push_event (ptid_t ptid, struct target_waitstatus *status);
> -
> /* Functions from hostio.c. */
> extern int handle_vFile (char *, int, int *);
>
>
--
Pedro Alves