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>.

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 (&current_target);
-  char *ptr = tdesc;
-  char *next;
 
-  if (tdesc == NULL)
+  gdb::optional<std::string> tdesc
+    = target_fetch_description_xml (&current_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);