From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lndn.lancelotsix.com (vps-42846194.vps.ovh.net [IPv6:2001:41d0:801:2000::2400]) by sourceware.org (Postfix) with ESMTPS id BEAA43847809 for ; Tue, 20 Jul 2021 23:03:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BEAA43847809 Received: from Plymouth (unknown [IPv6:2a02:390:9086:0:ff65:9830:391b:2038]) by lndn.lancelotsix.com (Postfix) with ESMTPSA id 3792782EC2; Tue, 20 Jul 2021 23:03:38 +0000 (UTC) Date: Wed, 21 Jul 2021 00:03:35 +0100 From: Lancelot SIX To: Simon Marchi Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 15/16] gdb: make cmd_list_element var an optional union Message-ID: <20210720230335.dcpfxbol2uwjre3b@Plymouth> References: <20210714045520.1623120-1-simon.marchi@polymtl.ca> <20210714045520.1623120-16-simon.marchi@polymtl.ca> <20210714120851.3pfew5pgcdp6ezn6@ubuntu.lan> <20210714171238.vzccwpurh2izbkps@ubuntu.lan> <20210714232112.wsn7pits6uuz3nf5@ubuntu.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.11 (lndn.lancelotsix.com [0.0.0.0]); Tue, 20 Jul 2021 23:03:38 +0000 (UTC) X-Spam-Status: No, score=-12.2 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: Tue, 20 Jul 2021 23:03:45 -0000 On Mon, Jul 19, 2021 at 03:52:30PM -0400, Simon Marchi wrote: > > I took a quick look, and that seems reasonable to me. I don't think it > > will conflict (in design) with my upcoming patch. My upcoming patch > > makes it so that some settings don't use the storage that is in > > cmd_list_element at all, but instead provide some getter/setter > > callback. This is to fix display of settings where the "source of > > truth" is not cmd_list_element::var. > > > > With my patch, the code at various places becomes, conceptually: > > > > if (cmd_list_element::var is set) > > use cmd_list_element::var > > else > > use the getter/setter > > > > I think it would be nice to make the first case (using > > cmd_list_element::var for storage) just a specific case of using the > > getter/setter. That is, when we create a setting that uses > > cmd_list_element::var for storage, we install a getter and a setter than > > get and set the value in cmd_list_element::var. But I haven't gotten > > there yet. You'll probably have some good ideas for achieving this :). > > > > But before posting that patch, I'd like to decide how the current patch > > series will end up, because that will very much affect how the next > > patch will look like. > > I realize I wasn't accurate here. The storage isn't actually in > cmd_list_element::var, cmd_list_element::var only contains pointers to > the storage. But the rest still stands. > > Simon Hi, I reworked a bit my previous implementation to see to what extent it can support what you are looking for. Short answer: it can (and it is completely transparent to the caller). In the previous draft version[1], I introduced a struct that groups together the var_type and the pointer to the actual data in order to abstract a setting (i.e. a value with a type that can be SET or SHOWn). This struct provides getters and setters in order to enforce that we cast the pointer to the adequate type. I have modified this data structure so we can register user-provided callbacks. If those functions are provided, the getter and setters will call them to generate retrieve the value instead of trying to dereference the pointer. For the moment, I just made sure that this compiles and I played with dummy settings in my local tree. It is not yet finalized nor fully operational because do_set_command will try to free the pointer returned by the getter when dealing with char* data. With user provided callbacks memory should probably not be managed at that level but by the getter / setter. Nothing worrying, you have a commit that plans to change the char* with std::string, this should solve the issue quite nicely. As for the previous version, this adds a significant amount of templated code that can make the implementation appear quite complex. I think I’ll wait for your series to land before I finalize this proposal (this is just a POC for the moment) unless you want to build on top of this approach to introduce user provided callbacks. Best, Lancelot. [1] https://sourceware.org/pipermail/gdb-patches/2021-July/180991.html --- >From 2217aeca7a963da7128311758fe3bbeddeaf43bd Mon Sep 17 00:00:00 2001 From: Lancelot SIX Date: Wed, 14 Jul 2021 22:30:14 +0000 Subject: [RFC PATCH v2] Add typesafe getter/setter for cmd_list_element.var cmd_list_element can contain a pointer to data that can be set and / or shown. This is achieved with a void* that points to the data that can be accessed, while the var_type (of type enum var_types) tells how to interpret the data pointed to. With this pattern, the user needs to know what the storage type associated with a given VAR_TYPES tag, and needs to do the proper casting. This looks something like: if (c->var_type == var_zuinteger) unsigned int v = *(unsigned int*)v->var_type; With this approach, it is easy to make an error. This patch aims at addressing the same problem as https://sourceware.org/pipermail/gdb-patches/2021-July/180920.html but uses a different approach. I am happy rebasing my work on top of the mentioned series if it is considered a good enough improvement (and I am happy dropping otherwise). This patch proposes to add an abstraction around the pair of the var_type and the void* pointer in order to prevent the user having to handle the cast. This is achieved by introducing the setting struct, and a set of templated member functions. The template parameter is the VAR_TYPES we want to the access the data for. The template ensures the return type of the get method and the parameter type of the set method matches the data type used to store the actual data. For example, instantiating the member functions with var_boolean will yield something similar to: bool get () const { gdb_assert (this->m_var_type == var_boolean); gdb_assert (this->m_var != nullptr); return *static_cast (this->m_var); } void set (bool var) { gdb_assert (this->m_var_type == var_boolean); gdb_assert (this->m_var != nullptr); *static_cast (this->m_var) = var; } With those new methods, the caller does not need to know the underlying storage nor does he need to perform the cast manually. This combined with the added dynamic checks (gdb_assert) makes this approach way safer in my opinion. Given that the processing of some options is common between VAR_TYPES, the templates can be instantiated with more than one VAR_TYPES. In such situation, all the template parameters need to describe options that share the same underlying storage type. Here is another example of what an instantiation looks like with 2 parameters: int get () { gdb_assert (this->m_var_type == var_integer || this->m_var_type == var_zinteger); gdb_assert (this->m_var != nullptr); return *static_cast (this->m_var); } void set (int v) { gdb_assert (this->m_var_type == var_integer || this->m_var_type == var_zinteger); gdb_assert (this->m_var != nullptr); *static_cast (this->m_var) = v; } With such abstractions available, the var_type and var members of the cmd_list_element are replaced with a setting instance. No user visible change is expected after this patch. Tested on GNU/Linux x86_64, with no regression noticed. --- gdb/auto-load.c | 2 +- gdb/cli/cli-cmds.c | 56 +++-- gdb/cli/cli-decode.c | 115 +++++----- gdb/cli/cli-decode.h | 6 +- gdb/cli/cli-setshow.c | 156 ++++++++----- gdb/command.h | 418 +++++++++++++++++++++++++++++++++++ gdb/guile/scm-param.c | 145 +++++++----- gdb/maint.c | 2 +- gdb/python/py-param.c | 16 +- gdb/python/python-internal.h | 2 +- gdb/python/python.c | 35 +-- gdb/remote.c | 2 +- 12 files changed, 741 insertions(+), 214 deletions(-) diff --git a/gdb/auto-load.c b/gdb/auto-load.c index 9cd70f174c3..033c448eff7 100644 --- a/gdb/auto-load.c +++ b/gdb/auto-load.c @@ -1408,7 +1408,7 @@ set_auto_load_cmd (const char *args, int from_tty) "otherwise check the auto-load sub-commands.")); for (list = *auto_load_set_cmdlist_get (); list != NULL; list = list->next) - if (list->var_type == var_boolean) + if (list->var.type () == var_boolean) { gdb_assert (list->type == set_cmd); do_set_command (args, from_tty, list); diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index 56ae12a0c19..3365a78860c 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -232,7 +232,7 @@ with_command_1 (const char *set_cmd_prefix, /*ignore_help_classes=*/ 1); gdb_assert (set_cmd != nullptr); - if (set_cmd->var == nullptr) + if (!set_cmd->var) error (_("Cannot use this setting with the \"with\" command")); std::string temp_value @@ -2090,29 +2090,29 @@ setting_cmd (const char *fnname, struct cmd_list_element *showlist, static struct value * value_from_setting (const cmd_list_element *cmd, struct gdbarch *gdbarch) { - switch (cmd->var_type) + switch (cmd->var.type ()) { case var_integer: - if (*(int *) cmd->var == INT_MAX) + if (cmd->var.get () == INT_MAX) return value_from_longest (builtin_type (gdbarch)->builtin_int, 0); else return value_from_longest (builtin_type (gdbarch)->builtin_int, - *(int *) cmd->var); + cmd->var.get ()); case var_zinteger: return value_from_longest (builtin_type (gdbarch)->builtin_int, - *(int *) cmd->var); + cmd->var.get ()); case var_boolean: return value_from_longest (builtin_type (gdbarch)->builtin_int, - *(bool *) cmd->var ? 1 : 0); + cmd->var.get () ? 1 : 0); case var_zuinteger_unlimited: return value_from_longest (builtin_type (gdbarch)->builtin_int, - *(int *) cmd->var); + cmd->var.get ()); case var_auto_boolean: { int val; - switch (*(enum auto_boolean*) cmd->var) + switch (cmd->var.get ()) { case AUTO_BOOLEAN_TRUE: val = 1; @@ -2130,24 +2130,34 @@ value_from_setting (const cmd_list_element *cmd, struct gdbarch *gdbarch) val); } case var_uinteger: - if (*(unsigned int *) cmd->var == UINT_MAX) + if (cmd->var.get () == UINT_MAX) return value_from_ulongest (builtin_type (gdbarch)->builtin_unsigned_int, 0); else return value_from_ulongest (builtin_type (gdbarch)->builtin_unsigned_int, - *(unsigned int *) cmd->var); + cmd->var.get ()); case var_zuinteger: return value_from_ulongest (builtin_type (gdbarch)->builtin_unsigned_int, - *(unsigned int *) cmd->var); + cmd->var.get ()); case var_string: case var_string_noescape: case var_optional_filename: case var_filename: case var_enum: - if (*(char **) cmd->var) - return value_cstring (*(char **) cmd->var, strlen (*(char **) cmd->var), - builtin_type (gdbarch)->builtin_char); + if (cmd->var) + { + const char * var; + if (cmd->var.type () == var_enum) + var = cmd->var.get (); + else + var = cmd->var.get (); + return value_cstring (var, strlen (var), + builtin_type (gdbarch)->builtin_char); + } else return value_cstring ("", 1, builtin_type (gdbarch)->builtin_char); @@ -2186,7 +2196,7 @@ gdb_maint_setting_internal_fn (struct gdbarch *gdbarch, static struct value * str_value_from_setting (const cmd_list_element *cmd, struct gdbarch *gdbarch) { - switch (cmd->var_type) + switch (cmd->var.type ()) { case var_integer: case var_zinteger: @@ -2211,9 +2221,19 @@ str_value_from_setting (const cmd_list_element *cmd, struct gdbarch *gdbarch) as this function handle some characters specially, e.g. by escaping quotes. So, we directly use the cmd->var string value, similarly to the value_from_setting code for these cases. */ - if (*(char **) cmd->var) - return value_cstring (*(char **) cmd->var, strlen (*(char **) cmd->var), - builtin_type (gdbarch)->builtin_char); + if (cmd->var) + { + const char *var; + if (cmd->var.type () == var_enum) + var = cmd->var.get (); + else + var = cmd->var.get (); + return value_cstring (var, strlen (var), + builtin_type (gdbarch)->builtin_char); + } else return value_cstring ("", 1, builtin_type (gdbarch)->builtin_char); diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c index 633a3ad9309..f15747cf337 100644 --- a/gdb/cli/cli-decode.c +++ b/gdb/cli/cli-decode.c @@ -481,12 +481,12 @@ empty_sfunc (const char *args, int from_tty, struct cmd_list_element *c) VAR is address of the variable being controlled by this command. DOC is the documentation string. */ +template static struct cmd_list_element * add_set_or_show_cmd (const char *name, enum cmd_types type, enum command_class theclass, - var_types var_type, - void *var, + typename detail::var_types_storage::type *var, const char *doc, struct cmd_list_element **list) { @@ -494,8 +494,7 @@ add_set_or_show_cmd (const char *name, gdb_assert (type == set_cmd || type == show_cmd); c->type = type; - c->var_type = var_type; - c->var = var; + c->var.set_p (var); /* This needs to be something besides NULL so that this isn't treated as a help class. */ set_cmd_sfunc (c, empty_sfunc); @@ -511,10 +510,11 @@ add_set_or_show_cmd (const char *name, Return the newly created set and show commands. */ +template static set_show_commands add_setshow_cmd_full (const char *name, enum command_class theclass, - var_types var_type, void *var, + typename detail::var_types_storage::type *var, const char *set_doc, const char *show_doc, const char *help_doc, cmd_const_sfunc_ftype *set_func, @@ -537,15 +537,15 @@ add_setshow_cmd_full (const char *name, full_set_doc = xstrdup (set_doc); full_show_doc = xstrdup (show_doc); } - set = add_set_or_show_cmd (name, set_cmd, theclass, var_type, var, - full_set_doc, set_list); + set = add_set_or_show_cmd (name, set_cmd, theclass, var, + full_set_doc, set_list); set->doc_allocated = 1; if (set_func != NULL) set_cmd_sfunc (set, set_func); - show = add_set_or_show_cmd (name, show_cmd, theclass, var_type, var, - full_show_doc, show_list); + show = add_set_or_show_cmd (name, show_cmd, theclass, var, + full_show_doc, show_list); show->doc_allocated = 1; show->show_value_func = show_func; /* Disable the default symbol completer. Doesn't make much sense @@ -574,10 +574,10 @@ add_setshow_enum_cmd (const char *name, struct cmd_list_element **show_list) { set_show_commands commands - = add_setshow_cmd_full (name, theclass, var_enum, var, - set_doc, show_doc, help_doc, - set_func, show_func, - set_list, show_list); + = add_setshow_cmd_full (name, theclass, var, + set_doc, show_doc, help_doc, + set_func, show_func, + set_list, show_list); commands.set->enums = enumlist; return commands; } @@ -602,10 +602,10 @@ add_setshow_auto_boolean_cmd (const char *name, struct cmd_list_element **show_list) { set_show_commands commands - = add_setshow_cmd_full (name, theclass, var_auto_boolean, var, - set_doc, show_doc, help_doc, - set_func, show_func, - set_list, show_list); + = add_setshow_cmd_full (name, theclass, var, + set_doc, show_doc, help_doc, + set_func, show_func, + set_list, show_list); commands.set->enums = auto_boolean_enums; @@ -631,10 +631,10 @@ add_setshow_boolean_cmd (const char *name, enum command_class theclass, bool *va struct cmd_list_element **show_list) { set_show_commands commands - = add_setshow_cmd_full (name, theclass, var_boolean, var, - set_doc, show_doc, help_doc, - set_func, show_func, - set_list, show_list); + = add_setshow_cmd_full (name, theclass, var, + set_doc, show_doc, help_doc, + set_func, show_func, + set_list, show_list); commands.set->enums = boolean_enums; @@ -655,10 +655,10 @@ add_setshow_filename_cmd (const char *name, enum command_class theclass, struct cmd_list_element **show_list) { set_show_commands commands - = add_setshow_cmd_full (name, theclass, var_filename, var, - set_doc, show_doc, help_doc, - set_func, show_func, - set_list, show_list); + = add_setshow_cmd_full (name, theclass, var, + set_doc, show_doc, help_doc, + set_func, show_func, + set_list, show_list); set_cmd_completer (commands.set, filename_completer); @@ -679,10 +679,10 @@ add_setshow_string_cmd (const char *name, enum command_class theclass, struct cmd_list_element **show_list) { set_show_commands commands - = add_setshow_cmd_full (name, theclass, var_string, var, - set_doc, show_doc, help_doc, - set_func, show_func, - set_list, show_list); + = add_setshow_cmd_full (name, theclass, var, + set_doc, show_doc, help_doc, + set_func, show_func, + set_list, show_list); /* Disable the default symbol completer. */ set_cmd_completer (commands.set, nullptr); @@ -704,10 +704,10 @@ add_setshow_string_noescape_cmd (const char *name, enum command_class theclass, struct cmd_list_element **show_list) { set_show_commands commands - = add_setshow_cmd_full (name, theclass, var_string_noescape, var, - set_doc, show_doc, help_doc, - set_func, show_func, - set_list, show_list); + = add_setshow_cmd_full (name, theclass, var, + set_doc, show_doc, help_doc, + set_func, show_func, + set_list, show_list); /* Disable the default symbol completer. */ set_cmd_completer (commands.set, nullptr); @@ -729,10 +729,10 @@ add_setshow_optional_filename_cmd (const char *name, enum command_class theclass struct cmd_list_element **show_list) { set_show_commands commands - = add_setshow_cmd_full (name, theclass, var_optional_filename, var, - set_doc, show_doc, help_doc, - set_func, show_func, - set_list, show_list); + = add_setshow_cmd_full (name, theclass, var, + set_doc, show_doc, help_doc, + set_func, show_func, + set_list, show_list); set_cmd_completer (commands.set, filename_completer); @@ -773,10 +773,10 @@ add_setshow_integer_cmd (const char *name, enum command_class theclass, struct cmd_list_element **show_list) { set_show_commands commands - = add_setshow_cmd_full (name, theclass, var_integer, var, - set_doc, show_doc, help_doc, - set_func, show_func, - set_list, show_list); + = add_setshow_cmd_full (name, theclass, var, + set_doc, show_doc, help_doc, + set_func, show_func, + set_list, show_list); set_cmd_completer (commands.set, integer_unlimited_completer); @@ -799,10 +799,10 @@ add_setshow_uinteger_cmd (const char *name, enum command_class theclass, struct cmd_list_element **show_list) { set_show_commands commands - = add_setshow_cmd_full (name, theclass, var_uinteger, var, - set_doc, show_doc, help_doc, - set_func, show_func, - set_list, show_list); + = add_setshow_cmd_full (name, theclass, var, + set_doc, show_doc, help_doc, + set_func, show_func, + set_list, show_list); set_cmd_completer (commands.set, integer_unlimited_completer); @@ -824,10 +824,10 @@ add_setshow_zinteger_cmd (const char *name, enum command_class theclass, struct cmd_list_element **set_list, struct cmd_list_element **show_list) { - return add_setshow_cmd_full (name, theclass, var_zinteger, var, - set_doc, show_doc, help_doc, - set_func, show_func, - set_list, show_list); + return add_setshow_cmd_full (name, theclass, var, + set_doc, show_doc, help_doc, + set_func, show_func, + set_list, show_list); } set_show_commands @@ -843,10 +843,11 @@ add_setshow_zuinteger_unlimited_cmd (const char *name, struct cmd_list_element **show_list) { set_show_commands commands - = add_setshow_cmd_full (name, theclass, var_zuinteger_unlimited, var, - set_doc, show_doc, help_doc, - set_func, show_func, - set_list, show_list); + = add_setshow_cmd_full (name, theclass, var, + set_doc, show_doc, + help_doc, set_func, + show_func, set_list, + show_list); set_cmd_completer (commands.set, integer_unlimited_completer); @@ -868,10 +869,10 @@ add_setshow_zuinteger_cmd (const char *name, enum command_class theclass, struct cmd_list_element **set_list, struct cmd_list_element **show_list) { - return add_setshow_cmd_full (name, theclass, var_zuinteger, var, - set_doc, show_doc, help_doc, - set_func, show_func, - set_list, show_list); + return add_setshow_cmd_full (name, theclass, var, + set_doc, show_doc, help_doc, + set_func, show_func, + set_list, show_list); } /* Remove the command named NAME from the command list. Return the diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h index 241535ae5b5..6799f965371 100644 --- a/gdb/cli/cli-decode.h +++ b/gdb/cli/cli-decode.h @@ -55,7 +55,6 @@ struct cmd_list_element allow_unknown (0), abbrev_flag (0), type (not_set_cmd), - var_type (var_boolean), doc (doc_) { memset (&function, 0, sizeof (function)); @@ -160,9 +159,6 @@ struct cmd_list_element or "show"). */ ENUM_BITFIELD (cmd_types) type : 2; - /* What kind of variable is *VAR? */ - ENUM_BITFIELD (var_types) var_type : 4; - /* Function definition of this command. NULL for command class names and for help topics that are not really commands. NOTE: cagney/2002-02-02: This function signature is evolving. For @@ -230,7 +226,7 @@ struct cmd_list_element /* Pointer to variable affected by "set" and "show". Doesn't matter if type is not_set. */ - void *var = nullptr; + setting var; /* Pointer to NULL terminated list of enumerated values (like argv). */ diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c index 5fd5fd15c6a..883ac30598e 100644 --- a/gdb/cli/cli-setshow.c +++ b/gdb/cli/cli-setshow.c @@ -133,7 +133,7 @@ deprecated_show_value_hack (struct ui_file *ignore_file, /* Print doc minus "Show " at start. Tell print_doc_line that this is for a 'show value' prefix. */ print_doc_line (gdb_stdout, c->doc + 5, true); - switch (c->var_type) + switch (c->var.type ()) { case var_string: case var_string_noescape: @@ -312,7 +312,7 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) if (arg == NULL) arg = ""; - switch (c->var_type) + switch (c->var.type ()) { case var_string: { @@ -353,11 +353,12 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) *q++ = '\0'; newobj = (char *) xrealloc (newobj, q - newobj); - if (*(char **) c->var == NULL - || strcmp (*(char **) c->var, newobj) != 0) + auto const var = c->var.get (); + if (var == nullptr + || strcmp (var, newobj) != 0) { - xfree (*(char **) c->var); - *(char **) c->var = newobj; + xfree (var); + c->var.set (newobj); option_changed = 1; } @@ -366,13 +367,17 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) } break; case var_string_noescape: - if (*(char **) c->var == NULL || strcmp (*(char **) c->var, arg) != 0) - { - xfree (*(char **) c->var); - *(char **) c->var = xstrdup (arg); + { + auto const var = c->var.get (); + if (var == nullptr + || strcmp (var, arg) != 0) + { + xfree (var); + c->var.set (xstrdup (arg)); - option_changed = 1; - } + option_changed = 1; + } + } break; case var_filename: if (*arg == '\0') @@ -381,6 +386,7 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) case var_optional_filename: { char *val = NULL; + auto const var = c->var.get (); if (*arg != '\0') { @@ -398,11 +404,11 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) else val = xstrdup (""); - if (*(char **) c->var == NULL - || strcmp (*(char **) c->var, val) != 0) + if (var == nullptr + || strcmp (var, val) != 0) { - xfree (*(char **) c->var); - *(char **) c->var = val; + xfree (val); + c->var.set (val); option_changed = 1; } @@ -416,9 +422,9 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) if (val < 0) error (_("\"on\" or \"off\" expected.")); - if (val != *(bool *) c->var) + if (val != c->var.get ()) { - *(bool *) c->var = val; + c->var.set (val); option_changed = 1; } @@ -428,9 +434,9 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) { enum auto_boolean val = parse_auto_binary_operation (arg); - if (*(enum auto_boolean *) c->var != val) + if (c->var.get () != val) { - *(enum auto_boolean *) c->var = val; + c->var.set (val); option_changed = 1; } @@ -439,11 +445,11 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) case var_uinteger: case var_zuinteger: { - unsigned int val = parse_cli_var_uinteger (c->var_type, &arg, true); + unsigned int val = parse_cli_var_uinteger (c->var.type (), &arg, true); - if (*(unsigned int *) c->var != val) + if (c->var.get () != val) { - *(unsigned int *) c->var = val; + c->var.set (val); option_changed = 1; } @@ -456,35 +462,35 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) if (*arg == '\0') { - if (c->var_type == var_integer) + if (c->var.type () == var_integer) error_no_arg (_("integer to set it to, or \"unlimited\".")); else error_no_arg (_("integer to set it to.")); } - if (c->var_type == var_integer && is_unlimited_literal (&arg, true)) + if (c->var.type () == var_integer && is_unlimited_literal (&arg, true)) val = 0; else val = parse_and_eval_long (arg); - if (val == 0 && c->var_type == var_integer) + if (val == 0 && c->var.type () == var_integer) val = INT_MAX; else if (val < INT_MIN /* For var_integer, don't let the user set the value to INT_MAX directly, as that exposes an implementation detail to the user interface. */ - || (c->var_type == var_integer && val >= INT_MAX) - || (c->var_type == var_zinteger && val > INT_MAX)) + || (c->var.type () == var_integer && val >= INT_MAX) + || (c->var.type () == var_zinteger && val > INT_MAX)) error (_("integer %s out of range"), plongest (val)); - if (*(int *) c->var != val) + if (c->var.get () != val) { - *(int *) c->var = val; + c->var.set (val); option_changed = 1; } - break; } + break; case var_enum: { const char *end_arg = arg; @@ -495,9 +501,9 @@ 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 (*(const char **) c->var != match) + if (c->var.get () != match) { - *(const char **) c->var = match; + c->var.set (match); option_changed = 1; } @@ -507,9 +513,9 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) { int val = parse_cli_var_zuinteger_unlimited (&arg, true); - if (*(int *) c->var != val) + if (c->var.get () != val) { - *(int *) c->var = val; + c->var.set (val); option_changed = 1; } } @@ -577,25 +583,35 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) xfree (cmds); - switch (c->var_type) + switch (c->var.type ()) { case var_string: case var_string_noescape: case var_filename: case var_optional_filename: case var_enum: - gdb::observers::command_param_changed.notify (name, *(char **) c->var); + { + const char *var; + if (c->var.type () == var_enum) + var = c->var.get (); + else + var = c->var.get (); + gdb::observers::command_param_changed.notify (name, var); + } break; case var_boolean: { - const char *opt = *(bool *) c->var ? "on" : "off"; + const char *opt = c->var.get () ? "on" : "off"; gdb::observers::command_param_changed.notify (name, opt); } break; case var_auto_boolean: { - const char *s = auto_boolean_enums[*(enum auto_boolean *) c->var]; + const char *s = auto_boolean_enums[c->var.get ()]; gdb::observers::command_param_changed.notify (name, s); } @@ -605,7 +621,7 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) { char s[64]; - xsnprintf (s, sizeof s, "%u", *(unsigned int *) c->var); + xsnprintf (s, sizeof s, "%u", c->var.get ()); gdb::observers::command_param_changed.notify (name, s); } break; @@ -615,7 +631,8 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c) { char s[64]; - xsnprintf (s, sizeof s, "%d", *(int *) c->var); + xsnprintf (s, sizeof s, "%d", + c->var.get ()); gdb::observers::command_param_changed.notify (name, s); } break; @@ -631,24 +648,36 @@ get_setshow_command_value_string (const cmd_list_element *c) { string_file stb; - switch (c->var_type) + switch (c->var.type ()) { case var_string: - if (*(char **) c->var) - stb.putstr (*(char **) c->var, '"'); + { + auto const var = c->var.get (); + if (var != nullptr) + stb.putstr (var, '"'); + } break; case var_string_noescape: case var_optional_filename: case var_filename: case var_enum: - if (*(char **) c->var) - stb.puts (*(char **) c->var); + { + const char *var; + if (c->var.type () == var_enum) + var = c->var.get (); + else + var = c->var.get (); + if (var != nullptr) + stb.puts (var); + } break; case var_boolean: - stb.puts (*(bool *) c->var ? "on" : "off"); + stb.puts (c->var.get () ? "on" : "off"); break; case var_auto_boolean: - switch (*(enum auto_boolean*) c->var) + switch (c->var.get ()) { case AUTO_BOOLEAN_TRUE: stb.puts ("on"); @@ -666,26 +695,33 @@ get_setshow_command_value_string (const cmd_list_element *c) break; case var_uinteger: case var_zuinteger: - if (c->var_type == var_uinteger - && *(unsigned int *) c->var == UINT_MAX) - stb.puts ("unlimited"); - else - stb.printf ("%u", *(unsigned int *) c->var); + { + auto const var = c->var.get (); + if (c->var.type () == var_uinteger + && var == UINT_MAX) + stb.puts ("unlimited"); + else + stb.printf ("%u", var); + } break; case var_integer: case var_zinteger: - if (c->var_type == var_integer - && *(int *) c->var == INT_MAX) - stb.puts ("unlimited"); - else - stb.printf ("%d", *(int *) c->var); + { + const auto var = c->var.get (); + if (c->var.type () == var_integer + && var == INT_MAX) + stb.puts ("unlimited"); + else + stb.printf ("%d", var); + } break; case var_zuinteger_unlimited: { - if (*(int *) c->var == -1) + auto const var = c->var.get (); + if (var == -1) stb.puts ("unlimited"); else - stb.printf ("%d", *(int *) c->var); + stb.printf ("%d", var); } break; default: diff --git a/gdb/command.h b/gdb/command.h index 711cbdcf43e..489ca9b1b65 100644 --- a/gdb/command.h +++ b/gdb/command.h @@ -125,6 +125,424 @@ typedef enum var_types } var_types; + +template +struct accessor_sigs +{ + using getter = T (*) (); + using setter = void (*) (T); +}; + + +union setting_getter +{ + typename accessor_sigs::getter get_bool; + typename accessor_sigs::getter get_int; + typename accessor_sigs::getter get_uint; + typename accessor_sigs::getter get_auto_boolean; + typename accessor_sigs::getter get_char_p; + typename accessor_sigs::getter get_const_char_p; +}; + +union setting_setter +{ + typename accessor_sigs::setter set_bool; + typename accessor_sigs::setter set_int; + typename accessor_sigs::setter set_uint; + typename accessor_sigs::setter set_auto_boolean; + typename accessor_sigs::setter set_char_p; + typename accessor_sigs::setter set_const_char_p; +}; + +namespace detail +{ + template + struct accessor_helper; + + template<> + struct accessor_helper + { + static accessor_sigs::getter &getter(setting_getter & getters) + { + return getters.get_bool; + } + + static accessor_sigs::setter &setter(setting_setter & setters) + { + return setters.set_bool; + } + }; + + template<> + struct accessor_helper + { + static accessor_sigs::getter &getter(setting_getter & getters) + { + return getters.get_int; + } + + static accessor_sigs::setter &setter(setting_setter & setters) + { + return setters.set_int; + } + }; + + template<> + struct accessor_helper + { + static accessor_sigs::getter &getter(setting_getter & getters) + { + return getters.get_uint; + } + + static accessor_sigs::setter &setter(setting_setter & setters) + { + return setters.set_uint; + } + }; + + template<> + struct accessor_helper + { + static accessor_sigs::getter &getter(setting_getter & getters) + { + return getters.get_auto_boolean; + } + + static accessor_sigs::setter &setter(setting_setter & setters) + { + return setters.set_auto_boolean; + } + }; + + template<> + struct accessor_helper + { + static accessor_sigs::getter &getter(setting_getter & getters) + { + return getters.get_char_p; + } + + static accessor_sigs::setter &setter(setting_setter & setters) + { + return setters.set_char_p; + } + }; + + template<> + struct accessor_helper + { + static accessor_sigs::getter &getter(setting_getter & getters) + { + return getters.get_const_char_p; + } + + static accessor_sigs::setter &setter(setting_setter & setters) + { + return setters.set_const_char_p; + } + }; + + /* Helper classes used to associate a storage type for each possible + var_type. */ + + template + struct var_types_storage; + + template<> + struct var_types_storage + { + using type = bool; + + }; + + template<> + struct var_types_storage + { + using type = enum auto_boolean; + }; + + template<> + struct var_types_storage + { + using type = unsigned int; + }; + + template<> + struct var_types_storage + { + using type = int; + }; + + template<> + struct var_types_storage + { + using type = char *; + }; + + template<> + struct var_types_storage + { + using type = char *; + }; + + template<> + struct var_types_storage + { + using type = char *; + }; + + template<> + struct var_types_storage + { + using type = char *; + }; + + template<> + struct var_types_storage + { + using type = int; + }; + + template<> + struct var_types_storage + { + using type = unsigned int; + }; + + template<> + struct var_types_storage + { + using type = int; + }; + + template<> + struct var_types_storage + { + using type = const char *; + }; + + /* Helper class used to check if multiple var_types are represented + using the same underlying type. This class is meant to be instantiated + using any number of var_types, and will be used to assess common properties + of the underlying storage type. + + Each template instantiating will define the following static members: + - value: True if and only if all the var_types are stored on the same + underlying storage type. + - covers_type (var_types t): True if and only if the parameter T is one + the templates parameter. + - type: Type alias of the underlying type if value is true, unspecified + otherwise. + */ + + template + struct var_types_have_same_storage; + + /* Specialization of var_types_have_same_storage when instantiated with only 1 + template parameter. */ + template + struct var_types_have_same_storage + { + static constexpr bool value = true; + + using type = typename var_types_storage::type; + + static constexpr bool covers_type (var_types t) + { + return t == T; + } + }; + + /* Specialization of var_types_have_same_storage when instantiated with exactly + 2 template parameters. */ + template + struct var_types_have_same_storage + { + static constexpr bool value + = std::is_same::type, + typename var_types_storage::type>::value; + + using type = typename var_types_storage::type; + + static constexpr bool covers_type (var_types t) + { + return var_types_have_same_storage::covers_type (t) + || var_types_have_same_storage::covers_type (t); + } + }; + + /* Specialization of var_types_have_same_storage when instantiated with 3 or more + template parameters. */ + template + struct var_types_have_same_storage + { + static constexpr bool value + = var_types_have_same_storage::value + && var_types_have_same_storage::value; + + using type = typename var_types_storage::type; + + static constexpr bool covers_type (var_types t) + { + return var_types_have_same_storage::covers_type (t) + || var_types_have_same_storage::covers_type (t); + } + }; +} /* namespace detail */ + +/* Abstraction that contains access to data that can be set or shown. + + The underlying data can be of an VAR_TYPES type. */ +struct base_setting_wrapper +{ + /* Access the type of the current var. */ + var_types type () const + { + return m_var_type; + } + + /* Return the current value (by pointer). + + The expected template parameter is the VAR_TYPES of the current instance. + This is enforced with a runtime check. + + If multiple template parameters are given, check that the underlying + pointer type associated with each parameter are the same. */ + template>> + typename detail::var_types_have_same_storage::type const *get_p() const + { + gdb_assert (detail::var_types_have_same_storage::covers_type + (this->m_var_type)); + gdb_assert (!empty ()); + // TODO + //gdb_assert (m_getter == nullptr && m_setter == nullptr); + + return static_cast< + typename detail::var_types_have_same_storage::type const *> + (this->m_var); + } + + /* Return the current value. + + See get_p for discussion on the return type. */ + template + typename detail::var_types_have_same_storage::type get() const + { + gdb_assert (detail::var_types_have_same_storage::covers_type + (this->m_var_type)); + auto getter = detail::accessor_helper< + typename detail::var_types_have_same_storage::type>::getter + (const_cast (this)->m_getter); + + if (getter != nullptr) + return (*getter) (); + else + { + gdb_assert (!empty ()); + return *get_p (); + } + } + + /* Sets the value V to the underlying buffer. If we have a user-provided + setter, use it to set the setting, otherwise set it to the internally + referenced buffer. + + If one template argument is given, it must be the VAR_TYPE of the current + instance. This is enforced at runtime. + + If multiple template parameters are given, they must all share the same + underlying storage type (this is checked at compile time), and THIS must + be of the type of one of the template parameters (this is checked at + runtime). */ + template>> + void set(typename detail::var_types_have_same_storage::type v) + { + /* Check that the current instance is of one of the supported types for + this instantiation. */ + gdb_assert (detail::var_types_have_same_storage::covers_type + (this->m_var_type)); + + auto setter = detail::accessor_helper< + typename detail::var_types_have_same_storage::type>::setter (this->m_setter); + + if (setter != nullptr) + { + (*setter) (v); + } + else + { + gdb_assert (!empty ()); + *static_cast::type *> + (this->m_var) = v; + } + } + + template + void set_accessors(typename accessor_sigs::type>::setter setter, + typename accessor_sigs::type>::getter getter) + { + m_var_type = T; + detail::accessor_helper::type>::setter (this->m_setter) = setter; + detail::accessor_helper::type>::getter (this->m_getter) = getter; + } + + /* A setting is valid (can be evaluated to true) if it contains user provided + getter and setter or has a pointer set to a setting. */ + explicit operator bool() const + { + // TODO refactor that to not rely on one of the options + return (m_getter.get_bool != nullptr && m_setter.set_bool != nullptr) || !this->empty(); + } + +protected: + /* The type of the variable M_VAR is pointing to. If M_VAR is nullptr, + M_VAR_TYPE is ignored. */ + var_types m_var_type { var_boolean }; + + /* Pointer to the enclosed variable. The type of the variable is encoded + in M_VAR_TYPE. Can be nullptr. */ + void *m_var { nullptr }; + + /* Pointer to a user provided getter. */ + union setting_getter m_getter { .get_bool { nullptr } }; + + /* Pointer to a user provided setter. */ + union setting_setter m_setter { .set_bool { nullptr } }; + + /* Indicates if the current instance has a underlying buffer. */ + bool empty () const + { + return m_var == nullptr;; + } + +}; + + +/* A augmented version of base_setting_wrapper with additional methods to set the + underlying buffer and declare the var_type. */ +struct setting final: base_setting_wrapper +{ + /* Set the type of the current variable. */ + void set_type (var_types type) + { + gdb_assert (empty ()); + this->m_var_type = type; + } + + /* Update the pointer to the underlying variable referenced by this + instance. */ + template + void set_p (typename detail::var_types_storage::type *v) + { + this->set_type (T); + this->m_var = static_cast (v); + } +}; + /* This structure records one command'd definition. */ struct cmd_list_element; diff --git a/gdb/guile/scm-param.c b/gdb/guile/scm-param.c index c052d04974a..83fb3a27072 100644 --- a/gdb/guile/scm-param.c +++ b/gdb/guile/scm-param.c @@ -113,6 +113,19 @@ struct param_smob SCM containing_scm; }; +/* Wraps a base_setting_wrapper around an existing param_smob. This abstraction + is used + to manipulate the value in S->VALUE in a type safe manner the same way as if + it was referenced by a setting. */ +struct setting_wrapper final: base_setting_wrapper +{ + explicit setting_wrapper (param_smob *s) + { + m_var_type = s->type; + m_var = &s->value; + } +}; + static const char param_smob_name[] = "gdb:parameter"; /* The tag Guile knows the param smob by. */ @@ -133,8 +146,7 @@ static SCM unlimited_keyword; static int pascm_is_valid (param_smob *); static const char *pascm_param_type_name (enum var_types type); -static SCM pascm_param_value (enum var_types type, void *var, - int arg_pos, const char *func_name); +static SCM pascm_param_value (base_setting_wrapper const &c, int arg_pos, const char *func_name); /* Administrivia for parameter smobs. */ @@ -151,10 +163,9 @@ pascm_print_param_smob (SCM self, SCM port, scm_print_state *pstate) if (! pascm_is_valid (p_smob)) scm_puts (" {invalid}", port); - gdbscm_printf (port, " %s ", pascm_param_type_name (p_smob->type)); + gdbscm_printf (port, " %s ", pascm_param_type_name (p_smob->type )); - value = pascm_param_value (p_smob->type, &p_smob->value, - GDBSCM_ARG_NONE, NULL); + value = pascm_param_value (setting_wrapper (p_smob), GDBSCM_ARG_NONE, NULL); scm_display (value, port); scm_puts (">", port); @@ -444,7 +455,7 @@ add_setshow_generic (enum var_types param_type, enum command_class cmd_class, show_doc, help_doc, set_func, show_func, set_list, show_list); /* Initialize the value, just in case. */ - self->value.cstringval = self->enumeration[0]; + setting_wrapper (self).set (self->enumeration[0]); break; default: @@ -566,14 +577,13 @@ pascm_param_type_name (enum var_types param_type) If TYPE is not supported, then a object is returned. */ static SCM -pascm_param_value (enum var_types type, void *var, - int arg_pos, const char *func_name) +pascm_param_value (base_setting_wrapper const &var, int arg_pos, const char *func_name) { /* Note: We *could* support var_integer here in case someone is trying to get the value of a Python-created parameter (which is the only place that still supports var_integer). To further discourage its use we do not. */ - switch (type) + switch (var.type ()) { case var_string: case var_string_noescape: @@ -581,16 +591,23 @@ pascm_param_value (enum var_types type, void *var, case var_filename: case var_enum: { - const char *str = *(char **) var; + const char *str; + if (var.type () == var_enum) + str = var.get (); + else + str = var.get (); - if (str == NULL) + if (str == nullptr) str = ""; return gdbscm_scm_from_host_string (str, strlen (str)); } case var_boolean: { - if (* (bool *) var) + if (var.get ()) return SCM_BOOL_T; else return SCM_BOOL_F; @@ -598,7 +615,7 @@ pascm_param_value (enum var_types type, void *var, case var_auto_boolean: { - enum auto_boolean ab = * (enum auto_boolean *) var; + enum auto_boolean ab = var.get (); if (ab == AUTO_BOOLEAN_TRUE) return SCM_BOOL_T; @@ -609,67 +626,84 @@ pascm_param_value (enum var_types type, void *var, } case var_zuinteger_unlimited: - if (* (int *) var == -1) + if (var.get () == -1) return unlimited_keyword; - gdb_assert (* (int *) var >= 0); + gdb_assert (var.get () >= 0); /* Fall through. */ case var_zinteger: - return scm_from_int (* (int *) var); + return scm_from_int (var.get ()); case var_uinteger: - if (* (unsigned int *) var == UINT_MAX) + if (var.get ()== UINT_MAX) return unlimited_keyword; /* Fall through. */ case var_zuinteger: - return scm_from_uint (* (unsigned int *) var); + return scm_from_uint (var.get ()); default: break; } return gdbscm_make_out_of_range_error (func_name, arg_pos, - scm_from_int (type), + scm_from_int (var.type ()), _("program error: unhandled type")); } -/* Set the value of a parameter of type TYPE in VAR from VALUE. +/* Set the value of a parameter of type P_SMOB->TYPE in P_SMOB->VAR from VALUE. ENUMERATION is the list of enum values for enum parameters, otherwise NULL. Throws a Scheme exception if VALUE_SCM is invalid for TYPE. */ static void -pascm_set_param_value_x (enum var_types type, union pascm_variable *var, +pascm_set_param_value_x (param_smob *p_smob, const char * const *enumeration, SCM value, int arg_pos, const char *func_name) { - switch (type) + setting_wrapper var { p_smob }; + + switch (var.type ()) { case var_string: case var_string_noescape: case var_optional_filename: case var_filename: SCM_ASSERT_TYPE (scm_is_string (value) - || (type != var_filename + || (var.type () != var_filename && gdbscm_is_false (value)), value, arg_pos, func_name, _("string or #f for non-PARAM_FILENAME parameters")); if (gdbscm_is_false (value)) { - xfree (var->stringval); - if (type == var_optional_filename) - var->stringval = xstrdup (""); + xfree (var.get ()); + if (var.type () == var_optional_filename) + var.set (xstrdup ("")); else - var->stringval = NULL; + var.set (nullptr); } else { SCM exception; gdb::unique_xmalloc_ptr string - = gdbscm_scm_to_host_string (value, NULL, &exception); - if (string == NULL) + = gdbscm_scm_to_host_string (value, nullptr, &exception); + if (string == nullptr) gdbscm_throw (exception); - xfree (var->stringval); - var->stringval = string.release (); + xfree (var.get ()); + var.set (string.release ()); } break; @@ -681,27 +715,27 @@ pascm_set_param_value_x (enum var_types type, union pascm_variable *var, SCM_ASSERT_TYPE (scm_is_string (value), value, arg_pos, func_name, _("string")); gdb::unique_xmalloc_ptr str - = gdbscm_scm_to_host_string (value, NULL, &exception); - if (str == NULL) + = gdbscm_scm_to_host_string (value, nullptr, &exception); + if (str == nullptr) gdbscm_throw (exception); for (i = 0; enumeration[i]; ++i) { if (strcmp (enumeration[i], str.get ()) == 0) break; } - if (enumeration[i] == NULL) + if (enumeration[i] == nullptr) { gdbscm_out_of_range_error (func_name, arg_pos, value, _("not member of enumeration")); } - var->cstringval = enumeration[i]; + var.set (enumeration[i]); break; } case var_boolean: SCM_ASSERT_TYPE (gdbscm_is_bool (value), value, arg_pos, func_name, _("boolean")); - var->boolval = gdbscm_is_true (value); + var.set (gdbscm_is_true (value)); break; case var_auto_boolean: @@ -710,19 +744,19 @@ pascm_set_param_value_x (enum var_types type, union pascm_variable *var, value, arg_pos, func_name, _("boolean or #:auto")); if (scm_is_eq (value, auto_keyword)) - var->autoboolval = AUTO_BOOLEAN_AUTO; + var.set (AUTO_BOOLEAN_AUTO); else if (gdbscm_is_true (value)) - var->autoboolval = AUTO_BOOLEAN_TRUE; + var.set (AUTO_BOOLEAN_TRUE); else - var->autoboolval = AUTO_BOOLEAN_FALSE; + var.set (AUTO_BOOLEAN_FALSE); break; case var_zinteger: case var_uinteger: case var_zuinteger: case var_zuinteger_unlimited: - if (type == var_uinteger - || type == var_zuinteger_unlimited) + if (var.type () == var_uinteger + || var.type () == var_zuinteger_unlimited) { SCM_ASSERT_TYPE (gdbscm_is_bool (value) || scm_is_eq (value, unlimited_keyword), @@ -730,10 +764,10 @@ pascm_set_param_value_x (enum var_types type, union pascm_variable *var, _("integer or #:unlimited")); if (scm_is_eq (value, unlimited_keyword)) { - if (type == var_uinteger) - var->intval = UINT_MAX; + if (var.type () == var_uinteger) + var.set (UINT_MAX); else - var->intval = -1; + var.set (-1); break; } } @@ -743,25 +777,25 @@ pascm_set_param_value_x (enum var_types type, union pascm_variable *var, _("integer")); } - if (type == var_uinteger - || type == var_zuinteger) + if (var.type () == var_uinteger + || var.type () == var_zuinteger) { unsigned int u = scm_to_uint (value); - if (type == var_uinteger && u == 0) + if (var.type () == var_uinteger && u == 0) u = UINT_MAX; - var->uintval = u; + var.set (u); } else { int i = scm_to_int (value); - if (type == var_zuinteger_unlimited && i < -1) + if (var.type () == var_zuinteger_unlimited && i < -1) { gdbscm_out_of_range_error (func_name, arg_pos, value, _("must be >= -1")); } - var->intval = i; + var.set (i); } break; @@ -934,7 +968,7 @@ gdbscm_make_parameter (SCM name_scm, SCM rest) if (gdbscm_is_exception (initial_value_scm)) gdbscm_throw (initial_value_scm); } - pascm_set_param_value_x (p_smob->type, &p_smob->value, enum_list, + pascm_set_param_value_x (p_smob, enum_list, initial_value_scm, initial_value_arg_pos, FUNC_NAME); } @@ -1030,8 +1064,7 @@ gdbscm_parameter_value (SCM self) param_smob *p_smob = pascm_get_param_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME); - return pascm_param_value (p_smob->type, &p_smob->value, - SCM_ARG1, FUNC_NAME); + return pascm_param_value (setting_wrapper (p_smob), SCM_ARG1, FUNC_NAME); } else { @@ -1062,13 +1095,13 @@ gdbscm_parameter_value (SCM self) gdbscm_out_of_range_error (FUNC_NAME, SCM_ARG1, self, _("parameter not found")); } - if (cmd->var == NULL) + if (!cmd->var) { gdbscm_out_of_range_error (FUNC_NAME, SCM_ARG1, self, _("not a parameter")); } - return pascm_param_value (cmd->var_type, cmd->var, SCM_ARG1, FUNC_NAME); + return pascm_param_value (cmd->var, SCM_ARG1, FUNC_NAME); } } @@ -1080,7 +1113,7 @@ gdbscm_set_parameter_value_x (SCM self, SCM value) param_smob *p_smob = pascm_get_param_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME); - pascm_set_param_value_x (p_smob->type, &p_smob->value, p_smob->enumeration, + pascm_set_param_value_x (p_smob, p_smob->enumeration, value, SCM_ARG2, FUNC_NAME); return SCM_UNSPECIFIED; diff --git a/gdb/maint.c b/gdb/maint.c index 4637fcbc86c..58a54c22940 100644 --- a/gdb/maint.c +++ b/gdb/maint.c @@ -1088,7 +1088,7 @@ set_per_command_cmd (const char *args, int from_tty) error (_("Bad value for 'mt set per-command no'.")); for (list = per_command_setlist; list != NULL; list = list->next) - if (list->var_type == var_boolean) + if (list->var.type () == var_boolean) { gdb_assert (list->type == set_cmd); do_set_command (args, from_tty, list); diff --git a/gdb/python/py-param.c b/gdb/python/py-param.c index f9dcb076c60..f2d251a19ea 100644 --- a/gdb/python/py-param.c +++ b/gdb/python/py-param.c @@ -88,6 +88,19 @@ struct parmpy_object const char **enumeration; }; +/* Wraps a base_setting_wrapper around an existing param_smob. This abstraction + is used + to manipulate the value in S->VALUE in a type safe manner the same way as if + it was referenced by a setting. */ +struct setting_wrapper final: base_setting_wrapper +{ + explicit setting_wrapper (parmpy_object *s) + { + m_var_type = s->type; + m_var = &s->value; + } +}; + extern PyTypeObject parmpy_object_type CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("parmpy_object"); @@ -110,7 +123,8 @@ get_attr (PyObject *obj, PyObject *attr_name) { parmpy_object *self = (parmpy_object *) obj; - return gdbpy_parameter_value (self->type, &self->value); + setting_wrapper wrapper { self }; + return gdbpy_parameter_value (wrapper); } return PyObject_GenericGetAttr (obj, attr_name); diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h index 690d2fb43c0..6b6ed009091 100644 --- a/gdb/python/python-internal.h +++ b/gdb/python/python-internal.h @@ -438,7 +438,7 @@ PyObject *gdbpy_create_ptid_object (ptid_t ptid); PyObject *gdbpy_selected_thread (PyObject *self, PyObject *args); PyObject *gdbpy_selected_inferior (PyObject *self, PyObject *args); PyObject *gdbpy_string_to_argv (PyObject *self, PyObject *args); -PyObject *gdbpy_parameter_value (enum var_types type, void *var); +PyObject *gdbpy_parameter_value (base_setting_wrapper const & var); gdb::unique_xmalloc_ptr gdbpy_parse_command_name (const char *name, struct cmd_list_element ***base_list, struct cmd_list_element **start_list); diff --git a/gdb/python/python.c b/gdb/python/python.c index e42cbc4fd5e..3b2210b5cc5 100644 --- a/gdb/python/python.c +++ b/gdb/python/python.c @@ -448,9 +448,9 @@ python_command (const char *arg, int from_tty) NULL (and set a Python exception) on error. Helper function for get_parameter. */ PyObject * -gdbpy_parameter_value (enum var_types type, void *var) +gdbpy_parameter_value (base_setting_wrapper const & var) { - switch (type) + switch (var.type ()) { case var_string: case var_string_noescape: @@ -458,16 +458,23 @@ gdbpy_parameter_value (enum var_types type, void *var) case var_filename: case var_enum: { - const char *str = *(char **) var; - - if (! str) + const char *str; + if (var.type () == var_enum) + str = var.get (); + else + str = var.get (); + + if (str == nullptr) str = ""; return host_string_to_python_string (str).release (); } case var_boolean: { - if (* (bool *) var) + if (var.get ()) Py_RETURN_TRUE; else Py_RETURN_FALSE; @@ -475,7 +482,7 @@ gdbpy_parameter_value (enum var_types type, void *var) case var_auto_boolean: { - enum auto_boolean ab = * (enum auto_boolean *) var; + enum auto_boolean ab = var.get (); if (ab == AUTO_BOOLEAN_TRUE) Py_RETURN_TRUE; @@ -486,16 +493,18 @@ gdbpy_parameter_value (enum var_types type, void *var) } case var_integer: - if ((* (int *) var) == INT_MAX) + if (var.get () == INT_MAX) Py_RETURN_NONE; /* Fall through. */ case var_zinteger: case var_zuinteger_unlimited: - return gdb_py_object_from_longest (* (int *) var).release (); + return gdb_py_object_from_longest + (var.get ()).release (); case var_uinteger: { - unsigned int val = * (unsigned int *) var; + unsigned int val = var.get (); if (val == UINT_MAX) Py_RETURN_NONE; @@ -504,7 +513,7 @@ gdbpy_parameter_value (enum var_types type, void *var) case var_zuinteger: { - unsigned int val = * (unsigned int *) var; + unsigned int val = var.get (); return gdb_py_object_from_ulongest (val).release (); } } @@ -541,10 +550,10 @@ gdbpy_parameter (PyObject *self, PyObject *args) return PyErr_Format (PyExc_RuntimeError, _("Could not find parameter `%s'."), arg); - if (! cmd->var) + if (!cmd->var) return PyErr_Format (PyExc_RuntimeError, _("`%s' is not a parameter."), arg); - return gdbpy_parameter_value (cmd->var_type, cmd->var); + return gdbpy_parameter_value (cmd->var); } /* Wrapper for target_charset. */ diff --git a/gdb/remote.c b/gdb/remote.c index adc53e324d0..f8cabe8ec87 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -2243,7 +2243,7 @@ show_remote_protocol_packet_cmd (struct ui_file *file, int from_tty, packet < &remote_protocol_packets[PACKET_MAX]; packet++) { - if (&packet->detect == c->var) + if (&packet->detect == c->var.get_p ()) { show_packet_config_cmd (packet); return; -- 2.31.1