From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <SRS0=z9qk=5A=lancelotsix.com=lsix@sourceware.org>
Received: from lndn.lancelotsix.com (lndn.lancelotsix.com [51.195.220.111])
	by sourceware.org (Postfix) with ESMTPS id C5F203858D1E
	for <gdb-patches@sourceware.org>; Tue,  3 Jan 2023 16:10:59 +0000 (GMT)
Received: from ubuntu.lan (unknown [IPv6:2a02:390:9086::146])
	by lndn.lancelotsix.com (Postfix) with ESMTPSA id 73D798079F;
	Tue,  3 Jan 2023 16:10:58 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=lancelotsix.com;
	s=2021; t=1672762258;
	bh=rvGwmI0J+QMdRtEfqrYchrobINpa7mBrVJVYdlOi40Q=;
	h=Date:From:To:Cc:Subject:References:In-Reply-To:From;
	b=lyq+RGNNnpz8wRRnDAw5FTD3EwzAfiQSI/cbNwsm+57jGnQZ7HOwjl8F7sc8gInPz
	 tgqyi8kR1NXtI1Fgx9SkGWGqc00yd+aA2JWW4NBd3RftlqZPTVRCTnjVfmemsjxe4j
	 r3QtiYTWtC+iEWqd0gQhI2ubnkcGwP6oKHk2mH7V6P7qeZuYyls0c5hFX9jd1Ldb2r
	 bfIKOQXphH+ZQrm51Vg3K+qr3YNEfozoxiqbAa5rCMVSxMP6P8XuBw15D/qhG8PoFW
	 sKxQB4RsEXj6pJu2F+j9rl+KBNbA1GrTfX9VTi/rtMhV92RwmFUUzosBZneXviLpCJ
	 4BpEaZWKYyt/Q==
Date: Tue, 3 Jan 2023 16:10:45 +0000
From: Lancelot SIX <lsix@lancelotsix.com>
To: Bruno Larsen <blarsen@redhat.com>
Cc: gdb-patches@sourceware.org, Eli Zaretskii <eliz@gnu.org>
Subject: Re: [PATCH v3] gdb: add 'maintenance print record-instruction'
 command
Message-ID: <20230103161045.ia4ldkemvsorbals@ubuntu.lan>
References: <20221222154338.2223678-1-blarsen@redhat.com>
 <20230102162645.oudy2wxxtmlm355r@ubuntu.lan>
 <1a74eebc-2ecd-7334-b702-52185c6820bf@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1a74eebc-2ecd-7334-b702-52185c6820bf@redhat.com>
X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.11 (lndn.lancelotsix.com [0.0.0.0]); Tue, 03 Jan 2023 16:10:58 +0000 (UTC)
X-Spam-Status: No, score=-5.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org
List-Id: <gdb-patches.sourceware.org>

Hi,

> > I do not think the rest of the test makes much sense if
> > [supports_process_record] returns false, does it?  I guess that
> > everything below this point should be be in the if block.
> > 
> > Another approach might to have the initial test (at the top of the file)
> > check for both supports_reverse and supports_process_record and ignore
> > the test if either feature is not supported.  WDYT?
> Good call, but since I do think it makes sense to test that GDB doesn't go
> wild even if recording is not supported, I think I'll make the test exit
> early here instead.

That seems good to me.

> > 
> > For what it is worth, the rest of the patch looks OK to me.
> Awesome, may I add your R-b tag, or do you want to see my change to the test
> first?

Sure, with the test modified as discussed above this seems OK to me.  Please add
Reviewed-By: Lancelot SIX <lancelot.six@amd.com>

Best,
Lancelot.