This is the mail archive of the gdb-cvs@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[binutils-gdb] C++ify xml-syscall.c


https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=5a9dcda14c240eccee5c1de53cf4ae936230ed0c

commit 5a9dcda14c240eccee5c1de53cf4ae936230ed0c
Author: Simon Marchi <simon.marchi@polymtl.ca>
Date:   Fri Oct 27 22:23:33 2017 -0400

    C++ify xml-syscall.c
    
    This patch C++ifies the structures in xml-syscall.c, by using
    std::vector instead of VEC, and std::string instead of char*.
    Using a unique_ptr in syscall_parse_xml allows to remove a cleanup.
    
    Something that seems strange with the existing code, if you look at
    syscalls_info_free_syscalls_desc and
    syscalls_info_free_syscall_group_desc, they free the structure elements
    (the strings and vectors), but they don't free the syscall_desc and
    syscall_group_desc structure themselves.  I don't see anything freeing
    those currently.  Any idea why?  According to the comment above
    syscalls_info_free_syscall_group_desc, it kinda looks like it's on
    purpose.  With this patch, those structures are deleted when the vector
    that contains them gets deleted.
    
    The only time I'm aware a syscalls_info structure gets deleted is in the
    case the data directory changes during runtime, in init_syscalls_info.
    If tried that use case (including under valgrind):
    
     (gdb) catch syscall
     (gdb) set data-directory another-data-directory
     (gdb) catch syscall
    
    I confirmed that the syscalls_info structure got deleted and recreated,
    and everything seemed fine.
    
    Regtested on the buildbot.
    
    gdb/ChangeLog:
    
    	* xml-syscall.c (struct syscall_desc): Add constructor.
    	<name>: Change type to std::string.
    	(syscall_desc_up): New typedef.
    	(syscall_desc_p): Remove typeder.
    	(DEF_VEC_P(syscall_desc_p)): Remove.
    	(struct syscall_group_desc): Add constructor.
    	<name>: Change type to std::string.
    	<syscalls>: Change type to std::vector.
    	(syscall_group_desc_up): New typedef.
    	(syscall_group_desc_p): Remove typedef.
    	(DEF_VEC_P(syscall_group_desc_p)): Remove.
    	(struct syscalls_info) <syscalls>: Change type to std::vector of
    	unique_ptr.
    	<groups>: Likewise.
    	<my_gdb_datadir>: Change type to std::string.
    	(syscalls_info_up): New typedef.
    	(allocate_syscalls_info): Remove.
    	(syscalls_info_free_syscalls_desc): Remove.
    	(syscalls_info_free_syscall_group_desc): Remove.
    	(free_syscalls_info): Remove.
    	(make_cleanup_free_syscalls_info): Remove.
    	(syscall_group_create_syscall_group_desc): Adjust.
    	(syscall_group_add_syscall): Adjust.
    	(syscall_create_syscall_desc): Adjust.
    	(syscall_parse_xml): Adjust, use unique_ptr instead of cleanup.
    	(init_syscalls_info): Adjust.
    	(syscall_group_get_group_by_name): Adjust.
    	(xml_get_syscall_number): Adjust.
    	(xml_get_syscall_name): Adjust.
    	(xml_list_of_syscalls): Adjust.
    	(xml_list_syscalls_by_group): Adjust.
    	(xml_list_of_groups): Adjust.

Diff:
---
 gdb/ChangeLog     |  35 +++++++++
 gdb/xml-syscall.c | 222 ++++++++++++++++++------------------------------------
 2 files changed, 108 insertions(+), 149 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9c4e409..51e3422 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,38 @@
