This is the mail archive of the
gdb-cvs@sourceware.org
mailing list for the GDB project.
[binutils-gdb] xml-support.c: Use std::string for growing string buffer
- From: Pedro Alves <palves at sourceware dot org>
- To: gdb-cvs at sourceware dot org
- Date: 18 Apr 2017 22:56:53 -0000
- Subject: [binutils-gdb] xml-support.c: Use std::string for growing string buffer
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=bd8a901f9e34191e0645a5527556d124ba5c345a
commit bd8a901f9e34191e0645a5527556d124ba5c345a
Author: Pedro Alves <palves@redhat.com>
Date: Tue Apr 18 21:39:25 2017 +0100
xml-support.c: Use std::string for growing string buffer
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:
2017-04-18 Pedro Alves <palves@redhat.com>
* tracefile-tfile.c (tfile_write_tdesc): Adjust to use
gdb::optional<std::string>.
* xml-support.c: Include <string>.
(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 <unordered_map> and <string>.
(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<std::string>, and adjust.
* xml-tdesc.h: Include "common/gdb_optional.h" and <string>.
(target_fetch_description_xml): Change return type to
gdb::optional<std::string>.
Diff:
---
gdb/ChangeLog | 29 ++++++++++++++
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 +++++++--
6 files changed, 117 insertions(+), 143 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e63ae39..2efc14f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,34 @@
2017-04-18 Pedro Alves <palves@redhat.com>
+ * tracefile-tfile.c (tfile_write_tdesc): Adjust to use
+ gdb::optional<std::string>.
+ * xml-support.c: Include <string>.
+ (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 <unordered_map> and <string>.
+ (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<std::string>, and adjust.
+ * xml-tdesc.h: Include "common/gdb_optional.h" and <string>.
+ (target_fetch_description_xml): Change return type to
+ gdb::optional<std::string>.
+
+2017-04-18 Pedro Alves <palves@redhat.com>
+
* Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
unittests/optional-selftests.c.
(SUBDIR_UNITTESTS_OBS): Add optional-selftests.o.
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<std::string> 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 d6c940c..3c5f9a1 100644
--- a/gdb/xml-support.c
+++ b/gdb/xml-support.c
@@ -23,6 +23,7 @@
#include "filestuff.h"
#include "safe-ctype.h"
#include <vector>
+#include <string>
/* 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 <xi:include> 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
- <xi:include> 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 <xi:include> 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 <unordered_map>
+#include <string>
/* 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<std::string, target_desc *> 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<std::string>
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<char>
+ 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 <http://www.gnu.org/licenses/>. */
+#ifndef XML_TDESC_H
+#define XML_TDESC_H
+
+#include "common/gdb_optional.h"
+#include <string>
+
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<std::string> target_fetch_description_xml (target_ops *ops);
+
+#endif /* XML_TDESC_H */
-char *target_fetch_description_xml (struct target_ops *ops);