C++ify osdata
authorSimon Marchi <simon.marchi@polymtl.ca>
Wed, 22 Nov 2017 21:12:06 +0000 (16:12 -0500)
committerSimon Marchi <simon.marchi@ericsson.com>
Wed, 22 Nov 2017 21:12:06 +0000 (16:12 -0500)
This patch c++ifies the osdata structure: osdata_column, osdata_item and
osdata.  char* are replaced with std::string and VEC are replaced with
std::vector.  This allows to get rid of a great deal of cleanup and
free'ing code.

I replaced the splay tree in list_available_thread_groups with an
std::map.  Unless there's a good advantage to keep using a splay tree,
I think using the standard type should make things simpler to
understand.

gdb/ChangeLog:

* osdata.h: Include vector isntead of vec.h.
(osdata_column_s): Remove typedef.
(struct osdata_column): Add constructor.
<name, value>: Change type to std::string.
(DEF_VEC_O (osdata_column_s)): Remove.
(osdata_item_s): Remove typedef.
(struct osdata_item) <columns>: Change type to std::vector.
(DEF_VEC_O (osdata_item_s)): Remove.
(struct osdata): Add constructor.
<type>: Change type to std::string.
<items>: Change type to std::vector.
(osdata_p): Remove typedef.
(DEF_VEC_P (osdata_p)): Remove.
(osdata_parse): Return a unique_ptr.
(osdata_free): Remove.
(make_cleanup_osdata_free): Remove.
(get_osdata): Return a unique_ptr.
(get_osdata_column): Return pointer to std::string, take a
reference to osdata_item as parameter.
* osdata.c (struct osdata_parsing_data) <osdata>: Change type to
unique_ptr.
<property_name>: Change type to std::string.
(osdata_start_osdata): Allocate osdata with new and adjust.
(osdata_start_item): Adjust.
(osdata_start_column): Adjust.
(osdata_end_column): Adjust.
(clear_parsing_data): Remove.
(osdata_parse): Return a unique_ptr and adjust, remove cleanup.
(osdata_item_clear): Remove.
(get_osdata): return a unique_ptr and adjust.
(get_osdata_column): Return a pointer to std::string and adjust.
(info_osdata): Adjust.
* mi/mi-main.c: Include <map>.
(free_vector_of_osdata_items): Remove.
(list_available_thread_groups): Adjust, use std::map instead of
splay tree.

gdb/ChangeLog
gdb/mi/mi-main.c
gdb/osdata.c
gdb/osdata.h

index bc8dd00e69fbda4ab63b572a0342f9b52b5ea145..ddf3852cfcdf243259cdc1ad960764cab8816e8c 100644 (file)
@@ -1,3 +1,42 @@
+2017-11-22  Simon Marchi  <simon.marchi@polymtl.ca>
+
+       * osdata.h: Include vector isntead of vec.h.
+       (osdata_column_s): Remove typedef.
+       (struct osdata_column): Add constructor.
+       <name, value>: Change type to std::string.
+       (DEF_VEC_O (osdata_column_s)): Remove.
+       (osdata_item_s): Remove typedef.
+       (struct osdata_item) <columns>: Change type to std::vector.
+       (DEF_VEC_O (osdata_item_s)): Remove.
+       (struct osdata): Add constructor.
+       <type>: Change type to std::string.
+       <items>: Change type to std::vector.
+       (osdata_p): Remove typedef.
+       (DEF_VEC_P (osdata_p)): Remove.
+       (osdata_parse): Return a unique_ptr.
+       (osdata_free): Remove.
+       (make_cleanup_osdata_free): Remove.
+       (get_osdata): Return a unique_ptr.
+       (get_osdata_column): Return pointer to std::string, take a
+       reference to osdata_item as parameter.
+       * osdata.c (struct osdata_parsing_data) <osdata>: Change type to
+       unique_ptr.
+       <property_name>: Change type to std::string.
+       (osdata_start_osdata): Allocate osdata with new and adjust.
+       (osdata_start_item): Adjust.
+       (osdata_start_column): Adjust.
+       (osdata_end_column): Adjust.
+       (clear_parsing_data): Remove.
+       (osdata_parse): Return a unique_ptr and adjust, remove cleanup.
+       (osdata_item_clear): Remove.
+       (get_osdata): return a unique_ptr and adjust.
+       (get_osdata_column): Return a pointer to std::string and adjust.
+       (info_osdata): Adjust.
+       * mi/mi-main.c: Include <map>.
+       (free_vector_of_osdata_items): Remove.
+       (list_available_thread_groups): Adjust, use std::map instead of
+       splay tree.
+
 2017-11-22  Simon Marchi  <simon.marchi@ericsson.com>
 
        * stack.c (iterate_over_block_locals): Add LOC_OPTIMIZED_OUT
