From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 68ED83858414 for ; Sat, 11 Mar 2023 02:57:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 68ED83858414 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca Received: from [10.0.0.11] (unknown [217.28.27.60]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id EA2DD1E128; Fri, 10 Mar 2023 21:57:55 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1678503476; bh=DPPrle4J2P+M1+1ci7q6sswtOHDz4TfuY04uOyrL6to=; h=Date:Subject:To:References:From:In-Reply-To:From; b=WqnwP02XNNC1S29U3wwA5LLkZOe4XzU8YnOY4AeMS+zI9l3HUirjP7QMpJcvLgJJW p510IK83eYxxuxJh9JMYzZB/yMybPYR1qa1u4AkdbktA8Y4uBuis7Lsl0Bamg/ptQq iJpc+PTfpxbYJJ7tnULCeJNUZ2y4uAR1GZefrlIQ= Message-ID: Date: Fri, 10 Mar 2023 21:57:55 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: Re: [PATCHv3 0/9] Add new gdbarch::displaced_step_buffer_length field Content-Language: en-US To: Andrew Burgess , gdb-patches@sourceware.org References: From: Simon Marchi In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-4.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,SPF_HELO_PASS,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: On 3/10/23 13:43, Andrew Burgess via Gdb-patches wrote: > Changes in V3: > > - Have now run 'black' on each patch in this series, so everything > should be formatted correctly, > > - Have fixed the out of date names in the last patch of this series. > Everything buildds and runs fine for me now, > > - Have removed the out of date text from the commit message in patch > #3, > > - Have fixed the missing comma Tom spotted in patch #4, > > - I have NOT removed the comments Simon pointed out in patch #4. > Removing these would require changing the generated code for more > Components than I already change in this commit. And so, I think, > if those comments are to go that would require a separate patch. > > That said, we do generate validation code within the getters for > many components, so I think having a comment in the components > where we don't generate validation makes sense. So for me, I > think the comments do add value, I'd suggest we should keep them. > > - And the big one. I've changed the 'invalid' default from False to > True. I know Simon suggested that False was the correct choice, > but I actually think that True makes more sense. I'd rather we > assume we should generate the validity checks and require than new > components explicitly choose to not have that check, rather than > just assuming we shouldn't check. > > However, in order to get to the default True state I ended up > having to fix some other issues. And, so, incase people really > would rather see False as the default, I've left the patch which > changes to default True as the penultimate patch in the series. > If you feel really strongly that False is best, I can just drop > the patch that switches over to use True. Just let me know. > > Let me know what you think, > > Thanks, > Andrew Thanks Andrew, it all LGTM: Approved-By: Simon Marchi Simon