Patchwork [03/25] Class-fy tdesc_reg tdesc_type and tdesc_feature

login
register
mail settings
Submitter Yao Qi
Date June 12, 2017, 8:41 a.m.
Message ID <1497256916-4958-4-git-send-email-yao.qi@linaro.org>
Download mbox | patch
Permalink /patch/20919/
State New
Headers show

Comments

Yao Qi - June 12, 2017, 8:41 a.m.
This patch class-fies them, adding ctor, dtor, and deleting
copy ctor and assignment operator.

gdb:

2017-05-20  Yao Qi  <yao.qi@linaro.org>

	* target-descriptions.c (tdesc_reg): Add ctor, dtor.
	Delete copy ctor and assignment operator.
	(tdesc_type): Likewise.
	(tdesc_feature): Likewise.
	(tdesc_free_reg): Remove.
	(tdesc_create_reg): Use new.
	(tdesc_free_type): Remove.
	(tdesc_create_vector): Use new.
	(tdesc_create_union): Likewise.
	(tdesc_create_flags): Likewise.
	(tdesc_create_enum): Likewise.
	(tdesc_free_feature): Delete.
	(free_target_description): Use delete.
---
 gdb/target-descriptions.c | 200 +++++++++++++++++++++++-----------------------
 1 file changed, 100 insertions(+), 100 deletions(-)
