public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Simon Marchi <simon.marchi@polymtl.ca>
To: gdb-patches@sourceware.org
Subject: [PATCH 15/16] gdb: make cmd_list_element var an optional union
Date: Wed, 14 Jul 2021 00:55:19 -0400	[thread overview]
Message-ID: <20210714045520.1623120-16-simon.marchi@polymtl.ca> (raw)
In-Reply-To: <20210714045520.1623120-1-simon.marchi@polymtl.ca>

The field cmd_list_element::var is used by set and show commands and
points to the variable affected by the set command and shown by the show
command.  It is of type `void *` and cast to a pointer of the
appriopriate type on each use depending on the type of the setting
(cmd_list_element::var_type).

I propose changing cmd_list_element to be a union of all the possible
pointer types.  Fundamentally, this doesn't change much what is
happening.  But I think this helps understand better how the field is
used and get a bit of type safety (while it is still possible to use the
wrong member at some point, we can't cast to something completely
unrealted).  Finally, when doing refactorings, such as one later in this
series which changes `char *` to `std::string`, it ensures that we don't
forget to update some spots.

I wrapped the union in an optional, because we need to check in some
spots whether var is set or not.  I think that conceptually, an optional
makes the most sense.  Another option would be to pick an arbitrary
member of the union and compare it to nullptr, whenever we want to know
whether var is set, but that seems a bit more hack-ish.

A note on the Guile situation: something that makes our life a little
bit complicated is that is possible to assign and read the value of a
Guile-created parameter that is not yet registered (see previous patch
that adds a test for this).  It would have been much more simple to say
that this is simply not supported anymore, but that could break existing
scripts, so I don't think it is a good idea.  In some cases, for example
when printing the value of a built-in parameter, we have access to a
show command and its union setting_variable.  When we have an
un-registered Guile param, we don't have a show command associated to
it, but we can pass the parameter's pascm_variable union, stored in
param_smob.  Because we have these two kinds of incompatible parameters,
we need two versions of the pascm_param_value function.

Change-Id: I74a6be2a9188520dbdac4cb123c08e00ebb40a91
---
 gdb/cli/cli-cmds.c           |  48 +++++++++-----
 gdb/cli/cli-decode.c         | 113 +++++++++++++++++++++++--------
 gdb/cli/cli-decode.h         |  31 ++++++++-
 gdb/cli/cli-setshow.c        |  87 +++++++++++++-----------
 gdb/guile/scm-param.c        | 125 +++++++++++++++++++++++++++++++----
 gdb/python/py-param.c        |   8 ++-
 gdb/python/python-internal.h |   2 +-
 gdb/python/python.c          |  35 +++++++---
 gdb/remote.c                 |   2 +-
 9 files changed, 340 insertions(+), 111 deletions(-)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 5ff0b77eb68f..a6b7ef637ab3 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.has_value ())
     error (_("Cannot use this setting with the \"with\" command"));
 
   std::string temp_value
@@ -2093,26 +2093,26 @@ value_from_setting (const cmd_list_element *cmd, struct gdbarch *gdbarch)
   switch (cmd->var_type)
     {
     case var_integer:
-      if (*(int *) cmd->var == INT_MAX)
+      if (*cmd->var->int_var == 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->int_var);
     case var_zinteger:
       return value_from_longest (builtin_type (gdbarch)->builtin_int,
-				 *(int *) cmd->var);
+				 *cmd->var->int_var);
     case var_boolean:
       return value_from_longest (builtin_type (gdbarch)->builtin_int,
-				 *(bool *) cmd->var ? 1 : 0);
+				 *cmd->var->bool_var ? 1 : 0);
     case var_zuinteger_unlimited:
       return value_from_longest (builtin_type (gdbarch)->builtin_int,
-				 *(int *) cmd->var);
+				 *cmd->var->int_var);
     case var_auto_boolean:
       {
 	int val;
 
-	switch (*(enum auto_boolean*) cmd->var)
+	switch (*cmd->var->auto_boolean_var)
 	  {
 	  case AUTO_BOOLEAN_TRUE:
 	    val = 1;
@@ -2130,23 +2130,32 @@ 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->unsigned_int_var == 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->unsigned_int_var);
     case var_zuinteger:
       return value_from_ulongest (builtin_type (gdbarch)->builtin_unsigned_int,
-				  *(unsigned int *) cmd->var);
+				  *cmd->var->unsigned_int_var);
     case var_string:
     case var_string_noescape:
     case var_optional_filename:
     case var_filename:
