* [PATCH] C++ify xml-syscall.c
@ 2017-10-21 7:43 Simon Marchi
2017-10-28 2:27 ` Simon Marchi
0 siblings, 1 reply; 2+ messages in thread
From: Simon Marchi @ 2017-10-21 7:43 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
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_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_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.
(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/xml-syscall.c | 220 ++++++++++++++++++------------------------------------
1 file changed, 71 insertions(+), 149 deletions(-)
diff --git a/gdb/xml-syscall.c b/gdb/xml-syscall.c
index fe036cbf5c..3c40ab1064 100644
--- a/gdb/xml-syscall.c
+++ b/gdb/xml-syscall.c
@@ -94,47 +94,58 @@ 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;
};
/* Callback data for syscall information parsing. */
@@ -145,66 +156,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 +163,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);
-
- groupdesc->name = xstrdup (group);
+ syscall_group_desc *groupdesc = new syscall_group_desc (group);
- VEC_safe_push (syscall_group_desc_p, syscalls_info->groups, groupdesc);
+ syscalls_info->groups.emplace_back (groupdesc);
return groupdesc;
}
@@ -228,19 +177,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 +199,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 +207,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 +280,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;
+ std::unique_ptr<syscalls_info> 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 +325,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 +346,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 +362,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 +374,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 +395,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 +412,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 +428,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 +465,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 +490,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;
--
2.14.2
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] C++ify xml-syscall.c
2017-10-21 7:43 [PATCH] C++ify xml-syscall.c Simon Marchi
@ 2017-10-28 2:27 ` Simon Marchi
0 siblings, 0 replies; 2+ messages in thread
From: Simon Marchi @ 2017-10-28 2:27 UTC (permalink / raw)
To: gdb-patches
On 2017-10-21 03:43, Simon Marchi wrote:
> 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.
I pushed this patch in.
Simon
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-10-28 2:27 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-21 7:43 [PATCH] C++ify xml-syscall.c Simon Marchi
2017-10-28 2:27 ` Simon Marchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).