From 479f8de1b3b7e69ca8d557bbe9d843c7d1bc89c5 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 22 Nov 2017 16:12:06 -0500 Subject: [PATCH] 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. : Change type to std::string. (DEF_VEC_O (osdata_column_s)): Remove. (osdata_item_s): Remove typedef. (struct osdata_item) : Change type to std::vector. (DEF_VEC_O (osdata_item_s)): Remove. (struct osdata): Add constructor. : Change type to std::string. : 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) : Change type to unique_ptr. : 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 . (free_vector_of_osdata_items): Remove. (list_available_thread_groups): Adjust, use std::map instead of splay tree. --- gdb/ChangeLog | 39 +++++++++ gdb/mi/mi-main.c | 113 ++++++++------------------ gdb/osdata.c | 207 ++++++++++------------------------------------- gdb/osdata.h | 42 +++++----- 4 files changed, 138 insertions(+), 263 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index bc8dd00e69f..ddf3852cfcd 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,42 @@ +2017-11-22 Simon Marchi + + * osdata.h: Include vector isntead of vec.h. + (osdata_column_s): Remove typedef. + (struct osdata_column): Add constructor. + : Change type to std::string. + (DEF_VEC_O (osdata_column_s)): Remove. + (osdata_item_s): Remove typedef. + (struct osdata_item) : Change type to std::vector. + (DEF_VEC_O (osdata_item_s)): Remove. + (struct osdata): Add constructor. + : Change type to std::string. + : 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) : Change type to + unique_ptr. + : 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 . + (free_vector_of_osdata_items): Remove. + (list_available_thread_groups): Adjust, use std::map instead of + splay tree. + 2017-11-22 Simon Marchi * stack.c (iterate_over_block_locals): Add LOC_OPTIMIZED_OUT diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index d3ea13987ff..c7db478434e 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -63,6 +63,7 @@ #include "common/rsp-low.h" #include #include +#include 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 &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 *> tree_; /* get_osdata will throw if it cannot return data. */ - data = get_osdata ("processes"); - cleanup = make_cleanup_osdata_free (data); + std::unique_ptr data = get_osdata ("processes"); if (recurse) { - struct osdata *threads = get_osdata ("threads"); + std::unique_ptr 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 *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; + 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 &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 *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 a8b106b2a79..f013d0fbd72 100644 --- a/gdb/osdata.c +++ b/gdb/osdata.c @@ -27,7 +27,7 @@ #if !defined(HAVE_LIBEXPAT) -struct osdata * +std::unique_ptr 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 osdata; + std::string property_name; +}; /* Handle the start of a 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 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 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 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_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 get_osdata (const char *type) { - struct osdata *osdata = NULL; + std::unique_ptr osdata; gdb::unique_xmalloc_ptr 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 = 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 5921384527d..58aaa3c07e7 100644 --- a/gdb/osdata.h +++ b/gdb/osdata.h @@ -20,35 +20,37 @@ #ifndef OSDATA_H #define OSDATA_H -#include "vec.h" +#include -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 columns; +}; struct osdata { - char *type; + osdata (std::string &&type_) + : type (std::move (type_)) + {} - VEC(osdata_item_s) *items; + std::string type; + std::vector 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_parse (const char *xml); +std::unique_ptr 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 -- 2.30.2