From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id gKAGH+mfB2TJvwcAWB0awg (envelope-from ) for ; Tue, 07 Mar 2023 15:34:49 -0500 Received: by simark.ca (Postfix, from userid 112) id 7CCD51E223; Tue, 7 Mar 2023 15:34:49 -0500 (EST) 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=j6gnw+/A; dkim-atps=neutral X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-6.3 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,NICE_REPLY_A, RCVD_IN_DNSWL_MED,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from 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 RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id D53431E128 for ; Tue, 7 Mar 2023 15:34:48 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 896F5385800C for ; Tue, 7 Mar 2023 20:34:48 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 896F5385800C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1678221288; bh=q91YqJt4rbXZpIt/fiTBKUFWlD2COPEDVeTjmOPc62A=; 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=j6gnw+/AmytMCqs01v6kn3TcdouqYuuq6TVoss+N3/GsYXiZ+8KZOOTDgVuGoHI6f QlfM7GHQBNDU9UXCpXj2+yhBQmZOSp14+TGZ5S05XpeaPuuUfTrgb7wJRzVGfflcIM LTAB+pjSnYzyD6WB1J36i/CpThr2XSwT7J6oiGz0= Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 70959385B522 for ; Tue, 7 Mar 2023 20:34:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 70959385B522 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 C20FE1E128; Tue, 7 Mar 2023 15:34:27 -0500 (EST) Message-ID: <2526b60c-8a86-dc29-6c23-79a00461c591@simark.ca> Date: Tue, 7 Mar 2023 15:34:27 -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: [PATCH 1/3] gdb: make get_interp_info return a reference Content-Language: en-US To: Tom Tromey , Simon Marchi via Gdb-patches Cc: Simon Marchi References: <20230302203224.118345-1-simon.marchi@efficios.com> <20230302203224.118345-2-simon.marchi@efficios.com> <874jqws6o5.fsf@tromey.com> In-Reply-To: <874jqws6o5.fsf@tromey.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit 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 3/7/23 14:27, Tom Tromey wrote: >>>>>> "Simon" == Simon Marchi via Gdb-patches writes: > > Simon> get_interp_info and get_current_interp_info always return non-nullptr, > Simon> so they can return a reference instead of a pointer. > > Maybe it would be better here if ui_interp_info also disabled copying. > Otherwise it seems easy to accidentally drop a "&" and wind up modifying > a new copy. > > Well, that's my first reaction anyway. I'm not sure how much we should > really be worried about it. Well, it's the language we have to work with, it does what we ask it to do. Here, we don't need for ui_interp_info to be copyable, to we can do it, but if we needed it to be copyable, it would be down to being careful. Here's what the patch looks like now: >From e446fa8ad63f2a33f6f7ffb2427d00998da3c781 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Thu, 2 Mar 2023 15:32:22 -0500 Subject: [PATCH] gdb: make get_interp_info return a reference get_interp_info and get_current_interp_info always return non-nullptr, so they can return a reference instead of a pointer. Since we don't need to copy it, make ui_interp_info non-copyiable, to avoid a copying it in a local variable, instead of getting a reference. Change-Id: I6d8dea92dc26a58ea340d04862db6b8d9cf906a0 --- gdb/interps.c | 76 ++++++++++++++++++++++++++------------------------- 1 file changed, 39 insertions(+), 37 deletions(-) diff --git a/gdb/interps.c b/gdb/interps.c index 9c7908bde1ca..9768f3cf8804 100644 --- a/gdb/interps.c +++ b/gdb/interps.c @@ -45,6 +45,9 @@ struct ui_interp_info { + ui_interp_info () = default; + DISABLE_COPY_AND_ASSIGN(ui_interp_info); + /* Each top level has its own independent set of interpreters. */ struct interp *interp_list; struct interp *current_interpreter; @@ -55,20 +58,19 @@ struct ui_interp_info struct interp *command_interpreter; }; -/* Get UI's ui_interp_info object. Never returns NULL. */ +/* Get UI's ui_interp_info object. */ -static struct ui_interp_info * +static ui_interp_info & get_interp_info (struct ui *ui) { if (ui->interp_info == NULL) - ui->interp_info = XCNEW (struct ui_interp_info); - return ui->interp_info; + ui->interp_info = new ui_interp_info; + return *ui->interp_info; } -/* Get the current UI's ui_interp_info object. Never returns - NULL. */ +/* Get the current UI's ui_interp_info object. */ -static struct ui_interp_info * +static ui_interp_info & get_current_interp_info (void) { return get_interp_info (current_ui); @@ -128,12 +130,12 @@ interp_factory_register (const char *name, interp_factory_func func) static void interp_add (struct ui *ui, struct interp *interp) { - struct ui_interp_info *ui_interp = get_interp_info (ui); + ui_interp_info &ui_interp = get_interp_info (ui); gdb_assert (interp_lookup_existing (ui, interp->name ()) == NULL); - interp->next = ui_interp->interp_list; - ui_interp->interp_list = interp; + interp->next = ui_interp.interp_list; + ui_interp.interp_list = interp; } /* This sets the current interpreter to be INTERP. If INTERP has not @@ -150,13 +152,13 @@ interp_add (struct ui *ui, struct interp *interp) static void interp_set (struct interp *interp, bool top_level) { - struct ui_interp_info *ui_interp = get_current_interp_info (); - struct interp *old_interp = ui_interp->current_interpreter; + ui_interp_info &ui_interp = get_current_interp_info (); + struct interp *old_interp = ui_interp.current_interpreter; /* If we already have an interpreter, then trying to set top level interpreter is kinda pointless. */ - gdb_assert (!top_level || !ui_interp->current_interpreter); - gdb_assert (!top_level || !ui_interp->top_level_interpreter); + gdb_assert (!top_level || !ui_interp.current_interpreter); + gdb_assert (!top_level || !ui_interp.top_level_interpreter); if (old_interp != NULL) { @@ -164,9 +166,9 @@ interp_set (struct interp *interp, bool top_level) old_interp->suspend (); } - ui_interp->current_interpreter = interp; + ui_interp.current_interpreter = interp; if (top_level) - ui_interp->top_level_interpreter = interp; + ui_interp.top_level_interpreter = interp; if (interpreter_p != interp->name ()) interpreter_p = interp->name (); @@ -203,10 +205,10 @@ interp_set (struct interp *interp, bool top_level) static struct interp * interp_lookup_existing (struct ui *ui, const char *name) { - struct ui_interp_info *ui_interp = get_interp_info (ui); + ui_interp_info &ui_interp = get_interp_info (ui); struct interp *interp; - for (interp = ui_interp->interp_list; + for (interp = ui_interp.interp_list; interp != NULL; interp = interp->next) { @@ -259,8 +261,8 @@ void current_interp_set_logging (ui_file_up logfile, bool logging_redirect, bool debug_redirect) { - struct ui_interp_info *ui_interp = get_current_interp_info (); - struct interp *interp = ui_interp->current_interpreter; + ui_interp_info &ui_interp = get_current_interp_info (); + struct interp *interp = ui_interp.current_interpreter; interp->set_logging (std::move (logfile), logging_redirect, debug_redirect); } @@ -269,12 +271,12 @@ current_interp_set_logging (ui_file_up logfile, bool logging_redirect, struct interp * scoped_restore_interp::set_interp (const char *name) { - struct ui_interp_info *ui_interp = get_current_interp_info (); + ui_interp_info &ui_interp = get_current_interp_info (); struct interp *interp = interp_lookup (current_ui, name); - struct interp *old_interp = ui_interp->current_interpreter; + struct interp *old_interp = ui_interp.current_interpreter; if (interp) - ui_interp->current_interpreter = interp; + ui_interp.current_interpreter = interp; return old_interp; } @@ -282,8 +284,8 @@ scoped_restore_interp::set_interp (const char *name) int current_interp_named_p (const char *interp_name) { - struct ui_interp_info *ui_interp = get_current_interp_info (); - struct interp *interp = ui_interp->current_interpreter; + ui_interp_info &ui_interp = get_current_interp_info (); + struct interp *interp = ui_interp.current_interpreter; if (interp != NULL) return (strcmp (interp->name (), interp_name) == 0); @@ -304,12 +306,12 @@ current_interp_named_p (const char *interp_name) struct interp * command_interp (void) { - struct ui_interp_info *ui_interp = get_current_interp_info (); + ui_interp_info &ui_interp = get_current_interp_info (); - if (ui_interp->command_interpreter != NULL) - return ui_interp->command_interpreter; + if (ui_interp.command_interpreter != NULL) + return ui_interp.command_interpreter; else - return ui_interp->current_interpreter; + return ui_interp.current_interpreter; } /* See interps.h. */ @@ -336,11 +338,11 @@ interp_supports_command_editing (struct interp *interp) void interp_exec (struct interp *interp, const char *command_str) { - struct ui_interp_info *ui_interp = get_current_interp_info (); + ui_interp_info &ui_interp = get_current_interp_info (); /* See `command_interp' for why we do this. */ scoped_restore save_command_interp - = make_scoped_restore (&ui_interp->command_interpreter, interp); + = make_scoped_restore (&ui_interp.command_interpreter, interp); interp->exec (command_str); } @@ -365,7 +367,7 @@ clear_interpreter_hooks (void) static void interpreter_exec_cmd (const char *args, int from_tty) { - struct ui_interp_info *ui_interp = get_current_interp_info (); + ui_interp_info &ui_interp = get_current_interp_info (); struct interp *old_interp, *interp_to_use; unsigned int nrules; unsigned int i; @@ -387,7 +389,7 @@ interpreter_exec_cmd (const char *args, int from_tty) if (nrules < 2) error (_("Usage: interpreter-exec INTERPRETER COMMAND...")); - old_interp = ui_interp->current_interpreter; + old_interp = ui_interp.current_interpreter; interp_to_use = interp_lookup (current_ui, prules[0]); if (interp_to_use == NULL) @@ -425,9 +427,9 @@ interpreter_completer (struct cmd_list_element *ignore, struct interp * top_level_interpreter (void) { - struct ui_interp_info *ui_interp = get_current_interp_info (); + ui_interp_info &ui_interp = get_current_interp_info (); - return ui_interp->top_level_interpreter; + return ui_interp.top_level_interpreter; } /* See interps.h. */ @@ -435,9 +437,9 @@ top_level_interpreter (void) struct interp * current_interpreter (void) { - struct ui_interp_info *ui_interp = get_interp_info (current_ui); + ui_interp_info &ui_interp = get_interp_info (current_ui); - return ui_interp->current_interpreter; + return ui_interp.current_interpreter; } /* This just adds the "interpreter-exec" command. */ base-commit: 4779ed9757fa71e6743fb7fc1f9eeae8267ae36c -- 2.39.2