From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id OGWwFl8vEGGAKgAAWB0awg (envelope-from ) for ; Sun, 08 Aug 2021 15:24:15 -0400 Received: by simark.ca (Postfix, from userid 112) id 5AD6C1EDF7; Sun, 8 Aug 2021 15:24:15 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-0.7 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RDNS_DYNAMIC,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from 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 448311E79C for ; Sun, 8 Aug 2021 15:24:14 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id C9E7C389003A for ; Sun, 8 Aug 2021 19:24:13 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C9E7C389003A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1628450653; bh=jZeirNTYTmfXjJQ4XlhFw0+OxsgPoRwchimNclQFv1E=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=wbGw9n+fByic7p4KvLAWi73KKboKbwbsUAtOXzX9/hmyjcrxbPEw+FgF+1yN2m7Du P+yHDTB5QhM7j91+B1dabtRGFjtTdNE0MuwE04b2e0aolI4Ctg+mUOJ9efw8yr72NP UC5MZ6AddaSEhwNzkvL+Tn51hToGnJGBzD9iXWy4= Received: from lndn.lancelotsix.com (vps-42846194.vps.ovh.net [IPv6:2001:41d0:801:2000::2400]) by sourceware.org (Postfix) with ESMTPS id 296BE3889C2E for ; Sun, 8 Aug 2021 19:23:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 296BE3889C2E Received: from Plymouth.lan (unknown [IPv6:2a02:390:9086:0:634a:c6a1:40ee:68b1]) by lndn.lancelotsix.com (Postfix) with ESMTPSA id 4A5FE84198; Sun, 8 Aug 2021 19:23:29 +0000 (UTC) To: gdb-patches@sourceware.org Subject: [PATCH v2 4/4] gdb: Setting setter return a bool to tell if the value changed Date: Sun, 8 Aug 2021 20:23:02 +0100 Message-Id: <20210808192302.3768766-5-lsix@lancelotsix.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210808192302.3768766-1-lsix@lancelotsix.com> References: <20210808192302.3768766-1-lsix@lancelotsix.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.11 (lndn.lancelotsix.com [0.0.0.0]); Sun, 08 Aug 2021 19:23:29 +0000 (UTC) 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: Lancelot SIX via Gdb-patches Reply-To: Lancelot SIX Cc: Lancelot SIX Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" GDB can notify observers when a parameter is changed. To do that, do_set_command (in gdb/cli/cli-setshow.c) compares the new value against the old one before updating it, and based on that notifies observers. This looks like something like: int valuechanged = 0; switch (cmd->var.type ()) { case var_integer: { LONGEST new_val = parse_and_eval_long (arg) if (new_val != cmd->var.get ()) { cmd->var.get (new_val); value_changes = 1; } } break; case var_string: { std::string val = std::string (arg); if (new_val != cmd->var.get ()) { cmd->var.get (new_val); value_changes = 1; } } break; case... /* And so on for all possible var_types. */ } This comparison is done for each possible var_type, which leads to unnecessary code duplication. In this patch I propose to move all those checks in one place within the setting setter method. This limits the code duplication and simplifies the do_set_command implementation. This patch also changes slightly the way a value change is detected. Instead of comparing the user provided value against the current value of the setting, we compare the value of the setting before and after the set operation. This is meant to handle edge cases where trying to set an unrecognized value would be equivalent to a noop (the actual value remains unchanged). There should be no user visible change introduced by this commit. Tested on x86_64 GNU/Linux. [1] https://review.lttng.org/c/binutils-gdb/+/5831/41 --- gdb/cli/cli-setshow.c | 96 +++++++++---------------------------------- gdb/command.h | 6 ++- 2 files changed, 25 insertions(+), 77 deletions(-) diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c index 3af99a5e917..591db466784 100644 --- a/gdb/cli/cli-setshow.c +++ b/gdb/cli/cli-setshow.c @@ -31,17 +31,17 @@ /* Return true if the change of command parameter should be notified. */ -static int -notify_command_param_changed_p (int param_changed, struct cmd_list_element *c) +static bool +notify_command_param_changed_p (bool param_changed, struct cmd_list_element *c) { - if (param_changed == 0) - return 0; + if (!param_changed) + return false; if (c->theclass == class_maintenance || c->theclass == class_deprecated || c->theclass == class_obscure) - return 0; + return false; - return 1; + return true; } @@ -305,7 +305,7 @@ void do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) { /* A flag to indicate the option is changed or not. */ - int option_changed = 0; + bool option_changed = false; gdb_assert (c->type == set_cmd); @@ -353,26 +353,12 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) *q++ = '\0'; newobj = (char *) xrealloc (newobj, q - newobj); - const std::string var = c->var.get (); - if (var != std::string (newobj)) - { - c->var.set (std::string (newobj)); - - option_changed = 1; - } + option_changed = c->var.set (std::string (newobj)); xfree (newobj); } break; case var_string_noescape: - { - const std::string var = c->var.get (); - if (var != arg) - { - c->var.set (std::string (arg)); - - option_changed = 1; - } - } + option_changed = c->var.set (std::string (arg)); break; case var_filename: if (*arg == '\0') @@ -398,13 +384,8 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) else val = xstrdup (""); - const std::string var = c->var.get (); - if (var != std::string (val)) - { - c->var.set (std::string (val)); - - option_changed = 1; - } + option_changed + = c->var.set (std::string (val)); xfree (val); } break; @@ -414,38 +395,18 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) if (val < 0) error (_("\"on\" or \"off\" expected.")); - if (val != c->var.get ()) - { - c->var.set (val); - option_changed = 1; - } + option_changed = c->var.set (val); } break; case var_auto_boolean: - { - enum auto_boolean val = parse_auto_binary_operation (arg); - - if (c->var.get () != val) - { - c->var.set (val); - - option_changed = 1; - } - } + option_changed = c->var.set (parse_auto_binary_operation (arg)); break; case var_uinteger: case var_zuinteger: - { - unsigned int val = parse_cli_var_uinteger (c->var.type (), &arg, true); - - if (c->var.get () != val) - { - c->var.set (val); - - option_changed = 1; - } - } + option_changed + = c->var.set (parse_cli_var_uinteger (c->var.type (), + &arg, true)); break; case var_integer: case var_zinteger: @@ -475,12 +436,7 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) || (c->var.type () == var_zinteger && val > INT_MAX)) error (_("integer %s out of range"), plongest (val)); - if (c->var.get () != val) - { - c->var.set (val); - - option_changed = 1; - } + option_changed = c->var.set (val); } break; case var_enum: @@ -493,24 +449,12 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) if (*after != '\0') error (_("Junk after item \"%.*s\": %s"), len, arg, after); - if (c->var.get () != match) - { - c->var.set (match); - - option_changed = 1; - } + option_changed = c->var.set (match); } break; case var_zuinteger_unlimited: - { - int val = parse_cli_var_zuinteger_unlimited (&arg, true); - - if (c->var.get () != val) - { - c->var.set (val); - option_changed = 1; - } - } + option_changed = c->var.set + (parse_cli_var_zuinteger_unlimited (&arg, true)); break; default: error (_("gdb internal error: bad var_type in do_setshow_command")); diff --git a/gdb/command.h b/gdb/command.h index 7b8fe5ea9f0..b30a6917211 100644 --- a/gdb/command.h +++ b/gdb/command.h @@ -420,19 +420,23 @@ struct base_setting The var_type of the setting must match T. */ template - void set (T v) + bool set (T v) { /* Check that the current instance is of one of the supported types for this instantiation. */ gdb_assert (var_type_uses (this->m_var_type)); gdb_assert (!this->empty ()); + const T old_value = this->get (); + auto setter = get_setting_setter (this->m_setter); if (setter != nullptr) (*setter) (v); else *static_cast (this->m_var) = v; + + return old_value != this->get (); } /* Set the user provided setter and getter functions. */ -- 2.31.1