Re: [PATCH 2/5] Query supported notifications by qSupported

Yao Qi
2013-01-22T08:41:00Z

On 01/22/2013 04:08 PM, Eli Zaretskii wrote:
> "by commas", in plural.
>
Fixed.

>> >+@cindex notifications, in remote protocol
>> >+The remote stub supports a string of notifications which the remote stub
>> >+supports.
> Something is wrong with this sentence...

How about this?

@cindex notifications, in remote protocol
The remote stub supports a string of notifications. @var{name} is
the name of the notification and @var{annex} is the name of the annex,
if the notification has the annex.

Below is the updated version with some minor non-doc cleanups.

--
Yao (é½å°§)

gdb/gdbserver:

2013-01-21 Yao Qi <yao@codesourcery.com>

* Makefile.in (SFILES): Add "common-notif.c".
(OBS): Add common-notif.o.
(common-notif.o): New rule.

* notif.c (notif_find_annex): New.
(notif_qsupported_record): New.
(+notif_qsupported_reply): New.
* notif.h (notif_qsupported_reply): Declare.
(notif_qsupported_record): Declare.
* server.c (notif_annex_stop): Update.
(handle_query): Call notif_qsupported_record and
notif_qsupported_reply.
gdb:

2013-01-21 Yao Qi <yao@codesourcery.com>

* common/common-notif.c: New.
* common/common-notif.h (struct notif_annex) <supported>: New
field.
* Makefile.in (iREMOTE_OBS): Append "common-notif.o".
(SFILES): Add "common-notif.c".

* remote-notif.c (remote_notif_parse_1): Call the parser of
annex if it is supported.
(remote_notif_qsupported): New.
(remote_notif_qsupported_reply): New.
* remote-notif.h (remote_notif_qsupported): Declare.
(remote_notif_qsupported_reply): Declare.

* remote.c (PACKET_notifications): New enum.
(remote_notifications_feature): New.
(remote_protocol_features): Add new element.
(remote_query_supported): Call remote_notif_qsupported and
append supported notifications to qSupported feature.
(notif_client_annex_stop): Update.

gdb/doc:

2013-01-21 Yao Qi <yao@codesourcery.com>

* gdb.texinfo (General Query Packets): Document the new feature
'notifications' of 'qSupported' packet.
Document the new feature in qSupported reply.
---
gdb/Makefile.in | 9 ++-
gdb/common/common-notif.c | 181 +++++++++++++++++++++++++++++++++++++++++++++
gdb/common/common-notif.h | 15 ++++
gdb/doc/gdb.texinfo | 19 +++++
gdb/gdbserver/Makefile.in | 7 +-
gdb/gdbserver/notif.c | 21 +++++
gdb/gdbserver/notif.h | 2 +
gdb/gdbserver/server.c | 15 ++++-
gdb/remote-notif.c | 32 +++++++-
gdb/remote-notif.h | 4 +
gdb/remote.c | 22 +++++-
11 files changed, 318 insertions(+), 9 deletions(-)
create mode 100644 gdb/common/common-notif.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 3bd4363..2e22ed0 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -508,7 +508,7 @@ 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-notif.o
+ remote-notif.o common-notif.o

# This is remote-sim.o if a simulator is to be linked in.
SIM_OBS = @SIM_OBS@
@@ -732,7 +732,8 @@ 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 remote-notif.c reverse.c \
+ regcache.c reggroups.c remote.c remote-fileio.c remote-notif.c \
+ common-notif.c reverse.c \
sentinel-frame.c \
serial.c ser-base.c ser-unix.c skip.c \
solib.c solib-target.c source.c \
@@ -1918,6 +1919,10 @@ common-utils.o: ${srcdir}/common/common-utils.c
$(COMPILE) $(srcdir)/common/common-utils.c
$(POSTCOMPILE)

