xml-support.c: Use std::string for growing string buffer
authorPedro Alves <palves@redhat.com>
Tue, 18 Apr 2017 20:39:25 +0000 (21:39 +0100)
committerPedro Alves <palves@redhat.com>
Tue, 18 Apr 2017 22:50:55 +0000 (23:50 +0100)
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>.

gdb/ChangeLog
gdb/tracefile-tfile.c
gdb/xml-support.c
gdb/xml-support.h
gdb/xml-tdesc.c
gdb/xml-tdesc.h

index e63ae3901f7bf8230217f9a2f8af0679a17dd8cf..2efc14f354598f0f78649101434ddccfbb6013e3 100644 (file)
@@ -1,3 +1,32 @@
+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
index 5d63c16e1503baf517ddbcbac107b1a8d27d05c8..255bbc9ef96466abf75a5d3f4e506198bd0956ef 100644 (file)
@@ -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
index d6c940c0de95c8dc41ec7d00f715ad454354c609..3c5f9a1ad0d0a905f5ccd128a5959f95ba34bdd5 100644 (file)
@@ -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 */
 \f
index de685e2d10a062c6d9d78f8dcdb18242aa3216df..8248a329a9981a9ae3214aa3fe84837e5892867b 100644 (file)
@@ -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.  */
 
index 522a9ba0987365bb0d532ad29cb130e6f42eee69..daa713f13f16835b3c04c0448caf1e010765b318 100644 (file)
@@ -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
 }
index 1637a899c24d160a93d0ed4200e6c5397fee8e33..237539562603222651c36d9642ffd15991faf8d4 100644 (file)
    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);