From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 63045 invoked by alias); 10 Sep 2017 17:40:41 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 62833 invoked by uid 89); 10 Sep 2017 17:40:40 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 10 Sep 2017 17:40:38 +0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 43339245F66; Sun, 10 Sep 2017 17:40:37 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 43339245F66 Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=sergiodj@redhat.com Received: from localhost (unused-10-15-17-193.yyz.redhat.com [10.15.17.193]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 0DC875C588; Sun, 10 Sep 2017 17:40:36 +0000 (UTC) From: Sergio Durigan Junior To: Simon Marchi Cc: Subject: Re: [PATCH 1/2] Make probe_ops::get_probes fill an std::vector References: <1505053377-10061-1-git-send-email-simon.marchi@ericsson.com> Date: Sun, 10 Sep 2017 17:40:00 -0000 In-Reply-To: <1505053377-10061-1-git-send-email-simon.marchi@ericsson.com> (Simon Marchi's message of "Sun, 10 Sep 2017 16:22:56 +0200") Message-ID: <87efreo3jf.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2017-09/txt/msg00274.txt.bz2 On Sunday, September 10 2017, Simon Marchi wrote: > This patch changes one usage of VEC to std::vector. It is a relatively > straightforward 1:1 change. The implementations of > sym_probe_fns::sym_get_probes return a borrowed reference to their probe > vectors, meaning that the caller should not free it. In the new code, I > made them return a const reference to the vector. > > This patch and the following one were tested by the buildbot. I didn't > see any failures that looked related to this one. Thanks for the patch, Simon. A few comments below. > gdb/ChangeLog: > > * probe.h (struct probe_ops) : Change parameter from > vec to std::vector. > * probe.c (parse_probes_in_pspace): Update. > (find_probes_in_objfile): Update. > (find_probe_by_pc): Update. > (collect_probes): Update. > (probe_any_get_probes): Update. > * symfile.h (struct sym_probe_fns) Change > return type to reference to std::vector. > * dtrace-probe.c (dtrace_process_dof_probe): Change parameter to > std::vector and update. > (dtrace_process_dof): Likewise. > (dtrace_get_probes): Likewise. > * elfread.c (elf_get_probes): Change return type to std::vector, > store an std::vector in bfd_data. > (probe_key_free): Update to std::vector. > * stap-probe.c (handle_stap_probe): Change parameter to > std::vector and update. > (stap_get_probes): Likewise. > * symfile-debug.c (debug_sym_get_probes): Change return type to > std::vector and update. > --- > gdb/dtrace-probe.c | 9 +++++---- > gdb/elfread.c | 27 ++++++++++----------------- > gdb/probe.c | 38 ++++++++++++++------------------------ > gdb/probe.h | 2 +- > gdb/stap-probe.c | 10 +++++----- > gdb/symfile-debug.c | 8 ++++---- > gdb/symfile.h | 7 ++----- > 7 files changed, 41 insertions(+), 60 deletions(-) > > diff --git a/gdb/dtrace-probe.c b/gdb/dtrace-probe.c > index c1a8cb0..f9209ec 100644 > --- a/gdb/dtrace-probe.c > +++ b/gdb/dtrace-probe.c > @@ -313,7 +313,8 @@ struct dtrace_dof_probe > > static void > dtrace_process_dof_probe (struct objfile *objfile, > - struct gdbarch *gdbarch, VEC (probe_p) **probesp, > + struct gdbarch *gdbarch, > + std::vector *probesp, Since you're doing this, what do you think about const-fying these occurrences of "std::vector"? > struct dtrace_dof_hdr *dof, > struct dtrace_dof_probe *probe, > struct dtrace_dof_provider *provider, > @@ -448,7 +449,7 @@ dtrace_process_dof_probe (struct objfile *objfile, > ret->enablers = VEC_copy (dtrace_probe_enabler_s, enablers); > > /* Successfully created probe. */ > - VEC_safe_push (probe_p, *probesp, (struct probe *) ret); > + probesp->push_back ((struct probe *) ret); > } > > do_cleanups (cleanup); > @@ -461,7 +462,7 @@ dtrace_process_dof_probe (struct objfile *objfile, > > static void > dtrace_process_dof (asection *sect, struct objfile *objfile, > - VEC (probe_p) **probesp, struct dtrace_dof_hdr *dof) > + std::vector *probesp, struct dtrace_dof_hdr *dof) > { > struct gdbarch *gdbarch = get_objfile_arch (objfile); > struct dtrace_dof_sect *section; > @@ -620,7 +621,7 @@ dtrace_get_arg (struct dtrace_probe *probe, unsigned n, > /* Implementation of the get_probes method. */ > > static void > -dtrace_get_probes (VEC (probe_p) **probesp, struct objfile *objfile) > +dtrace_get_probes (std::vector *probesp, struct objfile *objfile) > { > bfd *abfd = objfile->obfd; > asection *sect = NULL; > diff --git a/gdb/elfread.c b/gdb/elfread.c > index f3d4641..8a64865 100644 > --- a/gdb/elfread.c > +++ b/gdb/elfread.c > @@ -1309,35 +1309,30 @@ elf_symfile_init (struct objfile *objfile) > > /* Implementation of `sym_get_probes', as documented in symfile.h. */ > > -static VEC (probe_p) * > +static const std::vector & > elf_get_probes (struct objfile *objfile) > { > - VEC (probe_p) *probes_per_bfd; > + std::vector *probes_per_bfd; > > /* Have we parsed this objfile's probes already? */ > - probes_per_bfd = (VEC (probe_p) *) bfd_data (objfile->obfd, probe_key); > + probes_per_bfd = (std::vector *) bfd_data (objfile->obfd, probe_key); > > - if (!probes_per_bfd) > + if (probes_per_bfd == NULL) > { > int ix; > const struct probe_ops *probe_ops; > + probes_per_bfd = new std::vector; > > /* Here we try to gather information about all types of probes from the > objfile. */ > for (ix = 0; VEC_iterate (probe_ops_cp, all_probe_ops, ix, probe_ops); > ix++) > - probe_ops->get_probes (&probes_per_bfd, objfile); > - > - if (probes_per_bfd == NULL) > - { > - VEC_reserve (probe_p, probes_per_bfd, 1); > - gdb_assert (probes_per_bfd != NULL); > - } > + probe_ops->get_probes (probes_per_bfd, objfile); > > set_bfd_data (objfile->obfd, probe_key, probes_per_bfd); > } > > - return probes_per_bfd; > + return *probes_per_bfd; > } > > /* Helper function used to free the space allocated for storing SystemTap > @@ -1346,14 +1341,12 @@ elf_get_probes (struct objfile *objfile) > static void > probe_key_free (bfd *abfd, void *d) > { > - int ix; > - VEC (probe_p) *probes = (VEC (probe_p) *) d; > - struct probe *probe; > + std::vector *probes = (std::vector *) d; > > - for (ix = 0; VEC_iterate (probe_p, probes, ix, probe); ix++) > + for (struct probe *probe : *probes) > probe->pops->destroy (probe); > > - VEC_free (probe_p, probes); > + delete probes; > } > > > diff --git a/gdb/probe.c b/gdb/probe.c > index 686e90e..2d68437 100644 > --- a/gdb/probe.c > +++ b/gdb/probe.c > @@ -58,10 +58,6 @@ parse_probes_in_pspace (const struct probe_ops *probe_ops, > > ALL_PSPACE_OBJFILES (search_pspace, objfile) > { > - VEC (probe_p) *probes; > - struct probe *probe; > - int ix; > - > if (!objfile->sf || !objfile->sf->sym_probe_fns) > continue; > > @@ -71,9 +67,10 @@ parse_probes_in_pspace (const struct probe_ops *probe_ops, > objfile_namestr) != 0) > continue; > > - probes = objfile->sf->sym_probe_fns->sym_get_probes (objfile); > + const std::vector &probes > + = objfile->sf->sym_probe_fns->sym_get_probes (objfile); > > - for (ix = 0; VEC_iterate (probe_p, probes, ix, probe); ix++) > + for (struct probe *probe : probes) > { > if (probe_ops != &probe_ops_any && probe->pops != probe_ops) > continue; > @@ -211,15 +208,14 @@ VEC (probe_p) * > find_probes_in_objfile (struct objfile *objfile, const char *provider, > const char *name) Any reason for not converting this function's return as well? > { > - VEC (probe_p) *probes, *result = NULL; > - int ix; > - struct probe *probe; > + VEC (probe_p) *result = NULL; > > if (!objfile->sf || !objfile->sf->sym_probe_fns) > return NULL; > > - probes = objfile->sf->sym_probe_fns->sym_get_probes (objfile); > - for (ix = 0; VEC_iterate (probe_p, probes, ix, probe); ix++) > + const std::vector &probes > + = objfile->sf->sym_probe_fns->sym_get_probes (objfile); > + for (struct probe *probe : probes) > { > if (strcmp (probe->provider, provider) != 0) > continue; > @@ -246,17 +242,14 @@ find_probe_by_pc (CORE_ADDR pc) > > ALL_OBJFILES (objfile) > { > - VEC (probe_p) *probes; > - int ix; > - struct probe *probe; > - > if (!objfile->sf || !objfile->sf->sym_probe_fns > || objfile->sect_index_text == -1) > continue; > > /* If this proves too inefficient, we can replace with a hash. */ > - probes = objfile->sf->sym_probe_fns->sym_get_probes (objfile); > - for (ix = 0; VEC_iterate (probe_p, probes, ix, probe); ix++) > + const std::vector &probes > + = objfile->sf->sym_probe_fns->sym_get_probes (objfile); > + for (struct probe *probe : probes) > if (get_probe_address (probe, objfile) == pc) > { > result.objfile = objfile; > @@ -294,10 +287,6 @@ collect_probes (char *objname, char *provider, char *probe_name, > > ALL_OBJFILES (objfile) > { > - VEC (probe_p) *probes; > - struct probe *probe; > - int ix; > - > if (! objfile->sf || ! objfile->sf->sym_probe_fns) > continue; > > @@ -307,9 +296,10 @@ collect_probes (char *objname, char *provider, char *probe_name, > continue; > } > > - probes = objfile->sf->sym_probe_fns->sym_get_probes (objfile); > + const std::vector &probes > + = objfile->sf->sym_probe_fns->sym_get_probes (objfile); > > - for (ix = 0; VEC_iterate (probe_p, probes, ix, probe); ix++) > + for (struct probe *probe : probes) > { > struct bound_probe bound; > > @@ -907,7 +897,7 @@ probe_any_is_linespec (const char **linespecp) > /* Dummy method used for `probe_ops_any'. */ > > static void > -probe_any_get_probes (VEC (probe_p) **probesp, struct objfile *objfile) > +probe_any_get_probes (std::vector *probesp, struct objfile *objfile) > { > /* No probes can be provided by this dummy backend. */ > } > diff --git a/gdb/probe.h b/gdb/probe.h > index db7f1d1..61e3031 100644 > --- a/gdb/probe.h > +++ b/gdb/probe.h > @@ -64,7 +64,7 @@ struct probe_ops > > /* Function that should fill PROBES with known probes from OBJFILE. */ > > - void (*get_probes) (VEC (probe_p) **probes, struct objfile *objfile); > + void (*get_probes) (std::vector *probes, struct objfile *objfile); > > /* Compute the probe's relocated address. OBJFILE is the objfile > in which the probe originated. */ > diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c > index c0cc662..032f796 100644 > --- a/gdb/stap-probe.c > +++ b/gdb/stap-probe.c > @@ -1472,7 +1472,7 @@ stap_clear_semaphore (struct probe *probe_generic, struct objfile *objfile, > > static void > handle_stap_probe (struct objfile *objfile, struct sdt_note *el, > - VEC (probe_p) **probesp, CORE_ADDR base) > + std::vector *probesp, CORE_ADDR base) > { > bfd *abfd = objfile->obfd; > int size = bfd_get_arch_size (abfd) / 8; > @@ -1543,7 +1543,7 @@ handle_stap_probe (struct objfile *objfile, struct sdt_note *el, > ret->args_u.text = probe_args; > > /* Successfully created probe. */ > - VEC_safe_push (probe_p, *probesp, (struct probe *) ret); > + probesp->push_back ((struct probe *) ret); > } > > /* Helper function which tries to find the base address of the SystemTap > @@ -1588,7 +1588,7 @@ get_stap_base_address (bfd *obfd, bfd_vma *base) > SystemTap probes from OBJFILE. */ > > static void > -stap_get_probes (VEC (probe_p) **probesp, struct objfile *objfile) > +stap_get_probes (std::vector *probesp, struct objfile *objfile) > { > /* If we are here, then this is the first time we are parsing the > SystemTap probe's information. We basically have to count how many > @@ -1597,7 +1597,7 @@ stap_get_probes (VEC (probe_p) **probesp, struct objfile *objfile) > bfd *obfd = objfile->obfd; > bfd_vma base; > struct sdt_note *iter; > - unsigned save_probesp_len = VEC_length (probe_p, *probesp); > + unsigned save_probesp_len = probesp->size (); > > if (objfile->separate_debug_objfile_backlink != NULL) > { > @@ -1628,7 +1628,7 @@ stap_get_probes (VEC (probe_p) **probesp, struct objfile *objfile) > handle_stap_probe (objfile, iter, probesp, base); > } > > - if (save_probesp_len == VEC_length (probe_p, *probesp)) > + if (save_probesp_len == probesp->size ()) > { > /* If we are here, it means we have failed to parse every known > probe. */ > diff --git a/gdb/symfile-debug.c b/gdb/symfile-debug.c > index e7890c9..32dafa8 100644 > --- a/gdb/symfile-debug.c > +++ b/gdb/symfile-debug.c > @@ -384,20 +384,20 @@ static const struct quick_symbol_functions debug_sym_quick_functions = > > /* Debugging version of struct sym_probe_fns. */ > > -static VEC (probe_p) * > +static const std::vector & > debug_sym_get_probes (struct objfile *objfile) > { > const struct debug_sym_fns_data *debug_data > = ((const struct debug_sym_fns_data *) > objfile_data (objfile, symfile_debug_objfile_data_key)); > - VEC (probe_p) *retval; > > - retval = debug_data->real_sf->sym_probe_fns->sym_get_probes (objfile); > + const std::vector &retval > + = debug_data->real_sf->sym_probe_fns->sym_get_probes (objfile); > > fprintf_filtered (gdb_stdlog, > "probes->sym_get_probes (%s) = %s\n", > objfile_debug_name (objfile), > - host_address_to_string (retval)); > + host_address_to_string (retval.data ())); > > return retval; > } > diff --git a/gdb/symfile.h b/gdb/symfile.h > index bb47fdf..1f4460c 100644 > --- a/gdb/symfile.h > +++ b/gdb/symfile.h > @@ -314,11 +314,8 @@ struct quick_symbol_functions > > struct sym_probe_fns > { > - /* If non-NULL, return an array of probe objects. > - > - The returned value does not have to be freed and it has lifetime of the > - OBJFILE. */ > - VEC (probe_p) *(*sym_get_probes) (struct objfile *); > + /* If non-NULL, return a reference to vector of probe objects. */ > + const std::vector &(*sym_get_probes) (struct objfile *); > }; > > /* Structure to keep track of symbol reading functions for various > -- > 2.7.4 I know this patch is supposed to touch only probe_ops::get_probes, but I'd love to see the whole VEC (probe_p) replaced (a few places on breakpoint.c are also using it). Well, maybe on another patch :-). Thanks! -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/