From patchwork Tue Apr 14 19:12:02 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergio Durigan Junior X-Patchwork-Id: 6218 Received: (qmail 14532 invoked by alias); 14 Apr 2015 19:12:12 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 14469 invoked by uid 89); 14 Apr 2015 19:12:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 14 Apr 2015 19:12:09 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t3EJC5df006784 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Tue, 14 Apr 2015 15:12:05 -0400 Received: from localhost (unused-10-15-17-126.yyz.redhat.com [10.15.17.126]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t3EJC42E031779 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA256 bits=256 verify=NO); Tue, 14 Apr 2015 15:12:04 -0400 From: Sergio Durigan Junior To: Pedro Alves Cc: Doug Evans , Yao Qi , gdb-patches Subject: Re: [RFC] Unset tcl variable addr to avoid clashing References: <1428666671-12926-1-git-send-email-qiyaoltc@gmail.com> <87r3rqppgq.fsf@redhat.com> <552B7DA3.5070103@redhat.com> X-URL: http://blog.sergiodj.net Date: Tue, 14 Apr 2015 15:12:02 -0400 In-Reply-To: <552B7DA3.5070103@redhat.com> (Pedro Alves's message of "Mon, 13 Apr 2015 09:26:11 +0100") Message-ID: <87r3rmjzi5.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-IsSubscribed: yes 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 >>> >>> * 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, 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"