Re: [RFC] Unset tcl variable addr to avoid clashing

[Sergio Durigan Junior <sergiodj@redhat.com>]

On Monday, April 13 2015, Pedro Alves wrote:

>>> FWIW, there is not strong reason to keep the variable name as "addr" in
>>> the testcase.  So, an easier solution would be to rename the variable to
>>> something else, like "coredump_var_addr" (I think this name is unique
>>> enough).  The patch below does that.
>>>
>>> WDYT?
>>>
>>> --
>>> Sergio
>>> GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
>>> Please send encrypted e-mail if possible
>>> http://sergiodj.net/
>>>
>>> gdb/testsuite/ChangeLog:
>>> 2015-04-11  Sergio Durigan Junior  <sergiodj@redhat.com>
>>>
>>>         * gdb.base/coredump-filter.exp: Rename variable "addr" to
>>>         "coredump_var_addr" to avoid naming conflict with other
>>>         testcases.
>> 
>> Ok by me with one nit.
>> There's an issue here that still needs to be documented so it becomes
>> part of the collective lore.
>> Can this include a note about the need to give global array variables
>> unique names to either testsuite/README or
>> https://sourceware.org/gdb/wiki/Internals%20GDB-Testsuite-Coding-Standards
>
> I don't agree with this resolution.  Renaming is not sufficient.
>
> The same .exp file can be run in the same dejagnu invocation.  Most
> commonly, RUNTESTFLAGS="--target_board=unix\{-m64,-m32\}",
> but can also be mix of native and gdbserver, like
> RUNTESTFLAGS="--target_board='unix native-gdbserver'"
>
> So it's not just conflicting with other testcases that we need
> to worry about, but also a testcase conflicting with itself.  Even
> though in that "multiple boards" scenario the variable will be arrays in
> all invocations, we should still clear it to avoid stale
> contents, like e.g., here:
>
>   https://sourceware.org/ml/gdb-patches/2015-04/msg00261.html
>
> Therefore I think the solution must be that we unset/clear the
> variable.  And if we must do that, then the renaming to avoid
> naming conflicts is not a necessary condition.

Good point, thanks for mentioning this.

What do you think of the following patch (obvious, but I decided to send
anyway)?  This is just to make things 100% correct, and I don't think
it's worth reverting the renaming.

I will also expand/fix the update I did in the wiki page.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

gdb/testsuite/ChangeLog:
2015-04-14  Sergio Durigan Junior  <sergiodj@redhat.com>

	* gdb.base/coredump-filter.exp: Clear variable "coredump_var_addr"
	before using it.

diff --git a/gdb/testsuite/gdb.base/coredump-filter.exp b/gdb/testsuite/gdb.base/coredump-filter.exp
index f872de0..72f756a 100644
--- a/gdb/testsuite/gdb.base/coredump-filter.exp
+++ b/gdb/testsuite/gdb.base/coredump-filter.exp
@@ -170,6 +170,7 @@ gdb_test_multiple "print/x &main" "getting main's address" {
 
 # Obtain the address of each variable that will be checked on each
 # case.
+set coredump_var_addr ""
 foreach item $all_anon_corefiles {
     foreach name [list [lindex $item 3] [lindex $item 4]] {
 	set test "print/x $name"