From patchwork Mon Apr 10 12:22:26 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 19938 Received: (qmail 46237 invoked by alias); 10 Apr 2017 12:22:40 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 46164 invoked by uid 89); 10 Apr 2017 12:22:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=798 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 10 Apr 2017 12:22:34 +0000 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id CBF0D85543 for ; Mon, 10 Apr 2017 12:22:34 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com CBF0D85543 Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=palves@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com CBF0D85543 Received: from cascais.lan (ovpn04.gateway.prod.ext.phx2.redhat.com [10.5.9.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 26767B23FD for ; Mon, 10 Apr 2017 12:22:33 +0000 (UTC) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH 5/7] xml-support.c: Use std::string for growing string buffer Date: Mon, 10 Apr 2017 13:22:26 +0100 Message-Id: <1491826948-21874-6-git-send-email-palves@redhat.com> In-Reply-To: <1491826948-21874-1-git-send-email-palves@redhat.com> References: <1491826948-21874-1-git-send-email-palves@redhat.com> This main idea behind this patch is this change to xml-support.c:scope_level - /* Body text accumulation. This is an owning pointer. */ - struct obstack *body; + /* Body text accumulation. */ + std::string body; ... which allows simplifying other parts of the code. In target_fetch_description_xml, we want to distinguish between returning "success + empty std::string" and "no success", and gdb::optional is a natural fit for that. gdb/ChangeLog: yyyy-mm-dd Pedro Alves * tracefile-tfile.c (tfile_write_tdesc): Adjust to use gdb::optional. * xml-support.c: Include . (scope_level::scope_level(scope_level &&)) (scope_level::~scope_level): Delete. (scope_level::body): Now a std::string. (gdb_xml_body_text, gdb_xml_end_element): Adjust. (xinclude_parsing_data::xinclude_parsing_data): Add 'output' parameter. (xinclude_parsing_data::~xinclude_parsing_data): Delete. (xinclude_parsing_data::output): Now a std::string reference. (xinclude_start_include): Adjust. (xml_xinclude_default): Adjust. (xml_process_xincludes): Add 'output' parameter, and return bool. * xml-support.h (xml_process_xincludes): Add 'output' parameter, and return bool. * xml-tdesc.c: Include and . (tdesc_xml_cache): Delete. (tdesc_xml_cache_s): Delete. (xml_cache): Now an std::unordered_map. (tdesc_parse_xml): Adjust to use std::string and unordered_map. (target_fetch_description_xml): Change return type to gdb::optional, and adjust. * xml-tdesc.h: Include "common/gdb_optional.h" and . (target_fetch_description_xml): Change return type to gdb::optional. --- gdb/tracefile-tfile.c | 14 +++---- gdb/xml-support.c | 104 +++++++++++++++----------------------------------- gdb/xml-support.h | 15 ++++---- gdb/xml-tdesc.c | 81 ++++++++++++++------------------------- gdb/xml-tdesc.h | 17 +++++++-- 5 files changed, 88 insertions(+), 143 deletions(-) diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c index 5d63c16..255bbc9 100644 --- a/gdb/tracefile-tfile.c +++ b/gdb/tracefile-tfile.c @@ -275,17 +275,19 @@ tfile_write_tdesc (struct trace_file_writer *self) { struct tfile_trace_file_writer *writer = (struct tfile_trace_file_writer *) self; - char *tdesc = target_fetch_description_xml (¤t_target); - char *ptr = tdesc; - char *next; - if (tdesc == NULL) + gdb::optional tdesc + = target_fetch_description_xml (¤t_target); + + if (!tdesc) return; + const char *ptr = tdesc->c_str (); + /* Write tdesc line by line, prefixing each line with "tdesc ". */ while (ptr != NULL) { - next = strchr (ptr, '\n'); + const char *next = strchr (ptr, '\n'); if (next != NULL) { fprintf (writer->fp, "tdesc %.*s\n", (int) (next - ptr), ptr); @@ -299,8 +301,6 @@ tfile_write_tdesc (struct trace_file_writer *self) } ptr = next; } - - xfree (tdesc); } /* This is the implementation of trace_file_write_ops method diff --git a/gdb/xml-support.c b/gdb/xml-support.c index b19f81a..91e31d9 100644 --- a/gdb/xml-support.c +++ b/gdb/xml-support.c @@ -23,6 +23,7 @@ #include "filestuff.h" #include "safe-ctype.h" #include +#include /* Debugging flag. */ static int debug_xml; @@ -46,29 +47,9 @@ struct scope_level explicit scope_level (const gdb_xml_element *elements_ = NULL) : elements (elements_), element (NULL), - seen (0), - body (NULL) + seen (0) {} - scope_level (scope_level &&other) noexcept - : elements (other.elements), - element (other.element), - seen (other.seen), - body (other.body) - { - if (this != &other) - other.body = NULL; - } - - ~scope_level () - { - if (this->body) - { - obstack_free (this->body, NULL); - xfree (this->body); - } - } - /* Elements we allow at this level. */ const struct gdb_xml_element *elements; @@ -79,8 +60,8 @@ struct scope_level optional and repeatable checking). */ unsigned int seen; - /* Body text accumulation. This is an owning pointer. */ - struct obstack *body; + /* Body text accumulation. */ + std::string body; }; /* The parser itself, and our additional state. */ @@ -120,14 +101,7 @@ gdb_xml_body_text (void *data, const XML_Char *text, int length) return; scope_level &scope = parser->scopes.back (); - - if (scope.body == NULL) - { - scope.body = XCNEW (struct obstack); - obstack_init (scope.body); - } - - obstack_grow (scope.body, text, length); + scope.body.append (text, length); } /* Issue a debugging message from one of PARSER's handlers. */ @@ -390,29 +364,27 @@ gdb_xml_end_element (gdb_xml_parser *parser, const XML_Char *name) /* Call the element processor. */ if (scope->element != NULL && scope->element->end_handler) { - const char *scope_body; + const char *body; - if (scope->body == NULL) - scope_body = ""; + if (scope->body.empty ()) + body = ""; else { int length; - length = obstack_object_size (scope->body); - obstack_1grow (scope->body, '\0'); - char *body = (char *) obstack_finish (scope->body); + length = scope->body.size (); + body = scope->body.c_str (); /* Strip leading and trailing whitespace. */ - while (length > 0 && ISSPACE (body[length-1])) - body[--length] = '\0'; + while (length > 0 && ISSPACE (body[length - 1])) + length--; + scope->body.erase (length); while (*body && ISSPACE (*body)) body++; - - scope_body = body; } scope->element->end_handler (parser, scope->element, parser->user_data, - scope_body); + body); } else if (scope->element == NULL) XML_DefaultCurrent (parser->expat_parser); @@ -726,23 +698,18 @@ gdb_xml_parse_attr_enum (struct gdb_xml_parser *parser, struct xinclude_parsing_data { - xinclude_parsing_data (xml_fetch_another fetcher_, void *fetcher_baton_, + xinclude_parsing_data (std::string &output_, + xml_fetch_another fetcher_, void *fetcher_baton_, int include_depth_) - : skip_depth (0), + : output (output_), + skip_depth (0), include_depth (include_depth_), fetcher (fetcher_), fetcher_baton (fetcher_baton_) - { - obstack_init (&this->obstack); - } - - ~xinclude_parsing_data () - { - obstack_free (&this->obstack, NULL); - } + {} - /* The obstack to build the output in. */ - struct obstack obstack; + /* Where the output goes. */ + std::string &output; /* A count indicating whether we are in an element whose children should not be copied to the output, and if so, @@ -782,15 +749,11 @@ xinclude_start_include (struct gdb_xml_parser *parser, gdb_xml_error (parser, _("Could not load XML document \"%s\""), href); back_to = make_cleanup (xfree, text); - output = xml_process_xincludes (parser->name, text, data->fetcher, - data->fetcher_baton, - data->include_depth + 1); - if (output == NULL) + if (!xml_process_xincludes (data->output, parser->name, text, data->fetcher, + data->fetcher_baton, + data->include_depth + 1)) gdb_xml_error (parser, _("Parsing \"%s\" failed"), href); - obstack_grow (&data->obstack, output, strlen (output)); - xfree (output); - do_cleanups (back_to); data->skip_depth++; @@ -821,7 +784,7 @@ xml_xinclude_default (void *data_, const XML_Char *s, int len) /* Otherwise just add it to the end of the document we're building up. */ - obstack_grow (&data->obstack, s, len); + data->output.append (s, len); } static void XMLCALL @@ -871,14 +834,13 @@ const struct gdb_xml_element xinclude_elements[] = { /* The main entry point for processing. */ -char * -xml_process_xincludes (const char *name, const char *text, +bool +xml_process_xincludes (std::string &result, + const char *name, const char *text, xml_fetch_another fetcher, void *fetcher_baton, int depth) { - char *result = NULL; - - xinclude_parsing_data data (fetcher, fetcher_baton, depth); + xinclude_parsing_data data (result, fetcher, fetcher_baton, depth); gdb_xml_parser parser (name, xinclude_elements, &data); parser.is_xinclude = true; @@ -901,16 +863,12 @@ xml_process_xincludes (const char *name, const char *text, if (gdb_xml_parse (&parser, text) == 0) { - obstack_1grow (&data.obstack, '\0'); - result = xstrdup ((const char *) obstack_finish (&data.obstack)); - if (depth == 0) gdb_xml_debug (&parser, _("XInclude processing succeeded.")); + return true; } - else - result = NULL; - return result; + return false; } #endif /* HAVE_LIBEXPAT */ diff --git a/gdb/xml-support.h b/gdb/xml-support.h index de685e2..8248a32 100644 --- a/gdb/xml-support.h +++ b/gdb/xml-support.h @@ -54,17 +54,18 @@ extern const char *xml_builtin[][2]; typedef char *(*xml_fetch_another) (const char *href, void *baton); -/* Return a new string which is the expansion of TEXT after processing - tags. FETCHER will be called (with FETCHER_BATON) to - retrieve any new files. DEPTH should be zero on the initial call. +/* Append the expansion of TEXT after processing tags in + RESULT. FETCHER will be called (with FETCHER_BATON) to retrieve + any new files. DEPTH should be zero on the initial call. - On failure, this function uses NAME in a warning and returns NULL. + On failure, this function uses NAME in a warning and returns false. It may throw an exception, but does not for XML parsing problems. */ -char *xml_process_xincludes (const char *name, const char *text, - xml_fetch_another fetcher, void *fetcher_baton, - int depth); +bool xml_process_xincludes (std::string &result, + const char *name, const char *text, + xml_fetch_another fetcher, void *fetcher_baton, + int depth); /* Simplified XML parser infrastructure. */ diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c index 522a9ba..daa713f 100644 --- a/gdb/xml-tdesc.c +++ b/gdb/xml-tdesc.c @@ -26,6 +26,8 @@ #include "xml-tdesc.h" #include "osabi.h" #include "filenames.h" +#include +#include /* Maximum sizes. This is just to catch obviously wrong values. */ @@ -64,15 +66,7 @@ tdesc_parse_xml (const char *document, xml_fetch_another fetcher, then we will create unnecessary duplicate gdbarches. See gdbarch_list_lookup_by_info. */ -struct tdesc_xml_cache -{ - const char *xml_document; - struct target_desc *tdesc; -}; -typedef struct tdesc_xml_cache tdesc_xml_cache_s; -DEF_VEC_O(tdesc_xml_cache_s); - -static VEC(tdesc_xml_cache_s) *xml_cache; +static std::unordered_map xml_cache; /* Callback data for target description parsing. */ @@ -631,55 +625,42 @@ static struct target_desc * tdesc_parse_xml (const char *document, xml_fetch_another fetcher, void *fetcher_baton) { - struct cleanup *back_to, *result_cleanup; struct tdesc_parsing_data data; - struct tdesc_xml_cache *cache; - char *expanded_text; - int ix; /* Expand all XInclude directives. */ - expanded_text = xml_process_xincludes (_("target description"), - document, fetcher, fetcher_baton, 0); - if (expanded_text == NULL) + std::string expanded_text; + + if (!xml_process_xincludes (expanded_text, + _("target description"), + document, fetcher, fetcher_baton, 0)) { warning (_("Could not load XML target description; ignoring")); return NULL; } /* Check for an exact match in the list of descriptions we have - previously parsed. strcmp is a slightly inefficient way to - do this; an SHA-1 checksum would work as well. */ - for (ix = 0; VEC_iterate (tdesc_xml_cache_s, xml_cache, ix, cache); ix++) - if (strcmp (cache->xml_document, expanded_text) == 0) - { - xfree (expanded_text); - return cache->tdesc; - } - - back_to = make_cleanup (null_cleanup, NULL); + previously parsed. */ + const auto it = xml_cache.find (expanded_text); + if (it != xml_cache.end ()) + return it->second; memset (&data, 0, sizeof (struct tdesc_parsing_data)); data.tdesc = allocate_target_description (); - result_cleanup = make_cleanup_free_target_description (data.tdesc); - make_cleanup (xfree, expanded_text); + struct cleanup *result_cleanup + = make_cleanup_free_target_description (data.tdesc); if (gdb_xml_parse_quick (_("target description"), "gdb-target.dtd", - tdesc_elements, expanded_text, &data) == 0) + tdesc_elements, expanded_text.c_str (), &data) == 0) { /* Parsed successfully. */ - struct tdesc_xml_cache new_cache; - - new_cache.xml_document = expanded_text; - new_cache.tdesc = data.tdesc; - VEC_safe_push (tdesc_xml_cache_s, xml_cache, &new_cache); + xml_cache.emplace (std::move (expanded_text), data.tdesc); discard_cleanups (result_cleanup); - do_cleanups (back_to); return data.tdesc; } else { warning (_("Could not load XML target description; ignoring")); - do_cleanups (back_to); + do_cleanups (result_cleanup); return NULL; } } @@ -759,7 +740,7 @@ target_read_description_xml (struct target_ops *ops) includes, but not parsing it. Used to dump whole tdesc as a single XML file. */ -char * +gdb::optional target_fetch_description_xml (struct target_ops *ops) { #if !defined(HAVE_LIBEXPAT) @@ -772,28 +753,24 @@ target_fetch_description_xml (struct target_ops *ops) "disabled at compile time")); } - return NULL; + return {}; #else struct target_desc *tdesc; - char *tdesc_str; - char *expanded_text; - struct cleanup *back_to; - tdesc_str = fetch_available_features_from_target ("target.xml", ops); + gdb::unique_xmalloc_ptr + tdesc_str (fetch_available_features_from_target ("target.xml", ops)); if (tdesc_str == NULL) - return NULL; + return {}; - back_to = make_cleanup (xfree, tdesc_str); - expanded_text = xml_process_xincludes (_("target description"), - tdesc_str, - fetch_available_features_from_target, ops, 0); - do_cleanups (back_to); - if (expanded_text == NULL) + std::string output; + if (!xml_process_xincludes (output, + _("target description"), + tdesc_str.get (), + fetch_available_features_from_target, ops, 0)) { warning (_("Could not load XML target description; ignoring")); - return NULL; + return {}; } - - return expanded_text; + return output; #endif } diff --git a/gdb/xml-tdesc.h b/gdb/xml-tdesc.h index 1637a89..2375395 100644 --- a/gdb/xml-tdesc.h +++ b/gdb/xml-tdesc.h @@ -19,6 +19,12 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . */ +#ifndef XML_TDESC_H +#define XML_TDESC_H + +#include "common/gdb_optional.h" +#include + struct target_ops; struct target_desc; @@ -32,8 +38,11 @@ const struct target_desc *file_read_description_xml (const char *filename); const struct target_desc *target_read_description_xml (struct target_ops *); -/* Fetches an XML target description using OPS, processing - includes, but not parsing it. Used to dump whole tdesc - as a single XML file. */ +/* Fetches an XML target description using OPS, processing includes, + but not parsing it. Used to dump whole tdesc as a single XML file. + Returns the description on success, and a disengaged optional + otherwise. */ +gdb::optional target_fetch_description_xml (target_ops *ops); + +#endif /* XML_TDESC_H */ -char *target_fetch_description_xml (struct target_ops *ops);