+      if (*cmd->var->char_ptr_var != nullptr)
+	return value_cstring (*cmd->var->char_ptr_var,
+			      strlen (*cmd->var->char_ptr_var) + 1,
+			      builtin_type (gdbarch)->builtin_char);
+      else
+	return value_cstring ("", 1,
+			      builtin_type (gdbarch)->builtin_char);
+
     case var_enum:
-      if (*(char **) cmd->var)
-	return value_cstring (*(char **) cmd->var, strlen (*(char **) cmd->var),
+      if (*cmd->var->const_char_ptr_var != nullptr)
+	return value_cstring (*cmd->var->const_char_ptr_var,
+			      strlen (*cmd->var->const_char_ptr_var) + 1,
 			      builtin_type (gdbarch)->builtin_char);
       else
 	return value_cstring ("", 1,
@@ -2206,13 +2215,22 @@ str_value_from_setting (const cmd_list_element *cmd, struct gdbarch *gdbarch)
     case var_string_noescape:
     case var_optional_filename:
     case var_filename:
-    case var_enum:
       /* For these cases, we do not use get_setshow_command_value_string,
 	 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),
+      if (*cmd->var->char_ptr_var)
+	return value_cstring (*cmd->var->char_ptr_var,
+			      strlen (*cmd->var->char_ptr_var) + 1,
+			      builtin_type (gdbarch)->builtin_char);
+      else
+	return value_cstring ("", 1,
+			      builtin_type (gdbarch)->builtin_char);
+
+    case var_enum:
+      if (*cmd->var->const_char_ptr_var)
+	return value_cstring (*cmd->var->const_char_ptr_var,
+			      strlen (*cmd->var->const_char_ptr_var) + 1,
 			      builtin_type (gdbarch)->builtin_char);
       else
 	return value_cstring ("", 1,
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 06f3de0f038a..2db3ddc0551b 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -459,7 +459,6 @@ empty_func (const char *args, int from_tty, cmd_list_element *c)
    TYPE is set_cmd or show_cmd.
    CLASS is as in add_cmd.
    VAR_TYPE is the kind of thing we are setting.
-   VAR is address of the variable being controlled by this command.
    DOC is the documentation string.  */
 
 static struct cmd_list_element *
@@ -467,7 +466,6 @@ add_set_or_show_cmd (const char *name,
 		     enum cmd_types type,
 		     enum command_class theclass,
 		     var_types var_type,
-		     void *var,
 		     const char *doc,
 		     struct cmd_list_element **list)
 {
@@ -476,7 +474,6 @@ 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;
   /* This needs to be something besides NULL so that this isn't
      treated as a help class.  */
   c->func = empty_func;
@@ -485,8 +482,7 @@ add_set_or_show_cmd (const char *name,
 
 /* Add element named NAME to both the command SET_LIST and SHOW_LIST.
    CLASS is as in add_cmd.  VAR_TYPE is the kind of thing we are
-   setting.  VAR is address of the variable being controlled by this
-   command.  SET_FUNC and SHOW_FUNC are the callback functions (if
+   setting.  SET_FUNC and SHOW_FUNC are the callback functions (if
    non-NULL).  SET_DOC, SHOW_DOC and HELP_DOC are the documentation
    strings.
 
@@ -495,7 +491,7 @@ add_set_or_show_cmd (const char *name,
 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,
 		      const char *set_doc, const char *show_doc,
 		      const char *help_doc,
 		      cmd_func_ftype *set_func,
@@ -518,14 +514,14 @@ 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,
+  set = add_set_or_show_cmd (name, set_cmd, theclass, var_type,
 			     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,
+  show = add_set_or_show_cmd (name, show_cmd, theclass, var_type,
 			      full_show_doc, show_list);
   show->doc_allocated = 1;
   show->show_value_func = show_func;
@@ -555,11 +551,17 @@ 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,
+    =  add_setshow_cmd_full (name, theclass, var_enum,
 			     set_doc, show_doc, help_doc,
 			     set_func, show_func,
 			     set_list, show_list);
+
+  commands.set->var.emplace ();
+  commands.set->var->const_char_ptr_var = var;
   commands.set->enums = enumlist;
+  commands.show->var.emplace ();
+  commands.show->var->const_char_ptr_var = var;
+
   return commands;
 }
 
@@ -583,12 +585,16 @@ 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,
+    = add_setshow_cmd_full (name, theclass, var_auto_boolean,
 			    set_doc, show_doc, help_doc,
 			    set_func, show_func,
 			    set_list, show_list);
 
+  commands.set->var.emplace ();
+  commands.set->var->auto_boolean_var = var;
   commands.set->enums = auto_boolean_enums;
+  commands.show->var.emplace ();
+  commands.show->var->auto_boolean_var = var;
 
   return commands;
 }
@@ -612,12 +618,16 @@ 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,
+    = add_setshow_cmd_full (name, theclass, var_boolean,
 			    set_doc, show_doc, help_doc,
 			    set_func, show_func,
 			    set_list, show_list);
 
+  commands.set->var.emplace ();
+  commands.set->var->bool_var = var;
   commands.set->enums = boolean_enums;
+  commands.show->var.emplace ();
+  commands.show->var->bool_var = var;
 
   return commands;
 }
@@ -636,13 +646,18 @@ 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,
+    = add_setshow_cmd_full (name, theclass, var_filename,
 			    set_doc, show_doc, help_doc,
 			    set_func, show_func,
 			    set_list, show_list);
 
   set_cmd_completer (commands.set, filename_completer);
 
+  commands.set->var.emplace ();
+  commands.set->var->char_ptr_var = var;
+  commands.show->var.emplace ();
+  commands.show->var->char_ptr_var = var;
+
   return commands;
 }
 
@@ -660,7 +675,7 @@ 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,
+    = add_setshow_cmd_full (name, theclass, var_string,
 			    set_doc, show_doc, help_doc,
 			    set_func, show_func,
 			    set_list, show_list);
@@ -668,6 +683,11 @@ add_setshow_string_cmd (const char *name, enum command_class theclass,
   /* Disable the default symbol completer.  */
   set_cmd_completer (commands.set, nullptr);
 
+  commands.set->var.emplace ();
+  commands.set->var->char_ptr_var = var;
+  commands.show->var.emplace ();
+  commands.show->var->char_ptr_var = var;
+
   return commands;
 }
 
@@ -685,7 +705,7 @@ 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,
+    = add_setshow_cmd_full (name, theclass, var_string_noescape,
 			    set_doc, show_doc, help_doc,
 			    set_func, show_func,
 			    set_list, show_list);
@@ -693,6 +713,11 @@ add_setshow_string_noescape_cmd (const char *name, enum command_class theclass,
   /* Disable the default symbol completer.  */
   set_cmd_completer (commands.set, nullptr);
 
+  commands.set->var.emplace ();
+  commands.set->var->char_ptr_var = var;
+  commands.show->var.emplace ();
+  commands.show->var->char_ptr_var = var;
+
   return commands;
 }
 
@@ -710,13 +735,18 @@ 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,
+    = add_setshow_cmd_full (name, theclass, var_optional_filename,
 			    set_doc, show_doc, help_doc,
 			    set_func, show_func,
 			    set_list, show_list);
 		
   set_cmd_completer (commands.set, filename_completer);
 
+  commands.set->var.emplace ();
+  commands.set->var->char_ptr_var = var;
+  commands.show->var.emplace ();
+  commands.show->var->char_ptr_var = var;
+
   return commands;
 }
 
@@ -754,13 +784,18 @@ 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,
+    = add_setshow_cmd_full (name, theclass, var_integer,
 			    set_doc, show_doc, help_doc,
 			    set_func, show_func,
 			    set_list, show_list);
 
   set_cmd_completer (commands.set, integer_unlimited_completer);
 
+  commands.set->var.emplace ();
+  commands.set->var->int_var = var;
+  commands.show->var.emplace ();
+  commands.show->var->int_var = var;
+
   return commands;
 }
 
@@ -780,13 +815,18 @@ 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,
+    = add_setshow_cmd_full (name, theclass, var_uinteger,
 			    set_doc, show_doc, help_doc,
 			    set_func, show_func,
 			    set_list, show_list);
 
   set_cmd_completer (commands.set, integer_unlimited_completer);
 
+  commands.set->var.emplace ();
+  commands.set->var->unsigned_int_var = var;
+  commands.show->var.emplace ();
+  commands.show->var->unsigned_int_var = var;
+
   return commands;
 }
 
