C++ify osdata

  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/mi/mi-main.c | 113 ++++++++++--------------------
 gdb/osdata.c     | 207 +++++++++++--------------------------------------------
 gdb/osdata.h     |  42 +++++------
 3 files changed, 99 insertions(+), 263 deletions(-)
  

On 2017-11-18 06:38 PM, Simon Marchi wrote:
> 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/mi/mi-main.c | 113 ++++++++++--------------------
>  gdb/osdata.c     | 207 +++++++++++--------------------------------------------
>  gdb/osdata.h     |  42 +++++------
>  3 files changed, 99 insertions(+), 263 deletions(-)
> 
> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> index d3ea13987f..c7db478434 100644
> --- a/gdb/mi/mi-main.c
> +++ b/gdb/mi/mi-main.c
> @@ -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
> diff --git a/gdb/osdata.c b/gdb/osdata.c
> index a8b106b2a7..f013d0fbd7 100644
> --- a/gdb/osdata.c
> +++ b/gdb/osdata.c
> @@ -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
> diff --git a/gdb/osdata.h b/gdb/osdata.h
> index 5921384527..58aaa3c07e 100644
> --- a/gdb/osdata.h
> +++ b/gdb/osdata.h
> @@ -20,35 +20,37 @@
>  #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
> 

I pushed this in.

Simon
  

Hi Simon,

I think I spotted an issue here.

On 11/18/2017 11:38 PM, Simon Marchi wrote:
> +  /* 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_;

Isn't this leaking the heap-allocated vectors?

Why make it a map of pointers, actually?  Why not:

  std::map<int, std::vector<osdata_item>>

It's more efficient, and I think results in simpler code.  You
won't need the tree_.find() for example, can just do:

 tree_[pid_i].push_back (...);

Thanks,
Pedro Alves