From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 5Yg5JgNCMWUIIjIAWB0awg (envelope-from ) for ; Thu, 19 Oct 2023 10:49:39 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=polymtl.ca header.i=@polymtl.ca header.a=rsa-sha256 header.s=default header.b=QYzgTh6/; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 925D51E0C1; Thu, 19 Oct 2023 10:49:39 -0400 (EDT) Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 6886B1E00F for ; Thu, 19 Oct 2023 10:49:37 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id EA2B43858017 for ; Thu, 19 Oct 2023 14:49:36 +0000 (GMT) Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 424273858D1E for ; Thu, 19 Oct 2023 14:49:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 424273858D1E Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=polymtl.ca Authentication-Results: sourceware.org; spf=none smtp.mailfrom=polymtl.ca ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 424273858D1E Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=132.207.4.11 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1697726964; cv=none; b=sSPu3YC8u+2/+cZZjMX5TDXtQJNLnAn5ztWpZEkSgkcjWDbf0uH7UH7MMjtgB24lXzIh2h+RRxaGF3SSQLltoWWI3aYnazpxYFnChvbjb0+D/EGO7yYv74hMtg+wF3oDkde6YdLUeIvn3jiLK4DWdNF0vMJf/i4BZV5mWVKJEWg= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1697726964; c=relaxed/simple; bh=v9m+jbLe+tfBekPH0RWivxcSD43iua6kcm+Q0zE7Q78=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=HurCTFKVSqNbTIbgqoUxT0EGC6+14yfgzMvCh6NJK7EjxbF6DYxb06nOeO9XKEH4WVoUChkyi0hxiBaq6HXgdMCBwpyExsqhA8pJJ1PFKrs+s2Zj1CvVLbtBM7BPmzr74PFQnoJoYIaKz1z+6sfpdt9TRjrk02EXHRBUaUDdzHU= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 39JEnDeg032505 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 19 Oct 2023 10:49:18 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 39JEnDeg032505 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=polymtl.ca; s=default; t=1697726958; bh=8EE//sKRsLda0ilBsOGhnrKFCdAI7aGU8lslJRrZ1Ak=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=QYzgTh6/wL46B0/PAsvnPs7ubK22ppCvtwLa/KjvLJr5bLfFPfxoPRwTf+eUBAPCD fxf3jFdC4o61sJzONZHyNARynmstcgAJw+0PiKAVsLFp5xQPKzs/B7AH22HiKJ9XNp C+B2925pm4ldTVw8XrYFBSni9oVdjen8VQkE6y48= Received: from [172.16.0.192] (192-222-143-198.qc.cable.ebox.net [192.222.143.198]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 4B6B91E00F; Thu, 19 Oct 2023 10:49:13 -0400 (EDT) Message-ID: <560f3443-5089-4af2-94d5-efc7bbd92684@polymtl.ca> Date: Thu, 19 Oct 2023 10:49:12 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 4/24] gdb: replace some so_list parameters to use references Content-Language: fr To: Lancelot SIX , Simon Marchi Cc: gdb-patches@sourceware.org References: <20231010204213.111285-1-simon.marchi@efficios.com> <20231010204213.111285-5-simon.marchi@efficios.com> <20231019110750.34bk5fflxalsb4tw@khazad-dum> From: Simon Marchi In-Reply-To: <20231019110750.34bk5fflxalsb4tw@khazad-dum> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Thu, 19 Oct 2023 14:49:13 +0000 X-Spam-Status: No, score=-3037.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, SPF_NONE, 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 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org On 10/19/23 07:07, Lancelot SIX wrote: > Hi Simon, > > Some minor remarks below. > > On Tue, Oct 10, 2023 at 04:39:59PM -0400, Simon Marchi wrote: >> From: Simon Marchi >> >> A subsequent patch changes so_list to be linked using >> intrusive_list. Iterating an intrusive_list yields some references to >> the list elements. Convert some functions accepting so_list objects to >> take references, to make things easier and more natural. Add const >> where possible and convenient. >> >> Change-Id: Id5ab5339c3eb6432e809ad14782952d6a45806f3 >> --- >> gdb/breakpoint.c | 5 +- >> gdb/bsd-uthread.c | 12 ++--- >> gdb/exec.c | 4 +- >> gdb/interps.c | 4 +- >> gdb/interps.h | 8 +-- >> gdb/mi/mi-cmd-file.c | 2 +- >> gdb/mi/mi-interp.c | 26 ++++----- >> gdb/mi/mi-interp.h | 6 +-- >> gdb/nto-tdep.c | 6 +-- >> gdb/nto-tdep.h | 3 +- >> gdb/observable.h | 5 +- >> gdb/progspace.h | 4 +- >> gdb/solib-aix.c | 11 ++-- >> gdb/solib-darwin.c | 23 ++++---- >> gdb/solib-dsbt.c | 9 ++-- >> gdb/solib-frv.c | 9 ++-- >> gdb/solib-rocm.c | 8 +-- >> gdb/solib-svr4.c | 35 ++++++------ >> gdb/solib-target.c | 49 +++++++++-------- >> gdb/solib.c | 126 +++++++++++++++++++++---------------------- >> gdb/solib.h | 4 +- >> gdb/solist.h | 13 +++-- >> gdb/target-section.h | 2 +- >> 23 files changed, 183 insertions(+), 191 deletions(-) >> >> diff --git a/gdb/mi/mi-interp.h b/gdb/mi/mi-interp.h >> index f9af61f0a571..781a8dc6f466 100644 >> --- a/gdb/mi/mi-interp.h >> +++ b/gdb/mi/mi-interp.h >> @@ -60,8 +60,8 @@ class mi_interp final : public interp >> void on_record_changed (inferior *inf, int started, const char *method, >> const char *format) override; >> void on_target_resumed (ptid_t ptid) override; >> - void on_solib_loaded (so_list *so) override; >> - void on_solib_unloaded (so_list *so) override; >> + void on_solib_loaded (const so_list &so) override; >> + void on_solib_unloaded (const so_list &so) override; > > It is orthogonal to this change, but it would make sense for those > methods to be const as well. > > Doing this requires interp::interp_ui_out to be const as well (as done > in attached patch). Why do you think the interp object should be const (which is the effect of marking the methods const)? These methods are notifiers to let the interp know about certain events that occured. Whether they should modify the state of the interpreter or not is up to the particular implementation of the interpreter. Perhaps you're right, but I don't see it. In your patch, you make interp::interp_ui_out const, but it still returns a non-const ui_out. So when outputting something, the interpreter itself is not modified, but the ui_out behind it is. I presume (sometimes) the ui_out needs to be non-const, since for things like pagination it clearly needs to keep a state. Design-wise, is it "correct" to make a const intepreter able to return a non-const ui_out? >> void on_about_to_proceed () override; >> void on_traceframe_changed (int tfnum, int tpnum) override; >> void on_tsv_created (const trace_state_variable *tsv) override; >> diff --git a/gdb/observable.h b/gdb/observable.h >> index acb05e9b535c..5ed6ca547ce0 100644 >> --- a/gdb/observable.h >> +++ b/gdb/observable.h >> @@ -99,13 +99,12 @@ extern observable> /* The shared library specified by SOLIB has been loaded. Note that >> when gdb calls this observer, the library's symbols probably >> haven't been loaded yet. */ >> -extern observable solib_loaded; >> +extern observable solib_loaded; > > I am wondering, is there a reason to make the solib parameter const for > solib_unloaded but not for solib_loaded? > > Changing it to const seems to still compile just fine. If down the line > an observer needs to modify the SO, the observer's signature can be > adjusted. Just pragmatic reasons. Do you build with --enable-targets=all? It's because bsd_uthread_solib_loaded calls solib_read_symbols to read in the symbols of the library it is looking for, and that requires a non-const so_list. Simon