Re: [PATCH v4 02/10] Make gdbserver reg_defs a vector of objects

[Alan Hayward <Alan.Hayward@arm.com>]

> On 23 Mar 2018, at 17:04, Simon Marchi <simon.marchi@polymtl.ca> wrote:
> 
> On 2018-03-23 12:52, Alan Hayward wrote:
>>> The offset value is always 0 initially, so you can remove it and initialize it to 0.
>> Reason I added offset here was that in "Commonise tdesc_reg” this code
>> will get merged into init_target_desc().
>> At that point I’ll be creating a reg and setting and offset at the same time.
>> This was just so that I didn’t need to touch regdef.h again.
>> I can still remove offset from this patch if you want - given that I’m
>> not using it yet?
> 
> This it not terribly important, but I think it's better to only add it once you really need it.  The proposed patches could change, and the parameter could end up unused (not saying that's what will happen here, it's just an example).
> 
> Simon

Ok, pushed as suggested with offset initialised to 0.

Diff for reference:

diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c
index d6511fda650ca52688cd4d7db1acd94e822a3c0d..cbdf766df2c2b95f605198c617ffead9f9ac3775 100644
--- a/gdb/gdbserver/regcache.c
+++ b/gdb/gdbserver/regcache.c
@@ -196,9 +196,9 @@ regcache_cpy (struct regcache *dst, struct regcache *src)
   dst->registers_valid = src->registers_valid;
 }

-/* Return a pointer to the description of register N.  */
+/* Return a reference to the description of register N.  */

-static const struct reg *
+static const struct reg &
 find_register_by_number (const struct target_desc *tdesc, int n)
 {
   return tdesc->reg_defs[n];
@@ -251,7 +251,7 @@ find_regno (const struct target_desc *tdesc, const char *name)
 {
   for (int i = 0; i < tdesc->reg_defs.size (); ++i)
     {
-      if (strcmp (name, find_register_by_number (tdesc, i)->name) == 0)
+      if (strcmp (name, find_register_by_number (tdesc, i).name) == 0)
 	return i;
     }
   internal_error (__FILE__, __LINE__, "Unknown register %s requested",
@@ -288,7 +288,7 @@ register_cache_size (const struct target_desc *tdesc)
 int
 register_size (const struct target_desc *tdesc, int n)
 {
-  return find_register_by_number (tdesc, n)->size / 8;
+  return find_register_by_number (tdesc, n).size / 8;
 }

 /* See common/common-regcache.h.  */
@@ -303,7 +303,7 @@ static unsigned char *
 register_data (struct regcache *regcache, int n, int fetch)
 {
   return (regcache->registers
-	  + find_register_by_number (regcache->tdesc, n)->offset / 8);
+	  + find_register_by_number (regcache->tdesc, n).offset / 8);
 }

 /* Supply register N, whose contents are stored in BUF, to REGCACHE.
diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
index 4513ea74232a456cc86eb9a655904012ff117373..a62544341cd23a9a8ec6833e1eae73616a315d2d 100644
--- a/gdb/gdbserver/tdesc.h
+++ b/gdb/gdbserver/tdesc.h
@@ -34,7 +34,7 @@ struct target_desc : tdesc_feature
 {
   /* A vector of elements of register definitions that
      describe the inferior's register set.  */
-  std::vector<struct reg *> reg_defs;
+  std::vector<struct reg> reg_defs;

   /* The register cache size, in bytes.  */
   int registers_size;
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index e50a848e2f9f280a84ab139cfce4d1f17bd05884..cec7a66f9748f7295462c76fdae3d3e9029ee421 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -25,9 +25,6 @@ target_desc::~target_desc ()
 {
   int i;

-  for (reg *reg : reg_defs)
-    xfree (reg);
-
   xfree ((char *) arch);
   xfree ((char *) osabi);

@@ -40,18 +37,9 @@ target_desc::~target_desc ()

 bool target_desc::operator== (const target_desc &other) const
 {
-  if (reg_defs.size () != other.reg_defs.size ())
+  if (reg_defs != other.reg_defs)
     return false;

-  for (int i = 0; i < reg_defs.size (); ++i)
-    {
-      struct reg *reg = reg_defs[i];
-      struct reg *reg2 = other.reg_defs[i];
-
-      if (reg != reg2 && *reg != *reg2)
-	return false;
-    }
-
   /* Compare expedite_regs.  */
   int i = 0;
   for (; expedite_regs[i] != NULL; i++)
@@ -72,10 +60,10 @@ init_target_desc (struct target_desc *tdesc)
 {
   int offset = 0;

-  for (reg *reg : tdesc->reg_defs)
+  for (reg &reg : tdesc->reg_defs)
     {
-      reg->offset = offset;
-      offset += reg->size;
+      reg.offset = offset;
+      offset += reg.size;
     }

   tdesc->registers_size = offset / 8;
@@ -241,23 +229,12 @@ tdesc_create_reg (struct tdesc_feature *feature, const char *name,
 {
   struct target_desc *tdesc = (struct target_desc *) feature;

-  while (tdesc->reg_defs.size () < regnum)
-    {
-      struct reg *reg = XCNEW (struct reg);
-
-      reg->name = "";
-      reg->size = 0;
-      tdesc->reg_defs.push_back (reg);
-    }
-
-  gdb_assert (regnum == 0
-	      || regnum == tdesc->reg_defs.size ());
+  gdb_assert (regnum == 0 || regnum >= tdesc->reg_defs.size ());

-  struct reg *reg = XCNEW (struct reg);
+  if (regnum != 0)
+    tdesc->reg_defs.resize (regnum);

-  reg->name = name;
-  reg->size = bitsize;
-  tdesc->reg_defs.push_back (reg);
+  tdesc->reg_defs.emplace_back (name, bitsize);
 }

 /* See common/tdesc.h.  */
diff --git a/gdb/regformats/regdef.h b/gdb/regformats/regdef.h
index 262d03c0785f48e83b784b6177c52e2d253a6067..4775e863e9acc516c0d0ab90baf428604eb967c4 100644
--- a/gdb/regformats/regdef.h
+++ b/gdb/regformats/regdef.h
@@ -21,6 +21,18 @@

 struct reg
 {
+  reg ()
+    : name (""),
+      offset (0),
+      size (0)
+  {}
+
+  reg (const char *_name, int _size)
+    : name (_name),
+      offset (0),
+      size (_size)
+  {}
+
   /* The name of this register - NULL for pad entries.  */
   const char *name;