From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28968 invoked by alias); 17 Feb 2012 20:55:29 -0000 Received: (qmail 28959 invoked by uid 22791); 17 Feb 2012 20:55:28 -0000 X-SWARE-Spam-Status: No, hits=-6.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 17 Feb 2012 20:55:11 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q1HKtA39001924 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 17 Feb 2012 15:55:10 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q1HKt8JK007220; Fri, 17 Feb 2012 15:55:09 -0500 Message-ID: <4F3EBEAC.1070805@redhat.com> Date: Fri, 17 Feb 2012 21:00:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0) Gecko/20120131 Thunderbird/10.0 MIME-Version: 1.0 To: Tom Tromey CC: gdb-patches@sourceware.org, jan.kratochvil@redhat.com Subject: Re: [PATCH 1/2] gdb.threads/attach-into-signal.exp: cleanup References: <20120217193546.10029.74207.stgit@hit-nxdomain.opendns.com> <20120217193648.10029.26589.stgit@hit-nxdomain.opendns.com> <87pqdd9pjs.fsf@fleche.redhat.com> In-Reply-To: <87pqdd9pjs.fsf@fleche.redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-02/txt/msg00379.txt.bz2 On 02/17/2012 08:24 PM, Tom Tromey wrote: >>>>>> "Pedro" == Pedro Alves writes: > > Pedro> - build different executable files for the non-threaded and threaded > Pedro> cases. This was my motivation. I wanted to test the non-threaded > Pedro> case manually, but the threaded variant always clobbered the > Pedro> non-threaded executable. > > I think this ought to be a general rule. We need exceptions to it for > some executable-changed cases, but I think in general different tests > should build different executables, because this makes it easier to do > additional checking by hand. > > Pedro> + set save_pf_prefix $pf_prefix > Pedro> + lappend pf_prefix "$threadtype:" > > I think this should be append rather than lappend, as pf_prefix is just > a string, not a list. Hmm, the only thing dejagnu does with it is 'concat'. /usr/share/dejagnu/framework.exp:681: global pf_prefix /usr/share/dejagnu/framework.exp:688: if {[info exists pf_prefix]} { /usr/share/dejagnu/framework.exp:689: set message [concat $pf_prefix " " $message] Interesting data-point: $ grep -rn pf_prefix *| grep append | grep lappend | wc -l 40 $ grep -rn pf_prefix *| grep append | grep append | grep -v lappend | wc -l 2 'concat' actually takes lists as arguments, so it could be said that lappend is the correct thing to do. 'concat' also accepts strings all the same too, and everything is a string in tcl anyway, so it could go both ways. Hmm, having said that, with lists we can pop back one level without saving the previous state of pf_prefix. That is, all the "set save_pf_prefix $pf_prefix" are really unnecessary. So we could instead write: lappend pf_prefix "$threadtype:" ... set pf_prefix [lreplace $pf_prefix end end] or even better, hide that a bit in a couple of methods in lib/gdb.exp: proc push_prefix { $new_prefix} { lappend pf_prefix "$threadtype:" } proc pop_prefix {} { set pf_prefix [lreplace $pf_prefix end end] } All without any extra variables. > > Pedro> if [get_compiler_info ${binfile}] { > Pedro> + set pf_prefix $save_pf_prefix > Pedro> return -1 > Pedro> } > > I've occasionally wanted a wrapper like 'with_pf_prefix $whatever { body }'. > But not enough to write it :) Oh, that's a good idea! -- Pedro Alves