[PATCH 11/16] gdb: remove reggroup_next and reggroup_prev

Andrew Burgess via Gdb-patches
2022-03-31T21:10:17Z

Add a new function gdbarch_reggroups that returns a reference to a
vector containing all the reggroups for an architecture.

Make use of this function throughout GDB instead of the existing
reggroup_next and reggroup_prev functions.

Finally, delete the reggroup_next and reggroup_prev functions.

Most of these changes are pretty straight forward, using range based
for loops instead of the old style look using reggroup_next. There
are two places where the changes are less straight forward.

In gdb/python/py-registers.c, the register group iterator needed to
change slightly. As the iterator is tightly coupled to the gdbarch, I
just fetch the register group vector from the gdbarch when needed, and
use an index counter to find the next item from the vector when
needed.

In gdb/tui/tui-regs.c the tui_reg_next and tui_reg_prev functions are
just wrappers around reggroup_next and reggroup_prev respectively.
I've just inlined the logic of the old functions into the tui
functions. As the tui function had its own special twist (wrap around
behaviour) I think this is OK.

There should be no user visible changes after this commit.
---
gdb/completer.c | 6 +-
gdb/infcmd.c | 14 ++---
gdb/python/py-registers.c | 17 +++---
gdb/regcache-dump.c | 6 +-
gdb/reggroups.c | 118 ++++++++------------------------------
gdb/reggroups.h | 12 ++--
gdb/tui/tui-regs.c | 59 +++++++++++++------
7 files changed, 88 insertions(+), 144 deletions(-)