index d3ea13987ff6d8a41c21580fa6f05ca30a032a69..c7db478434e71cff5e59bcb3df31b98cbf151ca5 100644 (file)
@@ -63,6 +63,7 @@
 #include "common/rsp-low.h"
 #include <algorithm>
 #include <set>
+#include <map>
 
 enum
   {
@@ -708,89 +709,51 @@ output_cores (struct ui_out *uiout, const char *field_name, const char *xcores)
     uiout->field_string (NULL, p);
 }
 
-static void
-free_vector_of_osdata_items (splay_tree_value xvalue)
-{
-  VEC (osdata_item_s) *value = (VEC (osdata_item_s) *) xvalue;
-
-  /* We don't free the items itself, it will be done separately.  */
-  VEC_free (osdata_item_s, value);
-}
-
-static int
-splay_tree_int_comparator (splay_tree_key xa, splay_tree_key xb)
-{
-  int a = xa;
-  int b = xb;
-
-  return a - b;
-}
-
 static void
 list_available_thread_groups (const std::set<int> &ids, int recurse)
 {
-  struct osdata *data;
-  struct osdata_item *item;
-  int ix_items;
   struct ui_out *uiout = current_uiout;
-  struct cleanup *cleanup;
 
-  /* This keeps a map from integer (pid) to VEC (struct osdata_item *)*
-     The vector contains information about all threads for the given pid.
-     This is assigned an initial value to avoid "may be used uninitialized"
-     warning from gcc.  */
-  gdb_splay_tree_up tree;
+  /* This keeps a map from integer (pid) to vector of struct osdata_item.
+     The vector contains information about all threads for the given pid.  */
+  std::map<int, std::vector<osdata_item> *> tree_;
 
   /* get_osdata will throw if it cannot return data.  */
-  data = get_osdata ("processes");
-  cleanup = make_cleanup_osdata_free (data);
+  std::unique_ptr<osdata> data = get_osdata ("processes");
 
   if (recurse)
     {
-      struct osdata *threads = get_osdata ("threads");
+      std::unique_ptr<osdata> threads = get_osdata ("threads");
 
-      make_cleanup_osdata_free (threads);
-      tree.reset (splay_tree_new (splay_tree_int_comparator,
-                                 NULL,
-                                 free_vector_of_osdata_items));
-
-      for (ix_items = 0;
-          VEC_iterate (osdata_item_s, threads->items,
-                       ix_items, item);
-          ix_items++)
+      for (const osdata_item &item : threads->items)
        {
-         const char *pid = get_osdata_column (item, "pid");
-         int pid_i = strtoul (pid, NULL, 0);
-         VEC (osdata_item_s) *vec = 0;
+         const std::string *pid = get_osdata_column (item, "pid");
+         int pid_i = strtoul (pid->c_str (), NULL, 0);
+         std::vector<osdata_item> *vec;
 
-         splay_tree_node n = splay_tree_lookup (tree.get (), pid_i);
-         if (!n)
+         auto n = tree_.find (pid_i);
+         if (n == tree_.end ())
            {
-             VEC_safe_push (osdata_item_s, vec, item);
-             splay_tree_insert (tree.get (), pid_i, (splay_tree_value)vec);
+             vec = new std::vector<osdata_item>;
+             tree_[pid_i] = vec;
            }
          else
-           {
-             vec = (VEC (osdata_item_s) *) n->value;
-             VEC_safe_push (osdata_item_s, vec, item);
-             n->value = (splay_tree_value) vec;
-           }
+           vec = n->second;
+
+         vec->push_back (item);
        }
     }
 
   ui_out_emit_list list_emitter (uiout, "groups");
 
-  for (ix_items = 0;
-       VEC_iterate (osdata_item_s, data->items,
-                   ix_items, item);
-       ix_items++)
+  for (const osdata_item &item : data->items)
     {
-      const char *pid = get_osdata_column (item, "pid");
-      const char *cmd = get_osdata_column (item, "command");
-      const char *user = get_osdata_column (item, "user");
-      const char *cores = get_osdata_column (item, "cores");
+      const std::string *pid = get_osdata_column (item, "pid");
+      const std::string *cmd = get_osdata_column (item, "command");
+      const std::string *user = get_osdata_column (item, "user");
+      const std::string *cores = get_osdata_column (item, "cores");
 
-      int pid_i = strtoul (pid, NULL, 0);
+      int pid_i = strtoul (pid->c_str (), NULL, 0);
 
       /* At present, the target will return all available processes
         and if information about specific ones was required, we filter
@@ -800,43 +763,37 @@ list_available_thread_groups (const std::set<int> &ids, int recurse)
 
       ui_out_emit_tuple tuple_emitter (uiout, NULL);
 
-      uiout->field_fmt ("id", "%s", pid);
+      uiout->field_fmt ("id", "%s", pid->c_str ());
       uiout->field_string ("type", "process");
       if (cmd)
-       uiout->field_string ("description", cmd);
+       uiout->field_string ("description", cmd->c_str ());
       if (user)
-       uiout->field_string ("user", user);
+       uiout->field_string ("user", user->c_str ());
       if (cores)
-       output_cores (uiout, "cores", cores);
+       output_cores (uiout, "cores", cores->c_str ());
 
       if (recurse)
        {
-         splay_tree_node n = splay_tree_lookup (tree.get (), pid_i);
-         if (n)
+         auto n = tree_.find (pid_i);
+         if (n != tree_.end ())
            {
-             VEC (osdata_item_s) *children = (VEC (osdata_item_s) *) n->value;
-             struct osdata_item *child;
-             int ix_child;
+             std::vector<osdata_item> *children = n->second;
 
              ui_out_emit_list thread_list_emitter (uiout, "threads");
 
-             for (ix_child = 0;
-                  VEC_iterate (osdata_item_s, children, ix_child, child);
-                  ++ix_child)
+             for (const osdata_item &child : *children)
                {
                  ui_out_emit_tuple tuple_emitter (uiout, NULL);
-                 const char *tid = get_osdata_column (child, "tid");
-                 const char *tcore = get_osdata_column (child, "core");
+                 const std::string *tid = get_osdata_column (child, "tid");
+                 const std::string *tcore = get_osdata_column (child, "core");
 
-                 uiout->field_string ("id", tid);
+                 uiout->field_string ("id", tid->c_str ());
                  if (tcore)
-                   uiout->field_string ("core", tcore);
+                   uiout->field_string ("core", tcore->c_str ());
                }
            }
        }
     }
-
-  do_cleanups (cleanup);
 }
 
 void
index a8b106b2a794c347a4b58cfb0cbc1c4f4bfd94f4..f013d0fbd72758495e0dec1aec8d8c2ae5e6c095 100644 (file)
@@ -27,7 +27,7 @@
 
 #if !defined(HAVE_LIBEXPAT)
 
-struct osdata *
+std::unique_ptr<osdata>
 osdata_parse (const char *xml)
 {
   static int have_warned;
@@ -46,10 +46,10 @@ osdata_parse (const char *xml)
 
 /* Internal parsing data passed to all XML callbacks.  */
 struct osdata_parsing_data
-  {
-    struct osdata *osdata;
-    char *property_name;
-  };
+{
+  std::unique_ptr<struct osdata> osdata;
+  std::string property_name;
+};
 
 /* Handle the start of a <osdata> element.  */
 
@@ -59,16 +59,12 @@ osdata_start_osdata (struct gdb_xml_parser *parser,
                         void *user_data, VEC(gdb_xml_value_s) *attributes)
 {
   struct osdata_parsing_data *data = (struct osdata_parsing_data *) user_data;
-  char *type;
-  struct osdata *osdata;
 
-  if (data->osdata)
+  if (data->osdata != NULL)
     gdb_xml_error (parser, _("Seen more than on osdata element"));
 
-  type = (char *) xml_find_attribute (attributes, "type")->value;
-  osdata = XCNEW (struct osdata);
-  osdata->type = xstrdup (type);
-  data->osdata = osdata;
+  char *type = (char *) xml_find_attribute (attributes, "type")->value;
+  data->osdata.reset (new struct osdata (std::string (type)));
 }
 
 /* Handle the start of a <item> element.  */
@@ -79,9 +75,7 @@ osdata_start_item (struct gdb_xml_parser *parser,
                   void *user_data, VEC(gdb_xml_value_s) *attributes)
 {
   struct osdata_parsing_data *data = (struct osdata_parsing_data *) user_data;
-  struct osdata_item item = { NULL };
-
-  VEC_safe_push (osdata_item_s, data->osdata->items, &item);
+  data->osdata->items.emplace_back ();
 }
 
 /* Handle the start of a <column> element.  */
@@ -95,7 +89,7 @@ osdata_start_column (struct gdb_xml_parser *parser,
   const char *name
     = (const char *) xml_find_attribute (attributes, "name")->value;
 
-  data->property_name = xstrdup (name);
+  data->property_name.assign (name);
 }
 
 /* Handle the end of a <column> element.  */
@@ -105,29 +99,12 @@ osdata_end_column (struct gdb_xml_parser *parser,
                   const struct gdb_xml_element *element,
                   void *user_data, const char *body_text)
 {
-  struct osdata_parsing_data *data = (struct osdata_parsing_data *) user_data;
-  struct osdata *osdata = data->osdata;
-  struct osdata_item *item = VEC_last (osdata_item_s, osdata->items);
-  struct osdata_column *col = VEC_safe_push (osdata_column_s,
-                                            item->columns, NULL);
-
-  /* Transfer memory ownership.  NAME was already strdup'ed.  */
-  col->name = data->property_name;
-  col->value = xstrdup (body_text);
-  data->property_name = NULL;
-}
-
-/* Discard the constructed osdata (if an error occurs).  */
-
-static void
-clear_parsing_data (void *p)
-{
-  struct osdata_parsing_data *data = (struct osdata_parsing_data *) p;
+  osdata_parsing_data *data = (struct osdata_parsing_data *) user_data;
+  struct osdata *osdata = data->osdata.get ();
+  osdata_item &item = osdata->items.back ();
 
-  osdata_free (data->osdata);
-  data->osdata = NULL;
-  xfree (data->property_name);
-  data->property_name = NULL;
+  item.columns.emplace_back (std::move (data->property_name),
+                            std::move (std::string (body_text)));
 }
 
 /* The allowed elements and attributes for OS data object.
@@ -163,88 +140,26 @@ const struct gdb_xml_element osdata_elements[] = {
   { NULL, NULL, NULL, GDB_XML_EF_NONE, NULL, NULL }
 };
 
-struct osdata *
+std::unique_ptr<osdata>
 osdata_parse (const char *xml)
 {
-  struct cleanup *back_to;
-  struct osdata_parsing_data data = { NULL };
-
-  back_to = make_cleanup (clear_parsing_data, &data);
+  osdata_parsing_data data;
 
   if (gdb_xml_parse_quick (_("osdata"), "osdata.dtd",
                           osdata_elements, xml, &data) == 0)
     {
       /* Parsed successfully, don't need to delete the result.  */
-      discard_cleanups (back_to);
-      return data.osdata;
+      return std::move (data.osdata);
     }
 
-  do_cleanups (back_to);
   return NULL;
 }
 #endif
 
