This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA 2/3] gdb_breakpoint cleanup: Add "pending" option
- From: Pedro Alves <palves at redhat dot com>
- To: Keith Seitz <keiths at redhat dot com>
- Cc: "gdb-patches at sourceware dot org ml" <gdb-patches at sourceware dot org>
- Date: Mon, 29 Apr 2013 18:47:45 +0100
- Subject: Re: [RFA 2/3] gdb_breakpoint cleanup: Add "pending" option
- References: <517AE786 dot 1080509 at redhat dot com>
On 04/26/2013 09:45 PM, Keith Seitz wrote:
>
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index cdb7ca2..7a96309 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -342,6 +342,7 @@ proc gdb_start_cmd {args} {
> # temporary -- set a temporary breakpoint, e.g., "tbreak"
> # message -- print PASS messages*
> # no-message -- do not print FAIL messages*
> +# pending -- set a pending breakpoint (FAIL if it is not pending)
> #
> # The result is 1 for success, 0 for failure.
> #
> @@ -358,6 +359,12 @@ proc gdb_breakpoint { function args } {
> set pending_response y
> }
>
> + set require_pending 0
> + if {[lsearch -exact $args "pending"] != -1} {
> + set require_pending 1
> + set pending_response "y"
> + }
> +
> set break_command "break"
> set break_message "Breakpoint"
> if {[lsearch -exact $args temporary] != -1} {
> @@ -376,14 +383,34 @@ proc gdb_breakpoint { function args } {
> set print_pass 1
> }
>
> - set test_name "setting breakpoint at $function"
> + set what [string tolower $break_message]
> + if {$require_pending} {
> + set test_name "setting pending $what at $function"
> + } else {
> + set test_name "setting $what at $function"
> + }
This "$what" handling made me go "hmm, this looks like
an unrelated change". Notice:
set break_command "break"
set break_message "Breakpoint"
if {[lsearch -exact $args temporary] != -1} {
set break_command "tbreak"
set break_message "Temporary breakpoint"
^^^^^^^^^^^^^^^^^^^^
}
and:
> - -re "Make breakpoint pending.*y or \\\[n\\\]. $" {
> + -re "Make $what pending.*y or \\\[n\\\]. $" {
but, w/ current mainline:
$ ./gdb ~/gdb/tests/main
(gdb) tbreak foo
Function "foo" not defined.
Make breakpoint pending on future shared library load? (y or [n]) n
^^^^^^^^^^
Not "Make temporary breakpoint pending".
>
> send_gdb "$break_command $function\n"
> # The first two regexps are what we get with -g, the third is without -g.
> gdb_expect 30 {
> - -re "$break_message \[0-9\]* at .*: file .*, line $decimal.\r\n$gdb_prompt $" {}
> - -re "$break_message \[0-9\]*: file .*, line $decimal.\r\n$gdb_prompt $" {}
> - -re "$break_message \[0-9\]* at .*$gdb_prompt $" {}
> + -re "$break_message \[0-9\]* at .*: file .*, line $decimal.\r\n$gdb_prompt $" {
> + if {$require_pending} {
> + fail "$test_name (expected pending)"
Shouldn't these fails be wrapped in $print_fail checks?
> + return 0
> + }
> + }
> + -re "$break_message \[0-9\]*: file .*, line $decimal.\r\n$gdb_prompt $" {
> + if {$require_pending} {
> + fail "$test_name (expected pending)"
> + return 0
> + }
> + }
> + -re "$break_message \[0-9\]* at .*$gdb_prompt $" {
> + if {$require_pending} {
> + fail "$test_name (expected pending)"
> + return 0
> + }
> + }
> set bp2_loc [gdb_get_line_number "y = x + 4" ${libfile}.c]
> -gdb_test_multiple "break pendshr.c:$bp2_loc if x > 3" "Set pending breakpoint 2" {
> - -re ".*Make breakpoint pending.*y or \\\[n\\\]. $" {
> - gdb_test "y" "Breakpoint.*pendshr.c:$bp2_loc.*pending." \
> - "Set pending breakpoint 2"
> - }
> -}
> +gdb_breakpoint "pendshr.c:$bp2_loc if x > 3" pending message
This made me notice that the gdb_breakpoint API has another
weird aspect WRT test messages. It doesn't allow passing in a custom
fail/pass message text at all. So in this case, we'll now print the
line number ($bp2_loc) as part of the test message. We tend to avoid
it for the fact that line numbers aren't that stable over time.
Just noting it. I don't think this patch should be held for this.
--
Pedro Alves