@@ -805,10 +845,18 @@ 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);
+  set_show_commands commands
+    = add_setshow_cmd_full (name, theclass, var_zinteger,
+			    set_doc, show_doc, help_doc,
+			    set_func, show_func,
+			    set_list, show_list);
+
+  commands.set->var.emplace ();
+  commands.set->var->int_var = var;
+  commands.show->var.emplace ();
+  commands.show->var->int_var = var;
+
+  return commands;
 }
 
 set_show_commands
@@ -824,13 +872,18 @@ 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,
+    = add_setshow_cmd_full (name, theclass, var_zuinteger_unlimited,
 			    set_doc, show_doc, help_doc,
 			    set_func, show_func,
 			    set_list, show_list);
 
   set_cmd_completer (commands.set, integer_unlimited_completer);
 
+  commands.set->var.emplace ();
+  commands.set->var->int_var = var;
+  commands.show->var.emplace ();
+  commands.show->var->int_var = var;
+
   return commands;
 }
 
@@ -849,10 +902,18 @@ 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);
+  set_show_commands commands
+    = add_setshow_cmd_full (name, theclass, var_zuinteger,
+			    set_doc, show_doc, help_doc,
+			    set_func, show_func,
+			    set_list, show_list);
+
+  commands.set->var.emplace ();
+  commands.set->var->unsigned_int_var = var;
+  commands.show->var.emplace ();
+  commands.show->var->unsigned_int_var = var;
+
+  return commands;
 }
 
 /* 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 651d1ef8abb7..ba07d6a69f17 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -228,9 +228,34 @@ 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;
+  /* Pointer to variable affected by "set" and "show".
+
+     Not used if TYPE is not_set.
+
+     The effective member of the union depends on VAR_TYPE.  */
+  union setting_variable
+  {
+    /* Used by var_integer, var_zinteger and var_zuinteger_unlimited.  */
+    int *int_var;
+
+    /* Used by var_uinteger and var_zuinteger.  */
+    unsigned int *unsigned_int_var;
+
+    /* Used by var_boolean.  */
+    bool *bool_var;
+
+    /* Used by var_auto_boolean.  */
+    auto_boolean *auto_boolean_var;
+
+    /* Used by var_filename, var_string, var_string_noescape and
+       var_optional_filename.  */
+    char **char_ptr_var;
+
+    /* Used by var_enum.  */
+    const char **const_char_ptr_var;
+  };
+
+  gdb::optional<setting_variable> 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 0290acede7fa..8f456b6fc14b 100644
--- a/gdb/cli/cli-setshow.c
+++ b/gdb/cli/cli-setshow.c
@@ -353,11 +353,11 @@ 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)
+	if (*c->var->char_ptr_var == NULL
+	    || strcmp (*c->var->char_ptr_var, newobj) != 0)
 	  {
-	    xfree (*(char **) c->var);
-	    *(char **) c->var = newobj;
+	    xfree (*c->var->char_ptr_var);
+	    *c->var->char_ptr_var = newobj;
 
 	    option_changed = 1;
 	  }
