From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10399 invoked by alias); 21 Jul 2009 17:16:32 -0000 Received: (qmail 10390 invoked by uid 22791); 21 Jul 2009 17:16:31 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mx2.redhat.com (HELO mx2.redhat.com) (66.187.237.31) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 21 Jul 2009 17:16:24 +0000 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n6LHGMYH010461; Tue, 21 Jul 2009 13:16:22 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id n6LHGKvB004429; Tue, 21 Jul 2009 13:16:21 -0400 Received: from opsy.redhat.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n6LHGJRk021142; Tue, 21 Jul 2009 13:16:20 -0400 Received: by opsy.redhat.com (Postfix, from userid 500) id C705C3780F2; Tue, 21 Jul 2009 10:13:32 -0600 (MDT) To: Michael Snyder Cc: "gdb-patches\@sourceware.org" Subject: Re: [RFA] clean up temp sals in handle_inferior_event References: <4A63D5B4.8090505@vmware.com> From: Tom Tromey Reply-To: tromey@redhat.com Date: Tue, 21 Jul 2009 18:07:00 -0000 In-Reply-To: <4A63D5B4.8090505@vmware.com> (Michael Snyder's message of "Sun\, 19 Jul 2009 19\:25\:56 -0700") Message-ID: User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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: 2009-07/txt/msg00515.txt.bz2 >>>>> "Michael" == Michael Snyder writes: Michael> There were seven, and now eight, places in handle_inferior_event Michael> where a local struct symtab_and_line is declared and used briefly. Michael> One of them even opens and closes a block just for this purpose. Michael> This patch merges those all into a single local temp variable. Michael> They all initialize it, and all but one of them returns after Michael> using it, so they can't interact. Plus I ran the testsuites Michael> with no regressions. I find the old code clearer. In the old code, these temporary variables are mostly declared in small blocks surrounding their point of use. This means that it is very easy to reason about the variable: its entire lifetime fits onto the screen at once. After the patch, this is not so. And, given that handle_inferior_event is extremely long, I think this makes it trickier to understand. I don't really do much work in this code, though, so perhaps I'm speaking out of turn. I'd be interested to hear what Pedro has to say. Also, if there is a benefit other than aesthetics to changing this, that could change my opinion. Tom