* [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
* [PATCH 0/8, V4] A general notification in GDB RSP
@ 2012-12-11 6:40 Yao Qi
2012-12-11 6:40 ` [PATCH 2/8] de-couple %Stop from notification: gdbserver Yao Qi
` (7 more replies)
0 siblings, 8 replies; 19+ messages in thread
From: Yao Qi @ 2012-12-11 6:40 UTC (permalink / raw)
To: gdb-patches
Hi,
We have more and more requirements that remote target wants to notify
GDB via RSP at any time during connection when some states are changed
in remote target. However, GDB doesn't have such general notification
infrastructure. This is what this patch set tries to add.
Nowadays, notification mechanism exists in GDB for '%Stop' only,
and works pretty good. My rationale of this work is 'decouple
%Stop/vStopped from existing notification mechanism so that
notification mechanism can handle other types of notification.
First of all, let us look at how '%Stop' and 'vStopped' works,
gdb gdbserver
<- %Stop // [1] Send notification
[after process other packets]
-> vStopped // [2] Ack this notification
<- T05 thread:2 // [3] Reply
-> vStopped // [4] Continue to query
<- OK // [5] Done
We can generalize this protocol like this,
gdb gdbserver
<- %NOTIF // [1] Send notification
[after process other packets]
-> vACK // [2] Ack this notification
<- EVENT // [3] Event
-> vACK // [4] Continue to query
<- OK // [5] Done
For each type of notification, we only have to define three things,
1) NOTIF, the key word of notification, for example, "Stop"
2) ACK, the command GDB ack to this notification, "vStopped" for
example,
3) EVENT, the format of event to GDB. GDBserver should be able to
send packet to comply with EVENT, and GDB is able parse the packet,
and identify the packet is EVENT or not.
This is the V4 of this patch series, there are some changes compared
with V3,
- Move macros in common/queue.h from the bottom to to top.
Move the comments of functions to macros, as users should use
macros externally,
- Don't treat 'Stop' a special notification, and keep the global
queue of stop reply.
- Move 'ptid' field out of 'struct notif_reply' (it is 'struct notif_event'
now).
- Rename some struct
- Add some comments in the code.
This patch series depends on this patch,
[PATCH] Attach discard_pending_stop_replies to observer inferior_exit
http://sourceware.org/ml/gdb-patches/2012-12/msg00126.html
All these patches are tested on x86_64-linux {native, gdbserver} x
{sync, async}. No regression.
gdb/Makefile.in | 9 +-
gdb/NEWS | 4 +
gdb/common/queue.h | 303 ++++++++++++++++++++++++++++
gdb/doc/gdb.texinfo | 6 +
gdb/gdbserver/Makefile.in | 4 +-
gdb/gdbserver/linux-low.c | 10 +
gdb/gdbserver/notif.c | 180 +++++++++++++++++
gdb/gdbserver/notif.h | 68 +++++++
gdb/gdbserver/server.c | 164 +++++----------
gdb/gdbserver/server.h | 2 -
gdb/remote-notif.c | 324 ++++++++++++++++++++++++++++++
gdb/remote-notif.h | 85 ++++++++
gdb/remote.c | 491 ++++++++++++++++++++++++---------------------
gdb/remote.h | 3 +
14 files changed, 1310 insertions(+), 343 deletions(-)
create mode 100644 gdb/common/queue.h
create mode 100644 gdb/gdbserver/notif.c
create mode 100644 gdb/gdbserver/notif.h
create mode 100644 gdb/remote-notif.c
create mode 100644 gdb/remote-notif.h
--
1.7.7.6
^ 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
* [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
* [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 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
* [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
* [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
* [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
* 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 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
* 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
* 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
* 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 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
* Re: [PATCH 7/8] Cleanup pending queues before resume in all-stop
2012-12-11 6:41 ` [PATCH 7/8] Cleanup pending queues before resume in all-stop Yao Qi
@ 2012-12-12 18:22 ` Pedro Alves
0 siblings, 0 replies; 19+ messages in thread
From: Pedro Alves @ 2012-12-12 18:22 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>
>
> * remote.c (remote_resume): Call 'remote_notif_process' in
> all-stop mode.
OK.
--
Pedro Alves
^ 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
* 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
end of thread, other threads:[~2012-12-12 19:17 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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-12 17:06 ` Pedro Alves
2012-12-11 6:41 ` [PATCH 6/8] Handle notification for all-stop 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
2012-12-12 17:32 ` Pedro Alves
2012-12-11 6:41 ` [PATCH 8/8] RSP notification 'Test' Yao Qi
2012-12-11 6:41 ` [PATCH 5/8] Different notification from packets Yao Qi
2012-12-12 18:14 ` Pedro Alves
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
2012-12-12 18:59 ` Eli Zaretskii
2012-12-12 19:17 ` Pedro Alves
2012-12-11 6:41 ` [PATCH 7/8] Cleanup pending queues before resume in all-stop Yao Qi
2012-12-12 18:22 ` Pedro Alves
2012-12-11 6:41 ` [PATCH 1/8] new queue.h in common/ Yao Qi
2012-12-12 17:05 ` Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox