Re: [RFA 2/3] gdb_breakpoint cleanup: Add "pending" option

[Pedro Alves <palves 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