@@ -366,10 +366,10 @@ 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)
+      if (*c->var->char_ptr_var == NULL || strcmp (*c->var->char_ptr_var, arg) != 0)
 	{
-	  xfree (*(char **) c->var);
-	  *(char **) c->var = xstrdup (arg);
+	  xfree (*c->var->char_ptr_var);
+	  *c->var->char_ptr_var = xstrdup (arg);
 
 	  option_changed = 1;
 	}
@@ -398,11 +398,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 (*c->var->char_ptr_var == NULL
+	    || strcmp (*c->var->char_ptr_var, val) != 0)
 	  {
-	    xfree (*(char **) c->var);
-	    *(char **) c->var = val;
+	    xfree (*c->var->char_ptr_var);
+	    *c->var->char_ptr_var = val;
 
 	    option_changed = 1;
 	  }
@@ -416,9 +416,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->bool_var)
 	  {
-	    *(bool *) c->var = val;
+	    *c->var->bool_var = val;
 
 	    option_changed = 1;
 	  }
@@ -428,9 +428,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->auto_boolean_var != val)
 	  {
-	    *(enum auto_boolean *) c->var = val;
+	    *c->var->auto_boolean_var = val;
 
 	    option_changed = 1;
 	  }
@@ -441,9 +441,9 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
       {
 	unsigned int val = parse_cli_var_uinteger (c->var_type, &arg, true);
 
-	if (*(unsigned int *) c->var != val)
+	if (*c->var->unsigned_int_var != val)
 	  {
-	    *(unsigned int *) c->var = val;
+	    *c->var->unsigned_int_var = val;
 
 	    option_changed = 1;
 	  }
@@ -477,9 +477,9 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
 		 || (c->var_type == var_zinteger && val > INT_MAX))
 	  error (_("integer %s out of range"), plongest (val));
 
