diff options
author | Lancelot SIX <lsix@lancelotsix.com> | 2021-07-14 22:30:14 +0000 |
---|---|---|
committer | Lancelot SIX <lsix@lancelotsix.com> | 2021-08-11 23:28:53 +0100 |
commit | 9d80399a20c0997f2f891c8a26c2b2463a6339af (patch) | |
tree | d9e58b973dd81467d474bb2fa03b534e6c7a837c | |
parent | 0d81afad67091936f75457fac19f6c136669c2b8 (diff) | |
download | binutils-gdb-9d80399a20c0997f2f891c8a26c2b2463a6339af.tar.gz |
gdb: 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 of the cmd_list_element needs to know what
the storage type associated with a given VAR_TYPES tag, and needs to do
the proper casting. No automatic check at compile or run time enforces
that the adequate cast is done.
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.
This patch proposes to add an abstraction around the var_types and void*
pointer pair. The abstraction is meant to prevent the user from having
to handle the cast. This is achieved by introducing the setting
struct, and a set of templated get / set member functions. The template
parameter is the type of the variable that holds the referred variable.
For example, instantiating the member functions with bool will yield
something similar to:
bool get<bool> () const
{
gdb_assert (this->m_var_type == var_boolean);
gdb_assert (this->m_var != nullptr);
return *static_cast<bool *> (this->m_var);
}
void set<bool> (bool var)
{
gdb_assert (this->m_var_type == var_boolean);
gdb_assert (this->m_var != nullptr);
*static_cast<bool *> (this->m_var) = var;
}
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.
-rw-r--r-- | gdb/auto-load.c | 2 | ||||
-rw-r--r-- | gdb/cli/cli-cmds.c | 63 | ||||
-rw-r--r-- | gdb/cli/cli-decode.c | 112 | ||||
-rw-r--r-- | gdb/cli/cli-decode.h | 10 | ||||
-rw-r--r-- | gdb/cli/cli-setshow.c | 151 | ||||
-rw-r--r-- | gdb/command.h | 153 | ||||
-rw-r--r-- | gdb/guile/scm-param.c | 128 | ||||
-rw-r--r-- | gdb/maint.c | 2 | ||||
-rw-r--r-- | gdb/python/py-param.c | 15 | ||||
-rw-r--r-- | gdb/python/python-internal.h | 2 | ||||
-rw-r--r-- | gdb/python/python.c | 28 | ||||
-rw-r--r-- | gdb/remote.c | 2 |
12 files changed, 447 insertions, 221 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 5ff0b77eb68..9ee3ad82246 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.valid ()) 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> () == 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<int> ()); case var_zinteger: return value_from_longest (builtin_type (gdbarch)->builtin_int, - *(int *) cmd->var); + cmd->var.get<int> ()); case var_boolean: return value_from_longest (builtin_type (gdbarch)->builtin_int, - *(bool *) cmd->var ? 1 : 0); + cmd->var.get<bool> () ? 1 : 0); case var_zuinteger_unlimited: return value_from_longest (builtin_type (gdbarch)->builtin_int, - *(int *) cmd->var); + cmd->var.get<int> ()); case var_auto_boolean: { int val; - switch (*(enum auto_boolean*) cmd->var) + switch (cmd->var.get<enum auto_boolean> ()) { case AUTO_BOOLEAN_TRUE: val = 1; @@ -2130,27 +2130,35 @@ 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<unsigned int> () == 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<unsigned int> ()); case var_zuinteger: return value_from_ulongest (builtin_type (gdbarch)->builtin_unsigned_int, - *(unsigned int *) cmd->var); + cmd->var.get<unsigned int> ()); 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); - else - return value_cstring ("", 1, - builtin_type (gdbarch)->builtin_char); + { + const char *var; + if (cmd->var.type () == var_enum) + var = cmd->var.get<const char *> (); + else + var = cmd->var.get<char *> (); + + if (var != nullptr) + return value_cstring (var, strlen (var), + builtin_type (gdbarch)->builtin_char); + else + return value_cstring ("", 1, + builtin_type (gdbarch)->builtin_char); + } default: gdb_assert_not_reached ("bad var_type"); } @@ -2186,7 +2194,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,13 +2219,20 @@ 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); - else - return value_cstring ("", 1, - builtin_type (gdbarch)->builtin_char); + { + const char *var; + if (cmd->var.type () == var_enum) + var = cmd->var.get<const char *> (); + else + var = cmd->var.get<char *> (); + if (var != nullptr) + return value_cstring (var, strlen (var), + builtin_type (gdbarch)->builtin_char); + else + return value_cstring ("", 1, + builtin_type (gdbarch)->builtin_char); + } default: gdb_assert_not_reached ("bad var_type"); } diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c index 06f3de0f038..b59c6ffbda7 100644 --- a/gdb/cli/cli-decode.c +++ b/gdb/cli/cli-decode.c @@ -462,12 +462,13 @@ empty_func (const char *args, int from_tty, cmd_list_element *c) VAR is address of the variable being controlled by this command. DOC is the documentation string. */ +template<typename T> 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, + T *var, const char *doc, struct cmd_list_element **list) { @@ -475,8 +476,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<T> (var_type, var); /* This needs to be something besides NULL so that this isn't treated as a help class. */ c->func = empty_func; @@ -492,10 +492,11 @@ add_set_or_show_cmd (const char *name, Return the newly created set and show commands. */ +template<typename T> static set_show_commands add_setshow_cmd_full (const char *name, enum command_class theclass, - var_types var_type, void *var, + var_types var_type, T *var, const char *set_doc, const char *show_doc, const char *help_doc, cmd_func_ftype *set_func, @@ -518,15 +519,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<T> (name, set_cmd, theclass, var_type, var, + full_set_doc, set_list); set->doc_allocated = 1; if (set_func != NULL) set->func = 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<T> (name, show_cmd, theclass, var_type, 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 @@ -555,10 +556,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<const char *> (name, theclass, var_enum, var, + set_doc, show_doc, help_doc, + set_func, show_func, + set_list, show_list); commands.set->enums = enumlist; return commands; } @@ -583,10 +584,11 @@ 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<enum auto_boolean> (name, theclass, + var_auto_boolean,var, + set_doc, show_doc, help_doc, + set_func, show_func, + set_list, show_list); commands.set->enums = auto_boolean_enums; @@ -612,10 +614,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<bool> (name, theclass, var_boolean, var, + set_doc, show_doc, help_doc, + set_func, show_func, + set_list, show_list); commands.set->enums = boolean_enums; @@ -636,10 +638,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<char *> (name, theclass, var_filename, var, + set_doc, show_doc, help_doc, + set_func, show_func, + set_list, show_list); set_cmd_completer (commands.set, filename_completer); @@ -660,10 +662,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<char *> (name, theclass, var_string, 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); @@ -685,10 +687,9 @@ 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<char *> (name, theclass, var_string_noescape, 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); @@ -710,11 +711,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<char *> (name, theclass, var_optional_filename, + var, set_doc, show_doc, help_doc, + set_func, show_func, set_list, show_list); + set_cmd_completer (commands.set, filename_completer); return commands; @@ -754,10 +754,9 @@ 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<int> (name, theclass, var_integer, var, set_doc, + show_doc, help_doc, set_func, show_func, + set_list, show_list); set_cmd_completer (commands.set, integer_unlimited_completer); @@ -780,10 +779,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<unsigned int> (name, theclass, var_uinteger, var, + set_doc, show_doc, help_doc, + set_func, show_func, + set_list, show_list); set_cmd_completer (commands.set, integer_unlimited_completer); @@ -805,10 +804,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<int> (name, theclass, var_zinteger, var, + set_doc, show_doc, help_doc, + set_func, show_func, + set_list, show_list); } set_show_commands @@ -824,10 +823,9 @@ 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<int> (name, theclass, var_zuinteger_unlimited, var, + set_doc, show_doc, help_doc, set_func, + show_func, set_list, show_list); set_cmd_completer (commands.set, integer_unlimited_completer); @@ -849,10 +847,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<unsigned int> (name, theclass, var_zuinteger, + 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 651d1ef8abb..e21367ce8ab 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 @@ -228,9 +224,9 @@ struct cmd_list_element used to finalize the CONTEXT field, if needed. */ void (*destroyer) (struct cmd_list_element *self, void *context) = nullptr; - /* Pointer to variable affected by "set" and "show". Doesn't - matter if type is not_set. */ - void *var = nullptr; + /* Setting affected by "set" and "show". Doesn't matter if type is + not_set. */ + 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 0290acede7f..975c677fd52 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) + char * const var = c->var.get<char *> (); + if (var == nullptr + || strcmp (var, newobj) != 0) { - xfree (*(char **) c->var); - *(char **) c->var = newobj; + xfree (var); + c->var.set<char *> (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); + { + char * const var = c->var.get<char *> (); + if (var == nullptr + || strcmp (var, arg) != 0) + { + xfree (var); + c->var.set<char *> (xstrdup (arg)); - option_changed = 1; - } + option_changed = 1; + } + } break; case var_filename: if (*arg == '\0') @@ -398,11 +403,12 @@ 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) + char * const var = c->var.get<char *> (); + if (var == nullptr + || strcmp (var, val) != 0) { - xfree (*(char **) c->var); - *(char **) c->var = val; + xfree (var); + c->var.set<char *> (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> ()) { - *(bool *) c->var = val; + c->var.set<bool> (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<enum auto_boolean> () != val) { - *(enum auto_boolean *) c->var = val; + c->var.set<enum auto_boolean> (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<unsigned int> () != val) { - *(unsigned int *) c->var = val; + c->var.set<unsigned int> (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<int> () != val) { - *(int *) c->var = val; + c->var.set<int> (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<const char *> () != match) { - *(const char **) c->var = match; + c->var.set<const char *> (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<int> () != val) { - *(int *) c->var = val; + c->var.set<int> (val); option_changed = 1; } } @@ -578,25 +584,33 @@ 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<const char *> (); + else + var = c->var.get<char *> (); + 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<bool> () ? "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<enum auto_boolean> ()]; gdb::observers::command_param_changed.notify (name, s); } @@ -606,7 +620,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<unsigned int> ()); gdb::observers::command_param_changed.notify (name, s); } break; @@ -616,7 +630,7 @@ 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<int> ()); gdb::observers::command_param_changed.notify (name, s); } break; @@ -632,24 +646,34 @@ 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, '"'); + { + char * var = c->var.get<char *> (); + 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<const char *> (); + else + var = c->var.get<char *> (); + if (var != nullptr) + stb.puts (var); + } break; case var_boolean: - stb.puts (*(bool *) c->var ? "on" : "off"); + stb.puts (c->var.get<bool> () ? "on" : "off"); break; case var_auto_boolean: - switch (*(enum auto_boolean*) c->var) + switch (c->var.get<enum auto_boolean> ()) { case AUTO_BOOLEAN_TRUE: stb.puts ("on"); @@ -667,26 +691,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); + { + unsigned int const var = c->var.get<unsigned int> (); + 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); + { + int const var = c->var.get<int> (); + 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) + int const var = c->var.get<int> (); + 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 baf34401a07..a04d0b1c8c6 100644 --- a/gdb/command.h +++ b/gdb/command.h @@ -125,6 +125,159 @@ typedef enum var_types } var_types; +/* Return true if a setting of type VAR_TYPE is backed with type T. + + This function is left without definition intentionally. This template is + specialized for all valid types that are used to back var_types. Therefore + if one tries to instantiate this un-specialized template it means the T + parameter is not a type used to back a var_type and it is most likely a + programming error. */ +template<typename T> +inline bool var_type_uses (var_types var_type); + +/* Return true if a setting of type T is backed by a bool variable. */ +template<> +inline bool var_type_uses<bool> (var_types t) +{ + return t == var_boolean; +}; + +/* Return true if a setting of type T is backed by a auto_boolean variable. +*/ +template<> +inline bool var_type_uses<enum auto_boolean> (var_types t) +{ + return t == var_auto_boolean; +} + +/* Return true if a setting of type T is backed by an unsigned int variable. +*/ +template<> +inline bool var_type_uses<unsigned int> (var_types t) +{ + return (t == var_uinteger || t == var_zinteger || t == var_zuinteger); +} + +/* Return true if a setting of type T is backed by an int variable. */ +template<> +inline bool var_type_uses<int> (var_types t) +{ + return (t == var_integer || t == var_zinteger + || t == var_zuinteger_unlimited); +} + +/* Return true if a setting of type T is backed by a char * variable. */ +template<> +inline bool var_type_uses<char *> (var_types t) +{ + return (t == var_string || t == var_string_noescape + || t == var_optional_filename || t == var_filename); +} + +/* Return true if a setting of type T is backed by a const char * variable. +*/ +template<> +inline bool var_type_uses<const char *> (var_types t) +{ + return t == var_enum; +} + +/* Abstraction that contains access to data that can be set or shown. + + The underlying data can be of any VAR_TYPES type. */ +struct base_setting +{ + /* Access the type of the current var. */ + var_types type () const + { + return m_var_type; + } + + /* Return the current value (by pointer). + + The template parameter T is the type of the variable used to store the + setting. + + The returned value cannot be NULL (this is checked at runtime). */ + template<typename T> + T const * + get_p () const + { + gdb_assert (var_type_uses<T> (this->m_var_type)); + gdb_assert (this->m_var != nullptr); + + return static_cast<T const *> (this->m_var); + } + + /* Return the current value. + + See get_p for discussion on the return type and template parameters. */ + template<typename T> + T get () const + { + return *get_p<T> (); + } + + /* Sets the value V to the underlying buffer. + + The template parameter T indicates the type of the variable used to store + the setting. + + The var_type of the setting must match T. */ + template<typename T> + void set (T v) + { + /* Check that the current instance is of one of the supported types for + this instantiation. */ + gdb_assert (var_type_uses<T> (this->m_var_type)); + gdb_assert (this->m_var != nullptr); + + *static_cast<T *> (this->m_var) = v; + } + + /* A setting is valid if it contains a non null pointer to a memory buffer. + + On an instance which is not valid, any call to get, get_p or set is + guaranteed to generate a runtime error (using gdb_assert). + */ + bool valid () const + { + return this->m_var != nullptr; + } + +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 }; +}; + +/* A base_setting with additional methods to set the underlying buffer and + declare the var_type. */ +struct setting final: base_setting +{ + /* Set the type of the current variable. */ + void set_type (var_types type) + { + gdb_assert (this->m_var == nullptr); + this->m_var_type = type; + } + + /* Update the pointer to the underlying variable referenced by this + instance. */ + template<typename T> + void set_p (var_types var_type, T *v) + { + gdb_assert (v != nullptr); + gdb_assert (var_type_uses<T> (var_type)); + this->set_type (var_type); + this->m_var = static_cast<void *> (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 44ea167be27..d90429808e9 100644 --- a/gdb/guile/scm-param.c +++ b/gdb/guile/scm-param.c @@ -113,6 +113,18 @@ struct param_smob SCM containing_scm; }; +/* Wraps a base_param_ref 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 param_ref. */ +struct setting_wrapper final : public base_setting +{ + 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 +145,8 @@ 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 const &c, int arg_pos, + const char *func_name); /* Administrivia for parameter smobs. */ @@ -153,8 +165,7 @@ pascm_print_param_smob (SCM self, SCM port, scm_print_state *pstate) 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<const char *> (self->enumeration[0]); break; default: @@ -563,17 +574,17 @@ pascm_param_type_name (enum var_types param_type) } /* Return the value of a gdb parameter as a Scheme value. - If TYPE is not supported, then a <gdb:exception> object is returned. */ + If the var_type of VAR is not supported, then a <gdb:exception> 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 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 +592,20 @@ 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<const char *> (); + else + str = var.get<char *> (); - 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<bool> ()) return SCM_BOOL_T; else return SCM_BOOL_F; @@ -598,7 +613,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<enum auto_boolean> (); if (ab == AUTO_BOOLEAN_TRUE) return SCM_BOOL_T; @@ -609,67 +624,69 @@ pascm_param_value (enum var_types type, void *var, } case var_zuinteger_unlimited: - if (* (int *) var == -1) + if (var.get<int> () == -1) return unlimited_keyword; - gdb_assert (* (int *) var >= 0); + gdb_assert (var.get<int> () >= 0); /* Fall through. */ case var_zinteger: - return scm_from_int (* (int *) var); + return scm_from_int (var.get<int> ()); case var_uinteger: - if (* (unsigned int *) var == UINT_MAX) + if (var.get<unsigned int> ()== UINT_MAX) return unlimited_keyword; /* Fall through. */ case var_zuinteger: - return scm_from_uint (* (unsigned int *) var); + return scm_from_uint (var.get<unsigned int> ()); 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<char *> ()); + if (var.type () == var_optional_filename) + var.set<char *> (xstrdup ("")); else - var->stringval = NULL; + var.set<char *> (nullptr); } else { SCM exception; gdb::unique_xmalloc_ptr<char> 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<char *> ()); + var.set<char *> (string.release ()); } break; @@ -681,27 +698,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<char> 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<const char *> (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<bool> (gdbscm_is_true (value)); break; case var_auto_boolean: @@ -710,19 +727,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<enum auto_boolean> (AUTO_BOOLEAN_AUTO); else if (gdbscm_is_true (value)) - var->autoboolval = AUTO_BOOLEAN_TRUE; + var.set<enum auto_boolean> (AUTO_BOOLEAN_TRUE); else - var->autoboolval = AUTO_BOOLEAN_FALSE; + var.set<enum auto_boolean> (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 +747,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<unsigned int> (UINT_MAX); else - var->intval = -1; + var.set<int> (-1); break; } } @@ -743,25 +760,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<unsigned int> (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<int> (i); } break; @@ -934,7 +951,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 +1047,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 +1078,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.valid ()) { 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 +1096,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..905e91c76dd 100644 --- a/gdb/python/py-param.c +++ b/gdb/python/py-param.c @@ -88,6 +88,18 @@ struct parmpy_object const char **enumeration; }; +/* Wraps a base_setting around an existing parmpy_object. 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 param. */ +struct setting_wrapper final: base_setting +{ + 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 +122,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..7c60660facd 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 const & var); gdb::unique_xmalloc_ptr<char> 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..4ead73ce9b6 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 const & var) { - switch (type) + switch (var.type ()) { case var_string: case var_string_noescape: @@ -458,16 +458,20 @@ gdbpy_parameter_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<const char *> (); + else + str = var.get<char *> (); - if (! str) + if (str == nullptr) str = ""; return host_string_to_python_string (str).release (); } case var_boolean: { - if (* (bool *) var) + if (var.get<bool> ()) Py_RETURN_TRUE; else Py_RETURN_FALSE; @@ -475,7 +479,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<enum auto_boolean> (); if (ab == AUTO_BOOLEAN_TRUE) Py_RETURN_TRUE; @@ -486,16 +490,16 @@ gdbpy_parameter_value (enum var_types type, void *var) } case var_integer: - if ((* (int *) var) == INT_MAX) + if (var.get<int> () == 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<int> ()).release (); case var_uinteger: { - unsigned int val = * (unsigned int *) var; + unsigned int val = var.get<unsigned int> (); if (val == UINT_MAX) Py_RETURN_NONE; @@ -504,7 +508,7 @@ gdbpy_parameter_value (enum var_types type, void *var) case var_zuinteger: { - unsigned int val = * (unsigned int *) var; + unsigned int val = var.get<unsigned int> (); return gdb_py_object_from_ulongest (val).release (); } } @@ -541,10 +545,10 @@ gdbpy_parameter (PyObject *self, PyObject *args) return PyErr_Format (PyExc_RuntimeError, _("Could not find parameter `%s'."), arg); - if (! cmd->var) + if (!cmd->var.valid ()) 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 b6da6b086a2..429a5f5e630 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -2244,7 +2244,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<enum auto_boolean> ()) { show_packet_config_cmd (packet); return; |