Re: [PATCH 1/2] Refactor gdb.trace/save-trace.exp

[Pedro Alves <palves at redhat dot com>]

On 11/16/2015 06:44 PM, Simon Marchi wrote:
> Some code is duplicated, to run the test twice with absolute and
> relative paths, so I factored it out in a few procs.  It uses
> with_test_prefix to differentiate between test runs.

You also made each run generate a separate .tr file, which is good too.

> 
> I replaced usages of "save-tracepoints" with "save tracepoint", since
> the former is deprecated.
> 
> I also removed the "10.x", as it doesn't make much sense anymore.  It
> isn't used in general in the testsuite, and I don't think it's really
> useful.

> 
> gdb/testsuite/ChangeLog:
> 
> 	* save-trace.exp: Factor out code to these...
> 	(gdb_save_tracepoints): New.
> 	(gdb_load_tracepoints): New.
> 	(do_save_load_test): New.

Okay with nits below addressed.

> +proc gdb_save_tracepoints { save_path } {

Could you add an intro comment, mentioning what the parameter is for?

> +    set save_path_regexp [string_to_regexp $save_path]
> +    remote_file host delete $save_path
> +    gdb_test "save tracepoints $save_path" "Saved to file '$save_path_regexp'." "save tracepoint definitions"

Could you break overly long lines (throughout) with \, like done elsewhere in the file?

> +}
> +
> +proc gdb_load_tracepoints { save_path } {

Ditto.

Thanks,
Pedro Alves