+2017-10-27  Simon Marchi  <simon.marchi@polymtl.ca>
+
+	* xml-syscall.c (struct syscall_desc): Add constructor.
+	<name>: Change type to std::string.
+	(syscall_desc_up): New typedef.
+	(syscall_desc_p): Remove typeder.
+	(DEF_VEC_P(syscall_desc_p)): Remove.
+	(struct syscall_group_desc): Add constructor.
+	<name>: Change type to std::string.
+	<syscalls>: Change type to std::vector.
+	(syscall_group_desc_up): New typedef.
+	(syscall_group_desc_p): Remove typedef.
+	(DEF_VEC_P(syscall_group_desc_p)): Remove.
+	(struct syscalls_info) <syscalls>: Change type to std::vector of
+	unique_ptr.
+	<groups>: Likewise.
+	<my_gdb_datadir>: Change type to std::string.
+	(syscalls_info_up): New typedef.
+	(allocate_syscalls_info): Remove.
+	(syscalls_info_free_syscalls_desc): Remove.
+	(syscalls_info_free_syscall_group_desc): Remove.
+	(free_syscalls_info): Remove.
+	(make_cleanup_free_syscalls_info): Remove.
+	(syscall_group_create_syscall_group_desc): Adjust.
+	(syscall_group_add_syscall): Adjust.
+	(syscall_create_syscall_desc): Adjust.
+	(syscall_parse_xml): Adjust, use unique_ptr instead of cleanup.
+	(init_syscalls_info): Adjust.
+	(syscall_group_get_group_by_name): Adjust.
+	(xml_get_syscall_number): Adjust.
+	(xml_get_syscall_name): Adjust.
+	(xml_list_of_syscalls): Adjust.
+	(xml_list_syscalls_by_group): Adjust.
+	(xml_list_of_groups): Adjust.
+
 2017-10-27  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* probe.h: Don't include gdb_vecs.h.
diff --git a/gdb/xml-syscall.c b/gdb/xml-syscall.c
index fe036cb..307c304 100644
--- a/gdb/xml-syscall.c
+++ b/gdb/xml-syscall.c
@@ -94,49 +94,62 @@ get_syscall_group_names (struct gdbarch *gdbarch)
 #else /* ! HAVE_LIBEXPAT */
 
 /* Structure which describes a syscall.  */
-typedef struct syscall_desc
+struct syscall_desc
 {
+  syscall_desc (int number_, std::string name_)
+  : number (number_), name (name_)
+  {}
+
   /* The syscall number.  */
 
   int number;
 
   /* The syscall name.  */
 
-  char *name;
-} *syscall_desc_p;
-DEF_VEC_P(syscall_desc_p);
+  std::string name;
+};
+
+typedef std::unique_ptr<syscall_desc> syscall_desc_up;
 
 /* Structure of a syscall group.  */
-typedef struct syscall_group_desc
+struct syscall_group_desc
 {
+  syscall_group_desc (const std::string &name_)
+  : name (name_)
+  {}
+
   /* The group name.  */
 
-  char *name;
+  std::string name;
 
-  /* The syscalls that are part of the group.  */
+  /* The syscalls that are part of the group.  This is a non-owning
+     reference.  */
 
-  VEC(syscall_desc_p) *syscalls;
-} *syscall_group_desc_p;
-DEF_VEC_P(syscall_group_desc_p);
+  std::vector<syscall_desc *> syscalls;
+};
+
+typedef std::unique_ptr<syscall_group_desc> syscall_group_desc_up;
 
 /* Structure that represents syscalls information.  */
 struct syscalls_info
 {
   /* The syscalls.  */
 
-  VEC(syscall_desc_p) *syscalls;
+  std::vector<syscall_desc_up> syscalls;
 
   /* The syscall groups.  */
 
-  VEC(syscall_group_desc_p) *groups;
+  std::vector<syscall_group_desc_up> groups;
 
   /* Variable that will hold the last known data-directory.  This is
      useful to know whether we should re-read the XML info for the
      target.  */
 
-  char *my_gdb_datadir;
+  std::string my_gdb_datadir;
 };
 
+typedef std::unique_ptr<syscalls_info> syscalls_info_up;
+
 /* Callback data for syscall information parsing.  */
 struct syscall_parsing_data
 {
@@ -145,66 +158,6 @@ struct syscall_parsing_data
   struct syscalls_info *syscalls_info;
 };
 
