From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29107 invoked by alias); 15 Nov 2017 06:12:56 -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 29090 invoked by uid 89); 15 Nov 2017 06:12:55 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.7 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KB_WAM_FROM_NAME_SINGLEWORD,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; Wed, 15 Nov 2017 06:12:46 +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 822814E4C1; Wed, 15 Nov 2017 06:12:45 +0000 (UTC) Received: from localhost (unused-10-15-17-193.yyz.redhat.com [10.15.17.193]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 1BA1392A80; Wed, 15 Nov 2017 06:12:45 +0000 (UTC) From: Sergio Durigan Junior To: Simon Marchi Cc: GDB Patches Subject: Re: [PATCH 1/3] Convert generic probe interface to C++ (and perform some cleanups) References: <20171113175901.25367-1-sergiodj@redhat.com> <20171113175901.25367-2-sergiodj@redhat.com> <457199c5-ba70-d26b-7fcb-9471ad5c9d11@simark.ca> Date: Wed, 15 Nov 2017 06:12:00 -0000 In-Reply-To: <457199c5-ba70-d26b-7fcb-9471ad5c9d11@simark.ca> (Simon Marchi's message of "Tue, 14 Nov 2017 21:52:17 -0500") Message-ID: <87bmk45aub.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-11/txt/msg00270.txt.bz2 Hey Simon, Thanks for the review. Comments below. On Tuesday, November 14 2017, Simon Marchi wrote: > On 2017-11-13 12:58 PM, Sergio Durigan Junior wrote: >> This patch converts the generic probe interface (gdb/probe.[ch]) to >> C++, and also performs some cleanups that were on my TODO list for a >> while. >> >> The main changes were the conversion of 'struct probe' to 'class >> probe', and 'struct probe_ops' to 'class static_probe_ops'. The >> former now contains all the "dynamic", generic methods that act on a >> probe + the generic data related to it; the latter encapsulates a >> bunch of "static" methods that relate to the probe type, but not to a >> specific probe itself. > > Personally I'm fine with this. Maybe there would be a better C++-ish > design, but I'm not very good at that. If others have suggestions, I'd > like to hear them. Great! >> I've had to do a few renamings (e.g., on 'struct bound_probe' the >> field is called 'probe *prob' now, instead of 'struct probe *probe') >> because GCC was complaining about naming the field using the same name >> as the class. Nothing major, though. Generally speaking, the logic >> behind and the design behind the code are the same. >> >> Even though I'm sending a series of patches, they need to be tested >> and committed as a single unit, because of inter-dependencies. But it >> should be easier to review in separate logical units. >> >> I've regtested this patch on BuildBot, no regressions found. >> >> gdb/ChangeLog: >> 2017-11-13 Sergio Durigan Junior >> >> * break-catch-throw.c (fetch_probe_arguments): Use >> 'probe.prob' instead of 'probe.probe'. >> * breakpoint.c (create_longjmp_master_breakpoint): Call >> 'can_evaluate_arguments' and 'get_relocated_address' methods >> from probe. >> (create_exception_master_breakpoint): Likewise. >> (add_location_to_breakpoint): Use 'sal->prob' instead of >> 'sal->probe'. >> (bkpt_probe_insert_location): Call 'set_semaphore' method from >> probe. >> (bkpt_probe_remove_location): Likewise, for 'clear_semaphore'. >> * elfread.c (elf_get_probes): Use 'static_probe_ops' instead >> of 'probe_ops'. >> (probe_key_free): Call 'delete' on probe. >> (check_exception_resume): Use 'probe.prob' instead of >> 'probe.probe'. >> * location.c (string_to_event_location_basic): Call >> 'probe_linespec_to_static_ops'. >> * probe.c (class any_static_probe_ops): New class. >> (any_static_probe_ops any_static_probe_ops): New variable. >> (parse_probes_in_pspace): Receive 'static_probe_ops' as >> argument. Adjust code to reflect change. >> (parse_probes): Use 'static_probe_ops' instead of >> 'probe_ops'. Adjust code to reflect change. >> (find_probes_in_objfile): Call methods to get name and >> provider from probe. >> (find_probe_by_pc): Use 'result.prob' instead of >> 'result.probe'. Call 'get_relocated_address' method from >> probe. >> (collect_probes): Adjust comment and argument list to receive >> 'static_probe_ops' instead of 'probe_ops'. Adjust code to >> reflect change. Call necessary methods from probe. >> (compare_probes): Call methods to get name and provider from >> probes. >> (gen_ui_out_table_header_info): Receive 'static_probe_ops' >> instead of 'probe_ops'. Use 'std::vector' instead of VEC, >> adjust code accordingly. >> (print_ui_out_not_applicables): Likewise. >> (info_probes_for_ops): Rename to... >> (info_probes_for_spops): ...this. Receive 'static_probe_ops' >> as argument instead of 'probe_ops'. Adjust code. Call >> necessary methods from probe. >> (info_probes_command): Use 'info_probes_for_spops'. >> (enable_probes_command): Pass correct argument to >> 'collect_probes'. Call methods from probe. >> (disable_probes_command): Likewise. >> (get_probe_address): Move to 'any_static_probe_ops::get_address'. >> (get_probe_argument_count): Move to >> 'any_static_probe_ops::get_argument_count'. >> (can_evaluate_probe_arguments): Move to >> 'any_static_probe_ops::can_evaluate_arguments'. >> (evaluate_probe_argument): Move to >> 'any_static_probe_ops::evaluate_argument'. >> (probe_safe_evaluate_at_pc): Use 'probe.prob' instead of >> 'probe.probe'. >> (probe_linespec_to_ops): Rename to... >> (probe_linespec_to_static_ops): ...this. Adjust code. >> (probe_any_is_linespec): Rename to... >> (any_static_probe_ops::is_linespec): ...this. >> (probe_any_get_probes): Rename to... >> (any_static_probe_ops::get_probes): ...this. >> (any_static_probe_ops::type_name): New method. >> (any_static_probe_ops::emit_info_probes_extra_fields): New >> method. >> (any_static_probe_ops::gen_info_probes_table_header): New >> method. >> (compute_probe_arg): Use 'pc_probe.prob' instead of >> 'pc_probe.probe'. Call methods from probe. >> (compile_probe_arg): Likewise. >> (std::vector all_probe_ops): Delete. >> (std::vector all_static_probe_ops): >> New variable. >> (_initialize_probe): Use 'all_static_probe_ops' instead of >> 'all_probe_ops'. >> * probe.h (struct info_probe_column) : Delete >> extraneous newline >> (info_probe_column_s): Delete type and VEC. >> (struct probe_ops): Delete. Replace with... >> (class static_probe_ops): ...this and... >> (clas probe): ...this. >> (struct bound_probe) : Delete extraneous >> newline. Adjust constructor to receive 'probe' instead of >> 'struct probe'. >> : Rename to... >> : ...this. Delete extraneous newline. >> : Delete extraneous newline. >> (register_probe_ops): Delete unused prototype. >> (info_probes_for_ops): Rename to... >> (info_probes_for_spops): ...this. Adjust comment. >> (get_probe_address): Move to 'probe::get_address'. >> (get_probe_argument_count): Move to >> 'probe::get_argument_count'. >> (can_evaluate_probe_arguments): Move to >> 'probe::can_evaluate_arguments'. >> (evaluate_probe_argument): Move to 'probe::evaluate_argument'. >> * solib-svr4.c (struct svr4_info): Adjust comment. >> (struct probe_and_action) : Rename to... >> : ...this. >> (register_solib_event_probe): Receive 'probe' instead of >> 'struct probe' as argument. Use 'prob' instead of 'probe' >> when applicable. >> (solib_event_probe_action): Call 'get_argument_count' method >> from probe. Adjust comment. >> (svr4_handle_solib_event): Adjust comment. Call >> 'evaluate_argument' method from probe. >> (svr4_create_probe_breakpoints): Call 'get_relocated_address' >> from probe. >> (svr4_create_solib_event_breakpoints): Use 'probe' instead of >> 'struct probe'. Call 'can_evaluate_arguments' from probe. >> * symfile.h: Forward declare 'class probe' instead of 'struct >> probe'. >> * symtab.h: Likewise. >> (struct symtab_and_line) : Rename to... >> : ...this. >> * tracepoint.c (start_tracing): Use 'prob' when applicable. >> Call probe methods. >> (stop_tracing): Likewise. >> >> Signed-off-by: Sergio Durigan Junior >> Signed-off-by: Sergio Durigan Junior >> Signed-off-by: Sergio Durigan Junior >> --- >> gdb/break-catch-throw.c | 16 +- >> gdb/breakpoint.c | 20 +-- >> gdb/elfread.c | 4 +- >> gdb/infrun.c | 2 +- >> gdb/location.c | 2 +- >> gdb/probe.c | 435 ++++++++++++++++++++++-------------------------- >> gdb/probe.h | 363 ++++++++++++++++++++-------------------- >> gdb/solib-svr4.c | 36 ++-- >> gdb/symfile.h | 2 +- >> gdb/symtab.h | 4 +- >> gdb/tracepoint.c | 16 +- >> 11 files changed, 427 insertions(+), 473 deletions(-) >> >> diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c >> index fd8a113803..ffc100bb24 100644 >> --- a/gdb/break-catch-throw.c >> +++ b/gdb/break-catch-throw.c >> @@ -106,20 +106,20 @@ fetch_probe_arguments (struct value **arg0, struct value **arg1) >> unsigned n_args; >> >> pc_probe = find_probe_by_pc (pc); >> - if (pc_probe.probe == NULL >> - || strcmp (pc_probe.probe->provider, "libstdcxx") != 0 >> - || (strcmp (pc_probe.probe->name, "catch") != 0 >> - && strcmp (pc_probe.probe->name, "throw") != 0 >> - && strcmp (pc_probe.probe->name, "rethrow") != 0)) >> + if (pc_probe.prob == NULL >> + || strcmp (pc_probe.prob->get_provider (), "libstdcxx") != 0 >> + || (strcmp (pc_probe.prob->get_name (), "catch") != 0 >> + && strcmp (pc_probe.prob->get_name (), "throw") != 0 >> + && strcmp (pc_probe.prob->get_name (), "rethrow") != 0)) >> error (_("not stopped at a C++ exception catchpoint")); >> >> - n_args = get_probe_argument_count (pc_probe.probe, frame); >> + n_args = pc_probe.prob->get_argument_count (frame); >> if (n_args < 2) >> error (_("C++ exception catchpoint has too few arguments")); >> >> if (arg0 != NULL) >> - *arg0 = evaluate_probe_argument (pc_probe.probe, 0, frame); >> - *arg1 = evaluate_probe_argument (pc_probe.probe, 1, frame); >> + *arg0 = pc_probe.prob->evaluate_argument (0, frame); >> + *arg1 = pc_probe.prob->evaluate_argument (1, frame); >> >> if ((arg0 != NULL && *arg0 == NULL) || *arg1 == NULL) >> error (_("error computing probe argument at c++ exception catchpoint")); >> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c >> index b170a14cb5..1728aee838 100644 >> --- a/gdb/breakpoint.c >> +++ b/gdb/breakpoint.c >> @@ -3309,7 +3309,7 @@ create_longjmp_master_breakpoint (void) >> /* We are only interested in checking one element. */ >> probe *p = ret[0]; >> >> - if (!can_evaluate_probe_arguments (p)) >> + if (!p->can_evaluate_arguments ()) >> { >> /* We cannot use the probe interface here, because it does >> not know how to evaluate arguments. */ >> @@ -3329,7 +3329,7 @@ create_longjmp_master_breakpoint (void) >> struct breakpoint *b; >> >> b = create_internal_breakpoint (gdbarch, >> - get_probe_address (p, objfile), >> + p->get_relocated_address (objfile), >> bp_longjmp_master, >> &internal_breakpoint_ops); >> b->location = new_probe_location ("-probe-stap libc:longjmp"); >> @@ -3462,7 +3462,7 @@ create_exception_master_breakpoint (void) >> /* We are only interested in checking one element. */ >> probe *p = ret[0]; >> >> - if (!can_evaluate_probe_arguments (p)) >> + if (!p->can_evaluate_arguments ()) >> { >> /* We cannot use the probe interface here, because it does >> not know how to evaluate arguments. */ >> @@ -3482,7 +3482,7 @@ create_exception_master_breakpoint (void) >> struct breakpoint *b; >> >> b = create_internal_breakpoint (gdbarch, >> - get_probe_address (p, objfile), >> + p->get_relocated_address (objfile), >> bp_exception_master, >> &internal_breakpoint_ops); >> b->location = new_probe_location ("-probe-stap libgcc:unwind"); >> @@ -8694,7 +8694,7 @@ add_location_to_breakpoint (struct breakpoint *b, >> loc->requested_address = sal->pc; >> loc->address = adjusted_address; >> loc->pspace = sal->pspace; >> - loc->probe.probe = sal->probe; >> + loc->probe.prob = sal->prob; >> loc->probe.objfile = sal->objfile; >> gdb_assert (loc->pspace != NULL); >> loc->section = sal->section; >> @@ -12879,10 +12879,7 @@ bkpt_probe_insert_location (struct bp_location *bl) >> { >> /* The insertion was successful, now let's set the probe's semaphore >> if needed. */ >> - if (bl->probe.probe->pops->set_semaphore != NULL) >> - bl->probe.probe->pops->set_semaphore (bl->probe.probe, >> - bl->probe.objfile, >> - bl->gdbarch); >> + bl->probe.prob->set_semaphore (bl->probe.objfile, bl->gdbarch); >> } >> >> return v; >> @@ -12893,10 +12890,7 @@ bkpt_probe_remove_location (struct bp_location *bl, >> enum remove_bp_reason reason) >> { >> /* Let's clear the semaphore before removing the location. */ >> - if (bl->probe.probe->pops->clear_semaphore != NULL) >> - bl->probe.probe->pops->clear_semaphore (bl->probe.probe, >> - bl->probe.objfile, >> - bl->gdbarch); >> + bl->probe.prob->clear_semaphore (bl->probe.objfile, bl->gdbarch); >> >> return bkpt_remove_location (bl, reason); >> } >> diff --git a/gdb/elfread.c b/gdb/elfread.c >> index cdef5a8c59..f2483025ba 100644 >> --- a/gdb/elfread.c >> +++ b/gdb/elfread.c >> @@ -1324,7 +1324,7 @@ elf_get_probes (struct objfile *objfile) >> >> /* Here we try to gather information about all types of probes from the >> objfile. */ >> - for (const probe_ops *ops : all_probe_ops) >> + for (const static_probe_ops *ops : all_static_probe_ops) >> ops->get_probes (probes_per_bfd, objfile); >> >> set_bfd_data (objfile->obfd, probe_key, probes_per_bfd); >> @@ -1342,7 +1342,7 @@ probe_key_free (bfd *abfd, void *d) >> std::vector *probes = (std::vector *) d; >> >> for (probe *p : *probes) >> - p->pops->destroy (p); >> + delete p; >> >> delete probes; >> } >> diff --git a/gdb/infrun.c b/gdb/infrun.c >> index e2d1248e75..4592100962 100644 >> --- a/gdb/infrun.c >> +++ b/gdb/infrun.c >> @@ -7583,7 +7583,7 @@ check_exception_resume (struct execution_control_state *ecs, >> CFA and the HANDLER. We ignore the CFA, extract the handler, and >> set a breakpoint there. */ >> probe = find_probe_by_pc (get_frame_pc (frame)); >> - if (probe.probe) >> + if (probe.prob) >> { >> insert_exception_resume_from_probe (ecs->event_thread, &probe, frame); >> return; >> diff --git a/gdb/location.c b/gdb/location.c >> index c78778edc1..5ed3623f0a 100644 >> --- a/gdb/location.c >> +++ b/gdb/location.c >> @@ -844,7 +844,7 @@ string_to_event_location_basic (const char **stringp, >> >> /* Try the input as a probe spec. */ >> cs = *stringp; >> - if (cs != NULL && probe_linespec_to_ops (&cs) != NULL) >> + if (cs != NULL && probe_linespec_to_static_ops (&cs) != NULL) >> { >> location = new_probe_location (*stringp); >> *stringp += strlen (*stringp); >> diff --git a/gdb/probe.c b/gdb/probe.c >> index 829f6d18d7..0c47f904cb 100644 >> --- a/gdb/probe.c >> +++ b/gdb/probe.c >> @@ -38,11 +38,38 @@ >> #include >> #include "common/gdb_optional.h" >> >> +/* Class that implements the static probe methods for "any" probe. */ >> + >> +class any_static_probe_ops : public static_probe_ops >> +{ >> +public: >> + /* See probe.h. */ >> + bool is_linespec (const char **linespecp) const override; >> + >> + /* See probe.h. */ >> + void get_probes (std::vector *probesp, >> + struct objfile *objfile) const override; >> + >> + /* See probe.h. */ >> + const char *type_name () const override; >> + >> + /* See probe.h. */ >> + bool emit_info_probes_extra_fields () const override; > > I don't think we need this method (though maybe I'll get proven wrong > by the rest of the series). At least in this patch, we can just call > gen_info_probes_table_header, and if the probe type doesn't supply > any header, we'll just iterate on an empty vector, which is the same > as returning early. Ahm, you're right. Now that I think about it I don't recall why I added this method. I'll delete it in the next version. >> + >> + /* See probe.h. */ >> + void gen_info_probes_table_header >> + (std::vector *heads) const override; > > I am wondering if this can return the vector directly. Combined with > the suggestion above, it would simplify get_number_extra_fields, for example: > > static int > get_number_extra_fields (const static_probe_ops *spops) > { > return spops->gen_info_probes_table_header ().size (); > } I guess that works too. I'll make the change. >> +}; >> + >> +/* Static operations associated with a generic probe. */ >> + >> +const any_static_probe_ops any_static_probe_ops; >> + >> /* A helper for parse_probes that decodes a probe specification in >> SEARCH_PSPACE. It appends matching SALs to RESULT. */ >> >> static void >> -parse_probes_in_pspace (const struct probe_ops *probe_ops, >> +parse_probes_in_pspace (const static_probe_ops *spops, >> struct program_space *search_pspace, >> const char *objfile_namestr, >> const char *provider, >> @@ -67,21 +94,21 @@ parse_probes_in_pspace (const struct probe_ops *probe_ops, >> >> for (probe *p : probes) >> { >> - if (probe_ops != &probe_ops_any && p->pops != probe_ops) >> + if (spops != &any_static_probe_ops && p->get_static_ops () != spops) >> continue; >> >> - if (provider && strcmp (p->provider, provider) != 0) >> + if (provider && strcmp (p->get_provider (), provider) != 0) >> continue; >> >> - if (strcmp (p->name, name) != 0) >> + if (strcmp (p->get_name (), name) != 0) >> continue; >> >> symtab_and_line sal; >> - sal.pc = get_probe_address (p, objfile); >> + sal.pc = p->get_relocated_address (objfile); >> sal.explicit_pc = 1; >> sal.section = find_pc_overlay (sal.pc); >> sal.pspace = search_pspace; >> - sal.probe = p; >> + sal.prob = p; >> sal.objfile = objfile; >> >> result->push_back (std::move (sal)); >> @@ -98,15 +125,14 @@ parse_probes (const struct event_location *location, >> { >> char *arg_end, *arg; >> char *objfile_namestr = NULL, *provider = NULL, *name, *p; >> - const struct probe_ops *probe_ops; >> const char *arg_start, *cs; >> >> gdb_assert (event_location_type (location) == PROBE_LOCATION); >> arg_start = get_probe_location (location); >> >> cs = arg_start; >> - probe_ops = probe_linespec_to_ops (&cs); >> - if (probe_ops == NULL) >> + const static_probe_ops *spops = probe_linespec_to_static_ops (&cs); >> + if (spops == NULL) >> error (_("'%s' is not a probe linespec"), arg_start); >> >> arg = (char *) cs; >> @@ -159,7 +185,7 @@ parse_probes (const struct event_location *location, >> std::vector result; >> if (search_pspace != NULL) >> { >> - parse_probes_in_pspace (probe_ops, search_pspace, objfile_namestr, >> + parse_probes_in_pspace (spops, search_pspace, objfile_namestr, >> provider, name, &result); >> } >> else >> @@ -167,7 +193,7 @@ parse_probes (const struct event_location *location, >> struct program_space *pspace; >> >> ALL_PSPACES (pspace) >> - parse_probes_in_pspace (probe_ops, pspace, objfile_namestr, >> + parse_probes_in_pspace (spops, pspace, objfile_namestr, >> provider, name, &result); >> } >> >> @@ -206,10 +232,10 @@ find_probes_in_objfile (struct objfile *objfile, const char *provider, >> = objfile->sf->sym_probe_fns->sym_get_probes (objfile); >> for (probe *p : probes) >> { >> - if (strcmp (p->provider, provider) != 0) >> + if (strcmp (p->get_provider (), provider) != 0) >> continue; >> >> - if (strcmp (p->name, name) != 0) >> + if (strcmp (p->get_name (), name) != 0) >> continue; >> >> result.push_back (p); >> @@ -227,7 +253,7 @@ find_probe_by_pc (CORE_ADDR pc) >> struct bound_probe result; >> >> result.objfile = NULL; >> - result.probe = NULL; >> + result.prob = NULL; >> >> ALL_OBJFILES (objfile) >> { >> @@ -239,10 +265,10 @@ find_probe_by_pc (CORE_ADDR pc) >> const std::vector &probes >> = objfile->sf->sym_probe_fns->sym_get_probes (objfile); >> for (probe *p : probes) >> - if (get_probe_address (p, objfile) == pc) >> + if (p->get_relocated_address (objfile) == pc) >> { >> result.objfile = objfile; >> - result.probe = p; >> + result.prob = p; >> return result; >> } >> } >> @@ -253,12 +279,13 @@ find_probe_by_pc (CORE_ADDR pc) >> >> >> /* Make a vector of probes matching OBJNAME, PROVIDER, and PROBE_NAME. >> - If POPS is not NULL, only probes of this certain probe_ops will match. >> - Each argument is a regexp, or NULL, which matches anything. */ >> + If SPOPS is not &any_static_probe_ops, only probes related to this >> + specific static probe ops will match. Each argument is a regexp, >> + or NULL, which matches anything. */ >> >> static std::vector >> collect_probes (const std::string &objname, const std::string &provider, >> - const std::string &probe_name, const struct probe_ops *pops) >> + const std::string &probe_name, const static_probe_ops *spops) >> { >> struct objfile *objfile; >> std::vector result; >> @@ -290,15 +317,15 @@ collect_probes (const std::string &objname, const std::string &provider, >> >> for (probe *p : probes) >> { >> - if (pops != NULL && p->pops != pops) >> + if (spops != &any_static_probe_ops && p->get_static_ops () != spops) >> continue; >> >> if (prov_pat >> - && prov_pat->exec (p->provider, 0, NULL, 0) != 0) >> + && prov_pat->exec (p->get_provider (), 0, NULL, 0) != 0) >> continue; >> >> if (probe_pat >> - && probe_pat->exec (p->name, 0, NULL, 0) != 0) >> + && probe_pat->exec (p->get_name (), 0, NULL, 0) != 0) >> continue; >> >> result.emplace_back (p, objfile); >> @@ -315,16 +342,16 @@ compare_probes (const bound_probe &a, const bound_probe &b) >> { >> int v; >> >> - v = strcmp (a.probe->provider, b.probe->provider); >> + v = strcmp (a.prob->get_provider (), b.prob->get_provider ()); >> if (v != 0) >> return v < 0; >> >> - v = strcmp (a.probe->name, b.probe->name); >> + v = strcmp (a.prob->get_name (), b.prob->get_name ()); >> if (v != 0) >> return v < 0; >> >> - if (a.probe->address != b.probe->address) >> - return a.probe->address < b.probe->address; >> + if (a.prob->get_address () != b.prob->get_address ()) >> + return a.prob->get_address () < b.prob->get_address (); >> >> return strcmp (objfile_name (a.objfile), objfile_name (b.objfile)) < 0; >> } >> @@ -334,56 +361,37 @@ compare_probes (const bound_probe &a, const bound_probe &b) >> >> static void >> gen_ui_out_table_header_info (const std::vector &probes, >> - const struct probe_ops *p) >> + const static_probe_ops *spops) >> { >> /* `headings' refers to the names of the columns when printing `info >> probes'. */ >> - VEC (info_probe_column_s) *headings = NULL; >> - struct cleanup *c; >> - info_probe_column_s *column; >> - size_t headings_size; >> - int ix; >> + std::vector headings; >> >> - gdb_assert (p != NULL); >> + gdb_assert (spops != NULL); >> >> - if (p->gen_info_probes_table_header == NULL >> - && p->gen_info_probes_table_values == NULL) >> + if (!spops->emit_info_probes_extra_fields ()) >> return; >> >> - gdb_assert (p->gen_info_probes_table_header != NULL >> - && p->gen_info_probes_table_values != NULL); >> - >> - c = make_cleanup (VEC_cleanup (info_probe_column_s), &headings); >> - p->gen_info_probes_table_header (&headings); >> - >> - headings_size = VEC_length (info_probe_column_s, headings); >> + spops->gen_info_probes_table_header (&headings); >> >> - for (ix = 0; >> - VEC_iterate (info_probe_column_s, headings, ix, column); >> - ++ix) >> + for (struct info_probe_column column : headings) >> { >> - size_t size_max = strlen (column->print_name); >> + size_t size_max = strlen (column.print_name); >> >> for (const bound_probe &probe : probes) >> { >> /* `probe_fields' refers to the values of each new field that this >> probe will display. */ >> - VEC (const_char_ptr) *probe_fields = NULL; >> - struct cleanup *c2; >> - const char *val; >> - int kx; >> + std::vector probe_fields; >> >> - if (probe.probe->pops != p) >> + if (probe.prob->get_static_ops () != spops) >> continue; >> >> - c2 = make_cleanup (VEC_cleanup (const_char_ptr), &probe_fields); >> - p->gen_info_probes_table_values (probe.probe, &probe_fields); >> + probe.prob->gen_info_probes_table_values (&probe_fields); >> >> - gdb_assert (VEC_length (const_char_ptr, probe_fields) >> - == headings_size); >> + gdb_assert (probe_fields.size () == headings.size ()); >> >> - for (kx = 0; VEC_iterate (const_char_ptr, probe_fields, kx, val); >> - ++kx) >> + for (const char *val : probe_fields) >> { >> /* It is valid to have a NULL value here, which means that the >> backend does not have something to write and this particular >> @@ -393,128 +401,92 @@ gen_ui_out_table_header_info (const std::vector &probes, >> >> size_max = std::max (strlen (val), size_max); >> } >> - do_cleanups (c2); >> } >> >> current_uiout->table_header (size_max, ui_left, >> - column->field_name, column->print_name); >> + column.field_name, column.print_name); >> } >> - >> - do_cleanups (c); >> } >> >> /* Helper function to print not-applicable strings for all the extra >> - columns defined in a probe_ops. */ >> + columns defined in a static_probe_ops. */ >> >> static void >> -print_ui_out_not_applicables (const struct probe_ops *pops) >> +print_ui_out_not_applicables (const static_probe_ops *spops) >> { >> struct cleanup *c; > > This variable is now unused. Ops, removed. >> - VEC (info_probe_column_s) *headings = NULL; >> - info_probe_column_s *column; >> - int ix; >> + std::vector headings; >> >> - if (pops->gen_info_probes_table_header == NULL) >> + if (!spops->emit_info_probes_extra_fields ()) >> return; >> >> - c = make_cleanup (VEC_cleanup (info_probe_column_s), &headings); >> - pops->gen_info_probes_table_header (&headings); >> - >> - for (ix = 0; >> - VEC_iterate (info_probe_column_s, headings, ix, column); >> - ++ix) >> - current_uiout->field_string (column->field_name, _("n/a")); >> + spops->gen_info_probes_table_header (&headings); >> >> - do_cleanups (c); >> + for (struct info_probe_column column : headings) >> + current_uiout->field_string (column.field_name, _("n/a")); >> } >> >> /* Helper function to print extra information about a probe and an objfile >> represented by PROBE. */ >> >> static void >> -print_ui_out_info (struct probe *probe) >> +print_ui_out_info (probe *probe) >> { >> - int ix; >> - int j = 0; >> /* `values' refers to the actual values of each new field in the output >> of `info probe'. `headings' refers to the names of each new field. */ >> - VEC (const_char_ptr) *values = NULL; >> - VEC (info_probe_column_s) *headings = NULL; >> - info_probe_column_s *column; >> - struct cleanup *c; >> + std::vector headings; >> + std::vector values; >> + struct info_probe_column *column; > > This variable is unused (build probe.c with -Wunused to see all of them). OK, removed all the instances of unused vars, thanks. >> >> gdb_assert (probe != NULL); >> - gdb_assert (probe->pops != NULL); >> >> - if (probe->pops->gen_info_probes_table_header == NULL >> - && probe->pops->gen_info_probes_table_values == NULL) >> - return; >> + probe->get_static_ops ()->gen_info_probes_table_header (&headings); >> + probe->gen_info_probes_table_values (&values); >> >> - gdb_assert (probe->pops->gen_info_probes_table_header != NULL >> - && probe->pops->gen_info_probes_table_values != NULL); >> + gdb_assert (headings.size () == values.size ()); >> >> - c = make_cleanup (VEC_cleanup (info_probe_column_s), &headings); >> - make_cleanup (VEC_cleanup (const_char_ptr), &values); >> - >> - probe->pops->gen_info_probes_table_header (&headings); >> - probe->pops->gen_info_probes_table_values (probe, &values); >> - >> - gdb_assert (VEC_length (info_probe_column_s, headings) >> - == VEC_length (const_char_ptr, values)); >> - >> - for (ix = 0; >> - VEC_iterate (info_probe_column_s, headings, ix, column); >> - ++ix) >> + for (int ix = 0; ix < headings.size (); ++ix) >> { >> - const char *val = VEC_index (const_char_ptr, values, j++); >> + struct info_probe_column column = headings[ix]; >> + const char *val = values[ix]; >> >> if (val == NULL) >> - current_uiout->field_skip (column->field_name); >> + current_uiout->field_skip (column.field_name); >> else >> - current_uiout->field_string (column->field_name, val); >> + current_uiout->field_string (column.field_name, val); >> } >> - >> - do_cleanups (c); >> } >> >> /* Helper function that returns the number of extra fields which POPS will >> need. */ >> >> static int >> -get_number_extra_fields (const struct probe_ops *pops) >> +get_number_extra_fields (const static_probe_ops *spops) >> { >> - VEC (info_probe_column_s) *headings = NULL; >> - struct cleanup *c; >> - int n; >> + std::vector headings; >> >> - if (pops->gen_info_probes_table_header == NULL) >> + if (!spops->emit_info_probes_extra_fields ()) >> return 0; >> >> - c = make_cleanup (VEC_cleanup (info_probe_column_s), &headings); >> - pops->gen_info_probes_table_header (&headings); >> - >> - n = VEC_length (info_probe_column_s, headings); >> + spops->gen_info_probes_table_header (&headings); >> >> - do_cleanups (c); >> - >> - return n; >> + return headings.size (); >> } >> >> -/* Helper function that returns 1 if there is a probe in PROBES >> - featuring the given POPS. It returns 0 otherwise. */ >> +/* Helper function that returns true if there is a probe in PROBES >> + featuring the given SPOPS. It returns false otherwise. */ >> >> -static int >> -exists_probe_with_pops (const std::vector &probes, >> - const struct probe_ops *pops) >> +static bool >> +exists_probe_with_spops (const std::vector &probes, >> + const static_probe_ops *spops) >> { >> struct bound_probe *probe; >> - int ix; >> >> for (const bound_probe &probe : probes) >> - if (probe.probe->pops == pops) >> - return 1; >> + if (probe.prob->get_static_ops () == spops) >> + return true; >> >> - return 0; >> + return false; >> } >> >> /* Helper function that parses a probe linespec of the form [PROVIDER >> @@ -538,8 +510,8 @@ parse_probe_linespec (const char *str, std::string *provider, >> /* See comment in probe.h. */ >> >> void >> -info_probes_for_ops (const char *arg, int from_tty, >> - const struct probe_ops *pops) >> +info_probes_for_spops (const char *arg, int from_tty, >> + const static_probe_ops *spops) >> { >> std::string provider, probe_name, objname; >> int any_found; >> @@ -554,26 +526,28 @@ info_probes_for_ops (const char *arg, int from_tty, >> parse_probe_linespec (arg, &provider, &probe_name, &objname); >> >> std::vector probes >> - = collect_probes (objname, provider, probe_name, pops); >> + = collect_probes (objname, provider, probe_name, spops); >> >> - if (pops == NULL) >> + if (spops == &any_static_probe_ops) >> { >> - /* If the probe_ops is NULL, it means the user has requested a "simple" >> - `info probes', i.e., she wants to print all information about all >> - probes. For that, we have to identify how many extra fields we will >> - need to add in the ui_out table. >> - >> - To do that, we iterate over all probe_ops, querying each one about >> - its extra fields, and incrementing `ui_out_extra_fields' to reflect >> - that number. But note that we ignore the probe_ops for which no probes >> - are defined with the given search criteria. */ >> - >> - for (const probe_ops *po : all_probe_ops) >> - if (exists_probe_with_pops (probes, po)) >> + /* If SPOPS is &any_static_probe_ops, it means the user has >> + requested a "simple" `info probes', i.e., she wants to print >> + all information about all probes. For that, we have to >> + identify how many extra fields we will need to add in the >> + ui_out table. >> + >> + To do that, we iterate over all static_probe_ops, querying >> + each one about its extra fields, and incrementing >> + `ui_out_extra_fields' to reflect that number. But note that >> + we ignore the static_probe_ops for which no probes are >> + defined with the given search criteria. */ >> + >> + for (const static_probe_ops *po : all_static_probe_ops) >> + if (exists_probe_with_spops (probes, po)) >> ui_out_extra_fields += get_number_extra_fields (po); >> } >> else >> - ui_out_extra_fields = get_number_extra_fields (pops); >> + ui_out_extra_fields = get_number_extra_fields (spops); >> >> { >> ui_out_emit_table table_emitter (current_uiout, >> @@ -589,11 +563,12 @@ info_probes_for_ops (const char *arg, int from_tty, >> `name' and `objname'). */ >> for (const bound_probe &probe : probes) >> { >> - const char *probe_type = probe.probe->pops->type_name (probe.probe); >> + const char *probe_type = probe.prob->get_static_ops ()->type_name (); >> >> size_type = std::max (strlen (probe_type), size_type); >> - size_name = std::max (strlen (probe.probe->name), size_name); >> - size_provider = std::max (strlen (probe.probe->provider), size_provider); >> + size_name = std::max (strlen (probe.prob->get_name ()), size_name); >> + size_provider = std::max (strlen (probe.prob->get_provider ()), >> + size_provider); >> size_objname = std::max (strlen (objfile_name (probe.objfile)), >> size_objname); >> } >> @@ -604,44 +579,46 @@ info_probes_for_ops (const char *arg, int from_tty, >> current_uiout->table_header (size_name, ui_left, "name", _("Name")); >> current_uiout->table_header (size_addr, ui_left, "addr", _("Where")); >> >> - if (pops == NULL) >> + if (spops == &any_static_probe_ops) >> { >> /* We have to generate the table header for each new probe type >> that we will print. Note that this excludes probe types not >> having any defined probe with the search criteria. */ >> - for (const probe_ops *po : all_probe_ops) >> - if (exists_probe_with_pops (probes, po)) >> + for (const static_probe_ops *po : all_static_probe_ops) >> + if (exists_probe_with_spops (probes, po)) >> gen_ui_out_table_header_info (probes, po); >> } >> else >> - gen_ui_out_table_header_info (probes, pops); >> + gen_ui_out_table_header_info (probes, spops); >> >> current_uiout->table_header (size_objname, ui_left, "object", _("Object")); >> current_uiout->table_body (); >> >> for (const bound_probe &probe : probes) >> { >> - const char *probe_type = probe.probe->pops->type_name (probe.probe); >> + const char *probe_type = probe.prob->get_static_ops ()->type_name (); >> >> ui_out_emit_tuple tuple_emitter (current_uiout, "probe"); >> >> - current_uiout->field_string ("type",probe_type); >> - current_uiout->field_string ("provider", probe.probe->provider); >> - current_uiout->field_string ("name", probe.probe->name); >> - current_uiout->field_core_addr ("addr", probe.probe->arch, >> - get_probe_address (probe.probe, >> - probe.objfile)); >> + current_uiout->field_string ("type", probe_type); >> + current_uiout->field_string ("provider", probe.prob->get_provider ()); >> + current_uiout->field_string ("name", probe.prob->get_name ()); >> + current_uiout->field_core_addr ("addr", probe.prob->get_gdbarch (), >> + probe.prob->get_relocated_address >> + (probe.objfile)); >> >> - if (pops == NULL) >> + if (spops == &any_static_probe_ops) >> { >> - for (const probe_ops *po : all_probe_ops) >> - if (probe.probe->pops == po) >> - print_ui_out_info (probe.probe); >> - else if (exists_probe_with_pops (probes, po)) >> - print_ui_out_not_applicables (po); >> + for (const static_probe_ops *po : all_static_probe_ops) >> + { >> + if (probe.prob->get_static_ops () == po) >> + print_ui_out_info (probe.prob); >> + else if (exists_probe_with_spops (probes, po)) >> + print_ui_out_not_applicables (po); >> + } >> } >> else >> - print_ui_out_info (probe.probe); >> + print_ui_out_info (probe.prob); >> >> current_uiout->field_string ("object", >> objfile_name (probe.objfile)); >> @@ -660,7 +637,7 @@ info_probes_for_ops (const char *arg, int from_tty, >> static void >> info_probes_command (const char *arg, int from_tty) >> { >> - info_probes_for_ops (arg, from_tty, NULL); >> + info_probes_for_spops (arg, from_tty, &any_static_probe_ops); >> } >> >> /* Implementation of the `enable probes' command. */ >> @@ -673,7 +650,7 @@ enable_probes_command (const char *arg, int from_tty) >> parse_probe_linespec ((const char *) arg, &provider, &probe_name, &objname); >> >> std::vector probes >> - = collect_probes (objname, provider, probe_name, NULL); >> + = collect_probes (objname, provider, probe_name, &any_static_probe_ops); >> if (probes.empty ()) >> { >> current_uiout->message (_("No probes matched.\n")); >> @@ -684,17 +661,17 @@ enable_probes_command (const char *arg, int from_tty) >> notion of enabling a probe. */ >> for (const bound_probe &probe: probes) >> { >> - const struct probe_ops *pops = probe.probe->pops; >> - >> - if (pops->enable_probe != NULL) >> + if (probe.prob->can_enable ()) >> { >> - pops->enable_probe (probe.probe); >> + probe.prob->enable (); >> current_uiout->message (_("Probe %s:%s enabled.\n"), >> - probe.probe->provider, probe.probe->name); >> + probe.prob->get_provider (), >> + probe.prob->get_name ()); >> } >> else >> current_uiout->message (_("Probe %s:%s cannot be enabled.\n"), >> - probe.probe->provider, probe.probe->name); >> + probe.prob->get_provider (), >> + probe.prob->get_name ()); >> } >> } >> >> @@ -708,7 +685,7 @@ disable_probes_command (const char *arg, int from_tty) >> parse_probe_linespec ((const char *) arg, &provider, &probe_name, &objname); >> >> std::vector probes >> - = collect_probes (objname, provider, probe_name, NULL /* pops */); >> + = collect_probes (objname, provider, probe_name, &any_static_probe_ops); >> if (probes.empty ()) >> { >> current_uiout->message (_("No probes matched.\n")); >> @@ -719,55 +696,22 @@ disable_probes_command (const char *arg, int from_tty) >> notion of enabling a probe. */ >> for (const bound_probe &probe : probes) >> { >> - const struct probe_ops *pops = probe.probe->pops; >> - >> - if (pops->disable_probe != NULL) >> + if (probe.prob->can_enable ()) >> { >> - pops->disable_probe (probe.probe); >> + probe.prob->disable (); >> current_uiout->message (_("Probe %s:%s disabled.\n"), >> - probe.probe->provider, probe.probe->name); >> + probe.prob->get_provider (), >> + probe.prob->get_name ()); >> } >> else >> current_uiout->message (_("Probe %s:%s cannot be disabled.\n"), >> - probe.probe->provider, probe.probe->name); >> + probe.prob->get_provider (), >> + probe.prob->get_name ()); >> } >> } >> >> /* See comments in probe.h. */ >> >> -CORE_ADDR >> -get_probe_address (struct probe *probe, struct objfile *objfile) >> -{ >> - return probe->pops->get_probe_address (probe, objfile); >> -} >> - >> -/* See comments in probe.h. */ >> - >> -unsigned >> -get_probe_argument_count (struct probe *probe, struct frame_info *frame) >> -{ >> - return probe->pops->get_probe_argument_count (probe, frame); >> -} >> - >> -/* See comments in probe.h. */ >> - >> -int >> -can_evaluate_probe_arguments (struct probe *probe) >> -{ >> - return probe->pops->can_evaluate_probe_arguments (probe); >> -} >> - >> -/* See comments in probe.h. */ >> - >> -struct value * >> -evaluate_probe_argument (struct probe *probe, unsigned n, >> - struct frame_info *frame) >> -{ >> - return probe->pops->evaluate_probe_argument (probe, n, frame); >> -} >> - >> -/* See comments in probe.h. */ >> - >> struct value * >> probe_safe_evaluate_at_pc (struct frame_info *frame, unsigned n) >> { >> @@ -775,22 +719,22 @@ probe_safe_evaluate_at_pc (struct frame_info *frame, unsigned n) >> unsigned n_args; >> >> probe = find_probe_by_pc (get_frame_pc (frame)); >> - if (!probe.probe) >> + if (!probe.prob) >> return NULL; >> >> - n_args = get_probe_argument_count (probe.probe, frame); >> + n_args = probe.prob->get_argument_count (frame); >> if (n >= n_args) >> return NULL; >> >> - return evaluate_probe_argument (probe.probe, n, frame); >> + return probe.prob->evaluate_argument (n, frame); >> } >> >> /* See comment in probe.h. */ >> >> -const struct probe_ops * >> -probe_linespec_to_ops (const char **linespecp) >> +const struct static_probe_ops * >> +probe_linespec_to_static_ops (const char **linespecp) >> { >> - for (const probe_ops *ops : all_probe_ops) >> + for (const static_probe_ops *ops : all_static_probe_ops) >> if (ops->is_linespec (linespecp)) >> return ops; >> >> @@ -820,31 +764,48 @@ probe_is_linespec_by_keyword (const char **linespecp, const char *const *keyword >> return 0; >> } >> >> -/* Implementation of `is_linespec' method for `struct probe_ops'. */ >> +/* Implementation of `is_linespec' method. */ >> >> -static int >> -probe_any_is_linespec (const char **linespecp) >> +bool >> +any_static_probe_ops::is_linespec (const char **linespecp) const >> { >> static const char *const keywords[] = { "-p", "-probe", NULL }; >> >> return probe_is_linespec_by_keyword (linespecp, keywords); >> } >> >> -/* Dummy method used for `probe_ops_any'. */ >> +/* Implementation of 'get_probes' method. */ >> >> -static void >> -probe_any_get_probes (std::vector *probesp, struct objfile *objfile) >> +void >> +any_static_probe_ops::get_probes (std::vector *probesp, >> + struct objfile *objfile) const >> { >> /* No probes can be provided by this dummy backend. */ >> } >> >> -/* Operations associated with a generic probe. */ >> +/* Implementation of the 'type_name' method. */ >> >> -const struct probe_ops probe_ops_any = >> +const char * >> +any_static_probe_ops::type_name () const >> { >> - probe_any_is_linespec, >> - probe_any_get_probes, >> -}; >> + return NULL; >> +} >> + >> +/* Implementation of the 'emit_info_probes_extra_fields' method. */ >> + >> +bool >> +any_static_probe_ops::emit_info_probes_extra_fields () const >> +{ >> + return false; >> +} >> + >> +/* Implementation of the 'gen_info_probes_table_header' method. */ >> + >> +void >> +any_static_probe_ops::gen_info_probes_table_header >> + (std::vector *heads) const >> +{ >> +} >> >> /* See comments in probe.h. */ >> >> @@ -890,10 +851,10 @@ compute_probe_arg (struct gdbarch *arch, struct internalvar *ivar, >> gdb_assert (sel >= -1); >> >> pc_probe = find_probe_by_pc (pc); >> - if (pc_probe.probe == NULL) >> + if (pc_probe.prob == NULL) >> error (_("No probe at PC %s"), core_addr_to_string (pc)); >> >> - n_args = get_probe_argument_count (pc_probe.probe, frame); >> + n_args = pc_probe.prob->get_argument_count (frame); >> if (sel == -1) >> return value_from_longest (builtin_type (arch)->builtin_int, n_args); >> >> @@ -901,7 +862,7 @@ compute_probe_arg (struct gdbarch *arch, struct internalvar *ivar, >> error (_("Invalid probe argument %d -- probe has %u arguments available"), >> sel, n_args); >> >> - return evaluate_probe_argument (pc_probe.probe, sel, frame); >> + return pc_probe.prob->evaluate_argument (sel, frame); >> } >> >> /* This is called to compile one of the $_probe_arg* convenience >> @@ -922,10 +883,10 @@ compile_probe_arg (struct internalvar *ivar, struct agent_expr *expr, >> gdb_assert (sel >= -1); >> >> pc_probe = find_probe_by_pc (pc); >> - if (pc_probe.probe == NULL) >> + if (pc_probe.prob == NULL) >> error (_("No probe at PC %s"), core_addr_to_string (pc)); >> >> - n_args = get_probe_argument_count (pc_probe.probe, frame); >> + n_args = pc_probe.prob->get_argument_count (frame); >> >> if (sel == -1) >> { >> @@ -940,7 +901,7 @@ compile_probe_arg (struct internalvar *ivar, struct agent_expr *expr, >> error (_("Invalid probe argument %d -- probe has %d arguments available"), >> sel, n_args); >> >> - pc_probe.probe->pops->compile_to_ax (pc_probe.probe, expr, value, sel); >> + pc_probe.prob->compile_to_ax (expr, value, sel); >> } >> >> static const struct internalvar_funcs probe_funcs = >> @@ -951,12 +912,12 @@ static const struct internalvar_funcs probe_funcs = >> }; >> >> >> -std::vector all_probe_ops; >> +std::vector all_static_probe_ops; >> >> void >> _initialize_probe (void) >> { >> - all_probe_ops.push_back (&probe_ops_any); >> + all_static_probe_ops.push_back (&any_static_probe_ops); >> >> create_internalvar_type_lazy ("_probe_argc", &probe_funcs, >> (void *) (uintptr_t) -1); >> diff --git a/gdb/probe.h b/gdb/probe.h >> index 822e5c89a9..9ad183013b 100644 >> --- a/gdb/probe.h >> +++ b/gdb/probe.h >> @@ -30,7 +30,6 @@ struct info_probe_column >> { >> /* The internal name of the field. This string cannot be capitalized nor >> localized, e.g., "extra_field". */ >> - >> const char *field_name; >> >> /* The field name to be printed in the `info probes' command. This >> @@ -38,165 +37,206 @@ struct info_probe_column >> const char *print_name; >> }; >> >> -typedef struct info_probe_column info_probe_column_s; >> -DEF_VEC_O (info_probe_column_s); >> - >> -/* Operations associated with a probe. */ >> - >> -struct probe_ops >> - { >> - /* Method responsible for verifying if LINESPECP is a valid linespec for >> - a probe breakpoint. It should return 1 if it is, or zero if it is not. >> - It also should update LINESPECP in order to discard the breakpoint >> - option associated with this linespec. For example, if the option is >> - `-probe', and the LINESPECP is `-probe abc', the function should >> - return 1 and set LINESPECP to `abc'. */ >> - >> - int (*is_linespec) (const char **linespecp); >> - >> - /* Function that should fill PROBES with known probes from 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. */ >> - >> - CORE_ADDR (*get_probe_address) (struct probe *probe, >> - struct objfile *objfile); >> - >> - /* Return the number of arguments of PROBE. This function can >> - throw an exception. */ >> - >> - unsigned (*get_probe_argument_count) (struct probe *probe, >> - struct frame_info *frame); >> - >> - /* Return 1 if the probe interface can evaluate the arguments of probe >> - PROBE, zero otherwise. See the comments on >> - sym_probe_fns:can_evaluate_probe_arguments for more details. */ >> - >> - int (*can_evaluate_probe_arguments) (struct probe *probe); > > This could return a bool. This function was renamed to 'can_evaluate_arguments', but yeah, updated. >> - >> - /* Evaluate the Nth argument from the PROBE, returning a value >> - corresponding to it. The argument number is represented N. >> - This function can throw an exception. */ >> - >> - struct value *(*evaluate_probe_argument) (struct probe *probe, >> - unsigned n, >> - struct frame_info *frame); >> - >> - /* Compile the Nth argument of the PROBE to an agent expression. >> - The argument number is represented by N. */ >> - >> - void (*compile_to_ax) (struct probe *probe, struct agent_expr *aexpr, >> - struct axs_value *axs_value, unsigned n); >> +/* Operations that act on probes, but are specific to each backend. >> + These methods do not go into the 'class probe' because they do not >> + act on a single probe; instead, they are used to operate on many >> + probes at once, or to provide information about the probe backend >> + itself, instead of a single probe. >> >> - /* Set the semaphore associated with the PROBE. This function only makes >> - sense if the probe has a concept of semaphore associated to a >> - probe, otherwise it can be set to NULL. */ >> + Each probe backend needs to inherit this class and implement all of >> + the virtual functions specified here. Then, an object shall be >> + instantiated and added (or "registered") to the >> + ALL_STATIC_PROBE_OPS vector so that the frontend probe interface >> + can use it in the generic probe functions. */ >> >> - void (*set_semaphore) (struct probe *probe, struct objfile *objfile, >> - struct gdbarch *gdbarch); >> - >> - /* Clear the semaphore associated with the PROBE. This function only >> - makes sense if the probe has a concept of semaphore associated to >> - a probe, otherwise it can be set to NULL. */ >> - >> - void (*clear_semaphore) (struct probe *probe, struct objfile *objfile, >> - struct gdbarch *gdbarch); >> - >> - /* Function called to destroy PROBE's specific data. This function >> - shall not free PROBE itself. */ >> - >> - void (*destroy) (struct probe *probe); >> - >> - /* Return a pointer to a name identifying the probe type. This is >> - the string that will be displayed in the "Type" column of the >> - `info probes' command. */ >> +class static_probe_ops >> +{ >> +public: >> + /* Method responsible for verifying if LINESPECP is a valid linespec >> + for a probe breakpoint. It should return true if it is, or false >> + if it is not. It also should update LINESPECP in order to >> + discard the breakpoint option associated with this linespec. For >> + example, if the option is `-probe', and the LINESPECP is `-probe >> + abc', the function should return 1 and set LINESPECP to >> + `abc'. */ >> + virtual bool is_linespec (const char **linespecp) const = 0; >> + >> + /* Function that should fill PROBES with known probes from OBJFILE. */ >> + virtual void get_probes (std::vector *probes, >> + struct objfile *objfile) const = 0; >> + >> + /* Return a pointer to a name identifying the probe type. This is >> + the string that will be displayed in the "Type" column of the >> + `info probes' command. */ >> + virtual const char *type_name () const = 0; >> + >> + /* Return true if the probe type will emit extra fields when the >> + user issues a "info probes" command. If this function returns >> + false, then GEN_INFO_PROBES_TABLE_HEADER and >> + GEN_INFO_PROBES_TABLE_VALUES are not used. */ >> + virtual bool emit_info_probes_extra_fields () const = 0; >> + >> + /* Function responsible for providing the extra fields that will be >> + printed in the `info probes' command. It should fill HEADS >> + with whatever extra fields it needs. If no extra fields are >> + required by the probe backend, the method EMIT_INFO_PROBES_FIELDS >> + should return false. */ >> + virtual void gen_info_probes_table_header >> + (std::vector *heads) const = 0; >> +}; >> >> - const char *(*type_name) (struct probe *probe); >> +/* Definition of a vector of static_probe_ops. */ >> >> - /* Function responsible for providing the extra fields that will be >> - printed in the `info probes' command. It should fill HEADS >> - with whatever extra fields it needs. If the backend doesn't need >> - to print extra fields, it can set this method to NULL. */ >> +extern std::vector all_static_probe_ops; >> >> - void (*gen_info_probes_table_header) (VEC (info_probe_column_s) **heads); >> +/* Helper function that, given KEYWORDS, iterate over it trying to match >> + each keyword with LINESPECP. If it succeeds, it updates the LINESPECP >> + pointer and returns 1. Otherwise, nothing is done to LINESPECP and zero >> + is returned. */ >> >> - /* Function that will fill VALUES with the values of the extra fields >> - to be printed for PROBE. If the backend implements the >> - `gen_ui_out_table_header' method, then it should implement >> - this method as well. The backend should also guarantee that the >> - order and the number of values in the vector is exactly the same >> - as the order of the extra fields provided in the method >> - `gen_ui_out_table_header'. If a certain field is to be skipped >> - when printing the information, you can push a NULL value in that >> - position in the vector. */ >> +extern int probe_is_linespec_by_keyword (const char **linespecp, >> + const char *const *keywords); >> >> - void (*gen_info_probes_table_values) (struct probe *probe, >> - VEC (const_char_ptr) **values); >> +/* Return specific STATIC_PROBE_OPS * matching *LINESPECP and possibly >> + updating LINESPECP to skip its "-probe-type " prefix. Return >> + &static_probe_ops_any if LINESPECP matches "-probe ", that is any >> + unspecific probe. Return NULL if LINESPECP is not identified as >> + any known probe type, *LINESPECP is not modified in such case. */ >> >> - /* Enable a probe. The semantics of "enabling" a probe depend on >> - the specific backend and the field can be NULL in case enabling >> - probes is not supported. This function can throw an >> - exception. */ >> +extern const static_probe_ops * >> + probe_linespec_to_static_ops (const char **linespecp); >> >> - void (*enable_probe) (struct probe *probe); >> +/* The probe itself. The class contains generic information about the >> + probe. */ >> >> - /* Disable a probe. The semantics of "disabling" a probe depend >> - on the specific backend and the field can be NULL in case >> - disabling probes is not supported. This function can throw an >> - exception. */ >> +class probe >> +{ >> +public: >> + /* Default constructor for a probe. */ >> + probe (std::string &&name_, std::string &&provider_, CORE_ADDR address_, >> + struct gdbarch *arch_) >> + : m_name (std::move (name_)), m_provider (std::move (provider_)), >> + m_address (address_), m_arch (arch_) >> + {} >> >> - void (*disable_probe) (struct probe *probe); >> - }; >> + /* Virtual destructor. */ >> + virtual ~probe () >> + {} >> >> -/* Definition of a vector of probe_ops. */ >> + /* Compute the probe's relocated address. OBJFILE is the objfile >> + in which the probe originated. */ >> + virtual CORE_ADDR get_relocated_address (struct objfile *objfile) = 0; >> + >> + /* Return the number of arguments of the probe. This function can >> + throw an exception. */ >> + virtual unsigned get_argument_count (struct frame_info *frame) = 0; >> + >> + /* Return 1 if the probe interface can evaluate the arguments of >> + probe, zero otherwise. See the comments on >> + sym_probe_fns:can_evaluate_probe_arguments for more >> + details. */ >> + virtual int can_evaluate_arguments () const = 0; >> + >> + /* Evaluate the Nth argument from the probe, returning a value >> + corresponding to it. The argument number is represented N. >> + This function can throw an exception. */ >> + virtual struct value *evaluate_argument (unsigned n, >> + struct frame_info *frame) = 0; >> + >> + /* Compile the Nth argument of the probe to an agent expression. >> + The argument number is represented by N. */ >> + virtual void compile_to_ax (struct agent_expr *aexpr, >> + struct axs_value *axs_value, >> + unsigned n) = 0; >> + >> + /* Set the semaphore associated with the probe. This function only >> + makes sense if the probe has a concept of semaphore associated to >> + a probe. */ >> + virtual void set_semaphore (struct objfile *objfile, >> + struct gdbarch *gdbarch) >> + {} >> >> -extern std::vector all_probe_ops; >> + /* Clear the semaphore associated with the probe. This function >> + only makes sense if the probe has a concept of semaphore >> + associated to a probe. */ >> + virtual void clear_semaphore (struct objfile *objfile, >> + struct gdbarch *gdbarch) >> + {} >> >> -/* The probe_ops associated with the generic probe. */ >> + /* Return the pointer to the static_probe_ops instance related to >> + the probe type. */ >> + virtual const static_probe_ops *get_static_ops () const = 0; >> + >> + /* Function that will fill VALUES with the values of the extra >> + fields to be printed for the probe. >> + >> + If the backend implements the `gen_ui_out_table_header' method, >> + then it should implement this method as well. The backend should >> + also guarantee that the order and the number of values in the >> + vector is exactly the same as the order of the extra fields >> + provided in the method `gen_ui_out_table_header'. If a certain >> + field is to be skipped when printing the information, you can >> + push a NULL value in that position in the vector. */ >> + virtual void gen_info_probes_table_values >> + (std::vector *values) const >> + {} >> >> -extern const struct probe_ops probe_ops_any; >> + /* Return true if the probe can be enabled; false otherwise. */ >> + virtual bool can_enable () const >> + { >> + return false; >> + } >> >> -/* Helper function that, given KEYWORDS, iterate over it trying to match >> - each keyword with LINESPECP. If it succeeds, it updates the LINESPECP >> - pointer and returns 1. Otherwise, nothing is done to LINESPECP and zero >> - is returned. */ >> + /* Enable the probe. The semantics of "enabling" a probe depend on >> + the specific backend. This function can throw an exception. */ >> + virtual void enable () >> + {} >> >> -extern int probe_is_linespec_by_keyword (const char **linespecp, >> - const char *const *keywords); >> + /* Disable the probe. The semantics of "disabling" a probe depend >> + on the specific backend. This function can throw an >> + exception. */ >> + virtual void disable () >> + {} >> >> -/* Return specific PROBE_OPS * matching *LINESPECP and possibly updating >> - *LINESPECP to skip its "-probe-type " prefix. Return &probe_ops_any if >> - *LINESPECP matches "-probe ", that is any unspecific probe. Return NULL if >> - *LINESPECP is not identified as any known probe type, *LINESPECP is not >> - modified in such case. */ >> + /* Getter for M_NAME. */ >> + const char *get_name () const >> + { >> + return m_name.c_str (); >> + } >> >> -extern const struct probe_ops *probe_linespec_to_ops (const char **linespecp); >> + /* Getter for M_PROVIDER. */ >> + const char *get_provider () const >> + { >> + return m_provider.c_str (); >> + } > > I'd suggest making get_name and get_provider return an std::string. > Most usages use strcmp/strlen, which can be replaced with their C++ > equivalent. Other usages are used to pass to uiout methods, like > field_string. I think we could add an overload to these methods > that take a const std::string& instead of a const char *, for > convenience. But I'm fine if we do it in another patchset, so you > don't have to complicate things for nothing in this patchset. OK, no problem. Converted to const std::string&. >> >> -/* The probe itself. The struct contains generic information about the >> - probe, and then some specific information which should be stored in >> - the `probe_info' field. */ >> + /* Getter for M_ADDRESS. */ >> + CORE_ADDR get_address () const >> + { >> + return m_address; >> + } >> >> -struct probe >> + /* Getter for M_ARCH. */ >> + struct gdbarch *get_gdbarch () const >> { >> - /* The operations associated with this probe. */ >> - const struct probe_ops *pops; >> + return m_arch; >> + } >> >> - /* The probe's architecture. */ >> - struct gdbarch *arch; >> +private: >> + /* The name of the probe. */ >> + std::string m_name; >> >> - /* The name of the probe. */ >> - const char *name; >> + /* The provider of the probe. It generally defaults to the name of >> + the objfile which contains the probe. */ >> + std::string m_provider; >> >> - /* The provider of the probe. It generally defaults to the name of >> - the objfile which contains the probe. */ >> - const char *provider; >> + /* The address where the probe is inserted, relative to >> + SECT_OFF_TEXT. */ >> + CORE_ADDR m_address; >> >> - /* The address where the probe is inserted, relative to >> - SECT_OFF_TEXT. */ >> - CORE_ADDR address; >> - }; >> + /* The probe's architecture. */ >> + struct gdbarch *m_arch; >> +}; >> >> /* A bound probe holds a pointer to a probe and a pointer to the >> probe's defining objfile. This is needed because probes are >> @@ -206,22 +246,18 @@ struct probe >> struct bound_probe >> { >> /* Create an empty bound_probe object. */ >> - >> bound_probe () >> {} >> >> /* Create and initialize a bound_probe object using PROBE and OBJFILE. */ >> - >> - bound_probe (struct probe *probe_, struct objfile *objfile_) >> - : probe (probe_), objfile (objfile_) >> + bound_probe (probe *probe_, struct objfile *objfile_) >> + : prob (probe_), objfile (objfile_) >> {} >> >> /* The probe. */ >> - >> - struct probe *probe = NULL; >> + probe *prob = NULL; >> >> /* The objfile in which the probe originated. */ >> - >> struct objfile *objfile = NULL; >> }; >> >> @@ -234,11 +270,6 @@ extern std::vector parse_probes >> struct program_space *pspace, >> struct linespec_result *canon); >> >> -/* Helper function to register the proper probe_ops to a newly created probe. >> - This function is mainly called from `sym_get_probes'. */ >> - >> -extern void register_probe_ops (struct probe *probe); >> - >> /* Given a PC, find an associated probe. If a probe is found, return >> it. If no probe is found, return a bound probe whose fields are >> both NULL. */ >> @@ -253,13 +284,13 @@ extern std::vector find_probes_in_objfile (struct objfile *objfile, >> const char *provider, >> const char *name); >> >> -/* Generate a `info probes' command output for probe_ops represented by >> - POPS. If POPS is NULL it considers any probes types. It is a helper >> - function that can be used by the probe backends to print their >> - `info probe TYPE'. */ >> +/* Generate a `info probes' command output for probes associated with >> + SPOPS. If SPOPS is related to the "any probe" type, then all probe >> + types are considered. It is a helper function that can be used by >> + the probe backends to print their `info probe TYPE'. */ >> >> -extern void info_probes_for_ops (const char *arg, int from_tty, >> - const struct probe_ops *pops); >> +extern void info_probes_for_spops (const char *arg, int from_tty, >> + const static_probe_ops *spops); >> >> /* Return the `cmd_list_element' associated with the `info probes' command, >> or create a new one if it doesn't exist. Helper function that serves the >> @@ -268,34 +299,6 @@ extern void info_probes_for_ops (const char *arg, int from_tty, >> >> extern struct cmd_list_element **info_probes_cmdlist_get (void); >> >> -/* Compute the probe's relocated address. OBJFILE is the objfile in >> - which the probe originated. */ >> - >> -extern CORE_ADDR get_probe_address (struct probe *probe, >> - struct objfile *objfile); >> - >> -/* Return the argument count of the specified probe. >> - >> - This function can throw an exception. */ >> - >> -extern unsigned get_probe_argument_count (struct probe *probe, >> - struct frame_info *frame); >> - >> -/* Return 1 if the probe interface associated with PROBE can evaluate >> - arguments, zero otherwise. See the comments on the definition of >> - sym_probe_fns:can_evaluate_probe_arguments for more details. */ >> - >> -extern int can_evaluate_probe_arguments (struct probe *probe); >> - >> -/* Evaluate argument N of the specified probe. N must be between 0 >> - inclusive and get_probe_argument_count exclusive. >> - >> - This function can throw an exception. */ >> - >> -extern struct value *evaluate_probe_argument (struct probe *probe, >> - unsigned n, >> - struct frame_info *frame); >> - >> /* A convenience function that finds a probe at the PC in FRAME and >> evaluates argument N, with 0 <= N < number_of_args. If there is no >> probe at that location, or if the probe does not have enough arguments, >> diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c >> index 5ec606de43..6e834fb5d7 100644 >> --- a/gdb/solib-svr4.c >> +++ b/gdb/solib-svr4.c >> @@ -351,7 +351,7 @@ struct svr4_info >> /* Table of struct probe_and_action instances, used by the >> probes-based interface to map breakpoint addresses to probes >> and their associated actions. Lookup is performed using >> - probe_and_action->probe->address. */ >> + probe_and_action->prob->address. */ >> htab_t probes_table; >> >> /* List of objects loaded into the inferior, used by the probes- >> @@ -1664,7 +1664,7 @@ exec_entry_point (struct bfd *abfd, struct target_ops *targ) >> struct probe_and_action >> { >> /* The probe. */ >> - struct probe *probe; >> + probe *prob; >> >> /* The relocated address of the probe. */ >> CORE_ADDR address; >> @@ -1699,7 +1699,7 @@ equal_probe_and_action (const void *p1, const void *p2) >> probes table. */ >> >> static void >> -register_solib_event_probe (struct probe *probe, CORE_ADDR address, >> +register_solib_event_probe (probe *prob, CORE_ADDR address, >> enum probe_action action) >> { >> struct svr4_info *info = get_svr4_info (); >> @@ -1712,13 +1712,13 @@ register_solib_event_probe (struct probe *probe, CORE_ADDR address, >> equal_probe_and_action, >> xfree, xcalloc, xfree); >> >> - lookup.probe = probe; >> + lookup.prob = prob; >> lookup.address = address; >> slot = htab_find_slot (info->probes_table, &lookup, INSERT); >> gdb_assert (*slot == HTAB_EMPTY_ENTRY); >> >> pa = XCNEW (struct probe_and_action); >> - pa->probe = probe; >> + pa->prob = prob; >> pa->address = address; >> pa->action = action; >> >> @@ -1767,7 +1767,7 @@ solib_event_probe_action (struct probe_and_action *pa) >> arg2: struct link_map *new (optional, for incremental updates) */ >> TRY >> { >> - probe_argc = get_probe_argument_count (pa->probe, frame); >> + probe_argc = pa->prob->get_argument_count (frame); >> } >> CATCH (ex, RETURN_MASK_ERROR) >> { >> @@ -1776,11 +1776,11 @@ solib_event_probe_action (struct probe_and_action *pa) >> } >> END_CATCH >> >> - /* If get_probe_argument_count throws an exception, probe_argc will >> - be set to zero. However, if pa->probe does not have arguments, >> - then get_probe_argument_count will succeed but probe_argc will >> - also be zero. Both cases happen because of different things, but >> - they are treated equally here: action will be set to >> + /* If get_argument_count throws an exception, probe_argc will be set >> + to zero. However, if pa->prob does not have arguments, then >> + get_argument_count will succeed but probe_argc will also be zero. >> + Both cases happen because of different things, but they are >> + treated equally here: action will be set to >> PROBES_INTERFACE_FAILED. */ >> if (probe_argc == 2) >> action = FULL_RELOAD; >> @@ -1922,7 +1922,7 @@ svr4_handle_solib_event (void) >> return; >> } >> >> - /* evaluate_probe_argument looks up symbols in the dynamic linker >> + /* evaluate_argument looks up symbols in the dynamic linker >> using find_pc_section. find_pc_section is accelerated by a cache >> called the section map. The section map is invalidated every >> time a shared library is loaded or unloaded, and if the inferior >> @@ -1931,14 +1931,14 @@ svr4_handle_solib_event (void) >> We called find_pc_section in svr4_create_solib_event_breakpoints, >> so we can guarantee that the dynamic linker's sections are in the >> section map. We can therefore inhibit section map updates across >> - these calls to evaluate_probe_argument and save a lot of time. */ >> + these calls to evaluate_argument and save a lot of time. */ >> inhibit_section_map_updates (current_program_space); >> usm_chain = make_cleanup (resume_section_map_updates_cleanup, >> current_program_space); >> >> TRY >> { >> - val = evaluate_probe_argument (pa->probe, 1, frame); >> + val = pa->prob->evaluate_argument (1, frame); >> } >> CATCH (ex, RETURN_MASK_ERROR) >> { >> @@ -1979,7 +1979,7 @@ svr4_handle_solib_event (void) >> { >> TRY >> { >> - val = evaluate_probe_argument (pa->probe, 2, frame); >> + val = pa->prob->evaluate_argument (2, frame); >> } >> CATCH (ex, RETURN_MASK_ERROR) >> { >> @@ -2084,7 +2084,7 @@ svr4_create_probe_breakpoints (struct gdbarch *gdbarch, >> >> for (probe *p : probes[i]) >> { >> - CORE_ADDR address = get_probe_address (p, objfile); >> + CORE_ADDR address = p->get_relocated_address (objfile); >> >> create_solib_event_breakpoint (gdbarch, address); >> register_solib_event_probe (p, address, action); >> @@ -2126,7 +2126,7 @@ svr4_create_solib_event_breakpoints (struct gdbarch *gdbarch, >> for (int i = 0; i < NUM_PROBES; i++) >> { >> const char *name = probe_info[i].name; >> - struct probe *p; >> + probe *p; >> char buf[32]; >> >> /* Fedora 17 and Red Hat Enterprise Linux 6.2-6.4 >> @@ -2160,7 +2160,7 @@ svr4_create_solib_event_breakpoints (struct gdbarch *gdbarch, >> if (!checked_can_use_probe_arguments) >> { >> p = probes[i][0]; >> - if (!can_evaluate_probe_arguments (p)) >> + if (!p->can_evaluate_arguments ()) >> { >> all_probes_found = 0; >> break; >> diff --git a/gdb/symfile.h b/gdb/symfile.h >> index 3472aa0e7b..10b1504937 100644 >> --- a/gdb/symfile.h >> +++ b/gdb/symfile.h >> @@ -34,11 +34,11 @@ struct objfile; >> struct obj_section; >> struct obstack; >> struct block; >> -struct probe; >> struct value; >> struct frame_info; >> struct agent_expr; >> struct axs_value; >> +class probe; >> >> /* Comparison function for symbol look ups. */ >> >> diff --git a/gdb/symtab.h b/gdb/symtab.h >> index 7b8b5cc640..8bf9f3994f 100644 >> --- a/gdb/symtab.h >> +++ b/gdb/symtab.h >> @@ -41,10 +41,10 @@ struct axs_value; >> struct agent_expr; >> struct program_space; >> struct language_defn; >> -struct probe; >> struct common_block; >> struct obj_section; >> struct cmd_list_element; >> +class probe; >> struct lookup_name_info; >> >> /* How to match a lookup name against a symbol search name. */ >> @@ -1702,7 +1702,7 @@ struct symtab_and_line >> bool explicit_line = false; >> >> /* The probe associated with this symtab_and_line. */ >> - struct probe *probe = NULL; >> + probe *prob = NULL; >> /* If PROBE is not NULL, then this is the objfile in which the probe >> originated. */ >> struct objfile *objfile = NULL; >> diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c >> index 59a7b64ae8..76d05dfc13 100644 >> --- a/gdb/tracepoint.c >> +++ b/gdb/tracepoint.c >> @@ -1654,11 +1654,9 @@ start_tracing (const char *notes) >> t->number_on_target = b->number; >> >> for (loc = b->loc; loc; loc = loc->next) >> - if (loc->probe.probe != NULL >> - && loc->probe.probe->pops->set_semaphore != NULL) >> - loc->probe.probe->pops->set_semaphore (loc->probe.probe, >> - loc->probe.objfile, >> - loc->gdbarch); >> + if (loc->probe.prob != NULL) >> + loc->probe.prob->set_semaphore (loc->probe.objfile, >> + loc->gdbarch); >> >> if (bp_location_downloaded) >> observer_notify_breakpoint_modified (b); >> @@ -1754,11 +1752,9 @@ stop_tracing (const char *note) >> but we don't really care if this semaphore goes out of sync. >> That's why we are decrementing it here, but not taking care >> in other places. */ >> - if (loc->probe.probe != NULL >> - && loc->probe.probe->pops->clear_semaphore != NULL) >> - loc->probe.probe->pops->clear_semaphore (loc->probe.probe, >> - loc->probe.objfile, >> - loc->gdbarch); >> + if (loc->probe.prob != NULL) >> + loc->probe.prob->clear_semaphore (loc->probe.objfile, >> + loc->gdbarch); >> } >> } >> >> 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/