* [PATCH 1/2] gdb/testsuite: move get_maint_bp_addr to gdb-utils.exp @ 2022-06-19 12:32 Enze Li 2022-06-19 12:33 ` [PATCH 2/2] gdb/testsuite: add a clear test to py-breakpoint.exp Enze Li 2022-06-21 16:12 ` [PATCH 1/2] gdb/testsuite: move get_maint_bp_addr to gdb-utils.exp Andrew Burgess 0 siblings, 2 replies; 7+ messages in thread From: Enze Li @ 2022-06-19 12:32 UTC (permalink / raw) To: gdb-patches; +Cc: pedro, enze.li The get_maint_bp_addr procedure will be shared by other test suite, so move it to gdb-utils.exp. --- gdb/testsuite/gdb.base/clear_non_user_bp.exp | 23 -------------------- gdb/testsuite/lib/gdb-utils.exp | 23 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/gdb/testsuite/gdb.base/clear_non_user_bp.exp b/gdb/testsuite/gdb.base/clear_non_user_bp.exp index 26d7a31fa47..399a3a0f0dc 100644 --- a/gdb/testsuite/gdb.base/clear_non_user_bp.exp +++ b/gdb/testsuite/gdb.base/clear_non_user_bp.exp @@ -16,29 +16,6 @@ # Regression test for PR gdb/7161. Test that GDB cannot delete non-user # breakpoints with clear command. -# get_maint_bp_addr num -# -# Purpose: -# Get address of the specified internal breakpoint when using command -# "maint info breakpoints $num". -# -# Parameter: -# The parameter "num" indicates the number of the internal breakpoint -# to get. Note that this parameter must be a negative number. -# E.g., -1 means that we're gonna get the first internal breakpoint. -# -# Return: -# Internal breakpoint address. -# -proc get_maint_bp_addr { num } { - gdb_test_multiple "maint info break $num" "find address of internal bp $num" { - -re -wrap ".*(0x\[0-9a-f\]+).*" { - return $expect_out(1,string) - } - } - return "" -} - # get_first_maint_bp_num # # Purpose: diff --git a/gdb/testsuite/lib/gdb-utils.exp b/gdb/testsuite/lib/gdb-utils.exp index ffdfb75557c..bf2863722ce 100644 --- a/gdb/testsuite/lib/gdb-utils.exp +++ b/gdb/testsuite/lib/gdb-utils.exp @@ -72,3 +72,26 @@ proc style {str style} { } return "\033\\\[${style}m${str}\033\\\[m" } + +# get_maint_bp_addr num +# +# Purpose: +# Get address of the specified internal breakpoint when using command +# "maint info breakpoints $num". +# +# Parameter: +# The parameter "num" indicates the number of the internal breakpoint +# to get. Note that this parameter must be a negative number. +# E.g., -1 means that we're gonna get the first internal breakpoint. +# +# Return: +# Internal breakpoint address. +# +proc get_maint_bp_addr { num } { + gdb_test_multiple "maint info break $num" "find address of internal bp $num" { + -re -wrap ".*(0x\[0-9a-f\]+).*" { + return $expect_out(1,string) + } + } + return "" +} -- 2.36.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] gdb/testsuite: add a clear test to py-breakpoint.exp 2022-06-19 12:32 [PATCH 1/2] gdb/testsuite: move get_maint_bp_addr to gdb-utils.exp Enze Li @ 2022-06-19 12:33 ` Enze Li 2022-06-21 16:21 ` Andrew Burgess 2022-06-21 16:12 ` [PATCH 1/2] gdb/testsuite: move get_maint_bp_addr to gdb-utils.exp Andrew Burgess 1 sibling, 1 reply; 7+ messages in thread From: Enze Li @ 2022-06-19 12:33 UTC (permalink / raw) To: gdb-patches; +Cc: pedro, enze.li This patch adds a test case to try to clear an internal python breakpoint using the clear command. This was suggested by Pedro during a code review of the following commit. commit a5c69b1e49bae4d0dcb20f324cebb310c63495c6 Date: Sun Apr 17 15:09:46 2022 +0800 gdb: fix using clear command to delete non-user breakpoints(PR cli/7161) Tested on x86_64 openSUSE Tumbleweed. --- gdb/testsuite/gdb.python/py-breakpoint.exp | 41 ++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp index 58b1af3a0da..984c20f9dc2 100644 --- a/gdb/testsuite/gdb.python/py-breakpoint.exp +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp @@ -296,6 +296,27 @@ proc_with_prefix test_watchpoints { } { "Test watchpoint write" } +# get_maint_bp_num bp_name +# +# Purpose: +# Get the number of a specified internal breakpoint. +# +# Parameter: +# The parameter "bp_name" indicates the name of the internal breakpoint +# to get. +# +# Return: +# Internal breakpoint number. Empty string if fails. +# +proc get_maint_bp_num { bp_name } { + gdb_test_multiple "maint info break" "find $bp_name internal bp num" { + -re -wrap "0x.*(-\[0-9\]).*$bp_name.*" { + return $expect_out(1,string) + } + } + return "" +} + proc_with_prefix test_bkpt_internal { } { global srcfile testfile hex decimal @@ -309,6 +330,26 @@ proc_with_prefix test_bkpt_internal { } { if ![runto_main] then { return 0 } + delete_breakpoints + gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint (\"main\", type=gdb.BP_BREAKPOINT, wp_class=gdb.WP_WRITE, internal=True )" \ + "Set internal breakpoint" 0 + + set bp_num [get_maint_bp_num "main"] + set bp_addr [get_maint_bp_addr $bp_num] + + gdb_test "maint info break $bp_num" \ + "$bp_num.*$bp_addr.*" \ + "maint info breakpoint \$bp_num" + + gdb_test "python gdb.execute(\'clear *$bp_addr\')" \ + ".*No breakpoint at \\*$bp_addr.*" \ + "clear internal breakpoint" + + # Check again, make sure that GDB really didn't delete the internal breakpoint. + gdb_test "maint info break $bp_num" \ + "$bp_num.*$bp_addr.*" \ + "maint info breakpoint \$bp_num after clear" + delete_breakpoints gdb_py_test_silent_cmd "python wp1 = gdb.Breakpoint (\"result\", type=gdb.BP_WATCHPOINT, wp_class=gdb.WP_WRITE, internal=True )" \ "Set watchpoint" 0 -- 2.36.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] gdb/testsuite: add a clear test to py-breakpoint.exp 2022-06-19 12:33 ` [PATCH 2/2] gdb/testsuite: add a clear test to py-breakpoint.exp Enze Li @ 2022-06-21 16:21 ` Andrew Burgess 2022-06-23 13:29 ` Enze Li 0 siblings, 1 reply; 7+ messages in thread From: Andrew Burgess @ 2022-06-21 16:21 UTC (permalink / raw) To: Enze Li, gdb-patches; +Cc: pedro, enze.li Enze Li via Gdb-patches <gdb-patches@sourceware.org> writes: > This patch adds a test case to try to clear an internal python > breakpoint using the clear command. > > This was suggested by Pedro during a code review of the following > commit. > > commit a5c69b1e49bae4d0dcb20f324cebb310c63495c6 > Date: Sun Apr 17 15:09:46 2022 +0800 > > gdb: fix using clear command to delete non-user breakpoints(PR cli/7161) > > Tested on x86_64 openSUSE Tumbleweed. > --- > gdb/testsuite/gdb.python/py-breakpoint.exp | 41 ++++++++++++++++++++++ > 1 file changed, 41 insertions(+) > > diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp > index 58b1af3a0da..984c20f9dc2 100644 > --- a/gdb/testsuite/gdb.python/py-breakpoint.exp > +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp > @@ -296,6 +296,27 @@ proc_with_prefix test_watchpoints { } { > "Test watchpoint write" > } > > +# get_maint_bp_num bp_name > +# > +# Purpose: > +# Get the number of a specified internal breakpoint. > +# > +# Parameter: > +# The parameter "bp_name" indicates the name of the internal breakpoint > +# to get. > +# > +# Return: > +# Internal breakpoint number. Empty string if fails. > +# > +proc get_maint_bp_num { bp_name } { > + gdb_test_multiple "maint info break" "find $bp_name internal bp num" { > + -re -wrap "0x.*(-\[0-9\]).*$bp_name.*" { > + return $expect_out(1,string) > + } > + } > + return "" > +} > + > proc_with_prefix test_bkpt_internal { } { > global srcfile testfile hex decimal > > @@ -309,6 +330,26 @@ proc_with_prefix test_bkpt_internal { } { > if ![runto_main] then { > return 0 > } > + delete_breakpoints > + gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint (\"main\", type=gdb.BP_BREAKPOINT, wp_class=gdb.WP_WRITE, internal=True )" \ > + "Set internal breakpoint" 0 > + > + set bp_num [get_maint_bp_num "main"] Instead of adding get_maint_bp_num, would this work? set bp_num [get_python_valueof bp1.number "*DEFAULT*"] > + set bp_addr [get_maint_bp_addr $bp_num] > + > + gdb_test "maint info break $bp_num" \ > + "$bp_num.*$bp_addr.*" \ > + "maint info breakpoint \$bp_num" > + > + gdb_test "python gdb.execute(\'clear *$bp_addr\')" \ > + ".*No breakpoint at \\*$bp_addr.*" \ > + "clear internal breakpoint" You might also want to try calling clear without the gdb.execute, just in case the Python layer makes a difference. Thanks, Andrew > + > + # Check again, make sure that GDB really didn't delete the internal breakpoint. > + gdb_test "maint info break $bp_num" \ > + "$bp_num.*$bp_addr.*" \ > + "maint info breakpoint \$bp_num after clear" > + > delete_breakpoints > gdb_py_test_silent_cmd "python wp1 = gdb.Breakpoint (\"result\", type=gdb.BP_WATCHPOINT, wp_class=gdb.WP_WRITE, internal=True )" \ > "Set watchpoint" 0 > -- > 2.36.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] gdb/testsuite: add a clear test to py-breakpoint.exp 2022-06-21 16:21 ` Andrew Burgess @ 2022-06-23 13:29 ` Enze Li 2022-06-24 12:00 ` Andrew Burgess 0 siblings, 1 reply; 7+ messages in thread From: Enze Li @ 2022-06-23 13:29 UTC (permalink / raw) To: Andrew Burgess, gdb-patches; +Cc: pedro, enze.li Hi Andrew, Thanks for your review. Please see below. On Tue, 2022-06-21 at 17:21 +0100, Andrew Burgess wrote: > Enze Li via Gdb-patches <gdb-patches@sourceware.org> writes: > > > This patch adds a test case to try to clear an internal python > > breakpoint using the clear command. > > > > This was suggested by Pedro during a code review of the following > > commit. > > > > commit a5c69b1e49bae4d0dcb20f324cebb310c63495c6 > > Date: Sun Apr 17 15:09:46 2022 +0800 > > > > gdb: fix using clear command to delete non-user breakpoints(PR > > cli/7161) > > > > Tested on x86_64 openSUSE Tumbleweed. > > --- > > gdb/testsuite/gdb.python/py-breakpoint.exp | 41 > > ++++++++++++++++++++++ > > 1 file changed, 41 insertions(+) > > > > diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp > > b/gdb/testsuite/gdb.python/py-breakpoint.exp > > index 58b1af3a0da..984c20f9dc2 100644 > > --- a/gdb/testsuite/gdb.python/py-breakpoint.exp > > +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp > > @@ -296,6 +296,27 @@ proc_with_prefix test_watchpoints { } { > > "Test watchpoint write" > > } > > > > +# get_maint_bp_num bp_name > > +# > > +# Purpose: > > +# Get the number of a specified internal breakpoint. > > +# > > +# Parameter: > > +# The parameter "bp_name" indicates the name of the internal > > breakpoint > > +# to get. > > +# > > +# Return: > > +# Internal breakpoint number. Empty string if fails. > > +# > > +proc get_maint_bp_num { bp_name } { > > + gdb_test_multiple "maint info break" "find $bp_name internal > > bp num" { > > + -re -wrap "0x.*(-\[0-9\]).*$bp_name.*" { > > + return $expect_out(1,string) > > + } > > + } > > + return "" > > +} > > + > > proc_with_prefix test_bkpt_internal { } { > > global srcfile testfile hex decimal > > > > @@ -309,6 +330,26 @@ proc_with_prefix test_bkpt_internal { } { > > if ![runto_main] then { > > return 0 > > } > > + delete_breakpoints > > + gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint > > (\"main\", type=gdb.BP_BREAKPOINT, wp_class=gdb.WP_WRITE, > > internal=True )" \ > > + "Set internal breakpoint" 0 > > + > > + set bp_num [get_maint_bp_num "main"] > > Instead of adding get_maint_bp_num, would this work? > > set bp_num [get_python_valueof bp1.number "*DEFAULT*"] It works! There's no need to add a new procedure, and I'll drop it. > > > + set bp_addr [get_maint_bp_addr $bp_num] > > + > > + gdb_test "maint info break $bp_num" \ > > + "$bp_num.*$bp_addr.*" \ > > + "maint info breakpoint \$bp_num" > > + > > + gdb_test "python gdb.execute(\'clear *$bp_addr\')" \ > > + ".*No breakpoint at \\*$bp_addr.*" \ > > + "clear internal breakpoint" > > You might also want to try calling clear without the gdb.execute, > just > in case the Python layer makes a difference. > > Thanks, > Andrew In fact, there already exists a test case in the trunk which calling clear command without python extention. (See clear_non_user_bp.exp, line 86). However, if it is still needed here, pls let me know. Best Regards, Enze > > > + > > + # Check again, make sure that GDB really didn't delete the > > internal breakpoint. > > + gdb_test "maint info break $bp_num" \ > > + "$bp_num.*$bp_addr.*" \ > > + "maint info breakpoint \$bp_num after clear" > > + > > delete_breakpoints > > gdb_py_test_silent_cmd "python wp1 = gdb.Breakpoint > > (\"result\", type=gdb.BP_WATCHPOINT, wp_class=gdb.WP_WRITE, > > internal=True )" \ > > "Set watchpoint" 0 > > -- > > 2.36.1 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] gdb/testsuite: add a clear test to py-breakpoint.exp 2022-06-23 13:29 ` Enze Li @ 2022-06-24 12:00 ` Andrew Burgess 0 siblings, 0 replies; 7+ messages in thread From: Andrew Burgess @ 2022-06-24 12:00 UTC (permalink / raw) To: Enze Li, gdb-patches; +Cc: pedro, enze.li Enze Li via Gdb-patches <gdb-patches@sourceware.org> writes: > Hi Andrew, > > Thanks for your review. Please see below. > > On Tue, 2022-06-21 at 17:21 +0100, Andrew Burgess wrote: >> Enze Li via Gdb-patches <gdb-patches@sourceware.org> writes: >> >> > This patch adds a test case to try to clear an internal python >> > breakpoint using the clear command. >> > >> > This was suggested by Pedro during a code review of the following >> > commit. >> > >> > commit a5c69b1e49bae4d0dcb20f324cebb310c63495c6 >> > Date: Sun Apr 17 15:09:46 2022 +0800 >> > >> > gdb: fix using clear command to delete non-user breakpoints(PR >> > cli/7161) >> > >> > Tested on x86_64 openSUSE Tumbleweed. >> > --- >> > gdb/testsuite/gdb.python/py-breakpoint.exp | 41 >> > ++++++++++++++++++++++ >> > 1 file changed, 41 insertions(+) >> > >> > diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp >> > b/gdb/testsuite/gdb.python/py-breakpoint.exp >> > index 58b1af3a0da..984c20f9dc2 100644 >> > --- a/gdb/testsuite/gdb.python/py-breakpoint.exp >> > +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp >> > @@ -296,6 +296,27 @@ proc_with_prefix test_watchpoints { } { >> > "Test watchpoint write" >> > } >> > >> > +# get_maint_bp_num bp_name >> > +# >> > +# Purpose: >> > +# Get the number of a specified internal breakpoint. >> > +# >> > +# Parameter: >> > +# The parameter "bp_name" indicates the name of the internal >> > breakpoint >> > +# to get. >> > +# >> > +# Return: >> > +# Internal breakpoint number. Empty string if fails. >> > +# >> > +proc get_maint_bp_num { bp_name } { >> > + gdb_test_multiple "maint info break" "find $bp_name internal >> > bp num" { >> > + -re -wrap "0x.*(-\[0-9\]).*$bp_name.*" { >> > + return $expect_out(1,string) >> > + } >> > + } >> > + return "" >> > +} >> > + >> > proc_with_prefix test_bkpt_internal { } { >> > global srcfile testfile hex decimal >> > >> > @@ -309,6 +330,26 @@ proc_with_prefix test_bkpt_internal { } { >> > if ![runto_main] then { >> > return 0 >> > } >> > + delete_breakpoints >> > + gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint >> > (\"main\", type=gdb.BP_BREAKPOINT, wp_class=gdb.WP_WRITE, >> > internal=True )" \ >> > + "Set internal breakpoint" 0 >> > + >> > + set bp_num [get_maint_bp_num "main"] >> >> Instead of adding get_maint_bp_num, would this work? >> >> set bp_num [get_python_valueof bp1.number "*DEFAULT*"] > > It works! There's no need to add a new procedure, and I'll drop it. Excellent! > >> >> > + set bp_addr [get_maint_bp_addr $bp_num] >> > + >> > + gdb_test "maint info break $bp_num" \ >> > + "$bp_num.*$bp_addr.*" \ >> > + "maint info breakpoint \$bp_num" >> > + >> > + gdb_test "python gdb.execute(\'clear *$bp_addr\')" \ >> > + ".*No breakpoint at \\*$bp_addr.*" \ >> > + "clear internal breakpoint" >> >> You might also want to try calling clear without the gdb.execute, >> just >> in case the Python layer makes a difference. >> >> Thanks, >> Andrew > > In fact, there already exists a test case in the trunk which calling > clear command without python extention. (See clear_non_user_bp.exp, > line 86). However, if it is still needed here, pls let me know. I'll leave it up to you then. I think if you did add it, the test would be slightly different than anything else. But probably not in any significant way, so I don't really mind. Thanks, Andrew > > Best Regards, > Enze > >> >> > + >> > + # Check again, make sure that GDB really didn't delete the >> > internal breakpoint. >> > + gdb_test "maint info break $bp_num" \ >> > + "$bp_num.*$bp_addr.*" \ >> > + "maint info breakpoint \$bp_num after clear" >> > + >> > delete_breakpoints >> > gdb_py_test_silent_cmd "python wp1 = gdb.Breakpoint >> > (\"result\", type=gdb.BP_WATCHPOINT, wp_class=gdb.WP_WRITE, >> > internal=True )" \ >> > "Set watchpoint" 0 >> > -- >> > 2.36.1 >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] gdb/testsuite: move get_maint_bp_addr to gdb-utils.exp 2022-06-19 12:32 [PATCH 1/2] gdb/testsuite: move get_maint_bp_addr to gdb-utils.exp Enze Li 2022-06-19 12:33 ` [PATCH 2/2] gdb/testsuite: add a clear test to py-breakpoint.exp Enze Li @ 2022-06-21 16:12 ` Andrew Burgess 2022-06-23 13:12 ` Enze Li 1 sibling, 1 reply; 7+ messages in thread From: Andrew Burgess @ 2022-06-21 16:12 UTC (permalink / raw) To: Enze Li, gdb-patches; +Cc: pedro, enze.li Enze Li via Gdb-patches <gdb-patches@sourceware.org> writes: > The get_maint_bp_addr procedure will be shared by other test suite, so > move it to gdb-utils.exp. > --- > gdb/testsuite/gdb.base/clear_non_user_bp.exp | 23 -------------------- > gdb/testsuite/lib/gdb-utils.exp | 23 ++++++++++++++++++++ > 2 files changed, 23 insertions(+), 23 deletions(-) > > diff --git a/gdb/testsuite/gdb.base/clear_non_user_bp.exp b/gdb/testsuite/gdb.base/clear_non_user_bp.exp > index 26d7a31fa47..399a3a0f0dc 100644 > --- a/gdb/testsuite/gdb.base/clear_non_user_bp.exp > +++ b/gdb/testsuite/gdb.base/clear_non_user_bp.exp > @@ -16,29 +16,6 @@ > # Regression test for PR gdb/7161. Test that GDB cannot delete non-user > # breakpoints with clear command. > > -# get_maint_bp_addr num > -# > -# Purpose: > -# Get address of the specified internal breakpoint when using command > -# "maint info breakpoints $num". > -# > -# Parameter: > -# The parameter "num" indicates the number of the internal breakpoint > -# to get. Note that this parameter must be a negative number. > -# E.g., -1 means that we're gonna get the first internal breakpoint. > -# > -# Return: > -# Internal breakpoint address. > -# > -proc get_maint_bp_addr { num } { > - gdb_test_multiple "maint info break $num" "find address of internal bp $num" { > - -re -wrap ".*(0x\[0-9a-f\]+).*" { > - return $expect_out(1,string) > - } > - } > - return "" > -} > - > # get_first_maint_bp_num > # > # Purpose: > diff --git a/gdb/testsuite/lib/gdb-utils.exp b/gdb/testsuite/lib/gdb-utils.exp > index ffdfb75557c..bf2863722ce 100644 > --- a/gdb/testsuite/lib/gdb-utils.exp > +++ b/gdb/testsuite/lib/gdb-utils.exp > @@ -72,3 +72,26 @@ proc style {str style} { > } > return "\033\\\[${style}m${str}\033\\\[m" > } > + > +# get_maint_bp_addr num > +# > +# Purpose: > +# Get address of the specified internal breakpoint when using command > +# "maint info breakpoints $num". > +# > +# Parameter: > +# The parameter "num" indicates the number of the internal breakpoint > +# to get. Note that this parameter must be a negative number. > +# E.g., -1 means that we're gonna get the first internal breakpoint. > +# > +# Return: > +# Internal breakpoint address. > +# > +proc get_maint_bp_addr { num } { > + gdb_test_multiple "maint info break $num" "find address of internal bp $num" { > + -re -wrap ".*(0x\[0-9a-f\]+).*" { > + return $expect_out(1,string) > + } > + } > + return "" > +} Before the get_maint_bp_addr name gets spread around too much, I think we should rename this. I'd propose maybe gdb_get_bp_addr. The comment should be reworked to remove the mention of negative values, as I believe this function will work just as well with positive values. I think the comment should also be expanded to indicate that (currently) this function will only take integer values for NUM, and will return the first address for a particular breakpoint, e.g. we can't say 'get_maint_bp_addr 1.2' to get the address of the second location of breakpoint #1. I don't think that we need to make the actual implementation of get_maint_bp_addr better right now, but if we give the function a good name, and explain the limitations in the comments, then, if we ever need to, we can improve the function later. Thanks, Andrew > -- > 2.36.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] gdb/testsuite: move get_maint_bp_addr to gdb-utils.exp 2022-06-21 16:12 ` [PATCH 1/2] gdb/testsuite: move get_maint_bp_addr to gdb-utils.exp Andrew Burgess @ 2022-06-23 13:12 ` Enze Li 0 siblings, 0 replies; 7+ messages in thread From: Enze Li @ 2022-06-23 13:12 UTC (permalink / raw) To: Andrew Burgess, gdb-patches; +Cc: pedro, enze.li Hi Andrew, Thanks for your review. Naming is a hard thing for me. gdb_get_bp_addr seems to have more features than before, and starting with 'gdb' looks good. 🙂 V2 of this patch is on the way. Best Regards, Enze On Tue, 2022-06-21 at 17:12 +0100, Andrew Burgess wrote: > Enze Li via Gdb-patches <gdb-patches@sourceware.org> writes: > > > The get_maint_bp_addr procedure will be shared by other test suite, > > so > > move it to gdb-utils.exp. > > --- > > gdb/testsuite/gdb.base/clear_non_user_bp.exp | 23 ---------------- > > ---- > > gdb/testsuite/lib/gdb-utils.exp | 23 > > ++++++++++++++++++++ > > 2 files changed, 23 insertions(+), 23 deletions(-) > > > > diff --git a/gdb/testsuite/gdb.base/clear_non_user_bp.exp > > b/gdb/testsuite/gdb.base/clear_non_user_bp.exp > > index 26d7a31fa47..399a3a0f0dc 100644 > > --- a/gdb/testsuite/gdb.base/clear_non_user_bp.exp > > +++ b/gdb/testsuite/gdb.base/clear_non_user_bp.exp > > @@ -16,29 +16,6 @@ > > # Regression test for PR gdb/7161. Test that GDB cannot delete > > non-user > > # breakpoints with clear command. > > > > -# get_maint_bp_addr num > > -# > > -# Purpose: > > -# Get address of the specified internal breakpoint when using > > command > > -# "maint info breakpoints $num". > > -# > > -# Parameter: > > -# The parameter "num" indicates the number of the internal > > breakpoint > > -# to get. Note that this parameter must be a negative number. > > -# E.g., -1 means that we're gonna get the first internal > > breakpoint. > > -# > > -# Return: > > -# Internal breakpoint address. > > -# > > -proc get_maint_bp_addr { num } { > > - gdb_test_multiple "maint info break $num" "find address of > > internal bp $num" { > > - -re -wrap ".*(0x\[0-9a-f\]+).*" { > > - return $expect_out(1,string) > > - } > > - } > > - return "" > > -} > > - > > # get_first_maint_bp_num > > # > > # Purpose: > > diff --git a/gdb/testsuite/lib/gdb-utils.exp > > b/gdb/testsuite/lib/gdb-utils.exp > > index ffdfb75557c..bf2863722ce 100644 > > --- a/gdb/testsuite/lib/gdb-utils.exp > > +++ b/gdb/testsuite/lib/gdb-utils.exp > > @@ -72,3 +72,26 @@ proc style {str style} { > > } > > return "\033\\\[${style}m${str}\033\\\[m" > > } > > + > > +# get_maint_bp_addr num > > +# > > +# Purpose: > > +# Get address of the specified internal breakpoint when using > > command > > +# "maint info breakpoints $num". > > +# > > +# Parameter: > > +# The parameter "num" indicates the number of the internal > > breakpoint > > +# to get. Note that this parameter must be a negative number. > > +# E.g., -1 means that we're gonna get the first internal > > breakpoint. > > +# > > +# Return: > > +# Internal breakpoint address. > > +# > > +proc get_maint_bp_addr { num } { > > + gdb_test_multiple "maint info break $num" "find address of > > internal bp $num" { > > + -re -wrap ".*(0x\[0-9a-f\]+).*" { > > + return $expect_out(1,string) > > + } > > + } > > + return "" > > +} > > Before the get_maint_bp_addr name gets spread around too much, I > think > we should rename this. I'd propose maybe gdb_get_bp_addr. The > comment > should be reworked to remove the mention of negative values, as I > believe this function will work just as well with positive values. > > I think the comment should also be expanded to indicate that > (currently) > this function will only take integer values for NUM, and will return > the > first address for a particular breakpoint, e.g. we can't say > 'get_maint_bp_addr 1.2' to get the address of the second location of > breakpoint #1. > > I don't think that we need to make the actual implementation of > get_maint_bp_addr better right now, but if we give the function a > good > name, and explain the limitations in the comments, then, if we ever > need > to, we can improve the function later. > > Thanks, > Andrew > > > -- > > 2.36.1 > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-06-24 12:01 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-06-19 12:32 [PATCH 1/2] gdb/testsuite: move get_maint_bp_addr to gdb-utils.exp Enze Li 2022-06-19 12:33 ` [PATCH 2/2] gdb/testsuite: add a clear test to py-breakpoint.exp Enze Li 2022-06-21 16:21 ` Andrew Burgess 2022-06-23 13:29 ` Enze Li 2022-06-24 12:00 ` Andrew Burgess 2022-06-21 16:12 ` [PATCH 1/2] gdb/testsuite: move get_maint_bp_addr to gdb-utils.exp Andrew Burgess 2022-06-23 13:12 ` Enze Li
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).