Simon Marchi - June 19, 2017, 8:55 p.m.
On 2017-06-12 10:41, Yao Qi wrote:
> This patch class-fies them, adding ctor, dtor, and deleting
> copy ctor and assignment operator.
> 
> gdb:
> 
> 2017-05-20  Yao Qi  <yao.qi@linaro.org>
> 
> 	* target-descriptions.c (tdesc_reg): Add ctor, dtor.
> 	Delete copy ctor and assignment operator.
> 	(tdesc_type): Likewise.
> 	(tdesc_feature): Likewise.
> 	(tdesc_free_reg): Remove.
> 	(tdesc_create_reg): Use new.
> 	(tdesc_free_type): Remove.
> 	(tdesc_create_vector): Use new.
> 	(tdesc_create_union): Likewise.
> 	(tdesc_create_flags): Likewise.
> 	(tdesc_create_enum): Likewise.
> 	(tdesc_free_feature): Delete.
> 	(free_target_description): Use delete.
> ---
>  gdb/target-descriptions.c | 200 
> +++++++++++++++++++++++-----------------------
>  1 file changed, 100 insertions(+), 100 deletions(-)
> 
> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index 9a7e2dd..e2dcd1d 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -48,6 +48,32 @@ DEF_VEC_O(property_s);
> 
>  typedef struct tdesc_reg
>  {
> +public:

Since you are using the "struct" keyword, members are public by default, 
so you don't have to put "public:".  Same things for other types.

Otherwise it LGTM.  Of course, more c++ification is always possible, but 
I think it's fine to go in small increments.

Just FYI, I tested this on top of my "poison-xnew" branch, which points 
out the invalid use of XNEW/xfree on non-POD types, and it built fine.

Simon
Simon Marchi - June 19, 2017, 9:30 p.m.
On 2017-06-12 10:41, Yao Qi wrote:
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -48,6 +48,32 @@ DEF_VEC_O(property_s);
> 
>  typedef struct tdesc_reg
>  {
> +public:
> +  tdesc_reg (struct tdesc_feature *feature, const char *name_,
> +	     int regnum, int save_restore_, const char *group_,
> +	     int bitsize_, const char *type_)
> +    : name (xstrdup (name_)), target_regnum (regnum),
> +      save_restore (save_restore_),
> +      group (group_ ? xstrdup (group_) : NULL),
> +      bitsize (bitsize_),
> +      type (type_ ? xstrdup (type_) : xstrdup ("<unknown>"))

Oh, I forgot to mention: you could take this opportunity to make these 
expression respect our coding style (use != NULL to check for NULL 
pointer).

Simon

Patch

diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 9a7e2dd..e2dcd1d 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -48,6 +48,32 @@  DEF_VEC_O(property_s);
 
 typedef struct tdesc_reg
 {
+public:
+  tdesc_reg (struct tdesc_feature *feature, const char *name_,
+	     int regnum, int save_restore_, const char *group_,
+	     int bitsize_, const char *type_)
+    : name (xstrdup (name_)), target_regnum (regnum),
+      save_restore (save_restore_),
+      group (group_ ? xstrdup (group_) : NULL),
+      bitsize (bitsize_),
+      type (type_ ? xstrdup (type_) : xstrdup ("<unknown>"))
+  {
+    /* If the register's type is target-defined, look it up now.  We may not
+       have easy access to the containing feature when we want it later.  */
+    tdesc_type = tdesc_named_type (feature, type);
+  }
+
+  ~tdesc_reg ()
+  {
+    xfree (name);
+    xfree (type);
+    xfree (group);
+  }
+
+  /* Disable copying.  */
+  tdesc_reg (const tdesc_reg &) = delete;
+  tdesc_reg &operator= (const tdesc_reg &) = delete;
+
   /* The name of this register.  In standard features, it may be
      recognized by the architecture support code, or it may be purely
      for the user.  */
@@ -128,6 +154,43 @@  enum tdesc_type_kind
 
 typedef struct tdesc_type
 {
+public:
+  tdesc_type (const char *name_, enum tdesc_type_kind kind_)
+    : name (xstrdup (name_)), kind (kind_)
+  {
+    memset (&u, 0, sizeof (u));
+  }
+
+  ~tdesc_type ()
+  {
+    switch (kind)
+      {
+      case TDESC_TYPE_STRUCT:
+      case TDESC_TYPE_UNION:
+      case TDESC_TYPE_FLAGS:
+      case TDESC_TYPE_ENUM:
+	{
+	  struct tdesc_type_field *f;
+	  int ix;
+
+	  for (ix = 0;
+	       VEC_iterate (tdesc_type_field, u.u.fields, ix, f);
+	       ix++)
+	    xfree (f->name);
+
+	  VEC_free (tdesc_type_field, u.u.fields);
+	}
+	break;
+
+      default:
+	break;
+      }
+    xfree ((char *) name);
+  }
+  /* Disable copying.  */
+  tdesc_type (const tdesc_type &) = delete;
+  tdesc_type &operator= (const tdesc_type &) = delete;
+
   /* The name of this type.  If this type is a built-in type, this is
      a pointer to a constant string.  Otherwise, it's a
      malloc-allocated string (and thus must be freed).  */
@@ -161,15 +224,41 @@  DEF_VEC_P(tdesc_type_p);
 
 typedef struct tdesc_feature
 {
+public:
+  tdesc_feature (const char *name_)
+    : name (xstrdup (name_))
+  {}
+
+  ~tdesc_feature ()
+  {
+    struct tdesc_reg *reg;
+    struct tdesc_type *type;
+    int ix;
+
+    for (ix = 0; VEC_iterate (tdesc_reg_p, registers, ix, reg); ix++)
+      delete reg;
+    VEC_free (tdesc_reg_p, registers);
+
+    for (ix = 0; VEC_iterate (tdesc_type_p, types, ix, type); ix++)
+      delete type;
+    VEC_free (tdesc_type_p, types);
+
+    xfree (name);
+  }
+
+  /* Disable copying.  */
+  tdesc_feature (const tdesc_feature &) = delete;
+  tdesc_feature &operator= (const tdesc_feature &) = delete;
+
   /* The name of this feature.  It may be recognized by the architecture
      support code.  */
   char *name;
 
   /* The registers associated with this feature.  */
-  VEC(tdesc_reg_p) *registers;
+  VEC(tdesc_reg_p) *registers = NULL;
 
   /* The types associated with this feature.  */
-  VEC(tdesc_type_p) *types;
+  VEC(tdesc_type_p) *types = NULL;
 } *tdesc_feature_p;
 DEF_VEC_P(tdesc_feature_p);
 
@@ -1274,81 +1363,23 @@  tdesc_use_registers (struct gdbarch *gdbarch,
 }
 
 
-/* Methods for constructing a target description.  */
-
-static void
-tdesc_free_reg (struct tdesc_reg *reg)
-{
-  xfree (reg->name);
-  xfree (reg->type);
-  xfree (reg->group);
-  xfree (reg);
-}
-
 void
 tdesc_create_reg (struct tdesc_feature *feature, const char *name,
 		  int regnum, int save_restore, const char *group,
 		  int bitsize, const char *type)
 {
-  struct tdesc_reg *reg = XCNEW (struct tdesc_reg);
-
-  reg->name = xstrdup (name);
-  reg->target_regnum = regnum;
-  reg->save_restore = save_restore;
-  reg->group = group ? xstrdup (group) : NULL;
-  reg->bitsize = bitsize;
-  reg->type = type ? xstrdup (type) : xstrdup ("<unknown>");
-
-  /* If the register's type is target-defined, look it up now.  We may not
-     have easy access to the containing feature when we want it later.  */
-  reg->tdesc_type = tdesc_named_type (feature, reg->type);
+  tdesc_reg *reg = new tdesc_reg (feature, name, regnum, save_restore,
+				  group, bitsize, type);
 
   VEC_safe_push (tdesc_reg_p, feature->registers, reg);
 }
 
-/* Subroutine of tdesc_free_feature to simplify it.
-   Note: We do not want to free any referenced types here (e.g., types of
-   fields of a struct).  All types of a feature are recorded in
-   feature->types and are freed that way.  */
-
-static void
-tdesc_free_type (struct tdesc_type *type)
-{
-  switch (type->kind)
-    {
-    case TDESC_TYPE_STRUCT:
-    case TDESC_TYPE_UNION:
-    case TDESC_TYPE_FLAGS:
-    case TDESC_TYPE_ENUM:
-      {
-	struct tdesc_type_field *f;
-	int ix;
-
-	for (ix = 0;
-	     VEC_iterate (tdesc_type_field, type->u.u.fields, ix, f);
-	     ix++)
-	  xfree (f->name);
-
-	VEC_free (tdesc_type_field, type->u.u.fields);
-      }
-      break;
-
-    default:
-      break;
-    }
-
-  xfree ((char *) type->name);
-  xfree (type);
-}
-
 struct tdesc_type *
 tdesc_create_vector (struct tdesc_feature *feature, const char *name,
 		     struct tdesc_type *field_type, int count)
 {
-  struct tdesc_type *type = XCNEW (struct tdesc_type);
+  struct tdesc_type *type = new tdesc_type (name, TDESC_TYPE_VECTOR);
 
-  type->name = xstrdup (name);
-  type->kind = TDESC_TYPE_VECTOR;
   type->u.v.type = field_type;
   type->u.v.count = count;
 
@@ -1359,10 +1390,7 @@  tdesc_create_vector (struct tdesc_feature *feature, const char *name,
 struct tdesc_type *
 tdesc_create_struct (struct tdesc_feature *feature, const char *name)
 {
-  struct tdesc_type *type = XCNEW (struct tdesc_type);
-
-  type->name = xstrdup (name);
-  type->kind = TDESC_TYPE_STRUCT;
+  struct tdesc_type *type = new tdesc_type (name, TDESC_TYPE_STRUCT);
 
   VEC_safe_push (tdesc_type_p, feature->types, type);
   return type;
@@ -1383,10 +1411,7 @@  tdesc_set_struct_size (struct tdesc_type *type, int size)
 struct tdesc_type *
 tdesc_create_union (struct tdesc_feature *feature, const char *name)
 {
-  struct tdesc_type *type = XCNEW (struct tdesc_type);
-
-  type->name = xstrdup (name);
-  type->kind = TDESC_TYPE_UNION;
+  struct tdesc_type *type = new tdesc_type (name, TDESC_TYPE_UNION);
 
   VEC_safe_push (tdesc_type_p, feature->types, type);
   return type;
@@ -1396,12 +1421,10 @@  struct tdesc_type *
 tdesc_create_flags (struct tdesc_feature *feature, const char *name,
 		    int size)
 {
-  struct tdesc_type *type = XCNEW (struct tdesc_type);
+  struct tdesc_type *type = new tdesc_type (name, TDESC_TYPE_FLAGS);
 
   gdb_assert (size > 0);
 
-  type->name = xstrdup (name);
-  type->kind = TDESC_TYPE_FLAGS;
   type->u.u.size = size;
 
   VEC_safe_push (tdesc_type_p, feature->types, type);
@@ -1412,12 +1435,10 @@  struct tdesc_type *
 tdesc_create_enum (struct tdesc_feature *feature, const char *name,
 		   int size)
 {
-  struct tdesc_type *type = XCNEW (struct tdesc_type);
+  struct tdesc_type *type = new tdesc_type (name, TDESC_TYPE_ENUM);
 
   gdb_assert (size > 0);
 
-  type->name = xstrdup (name);
-  type->kind = TDESC_TYPE_ENUM;
   type->u.u.size = size;
 
   VEC_safe_push (tdesc_type_p, feature->types, type);
@@ -1521,31 +1542,10 @@  tdesc_add_enum_value (struct tdesc_type *type, int value,
   VEC_safe_push (tdesc_type_field, type->u.u.fields, &f);
 }
 
-static void
-tdesc_free_feature (struct tdesc_feature *feature)
-{
-  struct tdesc_reg *reg;
-  struct tdesc_type *type;
-  int ix;
-
-  for (ix = 0; VEC_iterate (tdesc_reg_p, feature->registers, ix, reg); ix++)
-    tdesc_free_reg (reg);
-  VEC_free (tdesc_reg_p, feature->registers);
-
-  for (ix = 0; VEC_iterate (tdesc_type_p, feature->types, ix, type); ix++)
-    tdesc_free_type (type);
-  VEC_free (tdesc_type_p, feature->types);
-
-  xfree (feature->name);
-  xfree (feature);
-}
-
 struct tdesc_feature *
 tdesc_create_feature (struct target_desc *tdesc, const char *name)
 {
-  struct tdesc_feature *new_feature = XCNEW (struct tdesc_feature);
-
-  new_feature->name = xstrdup (name);
+  struct tdesc_feature *new_feature = new tdesc_feature (name);
 
   VEC_safe_push (tdesc_feature_p, tdesc->features, new_feature);
   return new_feature;
@@ -1568,7 +1568,7 @@  free_target_description (void *arg)
   for (ix = 0;
        VEC_iterate (tdesc_feature_p, target_desc->features, ix, feature);
        ix++)
-    tdesc_free_feature (feature);
+    delete feature;
   VEC_free (tdesc_feature_p, target_desc->features);
 
   for (ix = 0;