diff --git a/gdb/completer.c b/gdb/completer.c
index a51c16ac7f8..8bb12f256af 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -1895,11 +1895,7 @@ reg_or_group_completer_1 (completion_tracker &tracker,

if ((targets & complete_reggroup_names) != 0)
{
- struct reggroup *group;
-
- for (group = reggroup_next (gdbarch, NULL);
- group != NULL;
- group = reggroup_next (gdbarch, group))
+ for (const struct reggroup *group : gdbarch_reggroups (gdbarch))
{
name = reggroup_name (group);
if (strncmp (word, name, len) == 0)
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index fbae4e14a5f..a28250a456d 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2295,17 +2295,17 @@ registers_info (const char *addr_exp, int fpregs)

/* A register group? */
{
- struct reggroup *group;
-
- for (group = reggroup_next (gdbarch, NULL);
- group != NULL;
- group = reggroup_next (gdbarch, group))
+ const struct reggroup *group = nullptr;
+ for (const struct reggroup *g : gdbarch_reggroups (gdbarch))
{
/* Don't bother with a length check. Should the user
enter a short register group name, go with the first
group that matches. */
- if (strncmp (start, reggroup_name (group), end - start) == 0)
- break;
+ if (strncmp (start, reggroup_name (g), end - start) == 0)
+ {
+ group = g;
+ break;
+ }
}
if (group != NULL)
{
diff --git a/gdb/python/py-registers.c b/gdb/python/py-registers.c
index 58d196c37b4..7a9d2e22403 100644
--- a/gdb/python/py-registers.c
+++ b/gdb/python/py-registers.c
@@ -65,8 +65,8 @@ extern PyTypeObject register_descriptor_object_type
struct reggroup_iterator_object {
PyObject_HEAD

- /* The last register group returned. Initially this will be NULL. */
- const struct reggroup *reggroup;
+ /* The index into GROUPS for the next group to return. */
+ std::vector<const reggroup *>::size_type index;

/* Pointer back to the architecture we're finding registers for. */
struct gdbarch *gdbarch;
@@ -225,17 +225,18 @@ gdbpy_reggroup_iter_next (PyObject *self)
{
reggroup_iterator_object *iter_obj
= (reggroup_iterator_object *) self;
- struct gdbarch *gdbarch = iter_obj->gdbarch;

- const reggroup *next_group = reggroup_next (gdbarch, iter_obj->reggroup);
- if (next_group == NULL)
+ const std::vector<const reggroup *> &groups
+ = gdbarch_reggroups (iter_obj->gdbarch);
+ if (iter_obj->index >= groups.size ())
{
PyErr_SetString (PyExc_StopIteration, _("No more groups"));
return NULL;
}

- iter_obj->reggroup = next_group;
- return gdbpy_get_reggroup (iter_obj->reggroup).release ();
+ const reggroup *group = groups[iter_obj->index];
+ iter_obj->index++;
+ return gdbpy_get_reggroup (group).release ();
}

/* Return a new gdb.RegisterGroupsIterator over all the register groups in
@@ -252,7 +253,7 @@ gdbpy_new_reggroup_iterator (struct gdbarch *gdbarch)
&reggroup_iterator_object_type);
if (iter == NULL)
return NULL;
- iter->reggroup = NULL;
+ iter->index = 0;
iter->gdbarch = gdbarch;
return (PyObject *) iter;
}
diff --git a/gdb/regcache-dump.c b/gdb/regcache-dump.c
index 2a2578558f7..409a868be76 100644
--- a/gdb/regcache-dump.c
+++ b/gdb/regcache-dump.c
@@ -192,11 +192,7 @@ class register_dump_groups : public register_dump
else
{
const char *sep = "";
- struct reggroup *group;
-
- for (group = reggroup_next (m_gdbarch, NULL);
- group != NULL;
- group = reggroup_next (m_gdbarch, group))
+ for (const struct reggroup *group : gdbarch_reggroups (m_gdbarch))
{
if (gdbarch_register_reggroup_p (m_gdbarch, regnum, group))
{
diff --git a/gdb/reggroups.c b/gdb/reggroups.c
index 262bf49cea8..4607c4b674c 100644
--- a/gdb/reggroups.c
+++ b/gdb/reggroups.c
@@ -100,7 +100,7 @@ struct reggroups

/* Return a reference to the list of all groups. */

- const std::vector<struct reggroup *> &
+ const std::vector<const struct reggroup *> &
groups () const
{
return m_groups;
@@ -108,7 +108,7 @@ struct reggroups

private:
/* The register groups. */
- std::vector<struct reggroup *> m_groups;
+ std::vector<const struct reggroup *> m_groups;
};

static struct gdbarch_data *reggroups_data;
@@ -146,62 +146,15 @@ reggroups_init (struct obstack *obstack)
return groups;
}

-/* A register group iterator. */
-
-struct reggroup *
-reggroup_next (struct gdbarch *gdbarch, const struct reggroup *last)
-{
- /* Don't allow this function to be called during architecture
- creation. If there are no groups, use the default groups list. */
- struct reggroups *groups
- = (struct reggroups *) gdbarch_data (gdbarch, reggroups_data);
- gdb_assert (groups != nullptr);
- gdb_assert (groups->size () > 0);
-
- /* Return the first/next reggroup. */
- if (last == nullptr)
- return groups->groups ().front ();
- for (int i = 0; i < groups->size (); ++i)
- {
- if (groups->groups ()[i] == last)
- {
- if (i + 1 < groups->size ())
- return groups->groups ()[i + 1];
- else
- return nullptr;
- }
- }
-
- return nullptr;
-}
-
/* See reggroups.h. */
-
-struct reggroup *
-reggroup_prev (struct gdbarch *gdbarch, const struct reggroup *curr)
+const std::vector<const reggroup *>
+gdbarch_reggroups (struct gdbarch *gdbarch)
{
- /* Don't allow this function to be called during architecture
- creation. If there are no groups, use the default groups list. */
struct reggroups *groups
= (struct reggroups *) gdbarch_data (gdbarch, reggroups_data);
gdb_assert (groups != nullptr);
gdb_assert (groups->size () > 0);
-
- /* Return the first/next reggroup. */
- if (curr == nullptr)
- return groups->groups ().back ();
- for (int i = groups->size () - 1; i >= 0; --i)
- {
- if (groups->groups ()[i] == curr)
- {
- if (i - 1 >= 0)
- return groups->groups ()[i - 1];
- else
- return nullptr;
- }
- }
-
- return nullptr;
+ return groups->groups ();
}

/* Is REGNUM a member of REGGROUP? */
@@ -239,11 +192,7 @@ default_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
const reggroup *
reggroup_find (struct gdbarch *gdbarch, const char *name)
{
- struct reggroup *group;
-
- for (group = reggroup_next (gdbarch, NULL);
- group != NULL;
- group = reggroup_next (gdbarch, group))
+ for (const struct reggroup *group : gdbarch_reggroups (gdbarch))
{
if (strcmp (name, reggroup_name (group)) == 0)
return group;
@@ -256,52 +205,35 @@ reggroup_find (struct gdbarch *gdbarch, const char *name)
static void
reggroups_dump (struct gdbarch *gdbarch, struct ui_file *file)
{
- struct reggroup *group = NULL;
+ static constexpr const char *fmt = " %-10s %-10s\n";

- do
+ gdb_printf (file, fmt, "Group", "Type");
+
+ for (const struct reggroup *group : gdbarch_reggroups (gdbarch))
{
/* Group name. */
- {
- const char *name;
-
- if (group == NULL)
- name = "Group";
- else
- name = reggroup_name (group);
- gdb_printf (file, " %-10s", name);
- }
+ const char *name = reggroup_name (group);

/* Group type. */
- {
- const char *type;
-
- if (group == NULL)
- type = "Type";
- else
- {
- switch (reggroup_type (group))
- {
- case USER_REGGROUP:
- type = "user";
- break;
- case INTERNAL_REGGROUP:
- type = "internal";
- break;
- default:
- internal_error (__FILE__, __LINE__, _("bad switch"));
- }
- }
- gdb_printf (file, " %-10s", type);
- }
+ const char *type;
+
+ switch (reggroup_type (group))
+ {
+ case USER_REGGROUP:
+ type = "user";
+ break;
+ case INTERNAL_REGGROUP:
+ type = "internal";
+ break;
+ default:
+ internal_error (__FILE__, __LINE__, _("bad switch"));
+ }

/* Note: If you change this, be sure to also update the
documentation. */
-
- gdb_printf (file, "\n");

- group = reggroup_next (gdbarch, group);
+ gdb_printf (file, fmt, name, type);
}
- while (group != NULL);
}

static void
diff --git a/gdb/reggroups.h b/gdb/reggroups.h
index bc87a7325f0..959191c4180 100644
--- a/gdb/reggroups.h
+++ b/gdb/reggroups.h
@@ -53,14 +53,10 @@ extern void reggroup_add (struct gdbarch *gdbarch, struct reggroup *group);
extern const char *reggroup_name (const struct reggroup *reggroup);
extern enum reggroup_type reggroup_type (const struct reggroup *reggroup);

-/* Iterators for the architecture's register groups. Pass in NULL, returns
- the first (for next), or last (for prev) group. Pass in a group,
- returns the next or previous group, or NULL when either the end or the
- beginning of the group list is reached. */
-extern struct reggroup *reggroup_next (struct gdbarch *gdbarch,
- const struct reggroup *last);
-extern struct reggroup *reggroup_prev (struct gdbarch *gdbarch,
- const struct reggroup *curr);
+/* Return the list of all register groups for GDBARCH. */
+extern const std::vector<const reggroup *>
+ gdbarch_reggroups (struct gdbarch *gdbarch);
+
/* Find a reggroup by name. */
extern const reggroup *reggroup_find (struct gdbarch *gdbarch,
const char *name);
diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index b968947fa1c..65ec8241370 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -523,10 +523,21 @@ tui_data_item_window::rerender (WINDOW *handle, int field_width)
static const reggroup *
tui_reg_next (const reggroup *current_group, struct gdbarch *gdbarch)
{
- const reggroup *group = reggroup_next (gdbarch, current_group);
- if (group == NULL)
- group = reggroup_next (gdbarch, NULL);
- return group;
+ const std::vector<const reggroup *> &groups = gdbarch_reggroups (gdbarch);
+
+ if (current_group != nullptr)
+ for (int i = 0; i < groups.size (); ++i)
+ {
+ if (groups[i] == current_group)
+ {
+ if (i + 1 < groups.size ())
+ return groups[i + 1];
+ else
+ return groups.front ();
+ }
+ }
+
+ return groups.front ();
}

/* Helper for "tui reg prev", wraps a call to REGGROUP_PREV, but adds wrap
@@ -538,10 +549,27 @@ tui_reg_next (const reggroup *current_group, struct gdbarch *gdbarch)
static const reggroup *
tui_reg_prev (const reggroup *current_group, struct gdbarch *gdbarch)
{
- const reggroup *group = reggroup_prev (gdbarch, current_group);
- if (group == NULL)
- group = reggroup_prev (gdbarch, NULL);
- return group;
+ const std::vector<const reggroup *> &groups = gdbarch_reggroups (gdbarch);
+
+ if (current_group != nullptr)
+ {
+ /* Iterate backwards. */
+ for (auto iter = groups.rbegin ();
+ iter != groups.rend ();
+ ++iter)
+ {
+ if (*iter == current_group)
+ {
+ ++iter;
+ if (iter == groups.rend ())
+ return groups.back ();
+ else
+ return *iter;
+ }
+ }
+ }
+
+ return groups.back ();
}

/* Implement the 'tui reg' command. Changes the register group displayed
@@ -555,7 +583,6 @@ tui_reg_command (const char *args, int from_tty)

if (args != NULL)
{
- const reggroup *group, *match = NULL;
size_t len = strlen (args);

/* Make sure the curses mode is enabled. */
@@ -569,6 +596,7 @@ tui_reg_command (const char *args, int from_tty)
if (TUI_DATA_WIN == NULL || !TUI_DATA_WIN->is_visible ())
tui_regs_layout ();

+ const reggroup *match = nullptr;
const reggroup *current_group = TUI_DATA_WIN->get_current_group ();
if (strncmp (args, "next", len) == 0)
match = tui_reg_next (current_group, gdbarch);
@@ -579,9 +607,7 @@ tui_reg_command (const char *args, int from_tty)
/* This loop matches on the initial part of a register group
name. If this initial part in ARGS matches only one register
group then the switch is made. */
- for (group = reggroup_next (gdbarch, NULL);
- group != NULL;
- group = reggroup_next (gdbarch, group))
+ for (const struct reggroup *group : gdbarch_reggroups (gdbarch))
{
if (strncmp (reggroup_name (group), args, len) == 0)
{
@@ -599,19 +625,16 @@ tui_reg_command (const char *args, int from_tty)
}
else
{
- const reggroup *group;
- int first;
-
gdb_printf (_("\"tui reg\" must be followed by the name of "
"either a register group,\nor one of 'next' "
"or 'prev'. Known register groups are:\n"));

- for (first = 1, group = reggroup_next (gdbarch, NULL);
- group != NULL;
- first = 0, group = reggroup_next (gdbarch, group))
+ bool first = true;
+ for (const struct reggroup *group : gdbarch_reggroups (gdbarch))
{
if (!first)
gdb_printf (", ");
+ first = false;
gdb_printf ("%s", reggroup_name (group));
}

--
2.25.4