From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lndn.lancelotsix.com (lndn.lancelotsix.com [51.195.220.111]) by sourceware.org (Postfix) with ESMTPS id C2B253858001 for ; Wed, 29 Sep 2021 21:50:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C2B253858001 Received: from Plymouth.lan (unknown [IPv6:2a02:390:9086:0:1f74:c407:abcc:344d]) by lndn.lancelotsix.com (Postfix) with ESMTPSA id E3112849A8; Wed, 29 Sep 2021 21:50:58 +0000 (UTC) From: Lancelot SIX To: gdb-patches@sourceware.org Cc: Lancelot SIX Subject: [PATCH v4 4/4] gdb: Setting setter return a bool to tell if the value changed Date: Wed, 29 Sep 2021 22:50:11 +0100 Message-Id: <20210929215011.1489639-5-lsix@lancelotsix.com> X-Mailer: git-send-email 2.33.0 In-Reply-To: <20210929215011.1489639-1-lsix@lancelotsix.com> References: <20210929215011.1489639-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]); Wed, 29 Sep 2021 21:50:59 +0000 (UTC) X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: , X-List-Received-Date: Wed, 29 Sep 2021 21:51:05 -0000 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; } } 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 = true; } } case... /* And so on for all possible var_types. */ } This comparison is done for each possible var_type, which leads to unnecessary logic 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). Doing this requires that the original value needs to be copied before the update, which can be non trivial for std::string. 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 Change-Id: If064b9cede3eb56275aacd2b286f74eceb1aed11 --- gdb/cli/cli-setshow.c | 83 +++++++------------------------------------ gdb/command.h | 6 +++- 2 files changed, 18 insertions(+), 71 deletions(-) diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c index a027891de05..f06c92731e0 100644 --- a/gdb/cli/cli-setshow.c +++ b/gdb/cli/cli-setshow.c @@ -360,26 +360,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 &cur_val = c->var->get (); - if (strcmp (cur_val.c_str(), newobj) != 0) - { - c->var->set (std::string (newobj)); - - option_changed = true; - } + option_changed = c->var->set (std::string (newobj)); xfree (newobj); } break; case var_string_noescape: - { - const std::string &cur_val = c->var->get (); - if (strcmp (cur_val.c_str (), arg) != 0) - { - c->var->set (std::string (arg)); - - option_changed = true; - } - } + option_changed = c->var->set (std::string (arg)); break; case var_filename: if (*arg == '\0') @@ -405,13 +391,8 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) else val = xstrdup (""); - const std::string &cur_val = c->var->get (); - if (strcmp (cur_val.c_str (), val) != 0) - { - c->var->set (std::string (val)); - - option_changed = true; - } + option_changed + = c->var->set (std::string (val)); xfree (val); } break; @@ -421,39 +402,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 = true; - } + 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 = true; - } - } + 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 = true; - } - } + option_changed + = c->var->set (parse_cli_var_uinteger (c->var->type (), + &arg, true)); break; case var_integer: case var_zinteger: @@ -483,12 +443,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 = true; - } + option_changed = c->var->set (val); } break; case var_enum: @@ -501,24 +456,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 = true; - } + 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 = true; - } - } + 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 5ab25c74a9f..2d4561318bc 100644 --- a/gdb/command.h +++ b/gdb/command.h @@ -315,12 +315,14 @@ struct setting The var_type of the setting must match T. */ template - void set (const T &v) + bool set (const T &v) { /* Check that the current instance is of one of the supported types for this instantiation. */ gdb_assert (var_type_uses (m_var_type)); + const T old_value = this->get (); + if (m_var == nullptr) { gdb_assert (m_setter != nullptr); @@ -329,6 +331,8 @@ struct setting } else *static_cast (m_var) = v; + + return old_value != this->get (); } private: -- 2.33.0