-static struct syscalls_info *
-allocate_syscalls_info (void)
-{
-  return XCNEW (struct syscalls_info);
-}
-
-static void
-syscalls_info_free_syscalls_desc (struct syscall_desc *sd)
-{
-  xfree (sd->name);
-}
-
-/* Free syscall_group_desc members but not the structure itself.  */
-
-static void
-syscalls_info_free_syscall_group_desc (struct syscall_group_desc *sd)
-{
-  VEC_free (syscall_desc_p, sd->syscalls);
-  xfree (sd->name);
-}
-
-static void
-free_syscalls_info (void *arg)
-{
-  struct syscalls_info *syscalls_info = (struct syscalls_info *) arg;
-  struct syscall_desc *sysdesc;
-  struct syscall_group_desc *groupdesc;
-  int i;
-
-  xfree (syscalls_info->my_gdb_datadir);
-
-  if (syscalls_info->syscalls != NULL)
-    {
-      for (i = 0;
-	   VEC_iterate (syscall_desc_p, syscalls_info->syscalls, i, sysdesc);
-	   i++)
-	syscalls_info_free_syscalls_desc (sysdesc);
-      VEC_free (syscall_desc_p, syscalls_info->syscalls);
-    }
-
-  if (syscalls_info->groups != NULL)
-    {
-      for (i = 0;
-	   VEC_iterate (syscall_group_desc_p,
-			syscalls_info->groups, i, groupdesc);
-	   i++)
-	syscalls_info_free_syscall_group_desc (groupdesc);
-
-      VEC_free (syscall_group_desc_p, syscalls_info->groups);
-    }
-
-  xfree (syscalls_info);
-}
-
-static struct cleanup *
-make_cleanup_free_syscalls_info (struct syscalls_info *syscalls_info)
-{
-  return make_cleanup (free_syscalls_info, syscalls_info);
-}
-
 /* Create a new syscall group.  Return pointer to the
    syscall_group_desc structure that represents the new group.  */
 
@@ -212,11 +165,9 @@ static struct syscall_group_desc *
 syscall_group_create_syscall_group_desc (struct syscalls_info *syscalls_info,
 					 const char *group)
 {
-  struct syscall_group_desc *groupdesc = XCNEW (struct syscall_group_desc);
+  syscall_group_desc *groupdesc = new syscall_group_desc (group);
 
-  groupdesc->name = xstrdup (group);
-
-  VEC_safe_push (syscall_group_desc_p, syscalls_info->groups, groupdesc);
+  syscalls_info->groups.emplace_back (groupdesc);
 
   return groupdesc;
 }
@@ -228,19 +179,21 @@ syscall_group_add_syscall (struct syscalls_info *syscalls_info,
 			   struct syscall_desc *syscall,
 			   const char *group)
 {
-  struct syscall_group_desc *groupdesc = NULL;
-  int i;
-
   /* Search for an existing group.  */
-  for (i = 0;
-       VEC_iterate (syscall_group_desc_p, syscalls_info->groups, i, groupdesc);
-       i++)
+  std::vector<syscall_group_desc_up>::iterator it
+    = syscalls_info->groups.begin ();
+
+  for (; it != syscalls_info->groups.end (); it++)
     {
-      if (strcmp (groupdesc->name, group) == 0)
+      if ((*it)->name == group)
 	break;
     }
 
-  if (groupdesc == NULL)
+  syscall_group_desc *groupdesc;
+
+  if (it != syscalls_info->groups.end ())
+    groupdesc = it->get ();
+  else
     {
       /* No group was found with this name.  We must create a new
 	 one.  */
@@ -248,7 +201,7 @@ syscall_group_add_syscall (struct syscalls_info *syscalls_info,
 							   group);
     }
 
-  VEC_safe_push (syscall_desc_p, groupdesc->syscalls, syscall);
+  groupdesc->syscalls.push_back (syscall);
 }
 
 static void
