From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8630 invoked by alias); 21 Oct 2009 10:54:12 -0000 Received: (qmail 8618 invoked by uid 22791); 21 Oct 2009 10:54:11 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 21 Oct 2009 10:54:07 +0000 Received: (qmail 11430 invoked from network); 21 Oct 2009 10:54:05 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 21 Oct 2009 10:54:05 -0000 From: Pedro Alves To: Michael Snyder Subject: Re: [RFA] Expand "info record" Date: Wed, 21 Oct 2009 10:54:00 -0000 User-Agent: KMail/1.9.10 Cc: "gdb-patches@sourceware.org" , Hui Zhu References: <4AD358E7.50009@vmware.com> <200910210122.06549.pedro@codesourcery.com> <4ADE647D.10607@vmware.com> In-Reply-To: <4ADE647D.10607@vmware.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200910211154.06992.pedro@codesourcery.com> X-IsSubscribed: yes 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-10/txt/msg00495.txt.bz2 On Wednesday 21 October 2009 02:31:41, Michael Snyder wrote: > Pedro Alves wrote: > > On Tuesday 20 October 2009 23:25:30, Michael Snyder wrote: > > > >>>> + /* Display instruction number for last instruction in the log. */ > >>>> + printf_filtered (_("Highest recorded instruction number is %llu.\n"), > >>>> + record_insn_count ? record_insn_count - 1 : 0); > >>> Why the conditional subtraction? > >> Because I don't want it to say "-1". > > > > Okay, that much is obvious, but how can you reach here > > with record_insn_count == 0, given that you check if you > > have a log at all a bit above? > > Maybe not -- but I'm a belt-and-suspenders guy. > I don't believe in not checking for something just because > it "can't happen". What if somebody changed the check above? That's why we comment non-obvious code, or add gdb_asserts (could be overkill), or warnings. The worse that can happen if somebody changes the check above, is the new bug is quickly exposed because gdb starting printing '(ULONGEST) -1'. Masking a buglet like that is wrong, IMO. > Yeah, you've convinced me. Thanks for the prodding, > and please see new revision. I'll make that conditional > go away too. ;-) Thanks. I could nit some more, but the all the info seems there. ;-) I'm fine with the patch. I don't see "info record" mentioned in the docs. Only: @kindex info record insn-number @item info record insn-number Show the current number of recorded instructions. Was there a corresponding documentation patch? FTR, it still pains me when I see this: > + if (current_target.to_stratum == record_stratum) > + { especially more so now that we have a stratum higher than record_statum... -- Pedro Alves