This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 6/6] New test: gdb.trace/mi-trace-frame-collected.exp
- From: Pedro Alves <palves at redhat dot com>
- To: Yao Qi <yao at codesourcery dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Tue, 25 Jun 2013 18:31:34 +0100
- Subject: Re: [PATCH 6/6] New test: gdb.trace/mi-trace-frame-collected.exp
- References: <1370610493-26468-1-git-send-email-yao at codesourcery dot com> <1371086914-8398-1-git-send-email-yao at codesourcery dot com> <1371086914-8398-7-git-send-email-yao at codesourcery dot com>
On 06/13/2013 02:28 AM, Yao Qi wrote:
> +# Define an action.
> +set pcreg ""
Suggest giving it an empty line after the comment, as the set
is not yet defining the action.
> +if [is_amd64_regs_target] {
> + set pcreg "rip"
> +} elseif [is_x86_like_target] {
> + set pcreg "eip"
> +} else {
> + # Other ports support tracepoing should set the name of pc
> + # register.
# Other ports that support tracepoints should set the name
# of the pc register here.
> + fail "set the number and the name of pc register"
> + return -1
> +}
> +
> +mi_gdb_test "-break-commands 3 \"collect gdb_char_test\" \"collect gdb_union1_test\" \"collect gdb_struct1_test.l\" \"collect gdb_arr_test\[0\]\" \"collect $${pcreg}\" \"teval \$tsv += 1\" \"collect \$tsv\" \"end\" " \
> + {\^done} "set action"
> +
> +mi_gdb_test "-break-insert -a gdb_c_test" \
> + "\\^done,bkpt=\{number=\"${decimal}\",type=\"tracepoint\".*\"\}" \
> + "insert tracepoint on gdb_c_test"
> +# Collect a global variable to make sure no registers are collected
> +# expect PC.
Typo: except. I do that one often too. :-)
Suggest s/make sure/be sure/.
> +# Test MI command '-trace-frame-collected' from different data source.
"from different" threw me off a second (different compared to what?).
Suggest:
# Test the MI command '-trace-frame-collected' with DATA_SOURCE as
# trace data source.
> +
> +proc test_trace_frame_collected { data_source } {
> + global decimal hex
> + global mi_gdb_prompt
> +
This looks OK to me with those changes.
Thanks!
--
Pedro Alves