+common-notif.o: ${srcdir}/common/common-notif.c
+ $(COMPILE) $(srcdir)/common/common-notif.c
+ $(POSTCOMPILE)
+
gdb_vecs.o: ${srcdir}/common/gdb_vecs.c
$(COMPILE) $(srcdir)/common/gdb_vecs.c
$(POSTCOMPILE)
diff --git a/gdb/common/common-notif.c b/gdb/common/common-notif.c
new file mode 100644
index 0000000..4f23865
--- /dev/null
+++ b/gdb/common/common-notif.c
@@ -0,0 +1,181 @@
+/* Copyright (C) 2013 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/>. */
+
+#ifdef GDBSERVER
+#include "server.h"
+#else
+#include "defs.h"
+#endif
+#include <string.h>
+#include "common-notif.h"
+#include "gdb_assert.h"
+
+/* Return a string about the notifications in array NOTIFS. NUM is
+ the number of elements in array NOTIFS. */
+
+char *
+notif_supported (struct notif_base *notifs[], int num)
+{
+ int i;
+ char * p = NULL;
+
+#define BUF_LEN 128
+
+ for (i = 0; i < num; i++)
+ {
+ struct notif_base *nb = notifs[i];
+
+ if (p == NULL)
+ {
+ p = xmalloc (BUF_LEN);
+ strcpy (p, nb->notif_name);
+ }
+ else
+ xsnprintf (p + strlen (p), BUF_LEN - strlen (p), ",%s",
+ nb->notif_name);
+
+ if (NOTIF_HAS_ANNEX (nb))
+ {
+ int j;
+
+ NOTIF_ITER_ANNEX (nb, j)
+ {
+ xsnprintf (p + strlen (p), BUF_LEN - strlen (p),
+ ".%s", nb->annexes[j].name);
+ }
+ }
+ }
+
+ return p;
+}
+
+/* Find annex in notification NB by name NAME and length LEN.
+ If found, return annex, otherwise return NULL. */
+
+static struct notif_annex *
+remote_notif_find_annex (struct notif_base *nb,
+ const char *name, int len)
+{
+ if (NOTIF_HAS_ANNEX (nb))
+ {
+ int j;
+
+ NOTIF_ITER_ANNEX (nb, j)
+ if (strncmp (name, nb->annexes[j].name, len) == 0
+ && len == strlen (nb->annexes[j].name))
+ return &nb->annexes[j];
+ }
+ return NULL;
+}
+
+/* Parse the REPLY, which is about supported annexes and
+ notifications in peer, and disable some annexes
+ if the remote stub doesn't support. */
+
+void
+notif_parse_supported (const char *reply,
+ struct notif_base *notifs[], int num)
+{
+ const char *p = reply;
+ int notif_num = 1;
+ char **notif_str;
+ int i;
+
+ for (i = 0; reply[i] != '\0'; i++)
+ if (reply[i] == ',')
+ notif_num++;
+
+ notif_str = xmalloc (notif_num * sizeof (char *));
+ for (i = 0; i < notif_num; i++)
+ {
+ char *end = strchr (p, ',');
+
+ if (end == NULL)
+ notif_str[i] = xstrdup (p);
+ else
+ {
+ /* Can't use xstrndup in GDBserver. */
+ notif_str[i] = strndup (p, end - p);
+ p = end + 1;
+ }
+ }
+
+ for (i = 0; i < notif_num; i++)
+ {
+ int j;
+ struct notif_base *nb = NULL;
+
+ p = notif_str[i];
+
+ for (j = 0; j < num; j++)
+ {
+ int name_len = strlen (notifs[j]->notif_name);
+
+ if (0 == strncmp (notifs[j]->notif_name, p, name_len)
+ && (p[name_len] == '.' || p[name_len] == 0))
+ {
+ nb = notifs[j];
+ p += name_len;
+ break;
+ }
+ }
+
+ if (nb != NULL)
+ {
+ if (p[0] == 0)
+ {
+ /* No annex. */
+ gdb_assert (!NOTIF_HAS_ANNEX (nb));
+ nb->annexes[0].supported = 1;
+ }
+ else if (p[0] == '.')
+ {
+ gdb_assert (NOTIF_HAS_ANNEX (nb));
+
+ p++;
+
+ /* Parse the rest of P and look for annexes. */
+ while (p != NULL)
+ {
+ char *end = strchr (p, '.');
+ struct notif_annex *annex = NULL;
+
+ if (end != NULL)
+ {
+ annex = remote_notif_find_annex (nb, p,
+ end - p);
+ p = end + 1;
+ }
+ else
+ {
+ annex = remote_notif_find_annex (nb, p,
+ strlen (p));
+ p = end;
+ }
+
+ /* If annex is known, mark it supported, otherwise
+ skip it because the peer knows the annex but we
+ don't know. */
+ if (annex != NULL)
+ annex->supported = 1;
+ }
+ }
+ else
+ warning (_("Unknown supported notification"));
+ }
+ }
+}
diff --git a/gdb/common/common-notif.h b/gdb/common/common-notif.h
index 1ffa26c..8e0e0c1 100644
--- a/gdb/common/common-notif.h
+++ b/gdb/common/common-notif.h
@@ -32,6 +32,11 @@ struct notif_annex
notification this annex belongs to. */
const char *name;