-static void
-osdata_item_clear (struct osdata_item *item)
-{
-  if (item->columns != NULL)
-    {
-      struct osdata_column *col;
-      int ix;
-
-      for (ix = 0;
-          VEC_iterate (osdata_column_s, item->columns,
-                       ix, col);
-          ix++)
-       {
-        xfree (col->name);
-        xfree (col->value);
-       }
-      VEC_free (osdata_column_s, item->columns);
-      item->columns = NULL;
-    }
-}
-
-void
-osdata_free (struct osdata *osdata)
-{
-  if (osdata == NULL)
-    return;
-
-  if (osdata->items != NULL)
-    {
-      struct osdata_item *item;
-      int ix;
-
-      for (ix = 0;
-          VEC_iterate (osdata_item_s, osdata->items,
-                       ix, item);
-          ix++)
-       osdata_item_clear (item);
-      VEC_free (osdata_item_s, osdata->items);
-    }
-
-  xfree (osdata);
-}
-
-static void
-osdata_free_cleanup (void *arg)
-{
-  struct osdata *osdata = (struct osdata *) arg;
-
-  osdata_free (osdata);
-}
-
-struct cleanup *
-make_cleanup_osdata_free (struct osdata *data)
-{
-  return make_cleanup (osdata_free_cleanup, data);
-}
-
-struct osdata *
+std::unique_ptr<osdata>
 get_osdata (const char *type)
 {
-  struct osdata *osdata = NULL;
+  std::unique_ptr<osdata> osdata;
   gdb::unique_xmalloc_ptr<char> xml = target_get_osdata (type);
 
   if (xml)
@@ -260,24 +175,18 @@ get_osdata (const char *type)
        osdata = osdata_parse (xml.get ());
     }
 