-	if (*(int *) c->var != val)
+	if (*c->var->int_var != val)
 	  {
-	    *(int *) c->var = val;
+	    *c->var->int_var = val;
 
 	    option_changed = 1;
 	  }
@@ -495,9 +495,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->const_char_ptr_var != match)
 	  {
-	    *(const char **) c->var = match;
+	    *c->var->const_char_ptr_var = match;
 
 	    option_changed = 1;
 	  }
@@ -507,9 +507,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->int_var != val)
 	  {
-	    *(int *) c->var = val;
+	    *c->var->int_var = val;
 	    option_changed = 1;
 	  }
       }
@@ -584,19 +584,23 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
 	case var_string_noescape:
 	case var_filename:
 	case var_optional_filename:
+	  gdb::observers::command_param_changed.notify
+	    (name, *c->var->char_ptr_var);
+	  break;
 	case var_enum:
-	  gdb::observers::command_param_changed.notify (name, *(char **) c->var);
+	  gdb::observers::command_param_changed.notify
+	    (name, *c->var->const_char_ptr_var);
 	  break;
 	case var_boolean:
 	  {
-	    const char *opt = *(bool *) c->var ? "on" : "off";
+	    const char *opt = *c->var->bool_var ? "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->auto_boolean_var];
 
 	    gdb::observers::command_param_changed.notify (name, s);
 	  }
@@ -606,7 +610,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->unsigned_int_var);
 	    gdb::observers::command_param_changed.notify (name, s);
 	  }
 	  break;
@@ -616,7 +620,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->int_var);
 	    gdb::observers::command_param_changed.notify (name, s);
 	  }
 	  break;
@@ -635,21 +639,24 @@ get_setshow_command_value_string (const cmd_list_element *c)
   switch (c->var_type)
     {
     case var_string:
-      if (*(char **) c->var)
-	stb.putstr (*(char **) c->var, '"');
+      if (*c->var->char_ptr_var)
+	stb.putstr (*c->var->char_ptr_var, '"');
       break;
     case var_string_noescape:
     case var_optional_filename:
     case var_filename:
+      if (*c->var->char_ptr_var)
+	stb.puts (*c->var->char_ptr_var);
+      break;
     case var_enum:
-      if (*(char **) c->var)
-	stb.puts (*(char **) c->var);
+      if (*c->var->const_char_ptr_var)
+	stb.puts (*c->var->const_char_ptr_var);
       break;
     case var_boolean:
-      stb.puts (*(bool *) c->var ? "on" : "off");
+      stb.puts (*c->var->bool_var ? "on" : "off");
       break;
     case var_auto_boolean:
-      switch (*(enum auto_boolean*) c->var)
+      switch (*c->var->auto_boolean_var)
 	{
 	case AUTO_BOOLEAN_TRUE:
 	  stb.puts ("on");
@@ -668,25 +675,25 @@ get_setshow_command_value_string (const cmd_list_element *c)
     case var_uinteger:
     case var_zuinteger:
       if (c->var_type == var_uinteger
-	  && *(unsigned int *) c->var == UINT_MAX)
+	  && *c->var->unsigned_int_var == UINT_MAX)
 	stb.puts ("unlimited");
       else
-	stb.printf ("%u", *(unsigned int *) c->var);
+	stb.printf ("%u", *c->var->unsigned_int_var);
       break;
     case var_integer:
     case var_zinteger:
       if (c->var_type == var_integer
-	  && *(int *) c->var == INT_MAX)
+	  && *c->var->int_var == INT_MAX)
 	stb.puts ("unlimited");
       else
-	stb.printf ("%d", *(int *) c->var);
+	stb.printf ("%d", *c->var->int_var);
       break;
     case var_zuinteger_unlimited:
       {
-	if (*(int *) c->var == -1)
+	if (*c->var->int_var == -1)
 	  stb.puts ("unlimited");
 	else
-	  stb.printf ("%d", *(int *) c->var);
+	  stb.printf ("%d", *c->var->int_var);
       }
       break;
     default:
diff --git a/gdb/guile/scm-param.c b/gdb/guile/scm-param.c
index 44ea167be277..57a8c7ec68a2 100644
--- a/gdb/guile/scm-param.c
+++ b/gdb/guile/scm-param.c
@@ -133,7 +133,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,
+static SCM pascm_param_value (enum var_types type,
+			      const pascm_variable *var,
 			      int arg_pos, const char *func_name);
 \f
 /* Administrivia for parameter smobs.  */