+ /* This annex is supported by the peer (GDB or GDBserver)? A
+ notification may have multiple annexes and some of them are
+ supported. Annex is the smallest unit of support. */
+ int supported;
+
#ifdef GDBSERVER
/* Write event EVENT to OWN_BUF. */
void (*write) (struct notif_event *event, char *own_buf);
@@ -53,6 +58,10 @@ struct notif_annex

#define NOTIF_HAS_ANNEX(NOTIF) ((NOTIF)->annexes[0].name != NULL)

+/* Whether the annex of notification N is supported. */
+
+#define NOTIF_ANNEX_SUPPORTED_P(N, INDEX) \
+ ((N).annexes[INDEX].supported)

/* "Base class" of a notification. It can be extended in both GDB
and GDBserver to represent a type of notification. */
@@ -71,3 +80,9 @@ struct notif_base
of the notification. */
struct notif_annex *annexes;
};
+
+char *notif_supported (struct notif_base *notifs[], int num);
+
+void notif_parse_supported (const char *reply,
+ struct notif_base *notifs[], int num);
+
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 8f08e82..0c25772 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -37113,6 +37113,14 @@ description.
This feature indicates whether @value{GDBN} supports the
@samp{qRelocInsn} packet (@pxref{Tracepoint Packets,,Relocate
instruction reply packet}).
+
+@item notifications
+@anchor{notifications feature}
+This feature indicates that @value{GDBN} supports the async remote
+notifications (@pxref{Notification Packets}). If the stub sees
+@samp{notifications=} with a string of supported notifications,
+separated by commas, it will report notifications supported by the
+stub.
@end table

Stubs should ignore any unknown values for
@@ -37301,6 +37309,11 @@ These are the currently defined stub features and their properties:
@tab @samp{-}
@tab No

+@item @samp{Notifications}
+@tab Yes
+@tab @samp{-}
+@tab No
+
@end multitable

These are the currently defined stub features, in more detail:
@@ -37455,6 +37468,12 @@ See @ref{Bytecode Descriptions} for details about the bytecode.
The remote stub supports running a breakpoint's command list itself,
rather than reporting the hit to @value{GDBN}.

+@item Notifications=@var{name}@r{[}.@var{annex}@r{]}@dots{}@r{[},@var{name}@r{[}.@var{annex}@r{]}@dots{}@r{]}@dots{}
+@cindex notifications, in remote protocol
+The remote stub supports a string of notifications. @var{name} is
+the name of the notification and @var{annex} is the name of the annex,
+if the notification has the annex.
+
@end table

@item qSymbol::
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index f8b1794..8533e54 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -155,7 +155,7 @@ SFILES= $(srcdir)/gdbreplay.c $(srcdir)/inferiors.c $(srcdir)/dll.c \
$(srcdir)/common/vec.c $(srcdir)/common/gdb_vecs.c \
$(srcdir)/common/common-utils.c $(srcdir)/common/xml-utils.c \
$(srcdir)/common/linux-osdata.c $(srcdir)/common/ptid.c \
- $(srcdir)/common/buffer.c
+ $(srcdir)/common/buffer.c $(srcdir)/common/common-notif.c

DEPFILES = @GDBSERVER_DEPFILES@