@@ -256,18 +209,14 @@ syscall_create_syscall_desc (struct syscalls_info *syscalls_info,
 			     const char *name, int number,
 			     char *groups)
 {
-  struct syscall_desc *sysdesc = XCNEW (struct syscall_desc);
-  char *group;
-
-  sysdesc->name = xstrdup (name);
-  sysdesc->number = number;
+  syscall_desc *sysdesc = new syscall_desc (number, name);
 
-  VEC_safe_push (syscall_desc_p, syscalls_info->syscalls, sysdesc);
+  syscalls_info->syscalls.emplace_back (sysdesc);
 
   /*  Add syscall to its groups.  */
   if (groups != NULL)
     {
-      for (group = strtok (groups, ",");
+      for (char *group = strtok (groups, ",");
 	   group != NULL;
 	   group = strtok (NULL, ","))
 	syscall_group_add_syscall (syscalls_info, sysdesc, group);
@@ -333,23 +282,20 @@ static struct syscalls_info *
 syscall_parse_xml (const char *document, xml_fetch_another fetcher,
                    void *fetcher_baton)
 {
-  struct cleanup *result_cleanup;
   struct syscall_parsing_data data;
+  syscalls_info_up sysinfo (new syscalls_info ());
 
-  data.syscalls_info = allocate_syscalls_info ();
-  result_cleanup = make_cleanup_free_syscalls_info (data.syscalls_info);
+  data.syscalls_info = sysinfo.get ();
 
   if (gdb_xml_parse_quick (_("syscalls info"), NULL,
 			   syselements, document, &data) == 0)
     {
       /* Parsed successfully.  */
-      discard_cleanups (result_cleanup);
-      return data.syscalls_info;
+      return sysinfo.release ();
     }
   else
     {
       warning (_("Could not load XML syscalls info; ignoring"));
-      do_cleanups (result_cleanup);
       return NULL;
     }
 }
@@ -381,12 +327,13 @@ init_syscalls_info (struct gdbarch *gdbarch)
   const char *xml_syscall_file = gdbarch_xml_syscall_file (gdbarch);
 
   /* Should we re-read the XML info for this target?  */
-  if (syscalls_info != NULL && syscalls_info->my_gdb_datadir != NULL
-      && filename_cmp (syscalls_info->my_gdb_datadir, gdb_datadir) != 0)
+  if (syscalls_info != NULL && !syscalls_info->my_gdb_datadir.empty ()
+      && filename_cmp (syscalls_info->my_gdb_datadir.c_str (),
+		       gdb_datadir) != 0)
     {
       /* The data-directory changed from the last time we used it.
 	 It means that we have to re-read the XML info.  */
-      free_syscalls_info (syscalls_info);
+      delete syscalls_info;
       syscalls_info = NULL;
       set_gdbarch_syscalls_info (gdbarch, NULL);
     }
@@ -401,9 +348,9 @@ init_syscalls_info (struct gdbarch *gdbarch)
      gdbarch->syscalls_info anyway, in order to store information
      about our attempt.  */
   if (syscalls_info == NULL)
-    syscalls_info = allocate_syscalls_info ();
+    syscalls_info = new struct syscalls_info ();
 
-  if (syscalls_info->syscalls == NULL)
+  if (syscalls_info->syscalls.empty ())
     {
       if (xml_syscall_file != NULL)
 	warning (_("Could not load the syscall XML file `%s/%s'."),
@@ -417,7 +364,7 @@ init_syscalls_info (struct gdbarch *gdbarch)
     }
 
   /* Saving the data-directory used to read this XML info.  */
-  syscalls_info->my_gdb_datadir = xstrdup (gdb_datadir);
+  syscalls_info->my_gdb_datadir.assign (gdb_datadir);
 
   set_gdbarch_syscalls_info (gdbarch, syscalls_info);
 }
@@ -429,22 +376,17 @@ static struct syscall_group_desc *
 syscall_group_get_group_by_name (const struct syscalls_info *syscalls_info,
 				 const char *group)
 {
-  struct syscall_group_desc *groupdesc;
-  int i;
-
   if (syscalls_info == NULL)
     return NULL;
 
   if (group == NULL)
     return NULL;
 
-   /* Search for existing group.  */
-  for (i = 0;
-       VEC_iterate (syscall_group_desc_p, syscalls_info->groups, i, groupdesc);
-       i++)
+  /* Search for existing group.  */
+  for (const syscall_group_desc_up &groupdesc : syscalls_info->groups)
     {
-      if (strcmp (groupdesc->name, group) == 0)
-	return groupdesc;
+      if (groupdesc->name == group)
+	return groupdesc.get ();
     }
 
   return NULL;
@@ -455,17 +397,13 @@ xml_get_syscall_number (struct gdbarch *gdbarch,
                         const char *syscall_name)
 {
   struct syscalls_info *syscalls_info = gdbarch_syscalls_info (gdbarch);
-  struct syscall_desc *sysdesc;
-  int i;
 
   if (syscalls_info == NULL
       || syscall_name == NULL)
     return UNKNOWN_SYSCALL;
 
-  for (i = 0;
-       VEC_iterate(syscall_desc_p, syscalls_info->syscalls, i, sysdesc);
-       i++)
-    if (strcmp (sysdesc->name, syscall_name) == 0)
+  for (const syscall_desc_up &sysdesc : syscalls_info->syscalls)
+    if (sysdesc->name == syscall_name)
       return sysdesc->number;
 
   return UNKNOWN_SYSCALL;
@@ -476,18 +414,14 @@ xml_get_syscall_name (struct gdbarch *gdbarch,
                       int syscall_number)
 {
   struct syscalls_info *syscalls_info = gdbarch_syscalls_info (gdbarch);
-  struct syscall_desc *sysdesc;
-  int i;
 
   if (syscalls_info == NULL
       || syscall_number < 0)
     return NULL;
 
-  for (i = 0;
-       VEC_iterate(syscall_desc_p, syscalls_info->syscalls, i, sysdesc);
-       i++)
+  for (const syscall_desc_up &sysdesc : syscalls_info->syscalls)
     if (sysdesc->number == syscall_number)
-      return sysdesc->name;
+      return sysdesc->name.c_str ();
 
   return NULL;
 }
@@ -496,21 +430,16 @@ static const char **
 xml_list_of_syscalls (struct gdbarch *gdbarch)
 {
   struct syscalls_info *syscalls_info = gdbarch_syscalls_info (gdbarch);
-  struct syscall_desc *sysdesc;
-  const char **names = NULL;
-  int nsyscalls;
-  int i;
 
   if (syscalls_info == NULL)
     return NULL;
 
-  nsyscalls = VEC_length (syscall_desc_p, syscalls_info->syscalls);
-  names = XNEWVEC (const char *, nsyscalls + 1);
+  int nsyscalls = syscalls_info->syscalls.size ();
+  const char **names = XNEWVEC (const char *, nsyscalls + 1);
 
-  for (i = 0;
-       VEC_iterate (syscall_desc_p, syscalls_info->syscalls, i, sysdesc);
-       i++)
-    names[i] = sysdesc->name;
+  int i;
+  for (i = 0; i < syscalls_info->syscalls.size (); i++)
+    names[i] = syscalls_info->syscalls[i]->name.c_str ();
 
   names[i] = NULL;
 
@@ -538,16 +467,14 @@ xml_list_syscalls_by_group (struct gdbarch *gdbarch, const char *group)
   if (groupdesc == NULL)
     return NULL;
 
-  nsyscalls = VEC_length (syscall_desc_p, groupdesc->syscalls);
+  nsyscalls = groupdesc->syscalls.size ();
   syscalls = (struct syscall*) xmalloc ((nsyscalls + 1)
 					* sizeof (struct syscall));
 
-  for (i = 0;
-       VEC_iterate (syscall_desc_p, groupdesc->syscalls, i, sysdesc);
-       i++)
+  for (i = 0; i < groupdesc->syscalls.size (); i++)
     {
-      syscalls[i].name = sysdesc->name;
-      syscalls[i].number = sysdesc->number;
+      syscalls[i].name = groupdesc->syscalls[i]->name.c_str ();
+      syscalls[i].number = groupdesc->syscalls[i]->number;
     }
 
   /* Add final element marker.  */
@@ -565,21 +492,18 @@ static const char **
 xml_list_of_groups (struct gdbarch *gdbarch)
 {
   struct syscalls_info *syscalls_info = gdbarch_syscalls_info (gdbarch);
-  struct syscall_group_desc *groupdesc;
   const char **names = NULL;
-  int i;
   int ngroups;
+  int i;
 
   if (syscalls_info == NULL)
     return NULL;
 
-  ngroups = VEC_length (syscall_group_desc_p, syscalls_info->groups);
+  ngroups = syscalls_info->groups.size ();
   names = (const char**) xmalloc ((ngroups + 1) * sizeof (char *));
 
-  for (i = 0;
-       VEC_iterate (syscall_group_desc_p, syscalls_info->groups, i, groupdesc);
-       i++)
-    names[i] = groupdesc->name;
+  for (i = 0; i < syscalls_info->groups.size (); i++)
+    names[i] = syscalls_info->groups[i]->name.c_str ();
 
   names[i] = NULL;


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]