@@ -563,10 +564,13 @@ 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 TYPE is not supported, then a <gdb:exception> object is returned.
+
+   This overload can be used on Guile-created parameters, even if they
+   haven't been registered yet.  */
 
 static SCM
-pascm_param_value (enum var_types type, void *var,
+pascm_param_value (enum var_types type, const pascm_variable *param_value,
 		   int arg_pos, const char *func_name)
 {
   /* Note: We *could* support var_integer here in case someone is trying to get
@@ -579,9 +583,18 @@ pascm_param_value (enum var_types type, void *var,
     case var_string_noescape:
     case var_optional_filename:
     case var_filename:
+      {
+	const char *str = param_value->stringval;
+
+	if (str == NULL)
+	  str = "";
+
+	return gdbscm_scm_from_host_string (str, strlen (str));
+      }
+
     case var_enum:
       {
-	const char *str = *(char **) var;
+	const char *str = param_value->cstringval;
 
 	if (str == NULL)
 	  str = "";
@@ -590,7 +603,7 @@ pascm_param_value (enum var_types type, void *var,
 
     case var_boolean:
       {
-	if (* (bool *) var)
+	if (param_value->boolval)
 	  return SCM_BOOL_T;
 	else
 	  return SCM_BOOL_F;
@@ -598,7 +611,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 = param_value->autoboolval;
 
 	if (ab == AUTO_BOOLEAN_TRUE)
 	  return SCM_BOOL_T;
@@ -609,19 +622,19 @@ pascm_param_value (enum var_types type, void *var,
       }
 
     case var_zuinteger_unlimited:
-      if (* (int *) var == -1)
+      if (param_value->intval == -1)
 	return unlimited_keyword;
-      gdb_assert (* (int *) var >= 0);
+      gdb_assert (param_value->intval >= 0);
       /* Fall through.  */
     case var_zinteger:
-      return scm_from_int (* (int *) var);
+      return scm_from_int (param_value->intval);
 
     case var_uinteger:
-      if (* (unsigned int *) var == UINT_MAX)
+      if (param_value->uintval == UINT_MAX)
 	return unlimited_keyword;
       /* Fall through.  */
     case var_zuinteger:
-      return scm_from_uint (* (unsigned int *) var);
+      return scm_from_uint (param_value->uintval);
 
     default:
       break;
@@ -632,6 +645,92 @@ pascm_param_value (enum var_types type, void *var,
 					 _("program error: unhandled type"));
 }
 
+/* Return the value of a gdb parameter as a Scheme value.
+   If SHOW_CMD::VAR_TYPE is not supported, then a <gdb:exception> object is
+   returned.
+
+   This overload can be used with any parameter that has an associated show
+   command, such as built-in commands and registered Guile parameters.  */
+
+static SCM
+pascm_param_value (const cmd_list_element *show_cmd,
+		   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.  */
+
+  gdb_assert (show_cmd->type == cmd_types::show_cmd);
+
+  switch (show_cmd->var_type)
+    {
+    case var_string:
+    case var_string_noescape:
+    case var_optional_filename:
+    case var_filename:
+      {
+	const char *str = *show_cmd->var->char_ptr_var;
+
+	if (str == NULL)
+	  str = "";
+
+	return gdbscm_scm_from_host_string (str, strlen (str));
+      }
+
+    case var_enum:
+      {
+	const char *str = *show_cmd->var->const_char_ptr_var;
+
+	if (str == NULL)
+	  str = "";
+
+	return gdbscm_scm_from_host_string (str, strlen (str));
+      }
+
+    case var_boolean:
+      {
+	if (*show_cmd->var->bool_var)
+	  return SCM_BOOL_T;
+	else
+	  return SCM_BOOL_F;
+      }
+
+    case var_auto_boolean:
+      {
+	enum auto_boolean ab = *show_cmd->var->auto_boolean_var;
+
+	if (ab == AUTO_BOOLEAN_TRUE)
+	  return SCM_BOOL_T;
+	else if (ab == AUTO_BOOLEAN_FALSE)
+	  return SCM_BOOL_F;
+	else
+	  return auto_keyword;
+      }
+
+    case var_zuinteger_unlimited:
+      if (*show_cmd->var->int_var == -1)
+	return unlimited_keyword;
+      gdb_assert (*show_cmd->var->int_var >= 0);
+      /* Fall through.  */
+    case var_zinteger:
+      return scm_from_int (*show_cmd->var->int_var);
+
+    case var_uinteger:
+      if (*show_cmd->var->unsigned_int_var == UINT_MAX)
+	return unlimited_keyword;
+      /* Fall through.  */
+    case var_zuinteger:
+      return scm_from_uint (*show_cmd->var->unsigned_int_var);
+
+    default:
+      break;
+    }
+
+  return gdbscm_make_out_of_range_error (func_name, arg_pos,
+					 scm_from_int (show_cmd->var_type),
+					 _("program error: unhandled type"));
+}
+
 /* Set the value of a parameter of type TYPE in 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.  */
@@ -1062,13 +1161,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.has_value ())
 	{
 	  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, SCM_ARG1, FUNC_NAME);
     }
 }
 