-  if (!osdata)
+  if (osdata == NULL)
     error (_("Can not fetch data now."));
 
-  return osdata;
+  return std::move (osdata);
 }
 
-const char *
-get_osdata_column (struct osdata_item *item, const char *name)
+const std::string *
+get_osdata_column (const osdata_item &item, const char *name)
 {
-  struct osdata_column *col;
-  int ix_cols; 
-  
-  for (ix_cols = 0;
-       VEC_iterate (osdata_column_s, item->columns,
-                   ix_cols, col);
-       ix_cols++)
-    if (strcmp (col->name, name) == 0)
-      return col->value;
+  for (const osdata_column &col : item.columns)
+    if (col.name == name)
+      return &col.value;
 
   return NULL;
 }
@@ -286,29 +195,24 @@ void
 info_osdata (const char *type)
 {
   struct ui_out *uiout = current_uiout;
-  struct osdata *osdata = NULL;
   struct osdata_item *last = NULL;
-  struct cleanup *old_chain;
   int ncols = 0;
-  int nrows;
   int col_to_skip = -1;
 
   if (type == NULL)
     type = "";
 
-  osdata = get_osdata (type);
-  old_chain = make_cleanup_osdata_free (osdata);
+  std::unique_ptr<osdata> osdata = get_osdata (type);
 
-  nrows = VEC_length (osdata_item_s, osdata->items);
+  int nrows = osdata->items.size ();
 
   if (*type == '\0' && nrows == 0)
     error (_("Available types of OS data not reported."));
   
-  if (!VEC_empty (osdata_item_s, osdata->items))
+  if (!osdata->items.empty ())
     {
-      last = VEC_last (osdata_item_s, osdata->items);
-      if (last->columns)
-        ncols = VEC_length (osdata_column_s, last->columns);
+      last = &osdata->items.back ();
+      ncols = last->columns.size ();
 
       /* As a special case, scan the listing of available data types
         for a column named "Title", and only include it with MI
@@ -316,14 +220,9 @@ info_osdata (const char *type)
         elements like menus, and it clutters up CLI output.  */
       if (*type == '\0' && !uiout->is_mi_like_p ())
        {
-         struct osdata_column *col;
-         int ix;
-
-         for (ix = 0;
-              VEC_iterate (osdata_column_s, last->columns, ix, col);
-              ix++)
+         for (int ix = 0; ix < last->columns.size (); ix++)
            {
-             if (strcmp (col->name, "Title") == 0)
+             if (last->columns[ix].name == "Title")
                col_to_skip = ix;
            }
          /* Be sure to reduce the total column count, otherwise
@@ -338,20 +237,11 @@ info_osdata (const char *type)
   /* With no columns/items, we just output an empty table, but we
      still output the table.  This matters for MI.  */
   if (ncols == 0)
-    {
-      do_cleanups (old_chain);
-      return;
-    }
+    return;
 
-  if (last && last->columns)
+  if (last != NULL && !last->columns.empty ())
     {
-      struct osdata_column *col;
-      int ix;
-
-      for (ix = 0;
-          VEC_iterate (osdata_column_s, last->columns,
-                       ix, col);
-          ix++)
+      for (int ix = 0; ix < last->columns.size (); ix++)
        {
          char col_name[32];
 
@@ -360,7 +250,7 @@ info_osdata (const char *type)
 
          snprintf (col_name, 32, "col%d", ix);
          uiout->table_header (10, ui_left,
-                              col_name, col->name);
+                              col_name, last->columns[ix].name.c_str ());
         }
     }
 
@@ -368,24 +258,12 @@ info_osdata (const char *type)
 
   if (nrows != 0)
     {
-      struct osdata_item *item;
-      int ix_items;
-
-      for (ix_items = 0;
-          VEC_iterate (osdata_item_s, osdata->items,
-                       ix_items, item);
-          ix_items++)
+      for (const osdata_item &item : osdata->items)
        {
-         int ix_cols;
-         struct osdata_column *col;
-
         {
           ui_out_emit_tuple tuple_emitter (uiout, "item");
 
-          for (ix_cols = 0;
-               VEC_iterate (osdata_column_s, item->columns,
-                            ix_cols, col);
-               ix_cols++)
+          for (int ix_cols = 0; ix_cols < item.columns.size (); ix_cols++)
             {
               char col_name[32];
 
@@ -393,15 +271,14 @@ info_osdata (const char *type)
                 continue;
 
               snprintf (col_name, 32, "col%d", ix_cols);
-              uiout->field_string (col_name, col->value);
+              uiout->field_string (col_name,
+                                   item.columns[ix_cols].value.c_str ());
             }
         }
 
          uiout->text ("\n");
        }
     }
-
-  do_cleanups (old_chain);
 }
 
 static void
index 5921384527d6c5aff8cf1f077bc01d54faea6cb3..58aaa3c07e7e21db4bb9329dd403ef987f20aafc 100644 (file)
 #ifndef OSDATA_H
 #define OSDATA_H
 
-#include "vec.h"
+#include <vector>
 
-typedef struct osdata_column
+struct osdata_column
 {
-  char *name;
-  char *value;
-} osdata_column_s;
-DEF_VEC_O(osdata_column_s);
+  osdata_column (std::string &&name_, std::string &&value_)
+  : name (std::move (name_)), value (std::move (value_))
+  {}
 
-typedef struct osdata_item
+  std::string name;
+  std::string value;
+};
+
+struct osdata_item
 {
-  VEC(osdata_column_s) *columns;
-} osdata_item_s;
-DEF_VEC_O(osdata_item_s);
+  std::vector<osdata_column> columns;
+};
 
 struct osdata
 {
-  char *type;
+  osdata (std::string &&type_)
+  : type (std::move (type_))
+  {}
 
-  VEC(osdata_item_s) *items;
+  std::string type;
+  std::vector<osdata_item> items;
 };
-typedef struct osdata *osdata_p;
-DEF_VEC_P(osdata_p);
-
-struct osdata *osdata_parse (const char *xml);
-void osdata_free (struct osdata *);
-struct cleanup *make_cleanup_osdata_free (struct osdata *data);
-struct osdata *get_osdata (const char *type);
-const char *get_osdata_column (struct osdata_item *item, const char *name);
+
+std::unique_ptr<osdata> osdata_parse (const char *xml);
+std::unique_ptr<osdata> get_osdata (const char *type);
+const std::string *get_osdata_column (const osdata_item &item,
+                                     const char *name);
 
 /* Dump TYPE info to the current uiout builder.  If TYPE is either
    NULL or empty, then dump the top level table that lists the