From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15629 invoked by alias); 30 Mar 2010 13:55:35 -0000 Received: (qmail 15525 invoked by uid 22791); 30 Mar 2010 13:55:33 -0000 X-SWARE-Spam-Status: No, hits=-1.6 required=5.0 tests=BAYES_00,SARE_SUB_OBFU_Q1,TW_QS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 30 Mar 2010 13:55:25 +0000 Received: (qmail 11756 invoked from network); 30 Mar 2010 13:55:23 -0000 Received: from unknown (HELO orlando.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 30 Mar 2010 13:55:23 -0000 From: Pedro Alves To: gdb-patches@sourceware.org, "H.J. Lu" Subject: Re: PATCH: Add xmlRegisters= to qsupported query Date: Tue, 30 Mar 2010 13:55:00 -0000 User-Agent: KMail/1.12.2 (Linux/2.6.31-20-generic; KDE/4.3.2; x86_64; ; ) References: <20100328204807.GA10649@intel.com> <20100329161910.GA10204@intel.com> <20100329180136.GA14704@intel.com> In-Reply-To: <20100329180136.GA14704@intel.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201003301455.21014.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-03/txt/msg01044.txt.bz2 On Monday 29 March 2010 19:01:36, H.J. Lu wrote: > 2010-03-29 H.J. Lu > > * i386-tdep.c: Include "remote.h". > (i386_gdbarch_init): Call register_remote_support_xml. > > * remote.c (remote_support_xml): New. > (register_remote_support_xml): Likewise. > (remote_query_supported_append): Likewise. > (remote_query_supported): Support remote_support_xml. > > * remote.h (register_remote_support_xml): New. > > gdb/doc/ > > 2010-03-29 H.J. Lu > > * gdb.texinfo (General Query Packets): Add xmlRegisters. > > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index 56dbe5d..15da725 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -30603,6 +30603,13 @@ extensions to the remote protocol. @value{GDBN} does not use such > extensions unless the stub also reports that it supports them by > including @samp{multiprocess+} in its @samp{qSupported} reply. > @xref{multiprocess extensions}, for details. > + > +@item xmlRegisters > +This feature indicates that @value{GDBN} supports the XML target > +description. If the stub sees @samp{xmlRegisters=} with target > +specific strings separated by a comma, it will send either an XML > +target description or an XML stub that states only the architecture > +and the OSABI. > @end table There's some confusion here. A target description can describe more than the target's register set. If the stub sees the xmlRegisters feature with a value included its architecture, it may safely include the register description in the target description, as GDB will understand it. I don't think it's a good idea to say here what _other_ things the stub will send. Why not explain the problem we're solving there? You could even copy Daniel's nice problem statement somewhere upthread. I think this should also be mentioned in NEWS. We document every new RSP packet and qSupported feature there. > > Stubs should ignore any unknown values for > diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c > index 83275ac..0199026 100644 > --- a/gdb/i386-tdep.c > +++ b/gdb/i386-tdep.c > @@ -44,6 +44,7 @@ > #include "value.h" > #include "dis-asm.h" > #include "disasm.h" > +#include "remote.h" > > #include "gdb_assert.h" > #include "gdb_string.h" > @@ -5943,6 +5944,9 @@ i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) > set_gdbarch_fast_tracepoint_valid_at (gdbarch, > i386_fast_tracepoint_valid_at); > > + /* Tell remote stub that we support XML target description. */ > + register_remote_support_xml ("i386"); > + > return gdbarch; > } Daniel suggested doing this from _initialize_i386_tdep, IIRC. Did that not work? That would make this new registration function only called once, instead of once for every i386 gdbarch. > > diff --git a/gdb/remote.c b/gdb/remote.c > index 0c791aa..25249d1 100644 > --- a/gdb/remote.c > +++ b/gdb/remote.c > @@ -3464,6 +3464,57 @@ static struct protocol_feature remote_protocol_features[] = { > PACKET_bs }, > }; > > +static char *remote_support_xml; > + > +/* Register string appended to "xmlRegisters=" in qSupported query. */ > + > +void > +register_remote_support_xml (const char *xml ATTRIBUTE_UNUSED) > +{ > +#if defined(HAVE_LIBEXPAT) > + if (remote_support_xml == NULL) > + remote_support_xml = concat ("xmlRegisters=", xml, NULL); > + else > + { > + const char *str = strstr (remote_support_xml, xml); > + int append; > + > + if (str) > + { > + if (str != remote_support_xml && *(str - 1) != ',') > + append = 0; > + else > + { > + size_t len = strlen (xml); > + append = xml[len] != ',' && xml[len] != '\0'; > + } I don't understand this. How can `xml[len]' ever be == ',' ? Did uou mean `src[len]' ? I'd have to say that using strtok to loop over all ',' separated archs, and strcmp to check if it's already there would probably avoid such confusions. Something like this: tok = strtok (remote_support_xml, ",")); do { if (strcmp (tok, xml) == 0) return; /* already there */ } while ((tok = strtok (NULL, ",")) != NULL); /* append */ char *p = concat (remote_support_xml, ",", xml, NULL); xfree (remote_support_xml); remote_support_xml = p; > + } > + else > + append = 1; > + > + if (append) > + { > + char *p = concat (remote_support_xml, ",", xml, NULL); > + xfree (remote_support_xml); > + remote_support_xml = p; > + } > + } > +#endif > +} > + > +static char * > +remote_query_supported_append (char *msg, const char *append) > +{ > + if (msg) > + { > + char *p = concat (msg, ";", append, NULL); > + xfree (msg); > + return p; > + } > + else > + return xstrdup (append); > +} > + > static void > remote_query_supported (void) > { > @@ -3482,24 +3533,27 @@ remote_query_supported (void) > rs->buf[0] = 0; > if (remote_protocol_packets[PACKET_qSupported].support != PACKET_DISABLE) > { > + char *q = NULL; > const char *qsupported = gdbarch_qsupported (target_gdbarch); > + > + if (rs->extended) > + q = remote_query_supported_append (q, "multiprocess+;"); Drop the ';' here as remote_query_supported_append takes care of adding it. -- Pedro Alves