* [PATCH 2/8] de-couple %Stop from notification: gdbserver
2012-12-11 6:40 [PATCH 0/8, V4] A general notification in GDB RSP Yao Qi
@ 2012-12-11 6:40 ` Yao Qi
2012-12-12 17:06 ` Pedro Alves
2012-12-11 6:41 ` [PATCH 6/8] Handle notification for all-stop Yao Qi
` (6 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Yao Qi @ 2012-12-11 6:40 UTC (permalink / raw)
To: gdb-patches
Hi
This patch de-couples %Stop notification out of notification
infrastructure, so that adding new notification is easier.
See details in the header comment in gdbserver/notif.c.
This version (V4) is mostly to address Pedro's comments, to do some
renaming, for example, 'struct notif' -> 'struct notif_server',
'struct notif_reply' -> 'struct notif_event'. In V4, we also move
'ptid' field out of 'struct notif_event' into its sub-class 'struct
stop_reply'.
Note that this patch doesn't rename 'vstop_notif'. We can change its
name is a separate patch if we want.
gdb/gdbserver:
2012-12-11 Yao Qi <yao@codesourcery.com>
* Makefile.in (OBS): Add notif.o.
(notif_h, queue_h): New.
(notif.o): New rule.
* notif.c, notif.h: New.
* server.c: Include "notif.h".
(struct vstop_notif) <next>: Remove.
<base>: New field.
(queue_stop_reply): Update.
(push_event, send_next_stop_reply): Remove.
(discard_queued_stop_replies): Update.
(notif_stop): New variable.
(handle_v_stopped): Remove.
(handle_v_requests): Don't call handle_v_stopped. Call
handle_ack_notif instead.
(queue_stop_reply_callback): Call notif_event_enque instead
of queue_stop_reply.
(handle_status): Don't call send_next_stop_reply, call
notif_write_event instead.
(kill_inferior_callback): Likewise.
(detach_or_kill_inferior_callback): Likewise.
(main): Call initialize_notif.
(process_serial_event): Call QUEUE_is_empty.
(handle_target_event): Call notif_push instead of push event.
* server.h: Remove declaration of push_event.
---
gdb/gdbserver/Makefile.in | 4 +-
gdb/gdbserver/notif.c | 168 +++++++++++++++++++++++++++++++++++++++++++++
gdb/gdbserver/notif.h | 66 ++++++++++++++++++
gdb/gdbserver/server.c | 164 ++++++++++++++-----------------------------
gdb/gdbserver/server.h | 2 -
5 files changed, 291 insertions(+), 113 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 d7dad29..9f440cc 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -168,7 +168,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
@@ -418,6 +418,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
@@ -442,6 +443,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)
diff --git a/gdb/gdbserver/notif.c b/gdb/gdbserver/notif.c
new file mode 100644
index 0000000..0713b36
--- /dev/null
+++ b/gdb/gdbserver/notif.c
@@ -0,0 +1,168 @@
+/* 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_server', in which there is a queue for events to GDB
+ represented by 'struct notif_event'. GDBserver writes (by means of
+ 'write' field) each event in the queue into the buffer and send the
+ contents in buffer to GDB. The contents in buffer is specified in
+ RSP. See more in the comments to field 'queue' of
+ 'struct notif_server'.
+
+ Here is the workflow of sending events and managing queue:
+ 1. At any time, when something interesting FOO happens, a object
+ of 'struct notif_event' or its sub-class EVENT is created for FOO.
+
+ 2. Enque EVENT to the 'queue' field of 'struct notif_server' for
+ FOO and send corresponding notification packet to GDB if EVENT is
+ the first one.
+ #1 and #2 are done by function 'notif_push'.
+
+ 3. EVENT is not deque'ed until the ack of FOO from GDB arrives.
+ Before ack of FOO arrives, FOO happens again, a new object of
+ EVENT is created and enque EVENT silently.
+ Once GDB has a chance to ack to FOO, it sends an ack to GDBserver,
+ and GDBserver repeatedly sends events to GDB and gets ack of FOO,
+ until queue is empty. Then, GDBserver sends 'OK' to GDB that all
+ queued notification events are done.
+
+ # 3 is done by function 'handle_notif_ack'. */
+
+#include "notif.h"
+
+static struct notif_server *notifs[] =
+{
+ ¬if_stop,
+};
+
+/* Write another event or an OK, if there are no more left, to
+ OWN_BUF. */
+
+void
+notif_write_event (struct notif_server *notif, char *own_buf)
+{
+ if (!QUEUE_is_empty (notif_event_p, notif->queue))
+ {
+ struct notif_event *event
+ = QUEUE_peek (notif_event_p, notif->queue);
+
+ notif->write (event, own_buf);
+ }
+ else
+ write_ok (own_buf);
+}
+
+/* Handle the ack in buffer OWN_BUF,and packet length is PACKET_LEN.
+ 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 packet_len)
+{
+ int i = 0;
+ struct notif_server *np = NULL;
+
+ for (i = 0; i < ARRAY_SIZE (notifs); i++)
+ {
+ np = notifs[i];
+ if (strncmp (own_buf, np->ack_name, strlen (np->ack_name)) == 0
+ && packet_len == strlen (np->ack_name))
+ break;
+ }
+
+ if (np == NULL)
+ return 0;
+
+ /* If we're waiting for GDB to acknowledge a pending event,
+ consider that done. */
+ if (!QUEUE_is_empty (notif_event_p, np->queue))
+ {
+ struct notif_event *head
+ = QUEUE_deque (notif_event_p, np->queue);
+
+ if (remote_debug)
+ fprintf (stderr, "%s: acking %d\n", np->ack_name,
+ QUEUE_length (notif_event_p, np->queue));
+
+ xfree (head);
+ }
+
+ notif_write_event (np, own_buf);
+
+ return 1;
+}
+
+/* Put EVENT to the queue of NOTIF. */
+
+void
+notif_event_enque (struct notif_server *notif,
+ struct notif_event *event)
+{
+ QUEUE_enque (notif_event_p, notif->queue, event);
+
+ if (remote_debug)
+ fprintf (stderr, "pending events: %s %d\n", notif->notif_name,
+ QUEUE_length (notif_event_p, notif->queue));
+
+}
+
+/* Push one event NEW_EVENT of notification NP into NP->queue. */
+
+void
+notif_push (struct notif_server *np, struct notif_event *new_event)
+{
+ int is_first_event = QUEUE_is_empty (notif_event_p, np->queue);
+
+ /* Something interesting. Tell GDB about it. */
+ notif_event_enque (np, new_event);
+
+ /* 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->write (new_event, p);
+ putpkt_notif (buf);
+ }
+}
+
+static void
+notif_event_xfree (struct notif_event *event)
+{
+ xfree (event);
+}
+
+void
+initialize_notif (void)
+{
+ int i = 0;
+
+ for (i = 0; i < ARRAY_SIZE (notifs); i++)
+ notifs[i]->queue
+ = QUEUE_alloc (notif_event_p, notif_event_xfree);
+}
diff --git a/gdb/gdbserver/notif.h b/gdb/gdbserver/notif.h
new file mode 100644
index 0000000..e153b48
--- /dev/null
+++ b/gdb/gdbserver/notif.h
@@ -0,0 +1,66 @@
+/* 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 related to a single event. We
+ keep a queue of these to push to GDB. It can be extended if
+ the event of given notification contains more information. */
+
+typedef struct notif_event
+{
+} *notif_event_p;
+
+DECLARE_QUEUE_P (notif_event_p);
+
+/* A type notification to GDB. An object of 'struct notif_server'
+ represents a type of notification. */
+
+typedef struct notif_server
+{
+ /* The name of ack packet, for example, 'vStopped'. */
+ const char *ack_name;
+
+ /* The notification packet, for example, '%Stop'. Note that '%' is
+ not in 'notif_name'. */
+ const char *notif_name;
+
+ /* A queue of events to GDB. A new notif_event 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_event_p) *queue;
+
+ /* Write event EVENT to OWN_BUF. */
+ void (*write) (struct notif_event *event, char *own_buf);
+} *notif_server_p;
+
+extern struct notif_server notif_stop;
+extern struct notif notif_trace;
+
+int handle_notif_ack (char *own_buf, int packet_len);
+void notif_write_event (struct notif_server *notif, char *own_buf);
+
+void notif_push (struct notif_server *np, struct notif_event *event);
+void notif_event_enque (struct notif_server *notif,
+ struct notif_event *event);
+
+void initialize_notif (void);
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index fae9199..fb7322c 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>
@@ -115,13 +116,13 @@ 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. */
+/* A sub-class of 'struct notif_event' for stop, holding information
+ relative to a single stop reply. We keep a queue of these to
+ push to GDB in non-stop mode. */
+
struct vstop_notif
{
- /* Pointer to next in list. */
- struct vstop_notif *next;
+ struct notif_event base;
/* Thread or process that got the event. */
ptid_t ptid;
@@ -130,66 +131,39 @@ struct vstop_notif
struct target_waitstatus status;
};
-/* The pending stop replies list head. */
-static struct vstop_notif *notif_queue = NULL;
+DEFINE_QUEUE_P (notif_event_p);
/* 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;
+ struct vstop_notif *new_notif = xmalloc (sizeof (*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);
- }
+ notif_event_enque (¬if_stop, (struct notif_event *) new_notif);
}
-/* 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)
+static int
+remove_all_on_match_pid (QUEUE (notif_event_p) *q,
+ QUEUE_ITER (notif_event_p) *iter,
+ struct notif_event *event,
+ void *data)
{
- gdb_assert (status->kind != TARGET_WAITKIND_IGNORE);
+ int *pid = data;
- 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)
+ if (*pid == -1
+ || ptid_get_pid (((struct vstop_notif *) event)->ptid) == *pid)
{
- char *p = own_buf;
- strcpy (p, "Stop:");
- p += strlen (p);
- prepare_resume_reply (p,
- notif_queue->ptid, ¬if_queue->status);
- putpkt_notif (own_buf);
+ if (q->free_func != NULL)
+ q->free_func (event);
+
+ QUEUE_remove_elem (notif_event_p, q, iter);
}
+
+ return 1;
}
/* Get rid of the currently pending stop replies for PID. If PID is
@@ -198,40 +172,23 @@ push_event (ptid_t ptid, struct target_waitstatus *status)
static void
discard_queued_stop_replies (int pid)
{
- 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;
- }
+ QUEUE_iterate (notif_event_p, notif_stop.queue,
+ remove_all_on_match_pid, &pid);
}
-/* If there are more stop replies to push, push one now. */
-
static void
-send_next_stop_reply (char *own_buf)
+vstop_notif_reply (struct notif_event *event, char *own_buf)
{
- if (notif_queue)
- prepare_resume_reply (own_buf,
- notif_queue->ptid,
- ¬if_queue->status);
- else
- write_ok (own_buf);
+ struct vstop_notif *vstop = (struct vstop_notif *) event;
+
+ prepare_resume_reply (own_buf, vstop->ptid, &vstop->status);
}
+struct notif_server notif_stop =
+{
+ "vStopped", "Stop", NULL, vstop_notif_reply,
+};
+
static int
target_running (void)
{
@@ -2169,29 +2126,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)
@@ -2252,11 +2186,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, packet_len))
+ return;
/* Otherwise we didn't know what packet it was. Say we didn't
understand it. */
@@ -2337,9 +2268,14 @@ 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->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_event_enque (¬if_stop,
+ (struct notif_event *) new_notif);
}
else
{
@@ -2420,7 +2356,7 @@ handle_status (char *own_buf)
/* 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_write_event (¬if_stop, own_buf);
}
else
{
@@ -2789,6 +2725,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. */
@@ -3400,7 +3338,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 (QUEUE_is_empty (notif_event_p, notif_stop.queue))
{
fprintf (stderr, "GDBserver exiting\n");
remote_close ();
@@ -3498,8 +3436,14 @@ handle_target_event (int err, gdb_client_data client_data)
}
else
{
- /* Something interesting. Tell GDB about it. */
- push_event (last_ptid, &last_status);
+ struct vstop_notif *vstop_notif
+ = xmalloc (sizeof (struct vstop_notif));
+
+ vstop_notif->status = last_status;
+ vstop_notif->ptid = last_ptid;
+ /* Push Stop notification. */
+ notif_push (¬if_stop,
+ (struct notif_event *) vstop_notif);
}
}
diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
index 929819e..7104ef7 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 *);
--
1.7.7.6
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 2/8] de-couple %Stop from notification: gdbserver
2012-12-11 6:40 ` [PATCH 2/8] de-couple %Stop from notification: gdbserver Yao Qi
@ 2012-12-12 17:06 ` Pedro Alves
0 siblings, 0 replies; 19+ messages in thread
From: Pedro Alves @ 2012-12-12 17:06 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
puOn 12/11/2012 06:40 AM, Yao Qi wrote:
> 2012-12-11 Yao Qi <yao@codesourcery.com>
>
> * Makefile.in (OBS): Add notif.o.
> (notif_h, queue_h): New.
Don't forget to remove this from the ChangeLog entry before
checking in, now that gdbserver tracks dependencies automatically.
> (notif.o): New rule.
>
> * notif.c, notif.h: New.
> * server.c: Include "notif.h".
> (struct vstop_notif) <next>: Remove.
> <base>: New field.
> (queue_stop_reply): Update.
> (push_event, send_next_stop_reply): Remove.
> (discard_queued_stop_replies): Update.
> (notif_stop): New variable.
> (handle_v_stopped): Remove.
> (handle_v_requests): Don't call handle_v_stopped. Call
> handle_ack_notif instead.
> (queue_stop_reply_callback): Call notif_event_enque instead
> of queue_stop_reply.
> (handle_status): Don't call send_next_stop_reply, call
> notif_write_event instead.
> (kill_inferior_callback): Likewise.
> (detach_or_kill_inferior_callback): Likewise.
> (main): Call initialize_notif.
> (process_serial_event): Call QUEUE_is_empty.
> (handle_target_event): Call notif_push instead of push event.
> * server.h: Remove declaration of push_event.
Write:
* server.h (push_event): Remove declaration.
Patch is okay.
Thanks!
--
Pedro Alves
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 6/8] Handle notification for all-stop.
2012-12-11 6:40 [PATCH 0/8, V4] A general notification in GDB RSP Yao Qi
2012-12-11 6:40 ` [PATCH 2/8] de-couple %Stop from notification: gdbserver Yao Qi
@ 2012-12-11 6:41 ` Yao Qi
2012-12-12 18:28 ` Pedro Alves
2012-12-11 6:41 ` [PATCH 3/8] de-couple %Stop from notification: gdb Yao Qi
` (5 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Yao Qi @ 2012-12-11 6:41 UTC (permalink / raw)
To: gdb-patches
Now, it is possible to get notifications in all-stop mode, so we
should use 'getpkt_or_notif_sane' instead of 'getpkt_sane'. Beside
this, once a notification arrives, GDB will return to core as this
event is not interesting and keep waiting back again.
gdb:
2012-12-11 Yao Qi <yao@codesourcery.com>
* remote.c (remote_wait_as): Call 'getpkt_or_notif_sane' instead
of 'getpkt_sane'.
Return minus_one_ptid early if gets a notification.
---
gdb/remote.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/gdb/remote.c b/gdb/remote.c
index f3ab5ce..5c8b20e 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5768,6 +5768,7 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options)
else
{
int ret;
+ int is_notif;
if (!target_is_async_p ())
{
@@ -5785,7 +5786,15 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options)
_never_ wait for ever -> test on target_is_async_p().
However, before we do that we need to ensure that the caller
knows how to take the target into/out of async mode. */
- ret = getpkt_sane (&rs->buf, &rs->buf_size, wait_forever_enabled_p);
+ ret = getpkt_or_notif_sane (&rs->buf, &rs->buf_size,
+ wait_forever_enabled_p, &is_notif);
+
+ if (ret != -1 && is_notif)
+ {
+ /* The target didn't really stop; keep waiting. */
+ rs->waiting_for_stop_reply = 1;
+ return minus_one_ptid;
+ }
if (!target_is_async_p ())
signal (SIGINT, ofunc);
}
--
1.7.7.6
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 6/8] Handle notification for all-stop.
2012-12-11 6:41 ` [PATCH 6/8] Handle notification for all-stop Yao Qi
@ 2012-12-12 18:28 ` Pedro Alves
0 siblings, 0 replies; 19+ messages in thread
From: Pedro Alves @ 2012-12-12 18:28 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 12/11/2012 06:40 AM, Yao Qi wrote:
> @@ -5785,7 +5786,15 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options)
> _never_ wait for ever -> test on target_is_async_p().
> However, before we do that we need to ensure that the caller
> knows how to take the target into/out of async mode. */
> - ret = getpkt_sane (&rs->buf, &rs->buf_size, wait_forever_enabled_p);
> + ret = getpkt_or_notif_sane (&rs->buf, &rs->buf_size,
> + wait_forever_enabled_p, &is_notif);
> +
> + if (ret != -1 && is_notif)
> + {
> + /* The target didn't really stop; keep waiting. */
> + rs->waiting_for_stop_reply = 1;
Why do we need to set this here? It's only cleared a bit below?
> + return minus_one_ptid;
> + }
> if (!target_is_async_p ())
> signal (SIGINT, ofunc);
> }
--
Pedro Alves
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/8] de-couple %Stop from notification: gdb
2012-12-11 6:40 [PATCH 0/8, V4] A general notification in GDB RSP Yao Qi
2012-12-11 6:40 ` [PATCH 2/8] de-couple %Stop from notification: gdbserver Yao Qi
2012-12-11 6:41 ` [PATCH 6/8] Handle notification for all-stop Yao Qi
@ 2012-12-11 6:41 ` Yao Qi
2012-12-12 17:32 ` Pedro Alves
2012-12-11 6:41 ` [PATCH 8/8] RSP notification 'Test' Yao Qi
` (4 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Yao Qi @ 2012-12-11 6:41 UTC (permalink / raw)
To: gdb-patches
gdb:
This patch is to de-couple vStopped/%Stop from notification in gdb
side.
I just put the necessary explanations into the comments in
remote-notif.c, so don't write down too much here.
In V4, some structs are renamed, 'struct notif' -> 'struct
notif_client', and 'struct notif_reply' -> 'struct notif_event'.
gdb:
2012-12-11 Yao Qi <yao@codesourcery.com>
* Makefile.in (REMOTE_OBS): Add "remote-notif.o".
(SFILES): Add "remote-notif.c".
(HFILES_NO_SRCDIR): Add "remote-notif.h" and "common/queue.h".
* remote-notif.c: New. Factored out from remote.c.
* remote-notif.h: New.
* remote.c: Include "remote-notif.h".
(stop_reply_xmalloc, do_stop_reply_xfree):
(remote_parse_stop_reply, remote_get_pending_stop_replies):
(remote_async_get_pending_events_handler): Remove declarations.
(remote_parse_stop_reply): Declare.
(pending_stop_reply): Remove.
(remote_async_get_pending_events_token): Move to
remote-notif.c.
(remote_close): Replace 'delete_async_event_handler' with
remote_notif_unregister_async_event_handler.
Don't call discard_pending_stop_replies.
(remote_start_remote): Replace code with remote_notif_parse
and remote_notif_get_pending_replies.
(remote_open_1): Replace 'create_async_event_handler' with
remote_notif_register_async_event_handler.
(extended_remote_attach_1): Call remote_notif_parse and
notif_stop_reply_push.
(struct stop_reply) <next>: Remove.
<base>: New field.
Callers update.
(stop_reply_queue): Change its type.
(stop_reply_xmalloc, do_stop_reply_xfree): Remove.
(remote_notif_remove_all): New.
(discard_pending_stop_replies): Update.
(remote_notif_stop_ack, stop_reply_dtr): New.
(remote_notif_stop_alloc_event): New.
(notif_client_stop): New variable.
(stop_reply_match_ptid, stop_reply_match_ptid_and_ws: New.
(queued_stop_reply, peek_stop_reply): Adjust.
(remote_get_pending_stop_replies): Rename to
remote_notif_get_pending_events.
(handle_notification): Move to remote-notif.c.
(remote_async_get_pending_events_handler): Likewise.
(remote_wait_as): Adjust to call remote_notif_parse.
(remote_wait): Call QUEUE_is_empty (notif_reply_p).
(_initialize_remote): Call QUEUE_alloc. Update caller.
* remote.h: Include "remote-notif.h".
(remote_notif_get_pending_replies):
---
gdb/Makefile.in | 9 +-
gdb/remote-notif.c | 270 ++++++++++++++++++++++++++++++++
gdb/remote-notif.h | 85 ++++++++++
gdb/remote.c | 442 +++++++++++++++++++++++++++-------------------------
gdb/remote.h | 3 +
5 files changed, 590 insertions(+), 219 deletions(-)
create mode 100644 gdb/remote-notif.c
create mode 100644 gdb/remote-notif.h
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 9f6f3a9..a2bbc59 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -506,7 +506,8 @@ SER_HARDWIRE = @SER_HARDWIRE@
# The `remote' debugging target is supported for most architectures,
# but not all (e.g. 960)
-REMOTE_OBS = remote.o dcache.o tracepoint.o ax-general.o ax-gdb.o remote-fileio.o
+REMOTE_OBS = remote.o dcache.o tracepoint.o ax-general.o ax-gdb.o remote-fileio.o \
+ remote-notif.o
# This is remote-sim.o if a simulator is to be linked in.
SIM_OBS = @SIM_OBS@
@@ -728,7 +729,7 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
p-exp.y p-lang.c p-typeprint.c p-valprint.c parse.c printcmd.c \
proc-service.list progspace.c \
prologue-value.c psymtab.c \
- regcache.c reggroups.c remote.c remote-fileio.c reverse.c \
+ regcache.c reggroups.c remote.c remote-fileio.c remote-notif.c reverse.c \
sentinel-frame.c \
serial.c ser-base.c ser-unix.c skip.c \
solib.c solib-target.c source.c \
@@ -773,7 +774,7 @@ c-lang.h d-lang.h go-lang.h frame.h event-loop.h block.h cli/cli-setshow.h \
cli/cli-decode.h cli/cli-cmds.h cli/cli-dump.h cli/cli-utils.h \
cli/cli-script.h macrotab.h symtab.h version.h \
gnulib/import/string.in.h gnulib/import/str-two-way.h \
-gnulib/import/stdint.in.h remote.h gdb.h sparc-nat.h \
+gnulib/import/stdint.in.h remote.h remote-notif.h gdb.h sparc-nat.h \
gdbthread.h dwarf2-frame.h dwarf2-frame-tailcall.h nbsd-nat.h dcache.h \
amd64-nat.h s390-tdep.h arm-linux-tdep.h exceptions.h macroscope.h \
gdbarch.h bsd-uthread.h common/gdb_stat.h memory-map.h memrange.h \
@@ -825,7 +826,7 @@ gnulib/import/extra/snippet/arg-nonnull.h gnulib/import/extra/snippet/c++defs.h
gnulib/import/extra/snippet/warn-on-use.h \
gnulib/import/stddef.in.h gnulib/import/inttypes.in.h inline-frame.h skip.h \
common/common-utils.h common/xml-utils.h common/buffer.h common/ptid.h \
-common/format.h common/host-defs.h utils.h \
+common/format.h common/host-defs.h utils.h common/queue.h \
common/linux-osdata.h gdb-dlfcn.h auto-load.h probe.h stap-probe.h gdb_bfd.h
# Header files that already have srcdir in them, or which are in objdir.
diff --git a/gdb/remote-notif.c b/gdb/remote-notif.c
new file mode 100644
index 0000000..5a72f40
--- /dev/null
+++ b/gdb/remote-notif.c
@@ -0,0 +1,270 @@
+/* Remote notification in GDB protocol
+
+ Copyright (C) 1988-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/>. */
+
+/* Remote async notification is sent from remote target over RSP.
+ Each type of notification is represented by an object of
+ 'struct notif', which has a field 'pending_reply'. It is not
+ NULL when GDB receives a notification from GDBserver, but hasn't
+ acknowledge yet. Before GDB acknowledges the notification,
+ GDBserver shouldn't send notification again (see the header comments
+ in gdbserver/notif.c).
+
+ Notifications are processed in an almost-unified approach for both
+ all-stop mode and non-stop mode, except the timing to process them.
+ In non-stop mode, notifications are processed in
+ remote_async_get_pending_events_handler, while in all-stop mode,
+ they are processed in remote_resume. */
+
+#include "defs.h"
+#include "remote.h"
+#include "remote-notif.h"
+#include "observer.h"
+#include "event-loop.h"
+#include "target.h"
+#include "inferior.h"
+
+#include <string.h>
+
+unsigned int notif_debug = 0;
+
+/* Supported clients of notifications. */
+
+static struct notif_client *notifs[] =
+{
+ ¬if_client_stop,
+};
+
+static void do_notif_event_xfree (void *arg);
+
+/* Parse the BUF for the expected notification NC, and send packet to
+ acknowledge. */
+
+void
+remote_notif_ack (struct notif_client *nc, char *buf)
+{
+ struct notif_event *event = nc->alloc_event ();
+ struct cleanup *old_chain
+ = make_cleanup (do_notif_event_xfree, event);
+
+ if (notif_debug)
+ fprintf_unfiltered (gdb_stdlog, "notif: ack '%s'\n",
+ nc->ack_command);
+
+ nc->parse (nc, buf, event);
+ nc->ack (nc, buf, event);
+
+ discard_cleanups (old_chain);
+}
+
+/* Parse the BUF for the expected notification NC. */
+
+struct notif_event *
+remote_notif_parse (struct notif_client *nc, char *buf)
+{
+ struct notif_event *event = nc->alloc_event ();
+ struct cleanup *old_chain
+ = make_cleanup (do_notif_event_xfree, event);
+
+ if (notif_debug)
+ fprintf_unfiltered (gdb_stdlog, "notif: parse '%s'\n", nc->name);
+
+ nc->parse (nc, buf, event);
+
+ discard_cleanups (old_chain);
+ return event;
+}
+
+DECLARE_QUEUE_P (notif_client_p);
+DEFINE_QUEUE_P (notif_client_p);
+
+static QUEUE(notif_client_p) *notif_queue;
+
+/* Process notifications one by one. EXCEPT is not expected in
+ the queue. */
+
+void
+remote_notif_process (struct notif_client *except)
+{
+ while (!QUEUE_is_empty (notif_client_p, notif_queue))
+ {
+ struct notif_client *nc = QUEUE_deque (notif_client_p,
+ notif_queue);
+
+ gdb_assert (nc != except);
+
+ if (nc->can_get_pending_events (nc))
+ remote_notif_get_pending_events (nc);
+ }
+}
+
+static void
+remote_async_get_pending_events_handler (gdb_client_data data)
+{
+ gdb_assert (non_stop);
+ remote_notif_process (NULL);
+}
+
+/* Asynchronous signal handle registered as event loop source for when
+ the remote sent us a notification. The registered callback
+ will do a ACK sequence to pull the rest of the events out of
+ the remote side into our event queue. */
+
+static struct async_event_handler *remote_async_get_pending_events_token;
+
+/* Register async_event_handler for notification. */
+
+void
+remote_notif_register_async_event_handler (void)
+{
+ remote_async_get_pending_events_token
+ = create_async_event_handler (remote_async_get_pending_events_handler,
+ NULL);
+}
+
+/* Unregister async_event_handler for notification. */
+
+void
+remote_notif_unregister_async_event_handler (void)
+{
+ if (remote_async_get_pending_events_token)
+ delete_async_event_handler (&remote_async_get_pending_events_token);
+}
+
+/* Remote notification handler. */
+
+void
+handle_notification (char *buf)
+{
+ struct notif_client *nc = NULL;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE (notifs); i++)
+ {
+ nc = notifs[i];
+ if (strncmp (buf, nc->name, strlen (nc->name)) == 0
+ && buf[strlen (nc->name)] == ':')
+ break;
+ }
+
+ /* We ignore notifications we don't recognize, for compatibility
+ with newer stubs. */
+ if (nc == NULL)
+ return;
+
+ if (nc->pending_event)
+ {
+ /* We've already parsed the in-flight reply, but the stub for some
+ reason thought we didn't, possibly due to timeout on its side.
+ Just ignore it. */
+ if (notif_debug)
+ fprintf_unfiltered (gdb_stdlog,
+ "notif: ignoring resent notification\n");
+ }
+ else
+ {
+ struct notif_event *event
+ = remote_notif_parse (nc, buf + strlen (nc->name) + 1);
+
+ /* Be careful to only set it after parsing, since an error
+ may be thrown then. */
+ nc->pending_event = event;
+
+ /* Notify the event loop there's a stop reply to acknowledge
+ and that there may be more events to fetch. */
+ QUEUE_enque (notif_client_p, notif_queue, nc);
+ if (non_stop)
+ {
+ /* In non-stop, We mark REMOTE_ASYNC_GET_PENDING_EVENTS_TOKEN
+ in order to go on what we were doing and postpone
+ querying notification events to some point safe to do so.
+ See details in the function comment of
+ remote.c:remote_notif_get_pending_events.
+
+ In all-stop, GDB may be blocked to wait for the reply, we
+ shouldn't return to event loop until the expected reply
+ arrives. For example:
+
+ 1.1) --> vCont;c
+ GDB expects getting stop reply 'T05 thread:2'.
+ 1.2) <-- %Notif
+ <GDB marks the REMOTE_ASYNC_GET_PENDING_EVENTS_TOKEN>
+
+ After step #1.2, we return to the event loop, which
+ notices there is a new event on the
+ REMOTE_ASYNC_GET_PENDING_EVENTS_TOKEN and calls the
+ handler, which will send 'vNotif' packet.
+ 1.3) --> vNotif
+ It is not safe to start a new sequence, because target
+ is still running and GDB is expecting the stop reply
+ from stub.
+
+ To solve this, whenever we parse a notification
+ successfully, we don't mark the
+ REMOTE_ASYNC_GET_PENDING_EVENTS_TOKEN and let GDB blocked
+ there as before to get the sequence done.
+
+ 2.1) --> vCont;c
+ GDB expects getting stop reply 'T05 thread:2'
+ 2.2) <-- %Notif
+ <Don't mark the REMOTE_ASYNC_GET_PENDING_EVENTS_TOKEN>
+ 2.3) <-- T05 thread:2
+
+ These pending notifications can be processed later. */
+ mark_async_event_handler (remote_async_get_pending_events_token);
+ }
+
+ if (notif_debug)
+ fprintf_unfiltered (gdb_stdlog,
+ "notif: Notification '%s' captured\n",
+ nc->name);
+ }
+}
+
+/* Cleanup wrapper. */
+
+static void
+do_notif_event_xfree (void *arg)
+{
+ struct notif_event *event = arg;
+
+ if (event && event->dtr)
+ event->dtr (event);
+
+ xfree (event);
+}
+
+static void
+notif_xfree (struct notif_client *notif)
+{
+ if (notif->pending_event != NULL
+ && notif->pending_event->dtr != NULL)
+ notif->pending_event->dtr (notif->pending_event);
+
+ xfree (notif->pending_event);
+ xfree (notif);
+}
+
+/* -Wmissing-prototypes */
+extern initialize_file_ftype _initialize_notif;
+
+void
+_initialize_notif (void)
+{
+ notif_queue = QUEUE_alloc (notif_client_p, notif_xfree);
+}
diff --git a/gdb/remote-notif.h b/gdb/remote-notif.h
new file mode 100644
index 0000000..92c4bb6
--- /dev/null
+++ b/gdb/remote-notif.h
@@ -0,0 +1,85 @@
+/* Remote notification in GDB protocol
+
+ Copyright (C) 1988-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/>. */
+
+#ifndef REMOTE_NOTIF_H
+#define REMOTE_NOTIF_H
+
+#include "queue.h"
+
+/* An event of a type of async remote notification. */
+
+struct notif_event
+{
+ /* Destructor. Release everything from SELF, but not SELF
+ itself. */
+ void (*dtr) (struct notif_event *self);
+};
+
+/* A client to a sort of async remote notification. */
+
+typedef struct notif_client
+{
+ /* The name of notification packet. */
+ const char *name;
+
+ /* The packet to acknowledge a previous reply. */
+ const char *ack_command;
+
+ /* Parse BUF to get the expected event and update EVENT. This
+ function may throw exception if contents in BUF is not the
+ expected event. */
+ void (*parse) (struct notif_client *self, char *buf,
+ struct notif_event *event);
+
+ /* Send field <ack_command> to remote, and do some checking. If
+ something wrong, throw an exception. */
+ void (*ack) (struct notif_client *self, char *buf,
+ struct notif_event *event);
+
+ /* Check this notification client can get pending events in
+ 'remote_notif_process'. */
+ int (*can_get_pending_events) (struct notif_client *self);
+
+ /* Allocate an event. */
+ struct notif_event *(*alloc_event) (void);
+
+ /* One pending event. This is where we keep it until it is
+ acknowledged. When there is a notification packet, parse it,
+ and create an object of 'struct notif_event' to assign to
+ it. This field is unchanged until GDB starts to ack this
+ notification (which is done by
+ remote.c:remote_notif_pending_replies). */
+ struct notif_event *pending_event;
+} *notif_client_p;
+
+void remote_notif_ack (struct notif_client *nc, char *buf);
+struct notif_event *remote_notif_parse (struct notif_client *nc,
+ char *buf);
+
+void handle_notification (char *buf);
+
+void remote_notif_register_async_event_handler (void);
+void remote_notif_unregister_async_event_handler (void);
+
+void remote_notif_process (struct notif_client *except);
+extern struct notif_client notif_client_stop;
+
+extern unsigned int notif_debug;
+
+#endif /* REMOTE_NOTIF_H */
diff --git a/gdb/remote.c b/gdb/remote.c
index 6d2f431..05b5b1f 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -34,6 +34,7 @@
#include "gdb-stabs.h"
#include "gdbthread.h"
#include "remote.h"
+#include "remote-notif.h"
#include "regcache.h"
#include "value.h"
#include "gdb_assert.h"
@@ -223,16 +224,12 @@ static void remote_check_symbols (struct objfile *objfile);
void _initialize_remote (void);
struct stop_reply;
-static struct stop_reply *stop_reply_xmalloc (void);
static void stop_reply_xfree (struct stop_reply *);
-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 int peek_stop_reply (ptid_t ptid);
+static void remote_parse_stop_reply (char *, struct stop_reply *);
static void remote_async_inferior_event_handler (gdb_client_data);
-static void remote_async_get_pending_events_handler (gdb_client_data);
static void remote_terminal_ours (void);
@@ -244,11 +241,6 @@ static int remote_supports_cond_breakpoints (void);
static int remote_can_run_breakpoint_commands (void);
-/* The non-stop remote protocol provisions for one pending stop reply.
- This is where we keep it until it is acknowledged. */
-
-static struct stop_reply *pending_stop_reply = NULL;
-
/* For "remote". */
static struct cmd_list_element *remote_cmdlist;
@@ -1401,12 +1393,6 @@ static struct async_signal_handler *sigint_remote_token;
static struct async_event_handler *remote_async_inferior_event_token;
-/* Asynchronous signal handle registered as event loop source for when
- the remote sent us a %Stop notification. The registered callback
- will do a vStopped sequence to pull the rest of the events out of
- the remote side into our event queue. */
-
-static struct async_event_handler *remote_async_get_pending_events_token;
\f
static ptid_t magic_null_ptid;
@@ -3023,8 +3009,8 @@ remote_close (int quitting)
if (remote_async_inferior_event_token)
delete_async_event_handler (&remote_async_inferior_event_token);
- if (remote_async_get_pending_events_token)
- delete_async_event_handler (&remote_async_get_pending_events_token);
+
+ remote_notif_unregister_async_event_handler ();
}
/* Query the remote side for the text, data and bss offsets. */
@@ -3446,19 +3432,13 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
mechanism. */
if (strcmp (rs->buf, "OK") != 0)
{
- struct stop_reply *stop_reply;
- struct cleanup *old_chain;
+ struct notif_client *notif = ¬if_client_stop;
- stop_reply = stop_reply_xmalloc ();
- old_chain = make_cleanup (do_stop_reply_xfree, stop_reply);
-
- remote_parse_stop_reply (rs->buf, stop_reply);
- discard_cleanups (old_chain);
-
- /* get_pending_stop_replies acks this one, and gets the rest
- out. */
- pending_stop_reply = stop_reply;
- remote_get_pending_stop_replies ();
+ /* remote_notif_get_pending_replies acks this one, and gets
+ the rest out. */
+ notif_client_stop.pending_event
+ = remote_notif_parse (notif, rs->buf);
+ remote_notif_get_pending_events (notif);
/* Make sure that threads that were stopped remain
stopped. */
@@ -4218,9 +4198,7 @@ remote_open_1 (char *name, int from_tty,
remote_async_inferior_event_token
= create_async_event_handler (remote_async_inferior_event_handler,
NULL);
- remote_async_get_pending_events_token
- = create_async_event_handler (remote_async_get_pending_events_handler,
- NULL);
+ remote_notif_register_async_event_handler ();
/* Reset the target state; these things will be queried either by
remote_query_supported or as they are needed. */
@@ -4475,14 +4453,10 @@ extended_remote_attach_1 (struct target_ops *target, char *args, int from_tty)
if (target_can_async_p ())
{
- struct stop_reply *stop_reply;
- struct cleanup *old_chain;
+ struct notif_event *reply
+ = remote_notif_parse (¬if_client_stop, wait_status);
- stop_reply = stop_reply_xmalloc ();
- old_chain = make_cleanup (do_stop_reply_xfree, stop_reply);
- remote_parse_stop_reply (wait_status, stop_reply);
- discard_cleanups (old_chain);
- push_stop_reply (stop_reply);
+ push_stop_reply ((struct stop_reply *) reply);
target_async (inferior_event_handler, 0);
}
@@ -5109,10 +5083,11 @@ typedef struct cached_reg
DEF_VEC_O(cached_reg_t);
-struct stop_reply
+typedef struct stop_reply
{
- struct stop_reply *next;
+ struct notif_event base;
+ /* The identifier of the thread about this event */
ptid_t ptid;
struct target_waitstatus ws;
@@ -5130,19 +5105,18 @@ struct stop_reply
int replay_event;
int core;
-};
+} *stop_reply_p;
-/* The list of already fetched and acknowledged stop events. */
-static struct stop_reply *stop_reply_queue;
-
-static struct stop_reply *
-stop_reply_xmalloc (void)
-{
- struct stop_reply *r = XMALLOC (struct stop_reply);
-
- r->next = NULL;
- return r;
-}
+DECLARE_QUEUE_P (stop_reply_p);
+DEFINE_QUEUE_P (stop_reply_p);
+/* The list of already fetched and acknowledged stop events. This
+ queue is used for notification Stop, and other notifications
+ don't need queue for their events, because the notification events
+ of Stop can't be consumed immediately, so that events should be
+ queued first, and be consumed by remote_wait_{ns,as} one per
+ time. Other notifications can consume their events immediately,
+ so queue is not needed for them. */
+static QUEUE (stop_reply_p) *stop_reply_queue;
static void
stop_reply_xfree (struct stop_reply *r)
@@ -5154,48 +5128,167 @@ stop_reply_xfree (struct stop_reply *r)
}
}
+static void
+remote_notif_stop_parse (struct notif_client *self, char *buf,
+ struct notif_event *event)
+{
+ remote_parse_stop_reply (buf, (struct stop_reply *) event);
+}
+
+static void
+remote_notif_stop_ack (struct notif_client *self, char *buf,
+ struct notif_event *event)
+{
+ struct stop_reply *stop_reply = (struct stop_reply *) event;
+
+ /* acknowledge */
+ putpkt ((char *) self->ack_command);
+
+ if (stop_reply->ws.kind == TARGET_WAITKIND_IGNORE)
+ /* We got an unknown stop reply. */
+ error (_("Unknown stop reply"));
+
+ push_stop_reply (stop_reply);
+}
+
+static int
+remote_notif_stop_can_get_pending_events (struct notif_client *self)
+{
+ /* We can't get pending events in remote_notif_process for
+ notification stop, and we have to do this in remote_wait_ns
+ instead. If we fetch all queued events from stub, remote stub
+ may exit and we have no chance to process them back in
+ remote_wait_ns. */
+ mark_async_event_handler (remote_async_inferior_event_token);
+ return 0;
+}
+
+static void
+stop_reply_dtr (struct notif_event *event)
+{
+ struct stop_reply *r = (struct stop_reply *) event;
+
+ VEC_free (cached_reg_t, r->regcache);
+}
+
+static struct notif_event *
+remote_notif_stop_alloc_reply (void)
+{
+ struct notif_event *r
+ = (struct notif_event *) XMALLOC (struct stop_reply);
+
+ r->dtr = stop_reply_dtr;
+
+ return r;
+}
+
+/* A client of notification Stop. */
+
+struct notif_client notif_client_stop =
+{
+ "Stop",
+ "vStopped",
+ remote_notif_stop_parse,
+ remote_notif_stop_ack,
+ remote_notif_stop_can_get_pending_events,
+ remote_notif_stop_alloc_reply,
+ NULL,
+};
+
+/* A parameter to pass data in and out. */
+
+struct queue_iter_param
+{
+ void *input;
+ struct stop_reply *output;
+};
+
+/* Remove all queue elements meet the condition it checks. */
+
+static int
+remote_notif_remove_all (QUEUE (stop_reply_p) *q,
+ QUEUE_ITER (stop_reply_p) *iter,
+ stop_reply_p event,
+ void *data)
+{
+ struct queue_iter_param *param = data;
+ int *pid = (int *) param->input;
+
+ if (ptid_get_pid (event->ptid) == *pid)
+ {
+ stop_reply_xfree (event);
+ QUEUE_remove_elem (stop_reply_p, q, iter);
+ }
+
+ return 1;
+}
+
/* Discard all pending stop replies of inferior INF. */
static void
discard_pending_stop_replies (struct inferior *inf)
{
- struct stop_reply *prev = NULL, *reply, *next;
+ int i;
+ struct queue_iter_param param;
+ struct stop_reply *reply
+ = (struct stop_reply *) notif_client_stop.pending_event;
/* Discard the in-flight notification. */
- if (pending_stop_reply != NULL
- && ptid_get_pid (pending_stop_reply->ptid) == inf->pid)
+ if (reply != NULL
+ && ptid_get_pid (reply->ptid) == inf->pid)
{
- stop_reply_xfree (pending_stop_reply);
- pending_stop_reply = NULL;
+ stop_reply_xfree (reply);
+ notif_client_stop.pending_event = NULL;
}
+ param.input = &inf->pid;
+ param.output = NULL;
/* Discard the stop replies we have already pulled with
vStopped. */
- for (reply = stop_reply_queue; reply; reply = next)
- {
- next = reply->next;
- if (ptid_get_pid (reply->ptid) == inf->pid)
- {
- if (reply == stop_reply_queue)
- stop_reply_queue = reply->next;
- else
- prev->next = reply->next;
+ QUEUE_iterate (stop_reply_p, stop_reply_queue,
+ remote_notif_remove_all, ¶m);
+}
- stop_reply_xfree (reply);
- }
- else
- prev = reply;
+/* A parameter to pass data in and out. */
+
+static int
+remote_notif_remove_once_on_match (QUEUE (stop_reply_p) *q,
+ QUEUE_ITER (stop_reply_p) *iter,
+ stop_reply_p event,
+ void *data)
+{
+ struct queue_iter_param *param = data;
+ ptid_t *ptid = param->input;
+
+ if (ptid_match (event->ptid, *ptid))
+ {
+ param->output = event;
+ QUEUE_remove_elem (stop_reply_p, q, iter);
+ return 0;
}
+
+ return 1;
}
-/* Cleanup wrapper. */
+/* Remove the first reply in 'stop_reply_queue' which matches
+ PTID. */
-static void
-do_stop_reply_xfree (void *arg)
+static struct stop_reply *
+remote_notif_remove_queued_reply (ptid_t ptid)
{
- struct stop_reply *r = arg;
+ struct queue_iter_param param;
+
+ param.input = &ptid;
+ param.output = NULL;
- stop_reply_xfree (r);
+ QUEUE_iterate (stop_reply_p, stop_reply_queue,
+ remote_notif_remove_once_on_match, ¶m);
+ if (notif_debug)
+ fprintf_unfiltered (gdb_stdlog,
+ "notif: discard queued event: 'Stop' in %s\n",
+ target_pid_to_str (ptid));
+
+ return param.output;
}
/* Look for a queued stop reply belonging to PTID. If one is found,
@@ -5206,29 +5299,13 @@ do_stop_reply_xfree (void *arg)
static struct stop_reply *
queued_stop_reply (ptid_t ptid)
{
- struct stop_reply *it;
- struct stop_reply **it_link;
-
- it = stop_reply_queue;
- it_link = &stop_reply_queue;
- while (it)
- {
- if (ptid_match (it->ptid, ptid))
- {
- *it_link = it->next;
- it->next = NULL;
- break;
- }
+ struct stop_reply *r = remote_notif_remove_queued_reply (ptid);
- it_link = &it->next;
- it = *it_link;
- }
-
- if (stop_reply_queue)
+ if (!QUEUE_is_empty (stop_reply_p, stop_reply_queue))
/* There's still at least an event left. */
mark_async_event_handler (remote_async_inferior_event_token);
- return it;
+ return r;
}
/* Push a fully parsed stop reply in the stop reply queue. Since we
@@ -5238,38 +5315,37 @@ queued_stop_reply (ptid_t ptid)
static void
push_stop_reply (struct stop_reply *new_event)
{
- struct stop_reply *event;
+ QUEUE_enque (stop_reply_p, stop_reply_queue, new_event);
- if (stop_reply_queue)
- {
- for (event = stop_reply_queue;
- event && event->next;
- event = event->next)
- ;
-
- event->next = new_event;
- }
- else
- stop_reply_queue = new_event;
+ if (notif_debug)
+ fprintf_unfiltered (gdb_stdlog,
+ "notif: push 'Stop' %s to queue %d\n",
+ target_pid_to_str (new_event->ptid),
+ QUEUE_length (stop_reply_p,
+ stop_reply_queue));
mark_async_event_handler (remote_async_inferior_event_token);
}
+static int
+stop_reply_match_ptid_and_ws (QUEUE (stop_reply_p) *q,
+ QUEUE_ITER (stop_reply_p) *iter,
+ struct stop_reply *event,
+ void *data)
+{
+ ptid_t *ptid = data;
+
+ return !(ptid_equal (*ptid, event->ptid)
+ && event->ws.kind == TARGET_WAITKIND_STOPPED);
+}
+
/* Returns true if we have a stop reply for PTID. */
static int
peek_stop_reply (ptid_t ptid)
{
- struct stop_reply *it;
-
- for (it = stop_reply_queue; it; it = it->next)
- if (ptid_equal (ptid, it->ptid))
- {
- if (it->ws.kind == TARGET_WAITKIND_STOPPED)
- return 1;
- }
-
- return 0;
+ return !QUEUE_iterate (stop_reply_p, stop_reply_queue,
+ stop_reply_match_ptid_and_ws, &ptid);
}
/* Parse the stop reply in BUF. Either the function succeeds, and the
@@ -5485,13 +5561,14 @@ Packet: '%s'\n"),
error (_("No process or thread specified in stop reply: %s"), buf);
}
-/* When the stub wants to tell GDB about a new stop reply, it sends a
- stop notification (%Stop). Those can come it at any time, hence,
- we have to make sure that any pending putpkt/getpkt sequence we're
- making is finished, before querying the stub for more events with
- vStopped. E.g., if we started a vStopped sequence immediatelly
- upon receiving the %Stop notification, something like this could
- happen:
+/* When the stub wants to tell GDB about a new notification reply, it
+ sends a notification (%Stop, for example). Those can come it at
+ any time, hence, we have to make sure that any pending
+ putpkt/getpkt sequence we're making is finished, before querying
+ the stub for more events with the corresponding ack command
+ (vStopped, for example). E.g., if we started a vStopped sequence
+ immediately upon receiving the notification, something like this
+ could happen:
1.1) --> Hg 1
1.2) <-- OK
@@ -5526,19 +5603,21 @@ Packet: '%s'\n"),
2.9) --> OK
*/
-static void
-remote_get_pending_stop_replies (void)
+void
+remote_notif_get_pending_events (struct notif_client *nc)
{
struct remote_state *rs = get_remote_state ();
- if (pending_stop_reply)
+ if (nc->pending_event)
{
- /* acknowledge */
- putpkt ("vStopped");
+ if (notif_debug)
+ fprintf_unfiltered (gdb_stdlog,
+ "notif: process: '%s' ack pending event\n",
+ nc->name);
- /* Now we can rely on it. */
- push_stop_reply (pending_stop_reply);
- pending_stop_reply = NULL;
+ /* acknowledge */
+ nc->ack (nc, rs->buf, nc->pending_event);
+ nc->pending_event = NULL;
while (1)
{
@@ -5546,31 +5625,18 @@ remote_get_pending_stop_replies (void)
if (strcmp (rs->buf, "OK") == 0)
break;
else
- {
- struct cleanup *old_chain;
- struct stop_reply *stop_reply = stop_reply_xmalloc ();
-
- old_chain = make_cleanup (do_stop_reply_xfree, stop_reply);
- remote_parse_stop_reply (rs->buf, stop_reply);
-
- /* acknowledge */
- putpkt ("vStopped");
-
- if (stop_reply->ws.kind != TARGET_WAITKIND_IGNORE)
- {
- /* Now we can rely on it. */
- discard_cleanups (old_chain);
- push_stop_reply (stop_reply);
- }
- else
- /* We got an unknown stop reply. */
- do_cleanups (old_chain);
- }
+ remote_notif_ack (nc, rs->buf);
}
}
+ else
+ {
+ if (notif_debug)
+ fprintf_unfiltered (gdb_stdlog,
+ "notif: process: '%s' no pending reply\n",
+ nc->name);
+ }
}
-
/* Called when it is decided that STOP_REPLY holds the info of the
event that is to be returned to the core. This function always
destroys STOP_REPLY. */
@@ -5653,8 +5719,8 @@ remote_wait_ns (ptid_t ptid, struct target_waitstatus *status, int options)
/* Acknowledge a pending stop reply that may have arrived in the
mean time. */
- if (pending_stop_reply != NULL)
- remote_get_pending_stop_replies ();
+ if (notif_client_stop.pending_event != NULL)
+ remote_notif_get_pending_events (¬if_client_stop);
/* If indeed we noticed a stop reply, we're done. */
stop_reply = queued_stop_reply (ptid);
@@ -5750,13 +5816,10 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options)
break;
case 'T': case 'S': case 'X': case 'W':
{
- struct stop_reply *stop_reply;
- struct cleanup *old_chain;
+ struct stop_reply *stop_reply
+ = (struct stop_reply *) remote_notif_parse (¬if_client_stop,
+ rs->buf);
- stop_reply = stop_reply_xmalloc ();
- old_chain = make_cleanup (do_stop_reply_xfree, stop_reply);
- remote_parse_stop_reply (buf, stop_reply);
- discard_cleanups (old_chain);
event_ptid = process_stop_reply (stop_reply, status);
break;
}
@@ -5837,7 +5900,7 @@ remote_wait (struct target_ops *ops,
{
/* If there are are events left in the queue tell the event loop
to return here. */
- if (stop_reply_queue)
+ if (!QUEUE_is_empty (stop_reply_p, stop_reply_queue))
mark_async_event_handler (remote_async_inferior_event_token);
}
@@ -6732,52 +6795,6 @@ remote_read_bytes (CORE_ADDR memaddr, gdb_byte *myaddr, int len)
/* Return what we have. Let higher layers handle partial reads. */
return i;
}
-\f
-
-/* Remote notification handler. */
-
-static void
-handle_notification (char *buf)
-{
- if (strncmp (buf, "Stop:", 5) == 0)
- {
- if (pending_stop_reply)
- {
- /* We've already parsed the in-flight stop-reply, but the
- stub for some reason thought we didn't, possibly due to
- timeout on its side. Just ignore it. */
- if (remote_debug)
- fprintf_unfiltered (gdb_stdlog, "ignoring resent notification\n");
- }
- else
- {
- struct cleanup *old_chain;
- struct stop_reply *reply = stop_reply_xmalloc ();
-
- old_chain = make_cleanup (do_stop_reply_xfree, reply);
-
- remote_parse_stop_reply (buf + 5, reply);
-
- discard_cleanups (old_chain);
-
- /* Be careful to only set it after parsing, since an error
- may be thrown then. */
- pending_stop_reply = reply;
-
- /* Notify the event loop there's a stop reply to acknowledge
- and that there may be more events to fetch. */
- mark_async_event_handler (remote_async_get_pending_events_token);
-
- if (remote_debug)
- fprintf_unfiltered (gdb_stdlog, "stop notification captured\n");
- }
- }
- else
- {
- /* We ignore notifications we don't recognize, for compatibility
- with newer stubs. */
- }
-}
\f
/* Read or write LEN bytes from inferior memory at MEMADDR,
@@ -11178,12 +11195,6 @@ remote_async_inferior_event_handler (gdb_client_data data)
}
static void
-remote_async_get_pending_events_handler (gdb_client_data data)
-{
- remote_get_pending_stop_replies ();
-}
-
-static void
remote_async (void (*callback) (enum inferior_event_type event_type,
void *context), void *context)
{
@@ -11338,6 +11349,7 @@ _initialize_remote (void)
init_remote_threadtests ();
#endif
+ stop_reply_queue = QUEUE_alloc (stop_reply_p, stop_reply_xfree);
/* set/show remote ... */
add_prefix_cmd ("remote", class_maintenance, set_remote_cmd, _("\
diff --git a/gdb/remote.h b/gdb/remote.h
index 3adc54e..4cd38f6 100644
--- a/gdb/remote.h
+++ b/gdb/remote.h
@@ -19,6 +19,8 @@
#ifndef REMOTE_H
#define REMOTE_H
+#include "remote-notif.h"
+
struct target_desc;
/* Read a packet from the remote machine, with error checking, and
@@ -59,4 +61,5 @@ extern int remote_register_number_and_offset (struct gdbarch *gdbarch,
int regnum, int *pnum,
int *poffset);
+extern void remote_notif_get_pending_events (struct notif_client *np);
#endif
--
1.7.7.6
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 3/8] de-couple %Stop from notification: gdb
2012-12-11 6:41 ` [PATCH 3/8] de-couple %Stop from notification: gdb Yao Qi
@ 2012-12-12 17:32 ` Pedro Alves
0 siblings, 0 replies; 19+ messages in thread
From: Pedro Alves @ 2012-12-12 17:32 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 12/11/2012 06:40 AM, Yao Qi wrote:
> 2012-12-11 Yao Qi <yao@codesourcery.com>
>
> * Makefile.in (REMOTE_OBS): Add "remote-notif.o".
> (SFILES): Add "remote-notif.c".
> (HFILES_NO_SRCDIR): Add "remote-notif.h" and "common/queue.h".
> * remote-notif.c: New. Factored out from remote.c.
> * remote-notif.h: New.
> * remote.c: Include "remote-notif.h".
> (stop_reply_xmalloc, do_stop_reply_xfree):
> (remote_parse_stop_reply, remote_get_pending_stop_replies):
> (remote_async_get_pending_events_handler): Remove declarations.
> (remote_parse_stop_reply): Declare.
> (pending_stop_reply): Remove.
> (remote_async_get_pending_events_token): Move to
> remote-notif.c.
> (remote_close): Replace 'delete_async_event_handler' with
> remote_notif_unregister_async_event_handler.
> Don't call discard_pending_stop_replies.
> (remote_start_remote): Replace code with remote_notif_parse
> and remote_notif_get_pending_replies.
> (remote_open_1): Replace 'create_async_event_handler' with
> remote_notif_register_async_event_handler.
> (extended_remote_attach_1): Call remote_notif_parse and
> notif_stop_reply_push.
> (struct stop_reply) <next>: Remove.
> <base>: New field.
> Callers update.
> (stop_reply_queue): Change its type.
> (stop_reply_xmalloc, do_stop_reply_xfree): Remove.
> (remote_notif_remove_all): New.
> (discard_pending_stop_replies): Update.
> (remote_notif_stop_ack, stop_reply_dtr): New.
> (remote_notif_stop_alloc_event): New.
> (notif_client_stop): New variable.
> (stop_reply_match_ptid, stop_reply_match_ptid_and_ws: New.
> (queued_stop_reply, peek_stop_reply): Adjust.
> (remote_get_pending_stop_replies): Rename to
> remote_notif_get_pending_events.
> (handle_notification): Move to remote-notif.c.
> (remote_async_get_pending_events_handler): Likewise.
> (remote_wait_as): Adjust to call remote_notif_parse.
> (remote_wait): Call QUEUE_is_empty (notif_reply_p).
> (_initialize_remote): Call QUEUE_alloc. Update caller.
> * remote.h: Include "remote-notif.h".
> (remote_notif_get_pending_replies):
Okay.
Thanks,
--
Pedro Alves
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 8/8] RSP notification 'Test'.
2012-12-11 6:40 [PATCH 0/8, V4] A general notification in GDB RSP Yao Qi
` (2 preceding siblings ...)
2012-12-11 6:41 ` [PATCH 3/8] de-couple %Stop from notification: gdb Yao Qi
@ 2012-12-11 6:41 ` Yao Qi
2012-12-11 6:41 ` [PATCH 5/8] Different notification from packets Yao Qi
` (3 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Yao Qi @ 2012-12-11 6:41 UTC (permalink / raw)
To: gdb-patches
This patch is only to present how do we add a new type of notifcation
in the future, and to test two types of notifications coexist together
for testing purpose. In each time GDB fetches registers, a new event
is pushed, and GDBserver will send a notification %Test to GDB.
This patch is not for committing to CVS trunk.
gdb/gdbserver:
2012-12-11 Yao Qi <yao@codesourcery.com>
* linux-low.c: Include "notif.h".
(linux_fetch_registers): Call notif_push.
* notif.c (notif_reply_test): New.
(notif_test): New variable.
(notifs): Add new element 'notif_test'.
notif.h (notif_test) Declaration.
gdb:
2012-12-11 Yao Qi <yao@codesourcery.com>
* remote-notif.c (remote_notif_parse_test): New.
(remote_notif_test_ack): New.
(remote_notif_test_alloc_event): New.
(notif_client_test): New variable.
(notifs): New element 'notif_client_test'.
---
gdb/gdbserver/linux-low.c | 10 ++++++++++
gdb/gdbserver/notif.c | 12 ++++++++++++
gdb/gdbserver/notif.h | 2 ++
gdb/remote-notif.c | 43 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 67 insertions(+), 0 deletions(-)
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index c697f6b..ef4b3a9 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 "gdb_wait.h"
#include <stdio.h>
@@ -4336,6 +4337,15 @@ linux_fetch_registers (struct regcache *regcache, int regno)
int use_regsets;
int all = 0;
+ /* Only for test. */
+ if (notif_test.queue != NULL)
+ {
+ struct notif_event *new_event
+ = xmalloc (sizeof (struct notif_event));
+
+ notif_push (¬if_test, new_event);
+ }
+
if (regno == -1)
{
if (the_low_target.fetch_register != NULL)
diff --git a/gdb/gdbserver/notif.c b/gdb/gdbserver/notif.c
index 0713b36..f9d4110 100644
--- a/gdb/gdbserver/notif.c
+++ b/gdb/gdbserver/notif.c
@@ -50,9 +50,21 @@
#include "notif.h"
+static void
+notif_reply_test (struct notif_event *event, char *own_buf)
+{
+ strcpy (own_buf, "CESHI");
+}
+
+struct notif_server notif_test =
+{
+ "vTested", "Test", NULL, notif_reply_test
+};
+
static struct notif_server *notifs[] =
{
¬if_stop,
+ ¬if_test,
};
/* Write another event or an OK, if there are no more left, to
diff --git a/gdb/gdbserver/notif.h b/gdb/gdbserver/notif.h
index e153b48..4223c0c 100644
--- a/gdb/gdbserver/notif.h
+++ b/gdb/gdbserver/notif.h
@@ -63,4 +63,6 @@ void notif_push (struct notif_server *np, struct notif_event *event);
void notif_event_enque (struct notif_server *notif,
struct notif_event *event);
+extern struct notif_server notif_test;
+
void initialize_notif (void);
diff --git a/gdb/remote-notif.c b/gdb/remote-notif.c
index c02394a..916e297 100644
--- a/gdb/remote-notif.c
+++ b/gdb/remote-notif.c
@@ -44,11 +44,54 @@
unsigned int notif_debug = 0;
+static void
+remote_notif_test_parse (struct notif_client *self, char *buf,
+ struct notif_event *event)
+{
+ if (strncmp (buf, "CESHI", 5) != 0)
+ error (_("'CESHI' is expected"));
+}
+
+static void
+remote_notif_test_ack (struct notif_client *self, char *buf,
+ struct notif_event *event)
+{
+ /* acknowledge */
+ putpkt ((char *) self->ack_command);
+}
+
+static int
+remote_notif_test_can_get_pending_events (struct notif_client *self)
+{
+ return 1;
+}
+
+static struct notif_event *
+remote_notif_test_alloc_event (void)
+{
+ struct notif_event *event = xmalloc (sizeof (struct notif_event));
+
+ event->dtr = NULL;
+
+ return event;
+}
+
+static struct notif_client notif_client_test =
+{
+ "Test", "vTested",
+ remote_notif_test_parse,
+ remote_notif_test_ack,
+ remote_notif_test_can_get_pending_events,
+ remote_notif_test_alloc_event,
+ NULL,
+};
+
/* Supported clients of notifications. */
static struct notif_client *notifs[] =
{
¬if_client_stop,
+ ¬if_client_test,
};
static void do_notif_event_xfree (void *arg);
--
1.7.7.6
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH 5/8] Different notification from packets.
2012-12-11 6:40 [PATCH 0/8, V4] A general notification in GDB RSP Yao Qi
` (3 preceding siblings ...)
2012-12-11 6:41 ` [PATCH 8/8] RSP notification 'Test' Yao Qi
@ 2012-12-11 6:41 ` Yao Qi
2012-12-12 18:14 ` Pedro Alves
2012-12-11 6:41 ` [PATCH 4/8] command 'set debug notification' Yao Qi
` (2 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Yao Qi @ 2012-12-11 6:41 UTC (permalink / raw)
To: gdb-patches
The next patch needs to know whether we get a normal packet or
notification from getpkt_or_notif_sane. This patch adds a boolean
pointer in argument to indicate which is got on return.
gdb:
2012-12-11 Yao Qi <yao@codesourcery.com>
* remote.c (getpkt_or_notif_sane): Add one more argument in
its declaration.
(getpkt_or_notif_sane_1): Add one more argument.
(getpkt_sane): Update caller.
(getpkt_or_notif_sane): Likewise. Update call
togetpkt_or_notif_sane_1.
(remote_wait_ns): Update caller.
---
gdb/remote.c | 29 +++++++++++++++++++----------
1 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/gdb/remote.c b/gdb/remote.c
index 05b5b1f..f3ab5ce 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -90,7 +90,7 @@ static void cleanup_sigint_signal_handler (void *dummy);
static void initialize_sigint_signal_handler (void);
static int getpkt_sane (char **buf, long *sizeof_buf, int forever);
static int getpkt_or_notif_sane (char **buf, long *sizeof_buf,
- int forever);
+ int forever, int *is_notif);
static void handle_remote_sigint (int);
static void handle_remote_sigint_twice (int);
@@ -5692,15 +5692,16 @@ remote_wait_ns (ptid_t ptid, struct target_waitstatus *status, int options)
struct remote_state *rs = get_remote_state ();
struct stop_reply *stop_reply;
int ret;
+ int is_notif = 0;
/* If in non-stop mode, get out of getpkt even if a
notification is received. */
ret = getpkt_or_notif_sane (&rs->buf, &rs->buf_size,
- 0 /* forever */);
+ 0 /* forever */, &is_notif);
while (1)
{
- if (ret != -1)
+ if (ret != -1 && !is_notif)
switch (rs->buf[0])
{
case 'E': /* Error of some sort. */
@@ -5737,7 +5738,7 @@ remote_wait_ns (ptid_t ptid, struct target_waitstatus *status, int options)
/* Otherwise do a blocking wait. */
ret = getpkt_or_notif_sane (&rs->buf, &rs->buf_size,
- 1 /* forever */);
+ 1 /* forever */, &is_notif);
}
}
@@ -7419,11 +7420,13 @@ getpkt (char **buf,
0, this function is allowed to time out gracefully and return an
indication of this to the caller. Otherwise return the number of
bytes read. If EXPECTING_NOTIF, consider receiving a notification
- enough reason to return to the caller. */
+ enough reason to return to the caller. On return, *IS_NOTIF stores
+ a boolean whether GDB reads a notification or not from remote.
+ IS_NOTIF can be NULL. */
static int
getpkt_or_notif_sane_1 (char **buf, long *sizeof_buf, int forever,
- int expecting_notif)
+ int expecting_notif, int *is_notif)
{
struct remote_state *rs = get_remote_state ();
int c;
@@ -7524,6 +7527,8 @@ getpkt_or_notif_sane_1 (char **buf, long *sizeof_buf, int forever,
/* Skip the ack char if we're in no-ack mode. */
if (!rs->noack_mode)
serial_write (remote_desc, "+", 1);
+ if (is_notif != NULL)
+ *is_notif = 0;
return val;
}
@@ -7545,13 +7550,15 @@ getpkt_or_notif_sane_1 (char **buf, long *sizeof_buf, int forever,
str);
do_cleanups (old_chain);
}
+ if (is_notif != NULL)
+ *is_notif = 1;
handle_notification (*buf);
/* Notifications require no acknowledgement. */
if (expecting_notif)
- return -1;
+ return val;
}
}
}
@@ -7559,13 +7566,15 @@ getpkt_or_notif_sane_1 (char **buf, long *sizeof_buf, int forever,
static int
getpkt_sane (char **buf, long *sizeof_buf, int forever)
{
- return getpkt_or_notif_sane_1 (buf, sizeof_buf, forever, 0);
+ return getpkt_or_notif_sane_1 (buf, sizeof_buf, forever, 0, NULL);
}
static int
-getpkt_or_notif_sane (char **buf, long *sizeof_buf, int forever)
+getpkt_or_notif_sane (char **buf, long *sizeof_buf, int forever,
+ int *is_notif)
{
- return getpkt_or_notif_sane_1 (buf, sizeof_buf, forever, 1);
+ return getpkt_or_notif_sane_1 (buf, sizeof_buf, forever, 1,
+ is_notif);
}
\f
--
1.7.7.6
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 5/8] Different notification from packets.
2012-12-11 6:41 ` [PATCH 5/8] Different notification from packets Yao Qi
@ 2012-12-12 18:14 ` Pedro Alves
0 siblings, 0 replies; 19+ messages in thread
From: Pedro Alves @ 2012-12-12 18:14 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
This is okay.
On 12/11/2012 06:40 AM, Yao Qi wrote:
> + enough reason to return to the caller. On return, *IS_NOTIF stores
> + a boolean whether GDB reads a notification or not from remote.
Spurious double-space --------------------------^^
I suggest:
On return, *IS_NOTIF stores a boolean indicating whether GDB has read
a notification or not from the remote.
Or my preferred:
*IS_NOTIF is an output boolean that indicates whether *BUF holds
a notification or a regular packet.
Thanks,
--
Pedro Alves
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 4/8] command 'set debug notification'.
2012-12-11 6:40 [PATCH 0/8, V4] A general notification in GDB RSP Yao Qi
` (4 preceding siblings ...)
2012-12-11 6:41 ` [PATCH 5/8] Different notification from packets Yao Qi
@ 2012-12-11 6:41 ` Yao Qi
2012-12-11 7:06 ` Eli Zaretskii
2012-12-11 6:41 ` [PATCH 7/8] Cleanup pending queues before resume in all-stop Yao Qi
2012-12-11 6:41 ` [PATCH 1/8] new queue.h in common/ Yao Qi
7 siblings, 1 reply; 19+ messages in thread
From: Yao Qi @ 2012-12-11 6:41 UTC (permalink / raw)
To: gdb-patches
gdb:
2012-12-07 Yao Qi <yao@codesourcery.com>
* remote-notif.c (_initialize_notif): Add new commands
'set debug notification' and 'show debug notification'.
* NEWS: Mention these new commands.
gdb/doc:
2012-12-07 Yao Qi <yao@codesourcery.com>
* gdb.texinfo (Debugging Output): Document 'set debug
notification' and 'show debug notification'.
---
gdb/NEWS | 4 ++++
gdb/doc/gdb.texinfo | 6 ++++++
gdb/remote-notif.c | 11 +++++++++++
3 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/gdb/NEWS b/gdb/NEWS
index 3b09e5f..e6f14f0 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -64,6 +64,10 @@ enable type-printer [name]...
disable type-printer [name]...
Enable or disable type printers.
+set debug notification
+show debug notification
+ Control display of debugging info for async remote notification.
+
* Removed commands
** For the Renesas Super-H architecture, the "regs" command has been removed
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 9ffdb77..9465047 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -21967,6 +21967,12 @@ Displays the current state of @value{GDBN} JIT debugging.
Turns on or off debugging messages from the Linux LWP debug support.
@item show debug lin-lwp
Show the current state of Linux LWP debugging messages.
+@item set debug notification
+@cindex remote async notification debugging info
+Turns on or off debugging messages about remote async notification.
+The default is off.
+@item show debug notification
+Displays the current state of remote async notification debugging messages.
@item set debug observer
@cindex observer debugging info
Turns on or off display of @value{GDBN} observer debugging. This
diff --git a/gdb/remote-notif.c b/gdb/remote-notif.c
index 5a72f40..c02394a 100644
--- a/gdb/remote-notif.c
+++ b/gdb/remote-notif.c
@@ -38,6 +38,7 @@
#include "event-loop.h"
#include "target.h"
#include "inferior.h"
+#include "gdbcmd.h"
#include <string.h>
@@ -267,4 +268,14 @@ void
_initialize_notif (void)
{
notif_queue = QUEUE_alloc (notif_client_p, notif_xfree);
+
+ add_setshow_boolean_cmd ("notification", no_class, ¬if_debug,
+ _("\
+Set debugging of async remote notification."), _("\
+Show debugging of async remote notification."), _("\
+When non-zero, async remote notification specific"
+" internal debugging is enabled."),
+ NULL,
+ NULL,
+ &setdebuglist, &showdebuglist);
}
--
1.7.7.6
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 4/8] command 'set debug notification'.
2012-12-11 6:41 ` [PATCH 4/8] command 'set debug notification' Yao Qi
@ 2012-12-11 7:06 ` Eli Zaretskii
2012-12-12 17:57 ` Pedro Alves
0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2012-12-11 7:06 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
> From: Yao Qi <yao@codesourcery.com>
> Date: Tue, 11 Dec 2012 14:40:13 +0800
>
> gdb:
>
> 2012-12-07 Yao Qi <yao@codesourcery.com>
>
> * remote-notif.c (_initialize_notif): Add new commands
> 'set debug notification' and 'show debug notification'.
> * NEWS: Mention these new commands.
> gdb/doc:
>
> 2012-12-07 Yao Qi <yao@codesourcery.com>
>
> * gdb.texinfo (Debugging Output): Document 'set debug
> notification' and 'show debug notification'.
These two parts are OK. Thanks.
> + add_setshow_boolean_cmd ("notification", no_class, ¬if_debug,
> + _("\
> +Set debugging of async remote notification."), _("\
> +Show debugging of async remote notification."), _("\
> +When non-zero, async remote notification specific"
> +" internal debugging is enabled."),
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
I think not "debugging" is enabled, but "debugging info" is enabled.
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 4/8] command 'set debug notification'.
2012-12-11 7:06 ` Eli Zaretskii
@ 2012-12-12 17:57 ` Pedro Alves
2012-12-12 18:59 ` Eli Zaretskii
0 siblings, 1 reply; 19+ messages in thread
From: Pedro Alves @ 2012-12-12 17:57 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Yao Qi, gdb-patches
On 12/11/2012 07:06 AM, Eli Zaretskii wrote:
>
>> > + add_setshow_boolean_cmd ("notification", no_class, ¬if_debug,
>> > + _("\
>> > +Set debugging of async remote notification."), _("\
>> > +Show debugging of async remote notification."), _("\
>> > +When non-zero, async remote notification specific"
>> > +" internal debugging is enabled."),
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> I think not "debugging" is enabled, but "debugging info" is enabled.
This command just enables/disables internal debugging/logging output,
it is not about debug info, as in dwarf, etc.
--
Pedro Alves
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 4/8] command 'set debug notification'.
2012-12-12 17:57 ` Pedro Alves
@ 2012-12-12 18:59 ` Eli Zaretskii
2012-12-12 19:17 ` Pedro Alves
0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2012-12-12 18:59 UTC (permalink / raw)
To: Pedro Alves; +Cc: yao, gdb-patches
> Date: Wed, 12 Dec 2012 17:56:52 +0000
> From: Pedro Alves <palves@redhat.com>
> CC: Yao Qi <yao@codesourcery.com>, gdb-patches@sourceware.org
>
> On 12/11/2012 07:06 AM, Eli Zaretskii wrote:
> >
> >> > + add_setshow_boolean_cmd ("notification", no_class, ¬if_debug,
> >> > + _("\
> >> > +Set debugging of async remote notification."), _("\
> >> > +Show debugging of async remote notification."), _("\
> >> > +When non-zero, async remote notification specific"
> >> > +" internal debugging is enabled."),
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > I think not "debugging" is enabled, but "debugging info" is enabled.
>
> This command just enables/disables internal debugging/logging output,
> it is not about debug info, as in dwarf, etc.
Granted, I understand that. If you suggest to say
When non-zero, debugging output about async remote notifications is
enabled.
then I have no objections. But saying that "internal debugging is
enabled" is certainly inaccurate and perhaps even misleading, IMO.
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 4/8] command 'set debug notification'.
2012-12-12 18:59 ` Eli Zaretskii
@ 2012-12-12 19:17 ` Pedro Alves
0 siblings, 0 replies; 19+ messages in thread
From: Pedro Alves @ 2012-12-12 19:17 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: yao, gdb-patches
On 12/12/2012 06:59 PM, Eli Zaretskii wrote:
>> Date: Wed, 12 Dec 2012 17:56:52 +0000
>> From: Pedro Alves <palves@redhat.com>
>> CC: Yao Qi <yao@codesourcery.com>, gdb-patches@sourceware.org
>>
>> On 12/11/2012 07:06 AM, Eli Zaretskii wrote:
>>>
>>>>> + add_setshow_boolean_cmd ("notification", no_class, ¬if_debug,
>>>>> + _("\
>>>>> +Set debugging of async remote notification."), _("\
>>>>> +Show debugging of async remote notification."), _("\
>>>>> +When non-zero, async remote notification specific"
>>>>> +" internal debugging is enabled."),
>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>> I think not "debugging" is enabled, but "debugging info" is enabled.
>>
>> This command just enables/disables internal debugging/logging output,
>> it is not about debug info, as in dwarf, etc.
>
> Granted, I understand that. If you suggest to say
>
> When non-zero, debugging output about async remote notifications is
> enabled.
>
> then I have no objections. But saying that "internal debugging is
> enabled" is certainly inaccurate and perhaps even misleading, IMO.
I like that text, thanks. Yao, the code parts are okay with that change.
Thanks,
--
Pedro Alves
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 7/8] Cleanup pending queues before resume in all-stop
2012-12-11 6:40 [PATCH 0/8, V4] A general notification in GDB RSP Yao Qi
` (5 preceding siblings ...)
2012-12-11 6:41 ` [PATCH 4/8] command 'set debug notification' Yao Qi
@ 2012-12-11 6:41 ` Yao Qi
2012-12-12 18:22 ` Pedro Alves
2012-12-11 6:41 ` [PATCH 1/8] new queue.h in common/ Yao Qi
7 siblings, 1 reply; 19+ messages in thread
From: Yao Qi @ 2012-12-11 6:41 UTC (permalink / raw)
To: gdb-patches
Hi,
Please read the comments to see why we need this change.
gdb:
2012-12-11 Yao Qi <yao@codesourcery.com>
* remote.c (remote_resume): Call 'remote_notif_process' in
all-stop mode.
---
gdb/remote.c | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/gdb/remote.c b/gdb/remote.c
index 5c8b20e..47b2c45 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -4750,6 +4750,15 @@ remote_resume (struct target_ops *ops,
struct remote_state *rs = get_remote_state ();
char *buf;
+ /* In all-stop, we can't mark REMOTE_ASYNC_GET_PENDING_EVENTS_TOKEN
+ (explained in remote-notif.c:handle_notification) so
+ remote_notif_process is not called. We need find a place where
+ it is safe to start a 'vNotif' sequence. It is good to do it
+ before resuming inferior, because inferior was stopped and no RSP
+ traffic at that moment. */
+ if (!non_stop)
+ remote_notif_process (¬if_client_stop);
+
last_sent_signal = siggnal;
last_sent_step = step;
--
1.7.7.6
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH 1/8] new queue.h in common/.
2012-12-11 6:40 [PATCH 0/8, V4] A general notification in GDB RSP Yao Qi
` (6 preceding siblings ...)
2012-12-11 6:41 ` [PATCH 7/8] Cleanup pending queues before resume in all-stop Yao Qi
@ 2012-12-11 6:41 ` Yao Qi
2012-12-12 17:05 ` Pedro Alves
7 siblings, 1 reply; 19+ messages in thread
From: Yao Qi @ 2012-12-11 6:41 UTC (permalink / raw)
To: gdb-patches
Hi,
When writing the patches for 'general notification', I find queue is
used in many places, so I think we may need a general queue. This
queue not only have typical operations enqueue and dequeue, but also
has some have some operations like remove and remove_all.
In V4, Pedro's comments are addressed, and the macros are moved from
the bottom of file to the top, along with comments.
gdb/
2012-12-11 Yao Qi <yao@codesourcery.com>
Doug Evans <dje@google.com>
* common/queue.h: New.
---
gdb/common/queue.h | 303 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 303 insertions(+), 0 deletions(-)
create mode 100644 gdb/common/queue.h
diff --git a/gdb/common/queue.h b/gdb/common/queue.h
new file mode 100644
index 0000000..bf48cdf
--- /dev/null
+++ b/gdb/common/queue.h
@@ -0,0 +1,303 @@
+/* General queue data structure for GDB, the GNU debugger.
+
+ Copyright (C) 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/>. */
+
+#ifndef QUEUE_H
+#define QUEUE_H
+
+#include "libiberty.h" /* xmalloc */
+#include "gdb_assert.h"
+
+/* These macros implement functions and structs for a general queue.
+ Macro 'DEFINE_QUEUE_P(TYPEDEF)' is to define the new queue type for
+ TYPEDEF', and macro 'DECLARE_QUEUE_P' is to declare external queue
+ APIs. The character P indicates TYPEDEF is a pointer (P). The
+ counterpart on object (O) and integer (I) are not implemented.
+
+ An example of their use would be,
+
+ typedef struct foo
+ {} *foo_p;
+
+ DEFINE_QUEUE_P (foo_p);
+ // A pointer to a queue of foo pointers. FOO_XFREE is a destructor
+ // function for foo instances in queue.
+ QUEUE(foo_p) *foo_queue = QUEUE_alloc (foo_p, foo_xfree);
+
+ foo_p foo_var_p;
+ // Enqueue and Dequeue
+ QUEUE_enque (foo_p, foo_queue, foo_var_p);
+ foo_var_p = QUEUE_deque (foo_p, foo_queue);
+
+ static int visit_foo (QUEUE (foo_p) *q, QUEUE_ITER (foo_p) *iter,
+ foo_p f, void *data)
+ {
+ return 1;
+ }
+ // Iterate over queue.
+ QUEUE_iterate (foo_p, foo_queue, visit_foo, ¶m);
+
+ QUEUE_free (foo_p, foo_queue); // Free queue. */
+
+/* Typical enqueue operation. Put V into queue Q. */
+#define QUEUE_enque(TYPE, Q, V) queue_ ## TYPE ## _enque ((Q), (V))
+
+/* Typical dequeue operation. Return head element of queue Q and
+ remove it. Q must not be empty. */
+#define QUEUE_deque(TYPE, Q) queue_ ## TYPE ## _deque (Q)
+
+/* Return the head element, but don't remove it from the queue.
+ Q must not be empty. */
+#define QUEUE_peek(TYPE, Q) queue_ ## TYPE ## _peek (Q)
+
+/* Return true if queue Q is empty. */
+#define QUEUE_is_empty(TYPE, Q) queue_ ## TYPE ## _is_empty (Q)
+
+/* Allocate memory for queue. FREE_FUNC is a function to release the
+ data put in each queue element. */
+#define QUEUE_alloc(TYPE, FREE_FUNC) queue_ ## TYPE ## _alloc (FREE_FUNC)
+
+/* Length of queue Q. */
+#define QUEUE_length(TYPE, Q) queue_ ## TYPE ## _length (Q)
+
+/* Free queue Q. Q's free_func is called once for each element. */
+#define QUEUE_free(TYPE, Q) queue_ ## TYPE ## _free (Q)
+
+/* Iterate over elements in the queue Q and call function OPERATE on
+ each element. It is allowed to remove element by OPERATE. OPERATE
+ returns false to terminate the iteration and true to continue the
+ iteration. Return false if iteration is terminated by function
+ OPERATE, otherwise return true. */
+#define QUEUE_iterate(TYPE, Q, OPERATE, PARAM) \
+ queue_ ## TYPE ## _iterate ((Q), (OPERATE), (PARAM))
+
+/* Remove the element per the state of iterator ITER from queue Q.
+ Leave the caller to release the data in the queue element. */
+#define QUEUE_remove_elem(TYPE, Q, ITER) \
+ queue_ ## TYPE ## _remove_elem ((Q), (ITER))
+
+/* Define a new queue implementation. */
+
+#define QUEUE(TYPE) struct queue_ ## TYPE
+#define QUEUE_ELEM(TYPE) struct queue_elem_ ## TYPE
+#define QUEUE_ITER(TYPE) struct queue_iter_ ## TYPE
+#define QUEUE_ITER_FUNC(TYPE) queue_ ## TYPE ## _operate_func
+
+#define DEFINE_QUEUE_P(TYPE) \
+QUEUE_ELEM (TYPE) \
+{ \
+ QUEUE_ELEM (TYPE) *next; \
+ \
+ TYPE data; \
+}; \
+ \
+/* Queue iterator. */ \
+QUEUE_ITER (TYPE) \
+{ \
+ /* The current element during traverse. */ \
+ QUEUE_ELEM (TYPE) *p; \
+ /* The previous element of P. */ \
+ QUEUE_ELEM (TYPE) *prev; \
+}; \
+ \
+QUEUE(TYPE) \
+{ \
+ /* The head and tail of the queue. */ \
+ QUEUE_ELEM (TYPE) *head; \
+ QUEUE_ELEM (TYPE) *tail; \
+ /* Function to release the data put in each \
+ queue element. */ \
+ void (*free_func) (TYPE); \
+}; \
+ \
+void \
+queue_ ## TYPE ## _enque (QUEUE (TYPE) *q, TYPE v) \
+{ \
+ QUEUE_ELEM (TYPE) *p \
+ = xmalloc (sizeof (QUEUE_ELEM (TYPE))); \
+ \
+ gdb_assert (q != NULL); \
+ p->data = v; \
+ p->next = NULL; \
+ if (q->tail == NULL) \
+ { \
+ q->tail = p; \
+ q->head = p; \
+ } \
+ else \
+ { \
+ q->tail->next = p; \
+ q->tail = p; \
+ } \
+} \
+ \
+TYPE \
+queue_ ## TYPE ## _deque (QUEUE (TYPE) *q) \
+{ \
+ QUEUE_ELEM (TYPE) *p; \
+ TYPE v; \
+ \
+ gdb_assert (q != NULL); \
+ p = q->head; \
+ gdb_assert (p != NULL); \
+ \
+ if (q->head == q->tail) \
+ { \
+ q->head = NULL; \
+ q->tail = NULL; \
+ } \
+ else \
+ q->head = q->head->next; \
+ \
+ v = p->data; \
+ \
+ xfree (p); \
+ return v; \
+} \
+ \
+TYPE \
+queue_ ## TYPE ## _peek (QUEUE (TYPE) *q) \
+{ \
+ gdb_assert (q != NULL); \
+ gdb_assert (q->head != NULL); \
+ return q->head->data; \
+} \
+ \
+int \
+queue_ ## TYPE ## _is_empty (QUEUE (TYPE) *q) \
+{ \
+ gdb_assert (q != NULL); \
+ return q->head == NULL; \
+} \
+ \
+void \
+queue_ ## TYPE ## _remove_elem (QUEUE (TYPE) *q, \
+ QUEUE_ITER (TYPE) *iter) \
+{ \
+ gdb_assert (q != NULL); \
+ gdb_assert (iter != NULL && iter->p != NULL); \
+ \
+ if (iter->p == q->head || iter->p == q->tail) \
+ { \
+ if (iter->p == q->head) \
+ q->head = iter->p->next; \
+ if (iter->p == q->tail) \
+ q->tail = iter->prev; \
+ } \
+ else \
+ iter->prev->next = iter->p->next; \
+ \
+ xfree (iter->p); \
+ /* Indicate that ITER->p has been deleted from QUEUE q. */ \
+ iter->p = NULL; \
+} \
+ \
+int \
+queue_ ## TYPE ## _iterate (QUEUE (TYPE) *q, \
+ QUEUE_ITER_FUNC (TYPE) operate, \
+ void *data) \
+{ \
+ QUEUE_ELEM (TYPE) *next = NULL; \
+ QUEUE_ITER (TYPE) iter = { NULL, NULL }; \
+ \
+ gdb_assert (q != NULL); \
+ \
+ for (iter.p = q->head; iter.p != NULL; iter.p = next) \
+ { \
+ next = iter.p->next; \
+ if (!operate (q, &iter, iter.p->data, data)) \
+ return 0; \
+ /* ITER.P was not deleted by function OPERATE. */ \
+ if (iter.p != NULL) \
+ iter.prev = iter.p; \
+ } \
+ return 1; \
+} \
+ \
+QUEUE (TYPE) * \
+queue_ ## TYPE ## _alloc (void (*free_func) (TYPE)) \
+{ \
+ QUEUE (TYPE) *q; \
+ \
+ q = (QUEUE (TYPE) *) xmalloc (sizeof (QUEUE (TYPE))); \
+ q->head = NULL; \
+ q->tail = NULL; \
+ q->free_func = free_func; \
+ return q; \
+} \
+ \
+int \
+queue_ ## TYPE ## _length (QUEUE (TYPE) *q) \
+{ \
+ QUEUE_ELEM (TYPE) *p; \
+ int len = 0; \
+ \
+ gdb_assert (q != NULL); \
+ \
+ for (p = q->head; p != NULL; p = p->next) \
+ len++; \
+ \
+ return len; \
+} \
+ \
+void \
+queue_ ## TYPE ## _free (QUEUE (TYPE) *q) \
+{ \
+ QUEUE_ELEM (TYPE) *p, *next; \
+ \
+ gdb_assert (q != NULL); \
+ \
+ for (p = q->head; p != NULL; p = next) \
+ { \
+ next = p->next; \
+ if (q->free_func) \
+ q->free_func (p->data); \
+ xfree (p); \
+ } \
+ xfree (q); \
+} \
+
+/* External declarations for queue functions. */
+#define DECLARE_QUEUE_P(TYPE) \
+QUEUE (TYPE); \
+QUEUE_ELEM (TYPE); \
+QUEUE_ITER (TYPE); \
+extern void \
+ queue_ ## TYPE ## _enque (QUEUE (TYPE) *q, TYPE v); \
+extern TYPE \
+ queue_ ## TYPE ## _deque (QUEUE (TYPE) *q); \
+extern int queue_ ## TYPE ## _is_empty (QUEUE (TYPE) *q); \
+extern QUEUE (TYPE) * \
+ queue_ ## TYPE ## _alloc (void (*free_func) (TYPE)); \
+extern int queue_ ## TYPE ## _length (QUEUE (TYPE) *q); \
+extern TYPE \
+ queue_ ## TYPE ## _peek (QUEUE (TYPE) *q); \
+extern void queue_ ## TYPE ## _free (QUEUE (TYPE) *q); \
+typedef int QUEUE_ITER_FUNC(TYPE) (QUEUE (TYPE) *, \
+ QUEUE_ITER (TYPE) *, \
+ TYPE, \
+ void *); \
+extern int \
+ queue_ ## TYPE ## _iterate (QUEUE (TYPE) *q, \
+ QUEUE_ITER_FUNC (TYPE) operate, \
+ void *); \
+extern void \
+ queue_ ## TYPE ## _remove_elem (QUEUE (TYPE) *q, \
+ QUEUE_ITER (TYPE) *iter); \
+
+#endif /* QUEUE_H */
--
1.7.7.6
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 1/8] new queue.h in common/.
2012-12-11 6:41 ` [PATCH 1/8] new queue.h in common/ Yao Qi
@ 2012-12-12 17:05 ` Pedro Alves
0 siblings, 0 replies; 19+ messages in thread
From: Pedro Alves @ 2012-12-12 17:05 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 12/11/2012 06:40 AM, Yao Qi wrote:
> Hi,
> When writing the patches for 'general notification', I find queue is
> used in many places, so I think we may need a general queue. This
> queue not only have typical operations enqueue and dequeue, but also
> has some have some operations like remove and remove_all.
>
> In V4, Pedro's comments are addressed, and the macros are moved from
> the bottom of file to the top, along with comments.
Thanks, I have no further comments.
--
Pedro Alves
^ permalink raw reply [flat|nested] 19+ messages in thread