From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2614 invoked by alias); 19 Oct 2011 00:44:27 -0000 Received: (qmail 2605 invoked by uid 22791); 19 Oct 2011 00:44:27 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 19 Oct 2011 00:44:13 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id B6AA52BB347; Tue, 18 Oct 2011 20:44:12 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id httjl1SOCpDG; Tue, 18 Oct 2011 20:44:12 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 8B4BF2BB346; Tue, 18 Oct 2011 20:44:12 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id A5C41145615; Tue, 18 Oct 2011 20:44:07 -0400 (EDT) Date: Wed, 19 Oct 2011 01:05:00 -0000 From: Joel Brobecker To: John Wehle Cc: gdb-patches@sourceware.org Subject: Re: GDB / SIM 7.3.1 Cosmetic patch for profile title Message-ID: <20111019004407.GD19246@adacore.com> References: <201110170428.p9H4SMZO025996@jwlab.FEITH.COM> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201110170428.p9H4SMZO025996@jwlab.FEITH.COM> User-Agent: Mutt/1.5.20 (2009-06-14) 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: 2011-10/txt/msg00520.txt.bz2 > The issue being that "Summary profiling results" is displayed multiple > times in a row. This patch fixes profile_info so that the title is > only displayed once. [...] > Mon Oct 17 00:14:40 EDT 2011 John Wehle (john@feith.com) > > * sim-profile.c (profile_info): Only print the title once. This is OK with one little nit: > - for (c = 0; c < MAX_NR_PROCESSORS; ++c) > + for (c = 0; c < MAX_NR_PROCESSORS && ! print_title_p; ++c) ^^^^^^^^^^^^^^^ No space after the '!'. As far as I can tell, you do not have a copyright assignment in place for GDB (it looks like you assigned some specific changes many many years ago, but nothing that covers those changes). I will commit them using the "small change" rule, but if you think you might contribute other changes, I invite you to start the process now, so that future patches do not stay held up just because of paperwork. As a side note, it took me a while to understand some of the logic. It looks like `print_title_p' really means `we-have-some-profile-data'. So, if it was me, I'd probably rename that variable to something like: profile_data_collected_p, and then rewrite the code such that the double-loop only sets that variable. And then have a simple if block: if (profile_data_collected_p) profile_printf (sd, cpu, "Summary profiling results:\n\n"); The only other place where this variable is reference would also benefit from that renaming, since... if (print_title_p && (PROFILE_INSN_P (cpu) || PROFILE_MODEL_P (cpu))) profile_print_addr_ranges (cpu); ... would become: if (profile_data_collected_p && (PROFILE_INSN_P (cpu) || PROFILE_MODEL_P (cpu))) profile_print_addr_ranges (cpu); -- Joel