This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/2] Refactor gdb.trace/save-trace.exp
- From: Pedro Alves <palves at redhat dot com>
- To: Simon Marchi <simon dot marchi at ericsson dot com>, gdb-patches at sourceware dot org
- Date: Wed, 18 Nov 2015 17:38:32 +0000
- Subject: Re: [PATCH 1/2] Refactor gdb.trace/save-trace.exp
- Authentication-results: sourceware.org; auth=none
- References: <1447699465-28821-1-git-send-email-simon dot marchi at ericsson 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