C++ify xml-syscall.c
authorSimon Marchi <simon.marchi@polymtl.ca>
Sat, 28 Oct 2017 02:23:33 +0000 (22:23 -0400)
committerSimon Marchi <simon.marchi@ericsson.com>
Sat, 28 Oct 2017 02:23:48 +0000 (22:23 -0400)
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.

gdb/ChangeLog
gdb/xml-syscall.c

index 9c4e409ef71b49551a36b5460777dbaa631d81a5..51e34227dc9e2849bbf4d84fa38ab1bda0116d95 100644 (file)
@@ -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.
index fe036cbf5cc5496e6bc53a1c237faa1fd9554666..307c3045126ba3b9d4e3579267f9e95e40a1ca7a 100644 (file)
@@ -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;