[PATCH 2/2] Make collect_probes return an std::vector

Simon Marchi
2017-09-10T14:23:16Z

Change collect_probes so it returns an std::vector<bound_probe> instead
of a VEC(bound_probe_s). This allows removing some cleanups. It also
seems like enable_probes_command and disable_probes_command were not
freeing that vector.

The comparison function compare_probes needs to be updated to return a
bool indicating whether the first parameter is "less than" the second
parameter.

I defined two constructors to bound_probe. The default constructor is
needed, for example, so the instance in struct bp_location can be
constructed without parameters. The constructor with parameters is
useful so we can use emplace_back and pass the values directly.

The s390 builder on the buildbot shows a weird failure that I can't
explain:

../../binutils-gdb/gdb/elfread.c: In function void probe_key_free(bfd*, void*):
../../binutils-gdb/gdb/elfread.c:1346:8: error: types may not be defined in a for-range-declaration [-Werror]
for (struct probe *probe : *probes)
^~~~~~

I guess it's a bug with that specific version< of the compiler, since no
other gcc gives me that error. It is using:

g++ (GCC) 6.3.1 20161221 (Red Hat 6.3.1-1)

Any idea about this problem?

gdb/ChangeLog:

* probe.h (struct bound_probe): Define constructors.
* probe.c (bound_probe_s): Remove typedef.
(DEF_VEC_O (bound_probe_s)): Remove VEC.
(collect_probes): Change return type to std::vector, remove
cleanup.
(compare_probes): Return bool, change parameter type. Change
semantic to "less than".
(gen_ui_out_table_header_info): Change parameter to std::vector
and update.
(exists_probe_with_pops): Likewise.
(info_probes_for_ops): Update to std::vector change.
(enable_probes_command): Likewise.
(disable_probes_command): Likewise.
---
gdb/probe.c | 147 ++++++++++++++++++++++++------------------------------------
gdb/probe.h | 19 +++++---
2 files changed, 72 insertions(+), 94 deletions(-)

diff --git a/gdb/probe.c b/gdb/probe.c
index 2d68437..384ea9d 100644
--- a/gdb/probe.c
+++ b/gdb/probe.c
@@ -38,11 +38,6 @@
#include <algorithm>
#include "common/gdb_optional.h"

-typedef struct bound_probe bound_probe_s;
-DEF_VEC_O (bound_probe_s);
-
-\f
-
/* A helper for parse_probes that decodes a probe specification in
SEARCH_PSPACE. It appends matching SALs to RESULT. */

@@ -267,17 +262,14 @@ find_probe_by_pc (CORE_ADDR pc)
If POPS is not NULL, only probes of this certain probe_ops will match.
Each argument is a regexp, or NULL, which matches anything. */

-static VEC (bound_probe_s) *
+static std::vector<bound_probe>
collect_probes (char *objname, char *provider, char *probe_name,
const struct probe_ops *pops)
{
struct objfile *objfile;
- VEC (bound_probe_s) *result = NULL;
- struct cleanup *cleanup;
+ std::vector<bound_probe> result;
gdb::optional<compiled_regex> obj_pat, prov_pat, probe_pat;

- cleanup = make_cleanup (VEC_cleanup (bound_probe_s), &result);
-
if (provider != NULL)
prov_pat.emplace (provider, REG_NOSUB, _("Invalid provider regexp"));
if (probe_name != NULL)
@@ -301,8 +293,6 @@ collect_probes (char *objname, char *provider, char *probe_name,

for (struct probe *probe : probes)
{
- struct bound_probe bound;
-
if (pops != NULL && probe->pops != pops)
continue;

@@ -314,46 +304,39 @@ collect_probes (char *objname, char *provider, char *probe_name,
&& probe_pat->exec (probe->name, 0, NULL, 0) != 0)
continue;

- bound.objfile = objfile;
- bound.probe = probe;
- VEC_safe_push (bound_probe_s, result, &bound);
+ result.emplace_back (probe, objfile);
}
}

- discard_cleanups (cleanup);
return result;
}

/* A qsort comparison function for bound_probe_s objects. */

