From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id Kb9+B9L9sWS0FCIAWB0awg (envelope-from ) for ; Fri, 14 Jul 2023 22:00:50 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=kjPUq8j6; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 0F70D1E0BE; Fri, 14 Jul 2023 22:00:50 -0400 (EDT) Received: from server2.sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id EF50A1E0B9 for ; Fri, 14 Jul 2023 22:00:47 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 59E2C385840C for ; Sat, 15 Jul 2023 02:00:47 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 59E2C385840C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1689386447; bh=hQtWF/VUNKt+MGiMyEfR95nB52EUXZ+ZoHmqlBlEEpw=; h=Date:Subject:To:Cc:References:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=kjPUq8j6r1z5aw33PZc7Qd9SMX5LOdXHtQnccu/ause7/VselJlJuYxkgb/vC5GlY hi9nKiKMuq/95TRuYJ6zM79HwRCD04jJloQNKN2ve+1BzxNS1HISbdrYgUV1/EuAE4 f7aiRcgT58R8TP1q8y6FNCqq6tv6Jo0qCL7A7mnU= Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id ECE733858CD1 for ; Sat, 15 Jul 2023 02:00:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org ECE733858CD1 Received: from [10.0.0.170] (modemcable238.237-201-24.mc.videotron.ca [24.201.237.238]) (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 077361E0AC; Fri, 14 Jul 2023 22:00:23 -0400 (EDT) Message-ID: Date: Fri, 14 Jul 2023 22:00:23 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [PATCH] gdb/amd-dbgapi-target: Use inf param in detach Content-Language: fr To: Pedro Alves , Lancelot SIX , gdb-patches@sourceware.org Cc: lsix@lancelotsix.com References: <20230616092528.69358-1-lancelot.six@amd.com> <2c7f2ef1-dea2-5dbe-8d3f-b9b885be3b72@palves.net> <05217693-1a01-bffb-e74a-503b3fe3a604@amd.com> <31b6963a-6b3a-f18e-9863-40acff23c546@palves.net> In-Reply-To: <31b6963a-6b3a-f18e-9863-40acff23c546@palves.net> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-4.2 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, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Simon Marchi via Gdb-patches Reply-To: Simon Marchi Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" On 7/14/23 15:18, Pedro Alves wrote: > On 2023-07-14 19:54, Lancelot SIX wrote: >> On 14/07/2023 19:35, Pedro Alves wrote: > >>> IMO this is a case of the target method's inferior * parameter having >>> been added too soon -- it would only make sense to have it if nothing in >>> the body of the target method implementations is relying on inf being >>> current_inferior on entry. But that is not the case, plenty of target_ops::detach >>> code has that assumption. The presence of an explicit inferior pointer should mean >>> that detach target method implementations that call code that depends >>> on the inferior being the current inferior, should be using a >>> scoped_restore_current_thread/inferior before calling such global-state-assuming >>> code. But, they don't do that, instead, we have this mixed situation. IMO, it would >>> be better to remove the parameter to avoid confusion and stick to the >>> (if explicit param, then switch global state to it if you need it) rule. I agree with this assessment. I was probably the one adding this parameter, before I understood that having a function receive the context by parameter _and_ depend on the global context is bad (worse than just depending on global context). >>> Anyhow, your patch doesn't make it worse, so it's fine with me. >>> >> >> I can also go the other way around and always use `current_inferior ()` instead of the `inf` parameter in this detach implementation. >> >> What bugged me here is the inconsistency from one line to the next. >> > > Please go ahead and merge your patch. I quickly looked at what it would like to remove > the parameter throughout, and I think we'd just end up doing: > > inferior *inf = current_inferior (); > > at the top of the function, so using "inf" in the remove_breakpoints_inf call anyhow, as > we have multiple references to the current inferior. Either fix is fine with me. Simon