[PATCH] gdb: Remove a non-const reference parameter
Andrew Burgess
andrew.burgess@embecosm.com
Wed Jul 17 15:32:00 GMT 2019
* Andrew Burgess <andrew.burgess@embecosm.com> [2019-07-02 16:21:19 +0100]:
> Non-const reference parameter should be avoided according to the GDB
> coding standard:
>
> https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Avoid_non-const_reference_parameters.2C_use_pointers_instead
>
> This commit updates the gdbarch method gdbarch_stap_adjust_register,
> and the one implementation i386_stap_adjust_register to avoid using a
> non-const reference parameter.
>
> I've also removed the kfail from the testsuite for bug 24541, as this
> issue is now resolved.
>
> gdb/ChangeLog:
>
> PR breakpoints/24541
> * gdbarch.c: Regenerate.
> * gdbarch.h: Regenerate.
> * gdbarch.sh: Adjust return type and parameter types for
> 'stap_adjust_register'.
> (i386_stap_adjust_register): Adjust signature and return new
> register name.
> * stap-probe.c (stap_parse_register_operand): Adjust use of
> 'gdbarch_stap_adjust_register'.
>
> gdb/testsuite/ChangeLog:
>
> PR breakpoints/24541
> * gdb.mi/mi-catch-cpp-exceptions.exp: Remove kfail due to
> 24541.
I went ahead and pushed this patch, after making the improvement Tom
suggested.
There didn't seem to be a rush of voices suggesting the non-const rule
be dropped. We can always revert this if the rule is later removed.
Thanks,
Andrew
> ---
> gdb/ChangeLog | 12 ++++++++++++
> gdb/gdbarch.c | 6 +++---
> gdb/gdbarch.h | 4 ++--
> gdb/gdbarch.sh | 2 +-
> gdb/i386-tdep.c | 12 ++++++++----
> gdb/stap-probe.c | 15 ++++++++-------
> gdb/testsuite/ChangeLog | 5 +++++
> gdb/testsuite/gdb.mi/mi-catch-cpp-exceptions.exp | 22 +---------------------
> 8 files changed, 40 insertions(+), 38 deletions(-)
>
> diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
> index cc7d0ace66c..725b98fcd9f 100644
> --- a/gdb/gdbarch.c
> +++ b/gdb/gdbarch.c
> @@ -4530,14 +4530,14 @@ gdbarch_stap_adjust_register_p (struct gdbarch *gdbarch)
> return gdbarch->stap_adjust_register != NULL;
> }
>
> -void
> -gdbarch_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p, std::string ®name, int regnum)
> +std::string
> +gdbarch_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p, const std::string ®name, int regnum)
> {
> gdb_assert (gdbarch != NULL);
> gdb_assert (gdbarch->stap_adjust_register != NULL);
> if (gdbarch_debug >= 2)
> fprintf_unfiltered (gdb_stdlog, "gdbarch_stap_adjust_register called\n");
> - gdbarch->stap_adjust_register (gdbarch, p, regname, regnum);
> + return gdbarch->stap_adjust_register (gdbarch, p, regname, regnum);
> }
>
> void
> diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
> index 0857d2f3027..c3556d38419 100644
> --- a/gdb/gdbarch.h
> +++ b/gdb/gdbarch.h
> @@ -1376,8 +1376,8 @@ extern void set_gdbarch_stap_parse_special_token (struct gdbarch *gdbarch, gdbar
>
> extern int gdbarch_stap_adjust_register_p (struct gdbarch *gdbarch);
>
> -typedef void (gdbarch_stap_adjust_register_ftype) (struct gdbarch *gdbarch, struct stap_parse_info *p, std::string ®name, int regnum);
> -extern void gdbarch_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p, std::string ®name, int regnum);
> +typedef std::string (gdbarch_stap_adjust_register_ftype) (struct gdbarch *gdbarch, struct stap_parse_info *p, const std::string ®name, int regnum);
> +extern std::string gdbarch_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p, const std::string ®name, int regnum);
> extern void set_gdbarch_stap_adjust_register (struct gdbarch *gdbarch, gdbarch_stap_adjust_register_ftype *stap_adjust_register);
>
> /* DTrace related functions.
> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
> index f3d1bf489ac..2f9fbbc56cd 100755
> --- a/gdb/gdbarch.sh
> +++ b/gdb/gdbarch.sh
> @@ -1053,7 +1053,7 @@ M;int;stap_parse_special_token;struct stap_parse_info *p;p
> # replace the register name from %ax to %eax.
> #
> # The rationale for this can be found at PR breakpoints/24541.
> -M;void;stap_adjust_register;struct stap_parse_info *p, std::string \®name, int regnum;p, regname, regnum
> +M;std::string;stap_adjust_register;struct stap_parse_info *p, const std::string \®name, int regnum;p, regname, regnum
>
> # DTrace related functions.
>
> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
> index 00c1f8d7499..b956604178f 100644
> --- a/gdb/i386-tdep.c
> +++ b/gdb/i386-tdep.c
> @@ -4389,9 +4389,9 @@ i386_stap_parse_special_token (struct gdbarch *gdbarch,
> /* Implementation of 'gdbarch_stap_adjust_register', as defined in
> gdbarch.h. */
>
> -static void
> +static std::string
> i386_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p,
> - std::string ®name, int regnum)
> + const std::string ®name, int regnum)
> {
> static const std::unordered_set<std::string> reg_assoc
> = { "ax", "bx", "cx", "dx",
> @@ -4402,14 +4402,18 @@ i386_stap_adjust_register (struct gdbarch *gdbarch, struct stap_parse_info *p,
> /* If we're dealing with a register whose size is greater or
> equal than the size specified by the "[-]N@" prefix, then we
> don't need to do anything. */
> - return;
> + return regname;
> }
>
> if (reg_assoc.find (regname) != reg_assoc.end ())
> {
> /* Use the extended version of the register. */
> - regname = "e" + regname;
> + std::string tmp = "e" + regname;
> + return tmp;
> }
> +
> + /* Use the requested register. */
> + return regname;
> }
>
>
> diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
> index aa1c8144d8a..747bbcfb8c1 100644
> --- a/gdb/stap-probe.c
> +++ b/gdb/stap-probe.c
> @@ -774,23 +774,24 @@ stap_parse_register_operand (struct stap_parse_info *p)
> code would like to perform on the register name. */
> if (gdbarch_stap_adjust_register_p (gdbarch))
> {
> - std::string oldregname = regname;
> + std::string newregname
> + = gdbarch_stap_adjust_register (gdbarch, p, regname, regnum);
>
> - gdbarch_stap_adjust_register (gdbarch, p, regname, regnum);
> -
> - if (regname != oldregname)
> + if (regname != newregname)
> {
> /* This is just a check we perform to make sure that the
> arch-dependent code has provided us with a valid
> register name. */
> - regnum = user_reg_map_name_to_regnum (gdbarch, regname.c_str (),
> - regname.size ());
> + regnum = user_reg_map_name_to_regnum (gdbarch, newregname.c_str (),
> + newregname.size ());
>
> if (regnum == -1)
> internal_error (__FILE__, __LINE__,
> _("Invalid register name '%s' after replacing it"
> " (previous name was '%s')"),
> - regname.c_str (), oldregname.c_str ());
> + newregname.c_str (), regname.c_str ());
> +
> + regname = newregname;
> }
> }
>
> diff --git a/gdb/testsuite/gdb.mi/mi-catch-cpp-exceptions.exp b/gdb/testsuite/gdb.mi/mi-catch-cpp-exceptions.exp
> index fa5b11e3e58..4b87e4b62ba 100644
> --- a/gdb/testsuite/gdb.mi/mi-catch-cpp-exceptions.exp
> +++ b/gdb/testsuite/gdb.mi/mi-catch-cpp-exceptions.exp
> @@ -130,27 +130,7 @@ with_test_prefix "all with invalid regexp" {
> setup_catchpoint "throw" "-r blahblah"
> setup_catchpoint "rethrow" "-r woofwoof"
> setup_catchpoint "catch" "-r miowmiow"
> -
> - # Would like to use 'continue_to_breakpoint_in_main' here, if
> - # there wasn't a bug that requires a use of kfail.
> -
> - mi_send_resuming_command "exec-continue" \
> - "exec-continue"
> - set testname "run until breakpoint in main"
> - gdb_expect {
> - -re "could not find minimal symbol for typeinfo address.*$mi_gdb_prompt$" {
> - kfail "gdb/24541" "${testname}"
> - }
> - -re "\\*stopped,bkptno=\"$decimal\",reason=\"breakpoint-hit\",disp=\"keep\".*func=\"__cxa_throw\".*$mi_gdb_prompt$" {
> - kfail "gdb/24541" "${testname}"
> - }
> - -re "\\*stopped,reason=\"breakpoint-hit\".*func=\"main\".*line=\"${main_lineno}\".*$mi_gdb_prompt$" {
> - pass "${testname}"
> - }
> - timeout {
> - fail "${testname} (timeout)"
> - }
> - }
> + continue_to_breakpoint_in_main
> }
>
> # Now check that all of the commands with a regexp that does match,
> --
> 2.14.5
>
More information about the Gdb-patches
mailing list