diff --git a/gdb/python/py-param.c b/gdb/python/py-param.c
index f9dcb076c603..b83e3d080eb3 100644
--- a/gdb/python/py-param.c
+++ b/gdb/python/py-param.c
@@ -86,6 +86,9 @@ struct parmpy_object
      allocated with xmalloc, as is each element.  It is
      NULL-terminated.  */
   const char **enumeration;
+
+  /* The commands created as a result of creating this parameter.  */
+  set_show_commands commands;
 };
 
 extern PyTypeObject parmpy_object_type
@@ -110,7 +113,7 @@ get_attr (PyObject *obj, PyObject *attr_name)
     {
       parmpy_object *self = (parmpy_object *) obj;
 
-      return gdbpy_parameter_value (self->type, &self->value);
+      return gdbpy_parameter_value (self->commands.show);
     }
 
   return PyObject_GenericGetAttr (obj, attr_name);
@@ -572,6 +575,9 @@ add_setshow_generic (int parmclass, enum command_class cmdclass,
   commands.set->set_context (self);
   commands.show->set_context (self);
 
+  /* Save the created set/show commands.  */
+  self->commands = commands;
+
   /* We (unfortunately) currently leak the command name.  */
   cmd_name.release ();
 }
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 690d2fb43c06..db467540c473 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 (const cmd_list_element *show_cmd);
 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 e42cbc4fd5ee..c5641c4db8f7 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -448,26 +448,38 @@ 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 (const cmd_list_element *show_cmd)
 {
-  switch (type)
+  gdb_assert (show_cmd->type == cmd_types::show_cmd);
+
+  switch (show_cmd->var_type)
     {
     case var_string:
     case var_string_noescape:
     case var_optional_filename:
     case var_filename:
+      {
+	const char *str = *show_cmd->var->char_ptr_var;
+
+	if (str == nullptr)
+	  str = "";
+
+	return host_string_to_python_string (str).release ();
+      }
+
     case var_enum:
       {
-	const char *str = *(char **) var;
+	const char *str = *show_cmd->var->const_char_ptr_var;
 
-	if (! str)
+	if (str == nullptr)
 	  str = "";
+
 	return host_string_to_python_string (str).release ();
       }
 
     case var_boolean:
       {
-	if (* (bool *) var)
+	if (*show_cmd->var->bool_var)
 	  Py_RETURN_TRUE;
 	else
 	  Py_RETURN_FALSE;
@@ -475,7 +487,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 = *show_cmd->var->auto_boolean_var;
 
 	if (ab == AUTO_BOOLEAN_TRUE)
 	  Py_RETURN_TRUE;
@@ -486,16 +498,16 @@ gdbpy_parameter_value (enum var_types type, void *var)
       }
 
     case var_integer:
-      if ((* (int *) var) == INT_MAX)
+      if (*show_cmd->var->int_var == 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 (*show_cmd->var->int_var).release ();
 
     case var_uinteger:
       {
-	unsigned int val = * (unsigned int *) var;
+	unsigned int val = *show_cmd->var->unsigned_int_var;
 
 	if (val == UINT_MAX)
 	  Py_RETURN_NONE;
@@ -504,7 +516,7 @@ gdbpy_parameter_value (enum var_types type, void *var)
 
     case var_zuinteger:
       {
-	unsigned int val = * (unsigned int *) var;
+	unsigned int val = *show_cmd->var->unsigned_int_var;
 	return gdb_py_object_from_ulongest (val).release ();
       }
     }
@@ -544,7 +556,8 @@ gdbpy_parameter (PyObject *self, PyObject *args)
   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);
 }
 
 /* Wrapper for target_charset.  */
diff --git a/gdb/remote.c b/gdb/remote.c
index e2a08c9a113d..87981330ce87 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->auto_boolean_var)
 	{
 	  show_packet_config_cmd (packet);
 	  return;
-- 
2.32.0


  parent reply	other threads:[~2021-07-14  4:57 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-14  4:55 [PATCH 00/16] Bunch of commands related cleanups Simon Marchi
2021-07-14  4:55 ` [PATCH 01/16] gdb/testsuite: split gdb.python/py-parameter.exp in procs Simon Marchi
2021-07-14  4:55 ` [PATCH 02/16] gdb.base/setshow.exp: use save_vars to save/restore gdb_prompt Simon Marchi
2021-07-14  4:55 ` [PATCH 03/16] gdb.base/setshow.exp: split in procs Simon Marchi
2021-07-14  4:55 ` [PATCH 04/16] gdb.base/setshow.exp: fix duplicate test name Simon Marchi
2021-07-14  4:55 ` [PATCH 05/16] gdb: un-share set_inferior_cwd declaration Simon Marchi
2021-07-14  4:55 ` [PATCH 06/16] gdb: remove inferior::{argc,argv} Simon Marchi
2021-07-14  4:55 ` [PATCH 07/16] gdb: add setter/getter for inferior arguments Simon Marchi
2021-07-14  4:55 ` [PATCH 08/16] gdb: add setter/getter for inferior cwd Simon Marchi
2021-07-14  4:55 ` [PATCH 09/16] gdb: make inferior::m_args an std::string Simon Marchi
2021-07-14  4:55 ` [PATCH 10/16] gdb: make inferior::m_cwd " Simon Marchi
2021-07-14  4:55 ` [PATCH 11/16] gdb: make inferior::m_terminal " Simon Marchi
2021-07-14  4:55 ` [PATCH 12/16] gdb: rename cfunc to simple_func Simon Marchi
2021-07-14  4:55 ` [PATCH 13/16] gdb: remove cmd_list_element::function::sfunc Simon Marchi
2021-07-28 19:10   ` Tom Tromey
2021-07-28 21:17     ` Simon Marchi
2021-07-29 17:33       ` Tom Tromey
2021-07-14  4:55 ` [PATCH 14/16] gdb/testsuite: test get/set value of unregistered Guile parameter Simon Marchi
2021-07-23 19:42   ` Simon Marchi
2021-07-14  4:55 ` Simon Marchi [this message]
2021-07-14 12:08   ` [PATCH 15/16] gdb: make cmd_list_element var an optional union Lancelot SIX
2021-07-14 17:12     ` Lancelot SIX
2021-07-14 19:22       ` Simon Marchi
2021-07-14 23:22         ` Lancelot SIX
2021-07-19 14:32           ` Simon Marchi
2021-07-19 19:52             ` Simon Marchi
2021-07-20 23:03               ` Lancelot SIX
2021-07-23 19:56                 ` Simon Marchi
2021-07-23 20:46                   ` Lancelot SIX
2021-07-23 21:15                     ` Simon Marchi
2021-07-23 22:55                       ` Lancelot SIX
2021-07-24  2:04                         ` Simon Marchi
2021-07-28 20:13                 ` Tom Tromey
2021-07-28 20:45                   ` Lancelot SIX
2021-07-29 17:47                     ` Tom Tromey
2021-07-29 20:12                       ` Lancelot SIX
2021-07-30  2:09                         ` Simon Marchi
2021-07-30 17:47                           ` Lancelot SIX
2021-07-18 15:44   ` Lancelot SIX
2021-07-19 14:19     ` Simon Marchi
2021-07-19 20:58       ` Lancelot SIX
2021-07-28 19:47   ` Tom Tromey
2021-07-28 20:59     ` Simon Marchi
2021-07-29 17:41       ` Tom Tromey
2021-07-29 17:44         ` Simon Marchi
2021-07-29 17:49           ` Tom Tromey
2021-07-29 18:00             ` Simon Marchi
2021-07-14  4:55 ` [PATCH 16/16] gdb: make string-like set show commands use std::string variable Simon Marchi
2021-07-28 20:27   ` Tom Tromey
2021-07-28 21:03     ` Simon Marchi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210714045520.1623120-16-simon.marchi@polymtl.ca \
    --to=simon.marchi@polymtl.ca \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).