Re: [PATCH v2] gdb.base/watchpoint-unaligned.exp: Always initialize wpoffset_to_wpnum

[Andrew Burgess <aburgess@redhat.com>]

Alexandra Hájková via Gdb-patches <gdb-patches@sourceware.org> writes:

> to avoid TCL error which happens in some aarch64 types.

Commit messages should ideally be written as proper sentences, so start
with a capital letter.

>
> ERROR: in testcase /root/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/watchpoint-unaligned.exp
> ERROR:  can't read "wpoffset_to_wpnum(1)": no such element in array
> ERROR:  tcl error code TCL READ VARNAME
> ERROR:  tcl error info:
> can't read "wpoffset_to_wpnum(1)": no such element in array
>     while executing
>
> Fixes bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30340

Looking through the commit log there's only one other instances of
'Fixes bug:', the GDB style is usually just 'Bug:' -- being consistent
makes it much easier to search for these labels.

Also, at one point we had to reference the traditional bug tag in order
to get this commit automatically linked with the bugzilla report - so we
needed to include something like 'PR gdb/30340'.  I don't know if this
is still needed or not -- I've never bothered to find out as I just
include both forms.

> ---
> The test uses gdb_test_multiple, and there are plenty of other reasons (see gdb_test_multiple in lib/gdb.exp) that a test using gdb_test_multiple might fail.  We're never going to recreate each of those in the gdb.base/watchpoint-unaligned.exp script just so that we can ensure the variable is initialized.  Instead we should just ensure the variable is always initialized.

It's not clear if this is intended to be part of the commit message or
not.  It seems like there's helpful text in here, but it's not wrapped
to a reasonable width.  That said, for such a small change, maybe we
don't need to include any additional text, folk can just figure out
what's going on from the code change.

>  
>  gdb/testsuite/gdb.base/watchpoint-unaligned.exp | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/testsuite/gdb.base/watchpoint-unaligned.exp b/gdb/testsuite/gdb.base/watchpoint-unaligned.exp
> index ce5a1e5bf66..d31a9cdc2c8 100644
> --- a/gdb/testsuite/gdb.base/watchpoint-unaligned.exp
> +++ b/gdb/testsuite/gdb.base/watchpoint-unaligned.exp
> @@ -103,6 +103,8 @@ foreach wpcount {4 7} {
>      for {set wpoffset 1} {$wpoffset <= $wpcount} {incr wpoffset} {
>  	set test "$rwatch data.u.size1\[$wpoffset\]"
>  	set wpnum ""
> +	# Initialize the result incase the test fails.
> +	set wpoffset_to_wpnum($wpoffset) 0
>  	gdb_test_multiple $test $test {
>  	    -re "$rwatch_exp (\[0-9\]+): .*\r\n$gdb_prompt $" {
>  		set wpoffset_to_wpnum($wpoffset) $expect_out(1,string)
> @@ -113,7 +115,6 @@ foreach wpcount {4 7} {
>  		    setup_xfail breakpoints/23131 "arm*-*-*"
>  		}
>  		fail $test
> -		set wpoffset_to_wpnum($wpoffset) 0
>  	    }
>  	}
>      }

The change itself looks great.

Reviewed-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew


> -- 
> 2.40.0