@@ -165,7 +165,7 @@ SOURCES = $(SFILES)
TAGFILES = $(SOURCES) ${HFILES} ${ALLPARAM} ${POSSLIBS}

OBS = agent.o ax.o inferiors.o regcache.o remote-utils.o server.o signals.o target.o \
- utils.o version.o vec.o gdb_vecs.o \
+ utils.o version.o vec.o gdb_vecs.o common-notif.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 notif.o \
@@ -517,6 +517,9 @@ linux-ptrace.o: ../common/linux-ptrace.c
common-utils.o: ../common/common-utils.c
$(COMPILE) $<
$(POSTCOMPILE)
+common-notif.o: ../common/common-notif.c
+ $(COMPILE) $<
+ $(POSTCOMPILE)
vec.o: ../common/vec.c
$(COMPILE) $<
$(POSTCOMPILE)
diff --git a/gdb/gdbserver/notif.c b/gdb/gdbserver/notif.c
index 168f77d..e6b3756 100644
--- a/gdb/gdbserver/notif.c
+++ b/gdb/gdbserver/notif.c
@@ -182,6 +182,27 @@ notif_event_xfree (struct notif_event *event)
xfree (event);
}

+/* Record the notifications supported by GDB. GDB_NOTIFICATIONS is a
+ string about notifications GDB supports. */
+
+void
+notif_qsupported_record (char *gdb_notifications)
+{
+ return notif_parse_supported (gdb_notifications,
+ (struct notif_base **) notifs,
+ ARRAY_SIZE (notifs));
+}
+
+/* Return a string about notifications that GDBserver supports.
+ Return NULL if no notification is supported. */
+
+char *
+notif_qsupported_reply (void)
+{
+ return notif_supported ((struct notif_base **) notifs,
+ ARRAY_SIZE (notifs));
+}
+
void
initialize_notif (void)
{
diff --git a/gdb/gdbserver/notif.h b/gdb/gdbserver/notif.h
index 5f9b6bc..72417aa 100644
--- a/gdb/gdbserver/notif.h
+++ b/gdb/gdbserver/notif.h
@@ -58,6 +58,8 @@ extern struct notif_server notif_stop;

int handle_notif_ack (char *own_buf, int packet_len);
void notif_write_event (struct notif_server *notif, char *own_buf);
+char* notif_qsupported_reply (void);
+void notif_qsupported_record (char *gdb_notifications);

void notif_push (struct notif_server *np, struct notif_event *event);
void notif_event_enque (struct notif_server *notif,
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index f98a780..61f08bd 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -185,7 +185,7 @@ vstop_notif_reply (struct notif_event *event, char *own_buf)

static struct notif_annex notif_annex_stop[] =
{
- { NULL, vstop_notif_reply, },
+ { NULL, 1, vstop_notif_reply, },
};

struct notif_server notif_stop =
@@ -1578,6 +1578,11 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
/* GDB supports relocate instruction requests. */
gdb_supports.qRelocInsn = 1;
}
+ else if (strncmp (p, "notifications=", 14) == 0)
+ {
+ /* Record what notifications GDB supports. */
+ notif_qsupported_record (&p[14]);
+ }
else
target_process_qsupported (p);

@@ -1659,6 +1664,14 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
if (target_supports_agent ())
strcat (own_buf, ";QAgent+");

+ p = notif_qsupported_reply ();
+
+ if (p != NULL)
+ {
+ strcat (own_buf, ";Notifications=");
+ strcat (own_buf, p);
+ xfree (p);
+ }
return;
}

diff --git a/gdb/remote-notif.c b/gdb/remote-notif.c
index e0bc745..ae05bbf 100644
--- a/gdb/remote-notif.c
+++ b/gdb/remote-notif.c
@@ -75,9 +75,16 @@ remote_notif_parse_1 (struct notif_client *nc,
contents in BUF. */
&& buf[strlen (m->name)] == ':')
{
- /* Pass BUF without annex and ':'. */
- m->parse (nc, buf + strlen (m->name) + 1, event);
- break;
+ if (m->supported)
+ {
+ /* Pass BUF without annex and ':'. */
+ m->parse (nc, buf + strlen (m->name) + 1, event);
+ break;
+ }
+ else
+ warning (_("GDB gets annex '%s' of notification '%s'"
+ "but remote stub doesn't support"),
+ base->notif_name, m->name);
}
m = NULL;
}
@@ -305,6 +312,25 @@ notif_xfree (struct notif_client *notif)
xfree (notif);
}

