From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12093 invoked by alias); 5 Mar 2013 20:07:38 -0000 Received: (qmail 12070 invoked by uid 22791); 5 Mar 2013 20:07:37 -0000 X-SWARE-Spam-Status: No, hits=-6.5 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_SPAMHAUS_DROP,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_BJ X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 05 Mar 2013 20:07:24 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r25K7M3V012349 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 5 Mar 2013 15:07:22 -0500 Received: from host2.jankratochvil.net (ovpn-116-50.ams2.redhat.com [10.36.116.50]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r25K7Gpa032345 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Tue, 5 Mar 2013 15:07:19 -0500 Date: Tue, 05 Mar 2013 20:07:00 -0000 From: Jan Kratochvil To: Markus Metzger Cc: gdb-patches@sourceware.org, markus.t.metzger@gmail.com Subject: Re: [patch v9 04/23] xml, btrace: define btrace xml document style Message-ID: <20130305200716.GD2386@host2.jankratochvil.net> References: <1362416770-19750-1-git-send-email-markus.t.metzger@intel.com> <1362416770-19750-5-git-send-email-markus.t.metzger@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1362416770-19750-5-git-send-email-markus.t.metzger@intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) 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: 2013-03/txt/msg00167.txt.bz2 On Mon, 04 Mar 2013 18:05:51 +0100, Markus Metzger wrote: > Define the xml document style for transferring branch trace data. > > Add a function to parse a btrace xml document into a vector of branch trace > blocks. > > 2013-03-04 Markus Metzger > > * features/btrace.dtd: New file. > * Makefile.in (XMLFILES): Add btrace.dtd. > * btrace.h (parse_xml_btrace): New declaration. > * btrace.c: Include xml-support.h. > (parse_xml_btrace): New function. > (parse_xml_btrace_block): New function. > (block_attributes): New struct. > (btrace_attributes): New struct. > (btrace_children): New struct. > (btrace_elements): New struct. [...] > --- a/gdb/btrace.c > +++ b/gdb/btrace.c > @@ -29,6 +29,7 @@ > #include "disasm.h" > #include "source.h" > #include "filenames.h" > +#include "xml-support.h" > > /* Print a record debug message. Use do ... while (0) to avoid ambiguities > when used in if statements. */ > @@ -465,3 +466,97 @@ btrace_free_objfile (struct objfile *objfile) > ALL_THREADS (tp) > btrace_clear (tp); > } > + > +#if defined(HAVE_LIBEXPAT) There should be space before '(' according to GNU Coding Standards. It is 7 times in the whole patchet, ther other instances I have not commented. Choose one: #ifdef HAVE_LIBEXPAT #if defined HAVE_LIBEXPAT #if defined (HAVE_LIBEXPAT) > + > +/* Check the btrace document version. */ > + > +static void > +check_xml_btrace_version (struct gdb_xml_parser *parser, > + const struct gdb_xml_element *element, > + void *user_data, VEC (gdb_xml_value_s) *attributes) > +{ > + const char *version = xml_find_attribute (attributes, "version")->value; > + > + if (strcmp (version, "1.0") != 0) > + gdb_xml_error (parser, _("Unsupported btrace version: \"%s\""), version); > +} > + > +/* Parse a btrace "block" xml record. */ > + > +static void > +parse_xml_btrace_block (struct gdb_xml_parser *parser, > + const struct gdb_xml_element *element, > + void *user_data, VEC (gdb_xml_value_s) *attributes) > +{ > + VEC (btrace_block_s) **btrace; > + struct btrace_block *block; > + ULONGEST *begin, *end; > + > + btrace = user_data; > + block = VEC_safe_push (btrace_block_s, *btrace, NULL); > + > + begin = xml_find_attribute (attributes, "begin")->value; > + end = xml_find_attribute (attributes, "end")->value; > + > + block->begin = *begin; > + block->end = *end; > +} > + > +static const struct gdb_xml_attribute block_attributes[] = { > + { "begin", GDB_XML_AF_NONE, gdb_xml_parse_attr_ulongest, NULL }, > + { "end", GDB_XML_AF_NONE, gdb_xml_parse_attr_ulongest, NULL }, > + { NULL, GDB_XML_AF_NONE, NULL, NULL } > +}; > + > +static const struct gdb_xml_attribute btrace_attributes[] = { > + { "version", GDB_XML_AF_NONE, NULL, NULL }, > + { NULL, GDB_XML_AF_NONE, NULL, NULL } > +}; > + > +static const struct gdb_xml_element btrace_children[] = { > + { "block", block_attributes, NULL, > + GDB_XML_EF_REPEATABLE | GDB_XML_EF_OPTIONAL, parse_xml_btrace_block, NULL }, > + { NULL, NULL, NULL, GDB_XML_EF_NONE, NULL, NULL } > +}; > + > +static const struct gdb_xml_element btrace_elements[] = { > + { "btrace", btrace_attributes, btrace_children, GDB_XML_EF_NONE, > + check_xml_btrace_version, NULL }, > + { NULL, NULL, NULL, GDB_XML_EF_NONE, NULL, NULL } > +}; > + > +#endif /* defined(HAVE_LIBEXPAT) */ > + > +/* See btrace.h. */ > + > +VEC (btrace_block_s) * > +parse_xml_btrace (const char *buffer) > +{ > + VEC (btrace_block_s) *btrace = NULL; > + struct cleanup *cleanup; > + int errcode; > + > +#if defined(HAVE_LIBEXPAT) > + > + cleanup = make_cleanup (VEC_cleanup (btrace_block_s), &btrace); > + errcode = gdb_xml_parse_quick (_("btrace"), "btrace.dtd", btrace_elements, > + buffer, &btrace); > + if (errcode != 0) > + { > + do_cleanups (cleanup); > + errno = errcode; gdb_xml_parse_quick returns 0 or -1, -1 is not valid errno code. Also I do not see errno used by the caller of parse_xml_btrace. Remove that errno setting, returning NULL is enough. > + return NULL; > + } > + > + /* Keep parse results. */ > + discard_cleanups (cleanup); > + > +#else /* !defined(HAVE_LIBEXPAT) */ > + > + error (_("Cannot process branch tracing result. XML parsing not supported.")); Two spaces after dot ('.'). Maybe also s/not/is not/. > + > +#endif /* !defined(HAVE_LIBEXPAT) */ > + > + return btrace; > +} > diff --git a/gdb/btrace.h b/gdb/btrace.h > index a1b01c8..405bcff 100644 > --- a/gdb/btrace.h > +++ b/gdb/btrace.h > @@ -132,4 +132,7 @@ extern void btrace_clear (struct thread_info *); > /* Clear the branch trace for all threads when an object file goes away. */ > extern void btrace_free_objfile (struct objfile *); > > +/* Parse a branch trace xml document into a block vector. */ > +extern VEC (btrace_block_s) *parse_xml_btrace (const char*); > + > #endif /* BTRACE_H */ > diff --git a/gdb/features/btrace.dtd b/gdb/features/btrace.dtd > new file mode 100644 > index 0000000..18c5b2a > --- /dev/null > +++ b/gdb/features/btrace.dtd > @@ -0,0 +1,12 @@ > + > + > + > + > + > + > + + end CDATA #REQUIRED> > -- > 1.7.1 OK for check in with those few small changes. Thanks, Jan