From patchwork Sat Oct 21 07:43:07 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 23747 Received: (qmail 109856 invoked by alias); 21 Oct 2017 07:43:17 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 108983 invoked by uid 89); 21 Oct 2017 07:43:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_SOFTFAIL autolearn=ham version=3.3.2 spammy=3469, 4177 X-HELO: barracuda.ebox.ca Received: from barracuda.ebox.ca (HELO barracuda.ebox.ca) (96.127.255.19) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 21 Oct 2017 07:43:13 +0000 X-ASG-Debug-ID: 1508571787-0c856e65d5380aab0001-fS2M51 Received: from smtp.electronicbox.net (smtp.electronicbox.net [96.127.255.82]) by barracuda.ebox.ca with ESMTP id YUCk0QR9VZKcGCyK (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 21 Oct 2017 03:43:07 -0400 (EDT) X-Barracuda-Envelope-From: simon.marchi@polymtl.ca X-Barracuda-RBL-Trusted-Forwarder: 96.127.255.82 Received: from simark.lan (cable-192.222.251.162.electronicbox.net [192.222.251.162]) by smtp.electronicbox.net (Postfix) with ESMTP id BB285441B21; Sat, 21 Oct 2017 03:43:07 -0400 (EDT) From: Simon Marchi X-Barracuda-Effective-Source-IP: cable-192.222.251.162.electronicbox.net[192.222.251.162] X-Barracuda-Apparent-Source-IP: 192.222.251.162 X-Barracuda-RBL-IP: 192.222.251.162 To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: [PATCH] C++ify xml-syscall.c Date: Sat, 21 Oct 2017 03:43:07 -0400 X-ASG-Orig-Subj: [PATCH] C++ify xml-syscall.c Message-Id: <20171021074307.26051-1-simon.marchi@polymtl.ca> X-Barracuda-Connect: smtp.electronicbox.net[96.127.255.82] X-Barracuda-Start-Time: 1508571787 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://96.127.255.19:443/cgi-mod/mark.cgi X-Barracuda-Scan-Msg-Size: 15635 X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.00 X-Barracuda-Spam-Status: No, SCORE=0.00 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=8.0 tests= X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.44080 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- X-IsSubscribed: yes 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. : Change type to std::string. (syscall_desc_p): Remove typeder. (DEF_VEC_P(syscall_desc_p)): Remove. (struct syscall_group_desc): Add constructor. : Change type to std::string. : Change type to std::vector. (syscall_group_desc_p): Remove typedef. (DEF_VEC_P(syscall_group_desc_p)): Remove. (struct syscalls_info) : Change type to std::vector of unique_ptr. : Likewise. : 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_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 syscalls; +}; + +typedef std::unique_ptr syscall_group_desc_up; /* Structure that represents syscalls information. */ struct syscalls_info { /* The syscalls. */ - VEC(syscall_desc_p) *syscalls; + std::vector syscalls; /* The syscall groups. */ - VEC(syscall_group_desc_p) *groups; + std::vector 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::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 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;