+/* Return a string of GDB supported features. */
+
+char *
+remote_notif_qsupported (void)
+{
+ return notif_supported ((struct notif_base **) notifs,
+ ARRAY_SIZE (notifs));
+}
+
+/* Parse the qSupported reply REPLY from the remote stub and disable
+ some notifications if the remote stub doesn't support. */
+
+void
+remote_notif_qsupported_reply (const char *reply)
+{
+ notif_parse_supported (reply, (struct notif_base **) notifs,
+ ARRAY_SIZE (notifs));
+}
+
/* -Wmissing-prototypes */
extern initialize_file_ftype _initialize_notif;

diff --git a/gdb/remote-notif.h b/gdb/remote-notif.h
index cb1636f..4859254 100644
--- a/gdb/remote-notif.h
+++ b/gdb/remote-notif.h
@@ -69,6 +69,10 @@ void remote_notif_register_async_event_handler (void);
void remote_notif_unregister_async_event_handler (void);

void remote_notif_process (struct notif_client *except);
+
+char * remote_notif_qsupported (void);
+void remote_notif_qsupported_reply (const char *reply);
+
extern struct notif_client notif_client_stop;

extern unsigned int notif_debug;
diff --git a/gdb/remote.c b/gdb/remote.c
index 92a0f27..e20ac80 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1284,6 +1284,7 @@ enum {
PACKET_qXfer_fdpic,
PACKET_QDisableRandomization,
PACKET_QAgent,
+ PACKET_notifications,
PACKET_MAX
};

@@ -3873,6 +3874,15 @@ remote_string_tracing_feature (const struct protocol_feature *feature,
rs->string_tracing = (support == PACKET_ENABLE);
}

+static void
+remote_notifications_feature (const struct protocol_feature *feature,
+ enum packet_support support,
+ const char *value)
+{
+ if (support == PACKET_ENABLE)
+ remote_notif_qsupported_reply (value);
+}
+
static struct protocol_feature remote_protocol_features[] = {
{ "PacketSize", PACKET_DISABLE, remote_packet_size, -1 },
{ "qXfer:auxv:read", PACKET_DISABLE, remote_supported_packet,
@@ -3938,6 +3948,8 @@ static struct protocol_feature remote_protocol_features[] = {
{ "QDisableRandomization", PACKET_DISABLE, remote_supported_packet,
PACKET_QDisableRandomization },
{ "QAgent", PACKET_DISABLE, remote_supported_packet, PACKET_QAgent},
+ { "Notifications", PACKET_DISABLE, remote_notifications_feature,
+ -1 },
{ "tracenz", PACKET_DISABLE,
remote_string_tracing_feature, -1 },
};
@@ -4004,6 +4016,7 @@ remote_query_supported (void)
if (remote_protocol_packets[PACKET_qSupported].support != PACKET_DISABLE)
{
char *q = NULL;
+ char *notifications = remote_notif_qsupported ();
struct cleanup *old_chain = make_cleanup (free_current_contents, &q);

q = remote_query_supported_append (q, "multiprocess+");
@@ -4013,6 +4026,10 @@ remote_query_supported (void)

q = remote_query_supported_append (q, "qRelocInsn+");

+ q = reconcat (q, q, ";notifications=", notifications,
+ (char *) NULL);
+ xfree (notifications);
+
q = reconcat (q, "qSupported:", q, (char *) NULL);
putpkt (q);

@@ -5211,7 +5228,10 @@ remote_notif_stop_alloc_reply (void)

static struct notif_annex notif_client_annex_stop[] =
{
- { NULL, remote_notif_stop_parse, },
+ /* Even the remote stub doesn't understand
+ 'qSupported:notifications=', it may still support notification
+ stop if it supports non-stop. */
+ { NULL, 1, remote_notif_stop_parse, },
};

/* A client of notification Stop. */
--
1.7.7.6