-static int
-compare_probes (const void *a, const void *b)
+static bool
+compare_probes (const bound_probe &a, const bound_probe &b)
{
- const struct bound_probe *pa = (const struct bound_probe *) a;
- const struct bound_probe *pb = (const struct bound_probe *) b;
int v;

- v = strcmp (pa->probe->provider, pb->probe->provider);
- if (v)
- return v;
+ v = strcmp (a.probe->provider, b.probe->provider);
+ if (v != 0)
+ return v < 0;

- v = strcmp (pa->probe->name, pb->probe->name);
- if (v)
- return v;
+ v = strcmp (a.probe->name, b.probe->name);
+ if (v != 0)
+ return v < 0;

- if (pa->probe->address < pb->probe->address)
- return -1;
- if (pa->probe->address > pb->probe->address)
- return 1;
+ if (a.probe->address != b.probe->address)
+ return a.probe->address < b.probe->address;

- return strcmp (objfile_name (pa->objfile), objfile_name (pb->objfile));
+ return strcmp (objfile_name (a.objfile), objfile_name (b.objfile)) < 0;
}

/* Helper function that generate entries in the ui_out table being
crafted by `info_probes_for_ops'. */

static void
-gen_ui_out_table_header_info (VEC (bound_probe_s) *probes,
+gen_ui_out_table_header_info (const std::vector<bound_probe> &probes,
const struct probe_ops *p)
{
/* `headings' refers to the names of the columns when printing `info
@@ -382,11 +365,9 @@ gen_ui_out_table_header_info (VEC (bound_probe_s) *probes,
VEC_iterate (info_probe_column_s, headings, ix, column);
++ix)
{
- struct bound_probe *probe;
- int jx;
size_t size_max = strlen (column->print_name);

- for (jx = 0; VEC_iterate (bound_probe_s, probes, jx, probe); ++jx)
+ for (const bound_probe &probe : probes)
{
/* `probe_fields' refers to the values of each new field that this
probe will display. */
@@ -395,11 +376,11 @@ gen_ui_out_table_header_info (VEC (bound_probe_s) *probes,
const char *val;
int kx;

- if (probe->probe->pops != p)
+ if (probe.probe->pops != p)
continue;

c2 = make_cleanup (VEC_cleanup (const_char_ptr), &probe_fields);
- p->gen_info_probes_table_values (probe->probe, &probe_fields);
+ p->gen_info_probes_table_values (probe.probe, &probe_fields);

gdb_assert (VEC_length (const_char_ptr, probe_fields)
== headings_size);
@@ -526,14 +507,14 @@ get_number_extra_fields (const struct probe_ops *pops)
featuring the given POPS. It returns 0 otherwise. */

static int
-exists_probe_with_pops (VEC (bound_probe_s) *probes,
+exists_probe_with_pops (const std::vector<bound_probe> &probes,
const struct probe_ops *pops)
{
struct bound_probe *probe;
int ix;

- for (ix = 0; VEC_iterate (bound_probe_s, probes, ix, probe); ++ix)
- if (probe->probe->pops == pops)
+ for (const bound_probe &probe : probes)
+ if (probe.probe->pops == pops)
return 1;

return 0;
@@ -565,15 +546,13 @@ info_probes_for_ops (const char *arg, int from_tty,
{
char *provider, *probe_name = NULL, *objname = NULL;
struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
- VEC (bound_probe_s) *probes;
- int i, any_found;
+ int any_found;
int ui_out_extra_fields = 0;
size_t size_addr;
size_t size_name = strlen ("Name");
size_t size_objname = strlen ("Object");
size_t size_provider = strlen ("Provider");
size_t size_type = strlen ("Type");
- struct bound_probe *probe;
struct gdbarch *gdbarch = get_current_arch ();

parse_probe_linespec (arg, &provider, &probe_name, &objname);
@@ -581,8 +560,8 @@ info_probes_for_ops (const char *arg, int from_tty,
make_cleanup (xfree, probe_name);
make_cleanup (xfree, objname);

- probes = collect_probes (objname, provider, probe_name, pops);
- make_cleanup (VEC_cleanup (probe_p), &probes);
+ std::vector<bound_probe> probes
+ = collect_probes (objname, provider, probe_name, pops);

if (pops == NULL)
{
@@ -609,27 +588,23 @@ info_probes_for_ops (const char *arg, int from_tty,
{
ui_out_emit_table table_emitter (current_uiout,
5 + ui_out_extra_fields,
- VEC_length (bound_probe_s, probes),
- "StaticProbes");
+ probes.size (), "StaticProbes");

- if (!VEC_empty (bound_probe_s, probes))
- qsort (VEC_address (bound_probe_s, probes),
- VEC_length (bound_probe_s, probes),
- sizeof (bound_probe_s), compare_probes);
+ std::sort (probes.begin (), probes.end (), compare_probes);

/* What's the size of an address in our architecture? */
size_addr = gdbarch_addr_bit (gdbarch) == 64 ? 18 : 10;

/* Determining the maximum size of each field (`type', `provider',
`name' and `objname'). */
- for (i = 0; VEC_iterate (bound_probe_s, probes, i, probe); ++i)
+ for (const bound_probe &probe : probes)
{
- const char *probe_type = probe->probe->pops->type_name (probe->probe);
+ const char *probe_type = probe.probe->pops->type_name (probe.probe);

size_type = std::max (strlen (probe_type), size_type);
- size_name = std::max (strlen (probe->probe->name), size_name);
- size_provider = std::max (strlen (probe->probe->provider), size_provider);
- size_objname = std::max (strlen (objfile_name (probe->objfile)),
+ size_name = std::max (strlen (probe.probe->name), size_name);
+ size_provider = std::max (strlen (probe.probe->provider), size_provider);
+ size_objname = std::max (strlen (objfile_name (probe.objfile)),
size_objname);
}

@@ -657,18 +632,18 @@ info_probes_for_ops (const char *arg, int from_tty,
current_uiout->table_header (size_objname, ui_left, "object", _("Object"));
current_uiout->table_body ();

- for (i = 0; VEC_iterate (bound_probe_s, probes, i, probe); ++i)
+ for (const bound_probe &probe : probes)
{
- const char *probe_type = probe->probe->pops->type_name (probe->probe);
+ const char *probe_type = probe.probe->pops->type_name (probe.probe);

ui_out_emit_tuple tuple_emitter (current_uiout, "probe");

current_uiout->field_string ("type",probe_type);
- current_uiout->field_string ("provider", probe->probe->provider);
- current_uiout->field_string ("name", probe->probe->name);
- current_uiout->field_core_addr (
- "addr", probe->probe->arch,
- get_probe_address (probe->probe, probe->objfile));
+ current_uiout->field_string ("provider", probe.probe->provider);
+ current_uiout->field_string ("name", probe.probe->name);
+ current_uiout->field_core_addr ("addr", probe.probe->arch,
+ get_probe_address (probe.probe,
+ probe.objfile));

if (pops == NULL)
{
@@ -677,20 +652,20 @@ info_probes_for_ops (const char *arg, int from_tty,

for (ix = 0; VEC_iterate (probe_ops_cp, all_probe_ops, ix, po);
++ix)
- if (probe->probe->pops == po)
- print_ui_out_info (probe->probe);
+ if (probe.probe->pops == po)
+ print_ui_out_info (probe.probe);
else if (exists_probe_with_pops (probes, po))
print_ui_out_not_applicables (po);
}
else
- print_ui_out_info (probe->probe);
+ print_ui_out_info (probe.probe);

current_uiout->field_string ("object",
- objfile_name (probe->objfile));
+ objfile_name (probe.objfile));
current_uiout->text ("\n");
}

- any_found = !VEC_empty (bound_probe_s, probes);
+ any_found = !probes.empty ();
}
do_cleanups (cleanup);

@@ -713,17 +688,15 @@ enable_probes_command (char *arg, int from_tty)
{
char *provider, *probe_name = NULL, *objname = NULL;
struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
- VEC (bound_probe_s) *probes;
- struct bound_probe *probe;
- int i;

parse_probe_linespec ((const char *) arg, &provider, &probe_name, &objname);
make_cleanup (xfree, provider);
make_cleanup (xfree, probe_name);
make_cleanup (xfree, objname);

- probes = collect_probes (objname, provider, probe_name, NULL);
- if (VEC_empty (bound_probe_s, probes))
+ std::vector<bound_probe> probes
+ = collect_probes (objname, provider, probe_name, NULL);
+ if (probes.empty ())
{
current_uiout->message (_("No probes matched.\n"));
do_cleanups (cleanup);
@@ -732,19 +705,19 @@ enable_probes_command (char *arg, int from_tty)

/* Enable the selected probes, provided their backends support the
notion of enabling a probe. */
- for (i = 0; VEC_iterate (bound_probe_s, probes, i, probe); ++i)
+ for (const bound_probe &probe: probes)
{
- const struct probe_ops *pops = probe->probe->pops;
+ const struct probe_ops *pops = probe.probe->pops;

if (pops->enable_probe != NULL)
{
- pops->enable_probe (probe->probe);
+ pops->enable_probe (probe.probe);
current_uiout->message (_("Probe %s:%s enabled.\n"),
- probe->probe->provider, probe->probe->name);
+ probe.probe->provider, probe.probe->name);
}
else
current_uiout->message (_("Probe %s:%s cannot be enabled.\n"),
- probe->probe->provider, probe->probe->name);
+ probe.probe->provider, probe.probe->name);
}

do_cleanups (cleanup);
@@ -757,17 +730,15 @@ disable_probes_command (char *arg, int from_tty)
{
char *provider, *probe_name = NULL, *objname = NULL;
struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
- VEC (bound_probe_s) *probes;
- struct bound_probe *probe;
- int i;

parse_probe_linespec ((const char *) arg, &provider, &probe_name, &objname);
make_cleanup (xfree, provider);
make_cleanup (xfree, probe_name);
make_cleanup (xfree, objname);

- probes = collect_probes (objname, provider, probe_name, NULL /* pops */);
- if (VEC_empty (bound_probe_s, probes))
+ std::vector<bound_probe> probes
+ = collect_probes (objname, provider, probe_name, NULL /* pops */);
+ if (probes.empty ())
{
current_uiout->message (_("No probes matched.\n"));
do_cleanups (cleanup);
@@ -776,19 +747,19 @@ disable_probes_command (char *arg, int from_tty)

/* Disable the selected probes, provided their backends support the
notion of enabling a probe. */
- for (i = 0; VEC_iterate (bound_probe_s, probes, i, probe); ++i)
+ for (const bound_probe &probe : probes)
{
- const struct probe_ops *pops = probe->probe->pops;
+ const struct probe_ops *pops = probe.probe->pops;

if (pops->disable_probe != NULL)
{
- pops->disable_probe (probe->probe);
+ pops->disable_probe (probe.probe);
current_uiout->message (_("Probe %s:%s disabled.\n"),
- probe->probe->provider, probe->probe->name);
+ probe.probe->provider, probe.probe->name);
}
else
current_uiout->message (_("Probe %s:%s cannot be disabled.\n"),
- probe->probe->provider, probe->probe->name);
+ probe.probe->provider, probe.probe->name);
}

do_cleanups (cleanup);
diff --git a/gdb/probe.h b/gdb/probe.h
index 61e3031..75e9a5c 100644
--- a/gdb/probe.h
+++ b/gdb/probe.h
@@ -214,15 +214,22 @@ struct probe
their point of use. */

struct bound_probe
- {
- /* The probe. */
+{
+ bound_probe ()
+ {}

- struct probe *probe;
+ bound_probe (struct probe *probe_, struct objfile *objfile_)
+ : probe (probe_), objfile (objfile_)
+ {}

- /* The objfile in which the probe originated. */
+ /* The probe. */

- struct objfile *objfile;
- };
+ struct probe *probe = NULL;
+
+ /* The objfile in which the probe originated. */
+
+ struct objfile *objfile = NULL;
+};

/* A helper for linespec that decodes a probe specification. It
returns a std::vector<symtab_and_line> object and updates LOC or
--
2.7.4