Re: [RFA 3/3] gdb_breakpoint cleanup: Add support for drpintf, trace, ftrace

[Pedro Alves <palves at redhat dot com>]

On 04/26/2013 09:46 PM, Keith Seitz wrote:

> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 7a96309..f5ee25e 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -343,6 +343,9 @@ proc gdb_start_cmd {args} {
>  # message -- print PASS messages*
>  # no-message -- do not print FAIL messages*
>  # pending -- set a pending breakpoint (FAIL if it is not pending)
> +# trace -- set a tracepoint, e.g., "trace"

Sorry to be picky, but this "e.g." confuses me.  What other examples
could there be?  Maybe you meant "i.e." ?  (Though that doesn't
seem to add anything over what the option name itself tells us?)

> +# ftrace -- set a fast tracepoint, e.g., "ftrace"
> +# dprintf -- set a dynamic printf, e.g., "dprintf"

Ditto.

I wonder if it wouldn't make it a cleaner API if we
split type/cmd out of args/options, and always required it
to be present, like:

-proc gdb_breakpoint { function args } {
+proc gdb_breakpoint { cmd function args } {

And then we'd use it like:

 gdb_breakpoint tbreak "foo" no-message
 gdb_breakpoint trace "foo" pending

The current uses of gdb_breakpoint, which look like:

 gdb_breakpoint "foo"

become:

 gdb_breakpoint break "foo"

Note the "temporary" option would disappear this way, replaced by
"tbreak".

Just brain storming.  Even if that would be agreed to be desirable,
I don't mind the options-based API going in -- it doesn't really make
such a change harder later.

> --- a/gdb/testsuite/gdb.trace/change-loc.exp
> +++ b/gdb/testsuite/gdb.trace/change-loc.exp
> @@ -187,11 +187,7 @@ proc tracepoint_change_loc_2 { trace_type } {
>  	gdb_start
>  	gdb_reinitialize_dir $srcdir/$subdir
>  
> -	gdb_test_multiple "${trace_type} set_tracepoint" "set pending tracepoint" {
> -	    -re ".*Make \(|fast \)tracepoint pending.*y or \\\[n\\\]. $" {
> -		gdb_test "y" "\(Fast t|T\)racepoint.*set_tracepoint.*pending." "set pending tracepoint"
> -	    }
> -	}
> +	gdb_breakpoint "set_tracepoint" pending trace message

It seems we lost $trace_type here.

Thanks,
-- 
Pedro Alves