* [PATCH v2 0/4] gdb: Refactor cmd_list_element.var
@ 2021-08-08 19:22 Lancelot SIX
2021-08-08 19:22 ` [PATCH v2 1/4] gdb: Add typesafe getter/setter for cmd_list_element.var Lancelot SIX
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Lancelot SIX @ 2021-08-08 19:22 UTC (permalink / raw)
To: gdb-patches; +Cc: Lancelot SIX, Tom Tromey, Simon Marchi
Hi,
This is a V2 for
https://sourceware.org/pipermail/gdb-patches/2021-July/181183.html
Noticeable changes since V1:
- Based on the feedbacks form Tom Tromey, the API have been simplified and
the amount of templated code have been reduced. Instead of using
something like
setting.get<var_integer, var_zinetger, var_zuinteger_unlimited> ()
in order to access the value of a setting backed by an int (which must
be either a var_integer, a var_zinetger or a var_zuinteger_unlimited),
we now write
setting.get<int> ()
The 'get' function still includes an assert that setting.var_type is one
of the var_types that is backed by an int. In this version, the call
site needs to know the mapping between var_types and underlying storage
type.
- Based on a comment from Simon Marchi, the name of the type that ties
together the var_type and the void * pointer has changed from 'param' to
'setting'.
- Minor formatting issues have been addressed.
---
This patch series proposes to refactor the way cmd_list_element.var is
handled. It is meant to be a cleanup series and is intended to offer a
basis on which to solve PR/28085.
Some commands contain a reference to a variable (setting) that can be set
or shown. To do that, the command element contains pieces of information:
a void* pointer to global variable, and a var_types field to indicate the
type of the setting. The var_type dictates what the data type of variable
pointed by the void* pointer must be.
When any piece of code within GDB tries to interact with such variable, it
first checks the type field and then casts the void* pointer into whatever
type pointer it needs to and dereferences it. There are few limitations
with this approach I try to address in this patch series:
- Any code that interact with a cmd_list_element.var is responsible to
do some cast to the appropriate type. This is done in multiple places
in the codebase and is quite easy to get wrong. The first patch tries
to cover that by introducing a safer abstraction around the void *
pointer. The accessors that hide the void * pointer adds runtime
checks that the pointer is casted to the appropriate type that matches
the var_type of the setting instance.
- The source of truth for any setting needs to be the variable pointed to.
However, as pointed out by Simon in PR/28085 this is not always true, and
there are bugs linked to this. In order to solve this problem, patch 3
allows getter and setter callbacks to be embedded in the abstraction
introduced in patch 1. When the setting is accessed, the callback is
called to compute or update the value based on the current state of GDB.
Patch 2, which was originally proposed by Simon Marchi[1] proposes to
change the variable type from char* to std::string for string like
setting. Finally Patch 4 does some more refactoring on how we detect
and notify observers when a given setting is updated.
None of those patches should introduce user visible changes.
The entire series have been tested on x86_64 and no regression have been
observed on the testsuite.
All feedbacks are welcome.
Lancelot SIX (3):
gdb: Add typesafe getter/setter for cmd_list_element.var
gdb: Have setter and getter callbacks for settings
gdb: Setting setter return a bool to tell if the value changed
Simon Marchi (1):
gdb: make string-like set show commands use std::string variable
gdb/auto-load.c | 50 ++---
gdb/breakpoint.c | 22 +-
gdb/build-id.c | 4 +-
gdb/cli/cli-cmds.c | 80 ++++---
gdb/cli/cli-decode.c | 425 +++++++++++++++++++++++++++++------
gdb/cli/cli-decode.h | 6 +-
gdb/cli/cli-logging.c | 23 +-
gdb/cli/cli-option.c | 9 +-
gdb/cli/cli-option.h | 4 +-
gdb/cli/cli-setshow.c | 188 +++++++---------
gdb/command.h | 419 +++++++++++++++++++++++++++++++++-
gdb/compile/compile.c | 46 ++--
gdb/corefile.c | 17 +-
gdb/defs.h | 4 +-
gdb/disasm.c | 11 +-
gdb/disasm.h | 2 +-
gdb/dwarf2/dwz.c | 2 +-
gdb/dwarf2/index-cache.c | 10 +-
gdb/dwarf2/read.c | 10 +-
gdb/event-top.c | 12 +-
gdb/fork-child.c | 7 +-
gdb/guile/scm-param.c | 163 ++++++++------
gdb/infcmd.c | 14 +-
gdb/linux-thread-db.c | 17 +-
gdb/main.c | 17 +-
gdb/maint-test-options.c | 11 +-
gdb/maint-test-settings.c | 8 +-
gdb/maint.c | 2 +-
gdb/mi/mi-cmd-env.c | 18 +-
gdb/proc-api.c | 5 +-
gdb/python/py-param.c | 58 +++--
gdb/python/python-internal.h | 2 +-
gdb/python/python.c | 31 +--
gdb/remote-sim.c | 7 +-
gdb/remote.c | 12 +-
gdb/serial.c | 8 +-
gdb/solib.c | 20 +-
gdb/source.c | 66 +++---
gdb/source.h | 5 +-
gdb/stack.c | 22 +-
gdb/symfile.c | 49 ++--
gdb/symtab.c | 46 ++--
gdb/target-descriptions.c | 2 +-
gdb/top.c | 112 +++++----
gdb/top.h | 2 +-
gdb/tracepoint.c | 29 +--
gdb/tracepoint.h | 2 +-
47 files changed, 1378 insertions(+), 701 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/4] gdb: Add typesafe getter/setter for cmd_list_element.var
2021-08-08 19:22 [PATCH v2 0/4] gdb: Refactor cmd_list_element.var Lancelot SIX
@ 2021-08-08 19:22 ` Lancelot SIX
2021-08-10 1:29 ` Simon Marchi
2021-08-10 12:22 ` Simon Marchi
2021-08-08 19:23 ` [PATCH v2 2/4] gdb: make string-like set show commands use std::string variable Lancelot SIX
` (2 subsequent siblings)
3 siblings, 2 replies; 15+ messages in thread
From: Lancelot SIX @ 2021-08-08 19:22 UTC (permalink / raw)
To: gdb-patches; +Cc: Lancelot SIX
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.
---
gdb/auto-load.c | 2 +-
gdb/cli/cli-cmds.c | 63 ++++++++------
gdb/cli/cli-decode.c | 112 ++++++++++++-------------
gdb/cli/cli-decode.h | 6 +-
gdb/cli/cli-setshow.c | 151 ++++++++++++++++++++-------------
gdb/command.h | 158 +++++++++++++++++++++++++++++++++++
gdb/guile/scm-param.c | 125 +++++++++++++++------------
gdb/maint.c | 2 +-
gdb/python/py-param.c | 15 +++-
gdb/python/python-internal.h | 2 +-
gdb/python/python.c | 28 ++++---
gdb/remote.c | 2 +-
12 files changed, 448 insertions(+), 218 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..84991fbda43 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> () == 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..1f2df58607d 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 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..644812c4d46 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -125,6 +125,164 @@ 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 t);
+
+/* 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->empty ());
+
+ 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->empty ());
+
+ *static_cast<T *> (this->m_var) = v;
+ }
+
+ /* A setting is valid (can be evaluated to true) if it contains a valid
+ reference to a memory buffer. */
+ explicit operator bool () const
+ {
+ return !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 };
+
+ /* Indicates if the current instance has a underlying buffer. */
+ bool empty () const
+ {
+ return 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->empty ());
+ 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..6bf1da17203 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);
\f
/* 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:
@@ -566,14 +577,13 @@ pascm_param_type_name (enum var_types param_type)
If TYPE 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 +591,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 +612,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 +623,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 +697,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 +726,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 +746,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 +759,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 +950,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 +1046,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 +1077,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 +1095,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..c89405c4e84 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)
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;
--
2.31.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 2/4] gdb: make string-like set show commands use std::string variable
2021-08-08 19:22 [PATCH v2 0/4] gdb: Refactor cmd_list_element.var Lancelot SIX
2021-08-08 19:22 ` [PATCH v2 1/4] gdb: Add typesafe getter/setter for cmd_list_element.var Lancelot SIX
@ 2021-08-08 19:23 ` Lancelot SIX
2021-08-08 19:23 ` [PATCH v2 3/4] gdb: Have setter and getter callbacks for settings Lancelot SIX
2021-08-08 19:23 ` [PATCH v2 4/4] gdb: Setting setter return a bool to tell if the value changed Lancelot SIX
3 siblings, 0 replies; 15+ messages in thread
From: Lancelot SIX @ 2021-08-08 19:23 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi, Lancelot SIX
From: Simon Marchi <simon.marchi@polymtl.ca>
String-like settings (var_string, var_filename, var_optional_filename,
var_string_noescape) currently take a pointer to a `char *` storage
variable (typically global) that holds the setting's value. I'd like to
"mordernize" this by changing them to use an std::string for storage.
An obvious reason is that string operations on std::string are often
easier to write than with C strings. And they avoid having to do any
manual memory management.
Another interesting reason is that, with `char *`, nullptr and an empty
string often both have the same meaning of "no value". String settings
are initially nullptr (unless initialized otherwise). But when setting
them to nothing, they point to an empty string. For example,
solib_search_path is nullptr at startup, but points to an empty string
after doing "set solib-search-path". This leads to some code that needs
to check for both to check for "no value". Or some code that converts
back and forth between NULL and "" when getting or setting the value. I
find this very error-prone, because it is very easy to forget one or the
other. With std::string, we at least know that the variable is not
"NULL". There is only one way of representing an empty string setting,
that is with an empty string.
I was wondering whether the distinction between NULL and "" would be
important for some setting, but it doesn't seem so. If that ever
happens, it would be more C++-y and self-descriptive to use
optional<string> anyway.
Actually, there's one spot where this distinction mattered, it's in
init_history, for the test gdb.base/gdbinit-history.exp. init_history
sets the history filename to the default ".gdb_history" if it sees that
the setting was never set - if history_filename is nullptr. If
history_filename is an empty string, it means the setting was explicitly
cleared, so it leaves it as-is. With the change to std::string, this
distinction doesn't exist anymore. This can be fixed by moving the code
that chooses a good default value for history_filename to
_initialize_top. This is ran before -ex commands are processed, so an
-ex command can then clear that value if needed (what
gdb.base/gdbinit-history.exp tests).
Another small improvement, in my opinion is that we can now easily
give string parameters initial values, by simply initializing the global
variables, instead of xstrdup-ing it in the _initialize function.
In Python and Guile, when registering a string-like parameter, we
allocate (with new) an std::string that is owned by the param_smob (in
Guile) and the parmpy_object (in Python) objects.
This patch started by changing all relevant add_setshow_* commands to
take an `std::string *` instead of a `char **` and fixing everything
that failed to build. That includes of course all string setting
variable and their uses.
string_option_def now uses an std::string also, because there's a
connection between options and settings (see
add_setshow_cmds_for_options).
The add_path function in source.c is really complex and twisted, I'd
rather not try to change it to work on an std::string right now.
Instead, I added an overload that copies the std:string to a `char *`
and back. This means more copying, but this is not used in a hot path
at all, so I think it is acceptable.
Change-Id: I92c50a1bdd8307141cdbacb388248e4e4fc08c93
Co-authored-by: Lancelot SIX <lsix@lancelotsix.com>
---
gdb/auto-load.c | 48 ++++++----------
gdb/breakpoint.c | 22 ++++----
gdb/build-id.c | 4 +-
gdb/cli/cli-cmds.c | 33 +++++------
gdb/cli/cli-decode.c | 38 +++++++------
gdb/cli/cli-logging.c | 23 ++++----
gdb/cli/cli-option.c | 9 ++-
gdb/cli/cli-option.h | 4 +-
gdb/cli/cli-setshow.c | 57 ++++++++-----------
gdb/command.h | 23 ++++----
gdb/compile/compile.c | 46 ++++++++--------
gdb/corefile.c | 17 +++---
gdb/defs.h | 4 +-
gdb/disasm.c | 11 ++--
gdb/disasm.h | 2 +-
gdb/dwarf2/dwz.c | 2 +-
gdb/dwarf2/index-cache.c | 10 ++--
gdb/dwarf2/read.c | 10 ++--
gdb/event-top.c | 12 ++--
gdb/fork-child.c | 7 +--
gdb/guile/scm-param.c | 62 ++++++++++++---------
gdb/infcmd.c | 14 ++---
gdb/linux-thread-db.c | 17 ++----
gdb/main.c | 17 ++----
gdb/maint-test-options.c | 11 +---
gdb/maint-test-settings.c | 8 +--
gdb/mi/mi-cmd-env.c | 18 +++---
gdb/proc-api.c | 5 +-
gdb/python/py-param.c | 45 +++++++++------
gdb/python/python.c | 16 +++---
gdb/remote-sim.c | 7 +--
gdb/remote.c | 10 ++--
gdb/serial.c | 8 +--
gdb/solib.c | 20 +++----
gdb/source.c | 66 ++++++++++++----------
gdb/source.h | 5 +-
gdb/stack.c | 22 ++++----
gdb/symfile.c | 49 ++++++++---------
gdb/symtab.c | 46 ++++++++--------
gdb/target-descriptions.c | 2 +-
gdb/top.c | 112 ++++++++++++++++++--------------------
gdb/top.h | 2 +-
gdb/tracepoint.c | 29 +++++-----
gdb/tracepoint.h | 2 +-
44 files changed, 468 insertions(+), 507 deletions(-)
diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index 033c448eff7..b2a1cb4d7ac 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -137,7 +137,7 @@ show_auto_load_local_gdbinit (struct ui_file *file, int from_tty,
/* Directory list from which to load auto-loaded scripts. It is not checked
for absolute paths but they are strongly recommended. It is initialized by
_initialize_auto_load. */
-static char *auto_load_dir;
+static std::string auto_load_dir = AUTO_LOAD_DIR;
/* "set" command for the auto_load_dir configuration variable. */
@@ -145,11 +145,8 @@ static void
set_auto_load_dir (const char *args, int from_tty, struct cmd_list_element *c)
{
/* Setting the variable to "" resets it to the compile time defaults. */
- if (auto_load_dir[0] == '\0')
- {
- xfree (auto_load_dir);
- auto_load_dir = xstrdup (AUTO_LOAD_DIR);
- }
+ if (auto_load_dir.empty ())
+ auto_load_dir = AUTO_LOAD_DIR;
}
/* "show" command for the auto_load_dir configuration variable. */
@@ -166,7 +163,7 @@ show_auto_load_dir (struct ui_file *file, int from_tty,
/* Directory list safe to hold auto-loaded files. It is not checked for
absolute paths but they are strongly recommended. It is initialized by
_initialize_auto_load. */
-static char *auto_load_safe_path;
+static std::string auto_load_safe_path = AUTO_LOAD_SAFE_PATH;
/* Vector of directory elements of AUTO_LOAD_SAFE_PATH with each one normalized
by tilde_expand and possibly each entries has added its gdb_realpath
@@ -181,7 +178,7 @@ auto_load_expand_dir_vars (const char *string)
{
char *s = xstrdup (string);
substitute_path_component (&s, "$datadir", gdb_datadir.c_str ());
- substitute_path_component (&s, "$debugdir", debug_file_directory);
+ substitute_path_component (&s, "$debugdir", debug_file_directory.c_str ());
if (debug_auto_load && strcmp (s, string) != 0)
auto_load_debug_printf ("Expanded $-variables to \"%s\".", s);
@@ -199,9 +196,10 @@ static void
auto_load_safe_path_vec_update (void)
{
auto_load_debug_printf ("Updating directories of \"%s\".",
- auto_load_safe_path);
+ auto_load_safe_path.c_str ());
- auto_load_safe_path_vec = auto_load_expand_dir_vars (auto_load_safe_path);
+ auto_load_safe_path_vec
+ = auto_load_expand_dir_vars (auto_load_safe_path.c_str ());
size_t len = auto_load_safe_path_vec.size ();
/* Apply tilde_expand and gdb_realpath to each AUTO_LOAD_SAFE_PATH_VEC
@@ -253,11 +251,8 @@ set_auto_load_safe_path (const char *args,
int from_tty, struct cmd_list_element *c)
{
/* Setting the variable to "" resets it to the compile time defaults. */
- if (auto_load_safe_path[0] == '\0')
- {
- xfree (auto_load_safe_path);
- auto_load_safe_path = xstrdup (AUTO_LOAD_SAFE_PATH);
- }
+ if (auto_load_safe_path.empty ())
+ auto_load_safe_path = AUTO_LOAD_SAFE_PATH;
auto_load_safe_path_vec_update ();
}
@@ -291,17 +286,14 @@ show_auto_load_safe_path (struct ui_file *file, int from_tty,
static void
add_auto_load_safe_path (const char *args, int from_tty)
{
- char *s;
-
if (args == NULL || *args == 0)
error (_("\
Directory argument required.\n\
Use 'set auto-load safe-path /' for disabling the auto-load safe-path security.\
"));
- s = xstrprintf ("%s%c%s", auto_load_safe_path, DIRNAME_SEPARATOR, args);
- xfree (auto_load_safe_path);
- auto_load_safe_path = s;
+ auto_load_safe_path = string_printf ("%s%c%s", auto_load_safe_path.c_str (),
+ DIRNAME_SEPARATOR, args);
auto_load_safe_path_vec_update ();
}
@@ -312,14 +304,11 @@ Use 'set auto-load safe-path /' for disabling the auto-load safe-path security.\
static void
add_auto_load_dir (const char *args, int from_tty)
{
- char *s;
-
if (args == NULL || *args == 0)
error (_("Directory argument required."));
- s = xstrprintf ("%s%c%s", auto_load_dir, DIRNAME_SEPARATOR, args);
- xfree (auto_load_dir);
- auto_load_dir = s;
+ auto_load_dir = string_printf ("%s%c%s", auto_load_dir.c_str (),
+ DIRNAME_SEPARATOR, args);
}
/* Implementation for filename_is_in_pattern overwriting the caller's FILENAME
@@ -459,7 +448,7 @@ file_is_auto_load_safe (const char *filename)
warning (_("File \"%ps\" auto-loading has been declined by your "
"`auto-load safe-path' set to \"%s\"."),
styled_string (file_name_style.style (), filename_real.get ()),
- auto_load_safe_path);
+ auto_load_safe_path.c_str ());
if (!advice_printed)
{
@@ -749,11 +738,11 @@ auto_load_objfile_script_1 (struct objfile *objfile, const char *realname,
directory. */
std::vector<gdb::unique_xmalloc_ptr<char>> vec
- = auto_load_expand_dir_vars (auto_load_dir);
+ = auto_load_expand_dir_vars (auto_load_dir.c_str ());
auto_load_debug_printf
("Searching 'set auto-load scripts-directory' path \"%s\".",
- auto_load_dir);
+ auto_load_dir.c_str ());
/* Convert Windows file name from c:/dir/file to /c/dir/file. */
if (HAS_DRIVE_SPEC (debugfile))
@@ -1542,8 +1531,6 @@ This option has security implications for untrusted inferiors."),
Usage: info auto-load local-gdbinit"),
auto_load_info_cmdlist_get ());
- auto_load_dir = xstrdup (AUTO_LOAD_DIR);
-
suffix = ext_lang_auto_load_suffix (get_ext_lang_defn (EXT_LANG_GDB));
gdb_name_help
= xstrprintf (_("\
@@ -1595,7 +1582,6 @@ Show the list of directories from which to load auto-loaded scripts."),
xfree (gdb_name_help);
xfree (guile_name_help);
- auto_load_safe_path = xstrdup (AUTO_LOAD_SAFE_PATH);
auto_load_safe_path_vec_update ();
add_setshow_optional_filename_cmd ("safe-path", class_support,
&auto_load_safe_path, _("\
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 89af44ee4c6..d07084f1d52 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -276,7 +276,7 @@ static const char *dprintf_style = dprintf_style_gdb;
copied into the command, so it can be anything that GDB can
evaluate to a callable address, not necessarily a function name. */
-static char *dprintf_function;
+static std::string dprintf_function = "printf";
/* The channel to use for dynamic printf if the preferred style is to
call into the inferior; if a nonempty string, it will be passed to
@@ -286,7 +286,7 @@ static char *dprintf_function;
"stderr", this could be an app-specific expression like
"mystreams[curlogger]". */
-static char *dprintf_channel;
+static std::string dprintf_channel;
/* True if dprintf commands should continue to operate even if GDB
has disconnected. */
@@ -6672,7 +6672,7 @@ default_collect_info (void)
/* If it has no value (which is frequently the case), say nothing; a
message like "No default-collect." gets in user's face when it's
not wanted. */
- if (!*default_collect)
+ if (default_collect.empty ())
return;
/* The following phrase lines up nicely with per-tracepoint collect
@@ -8773,17 +8773,17 @@ update_dprintf_command_list (struct breakpoint *b)
printf_line = xstrprintf ("printf %s", dprintf_args);
else if (strcmp (dprintf_style, dprintf_style_call) == 0)
{
- if (!dprintf_function)
+ if (dprintf_function.empty ())
error (_("No function supplied for dprintf call"));
- if (dprintf_channel && strlen (dprintf_channel) > 0)
+ if (!dprintf_channel.empty ())
printf_line = xstrprintf ("call (void) %s (%s,%s)",
- dprintf_function,
- dprintf_channel,
+ dprintf_function.c_str (),
+ dprintf_channel.c_str (),
dprintf_args);
else
printf_line = xstrprintf ("call (void) %s (%s)",
- dprintf_function,
+ dprintf_function.c_str (),
dprintf_args);
}
else if (strcmp (dprintf_style, dprintf_style_agent) == 0)
@@ -15116,8 +15116,8 @@ save_breakpoints (const char *filename, int from_tty,
}
}
- if (extra_trace_bits && *default_collect)
- fp.printf ("set default-collect %s\n", default_collect);
+ if (extra_trace_bits && !default_collect.empty ())
+ fp.printf ("set default-collect %s\n", default_collect.c_str ());
if (from_tty)
printf_filtered (_("Saved to file '%s'.\n"), expanded_filename.get ());
@@ -16028,7 +16028,6 @@ output stream by setting dprintf-function and dprintf-channel."),
update_dprintf_commands, NULL,
&setlist, &showlist);
- dprintf_function = xstrdup ("printf");
add_setshow_string_cmd ("dprintf-function", class_support,
&dprintf_function, _("\
Set the function to use for dynamic printf."), _("\
@@ -16036,7 +16035,6 @@ Show the function to use for dynamic printf."), NULL,
update_dprintf_commands, NULL,
&setlist, &showlist);
- dprintf_channel = xstrdup ("");
add_setshow_string_cmd ("dprintf-channel", class_support,
&dprintf_channel, _("\
Set the channel to use for dynamic printf."), _("\
diff --git a/gdb/build-id.c b/gdb/build-id.c
index d2f2576d96d..553d6cec3d2 100644
--- a/gdb/build-id.c
+++ b/gdb/build-id.c
@@ -130,7 +130,7 @@ build_id_to_bfd_suffix (size_t build_id_len, const bfd_byte *build_id,
cause "/.build-id/..." lookups. */
std::vector<gdb::unique_xmalloc_ptr<char>> debugdir_vec
- = dirnames_to_char_ptr_vec (debug_file_directory);
+ = dirnames_to_char_ptr_vec (debug_file_directory.c_str ());
for (const gdb::unique_xmalloc_ptr<char> &debugdir : debugdir_vec)
{
@@ -167,7 +167,7 @@ build_id_to_bfd_suffix (size_t build_id_len, const bfd_byte *build_id,
Don't do it if the sysroot is the target system ("target:"). It
could work in theory, but the lrealpath in build_id_to_debug_bfd_1
only works with local paths. */
- if (strcmp (gdb_sysroot, TARGET_SYSROOT_PREFIX) != 0)
+ if (gdb_sysroot != TARGET_SYSROOT_PREFIX)
{
link = gdb_sysroot + link;
debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id);
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 84991fbda43..944771ef0d1 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -645,7 +645,7 @@ find_and_open_script (const char *script_file, int search_path)
/* Search for and open 'file' on the search path used for source
files. Put the full location in *FULL_PATHP. */
gdb::unique_xmalloc_ptr<char> full_path;
- fd = openp (source_path, search_flags,
+ fd = openp (source_path.c_str (), search_flags,
file.get (), O_RDONLY, &full_path);
if (fd == -1)
@@ -1037,12 +1037,7 @@ edit_command (const char *arg, int from_tty)
struct pipe_cmd_opts
{
/* For "-d". */
- char *delimiter = nullptr;
-
- ~pipe_cmd_opts ()
- {
- xfree (delimiter);
- }
+ std::string delimiter;
};
static const gdb::option::option_def pipe_cmd_option_defs[] = {
@@ -1079,8 +1074,8 @@ pipe_command (const char *arg, int from_tty)
(&arg, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, grp);
const char *delim = "|";
- if (opts.delimiter != nullptr)
- delim = opts.delimiter;
+ if (!opts.delimiter.empty ())
+ delim = opts.delimiter.c_str ();
const char *command = arg;
if (command == nullptr)
@@ -1143,8 +1138,8 @@ pipe_command_completer (struct cmd_list_element *ignore,
return;
const char *delimiter = "|";
- if (opts.delimiter != nullptr)
- delimiter = opts.delimiter;
+ if (!opts.delimiter.empty ())
+ delimiter = opts.delimiter.c_str ();
/* Check if we're past option values already. */
if (text > org_text && !isspace (text[-1]))
@@ -2146,14 +2141,14 @@ value_from_setting (const cmd_list_element *cmd, struct gdbarch *gdbarch)
case var_filename:
case var_enum:
{
- const char *var;
+ std::string var;
if (cmd->var.type () == var_enum)
var = cmd->var.get<const char *> ();
else
- var = cmd->var.get<char *> ();
+ var = cmd->var.get<std::string> ();
- if (var != nullptr)
- return value_cstring (var, strlen (var),
+ if (!var.empty ())
+ return value_cstring (var.c_str (), var.length (),
builtin_type (gdbarch)->builtin_char);
else
return value_cstring ("", 1,
@@ -2220,14 +2215,14 @@ str_value_from_setting (const cmd_list_element *cmd, struct gdbarch *gdbarch)
escaping quotes. So, we directly use the cmd->var string value,
similarly to the value_from_setting code for these cases. */
{
- const char *var;
+ std::string var;
if (cmd->var.type () == var_enum)
var = cmd->var.get<const char *> ();
else
- var = cmd->var.get<char *> ();
+ var = cmd->var.get<std::string> ();
- if (var != nullptr)
- return value_cstring (var, strlen (var),
+ if (!var.empty ())
+ return value_cstring (var.c_str (), var.length (),
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 b59c6ffbda7..865e1331c8f 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -629,7 +629,7 @@ add_setshow_boolean_cmd (const char *name, enum command_class theclass, bool *va
set_show_commands
add_setshow_filename_cmd (const char *name, enum command_class theclass,
- char **var,
+ std::string *var,
const char *set_doc, const char *show_doc,
const char *help_doc,
cmd_func_ftype *set_func,
@@ -638,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<char *> (name, theclass, var_filename, var,
- set_doc, show_doc, help_doc,
- set_func, show_func,
- set_list, show_list);
+ = add_setshow_cmd_full<std::string> (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);
@@ -653,7 +653,7 @@ add_setshow_filename_cmd (const char *name, enum command_class theclass,
set_show_commands
add_setshow_string_cmd (const char *name, enum command_class theclass,
- char **var,
+ std::string *var,
const char *set_doc, const char *show_doc,
const char *help_doc,
cmd_func_ftype *set_func,
@@ -662,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<char *> (name, theclass, var_string, var,
- set_doc, show_doc, help_doc,
- set_func, show_func,
- set_list, show_list);
+ = add_setshow_cmd_full<std::string> (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);
@@ -678,7 +678,7 @@ add_setshow_string_cmd (const char *name, enum command_class theclass,
set_show_commands
add_setshow_string_noescape_cmd (const char *name, enum command_class theclass,
- char **var,
+ std::string *var,
const char *set_doc, const char *show_doc,
const char *help_doc,
cmd_func_ftype *set_func,
@@ -687,9 +687,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<char *> (name, theclass, var_string_noescape, var,
- set_doc, show_doc, help_doc, set_func,
- show_func, set_list, show_list);
+ = add_setshow_cmd_full<std::string> (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);
@@ -702,7 +703,7 @@ add_setshow_string_noescape_cmd (const char *name, enum command_class theclass,
set_show_commands
add_setshow_optional_filename_cmd (const char *name, enum command_class theclass,
- char **var,
+ std::string *var,
const char *set_doc, const char *show_doc,
const char *help_doc,
cmd_func_ftype *set_func,
@@ -711,9 +712,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<char *> (name, theclass, var_optional_filename,
- var, set_doc, show_doc, help_doc,
- set_func, show_func, set_list, show_list);
+ = add_setshow_cmd_full<std::string> (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);
diff --git a/gdb/cli/cli-logging.c b/gdb/cli/cli-logging.c
index dfedd7599a2..c9093520a71 100644
--- a/gdb/cli/cli-logging.c
+++ b/gdb/cli/cli-logging.c
@@ -25,7 +25,7 @@
static char *saved_filename;
-static char *logging_filename;
+static std::string logging_filename = "gdb.txt";
static void
show_logging_filename (struct ui_file *file, int from_tty,
struct cmd_list_element *c, const char *value)
@@ -102,7 +102,7 @@ handle_redirections (int from_tty)
}
stdio_file_up log (new no_terminal_escape_file ());
- if (!log->open (logging_filename, logging_overwrite ? "w" : "a"))
+ if (!log->open (logging_filename.c_str (), logging_overwrite ? "w" : "a"))
perror_with_name (_("set logging"));
/* Redirects everything to gdb_stdout while this is running. */
@@ -110,20 +110,20 @@ handle_redirections (int from_tty)
{
if (!logging_redirect)
fprintf_unfiltered (gdb_stdout, "Copying output to %s.\n",
- logging_filename);
+ logging_filename.c_str ());
else
fprintf_unfiltered (gdb_stdout, "Redirecting output to %s.\n",
- logging_filename);
+ logging_filename.c_str ());
if (!debug_redirect)
fprintf_unfiltered (gdb_stdout, "Copying debug output to %s.\n",
- logging_filename);
+ logging_filename.c_str ());
else
fprintf_unfiltered (gdb_stdout, "Redirecting debug output to %s.\n",
- logging_filename);
+ logging_filename.c_str ());
}
- saved_filename = xstrdup (logging_filename);
+ saved_filename = xstrdup (logging_filename.c_str ());
/* Let the interpreter do anything it needs. */
current_interp_set_logging (std::move (log), logging_redirect,
@@ -145,10 +145,8 @@ set_logging_on (const char *args, int from_tty)
const char *rest = args;
if (rest && *rest)
- {
- xfree (logging_filename);
- logging_filename = xstrdup (rest);
- }
+ logging_filename = rest;
+
handle_redirections (from_tty);
}
@@ -201,6 +199,7 @@ If debug redirect is on, debug will go only to the log file."),
set_logging_redirect,
show_logging_redirect,
&set_logging_cmdlist, &show_logging_cmdlist);
+
add_setshow_filename_cmd ("file", class_support, &logging_filename, _("\
Set the current logfile."), _("\
Show the current logfile."), _("\
@@ -212,6 +211,4 @@ The logfile is used when directing GDB's output."),
_("Enable logging."), &set_logging_cmdlist);
add_cmd ("off", class_support, set_logging_off,
_("Disable logging."), &set_logging_cmdlist);
-
- logging_filename = xstrdup ("gdb.txt");
}
diff --git a/gdb/cli/cli-option.c b/gdb/cli/cli-option.c
index ab76f194c3d..846b8198985 100644
--- a/gdb/cli/cli-option.c
+++ b/gdb/cli/cli-option.c
@@ -45,7 +45,7 @@ union option_value
const char *enumeration;
/* For var_string options. This is malloc-allocated. */
- char *string;
+ std::string *string;
};
/* Holds an options definition and its value. */
@@ -87,7 +87,7 @@ struct option_def_and_value
if (value.has_value ())
{
if (option.type == var_string)
- xfree (value->string);
+ delete value->string;
}
}
@@ -439,7 +439,7 @@ parse_option (gdb::array_view<const option_def_group> options_group,
error (_("-%s requires an argument"), match->name);
option_value val;
- val.string = xstrdup (str.c_str ());
+ val.string = new std::string (std::move (str));
return option_def_and_value {*match, match_ctx, val};
}
@@ -603,8 +603,7 @@ save_option_value_in_ctx (gdb::optional<option_def_and_value> &ov)
break;
case var_string:
*ov->option.var_address.string (ov->option, ov->ctx)
- = ov->value->string;
- ov->value->string = nullptr;
+ = std::move (*ov->value->string);
break;
default:
gdb_assert_not_reached ("unhandled option type");
diff --git a/gdb/cli/cli-option.h b/gdb/cli/cli-option.h
index aa2ccbed5d0..b7ede458748 100644
--- a/gdb/cli/cli-option.h
+++ b/gdb/cli/cli-option.h
@@ -86,7 +86,7 @@ struct option_def
unsigned int *(*uinteger) (const option_def &, void *ctx);
int *(*integer) (const option_def &, void *ctx);
const char **(*enumeration) (const option_def &, void *ctx);
- char **(*string) (const option_def &, void *ctx);
+ std::string *(*string) (const option_def &, void *ctx);
}
var_address;
@@ -268,7 +268,7 @@ template<typename Context>
struct string_option_def : option_def
{
string_option_def (const char *long_option_,
- char **(*get_var_address_cb_) (Context *),
+ std::string *(*get_var_address_cb_) (Context *),
show_value_ftype *show_cmd_cb_,
const char *set_doc_,
const char *show_doc_ = nullptr,
diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c
index 975c677fd52..3af99a5e917 100644
--- a/gdb/cli/cli-setshow.c
+++ b/gdb/cli/cli-setshow.c
@@ -353,27 +353,22 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
*q++ = '\0';
newobj = (char *) xrealloc (newobj, q - newobj);
- char * const var = c->var.get<char *> ();
- if (var == nullptr
- || strcmp (var, newobj) != 0)
+ const std::string var = c->var.get<std::string> ();
+ if (var != std::string (newobj))
{
- xfree (var);
- c->var.set<char *> (newobj);
+ c->var.set<std::string> (std::string (newobj));
option_changed = 1;
}
- else
- xfree (newobj);
+ xfree (newobj);
}
break;
case var_string_noescape:
{
- char * const var = c->var.get<char *> ();
- if (var == nullptr
- || strcmp (var, arg) != 0)
+ const std::string var = c->var.get<std::string> ();
+ if (var != arg)
{
- xfree (var);
- c->var.set<char *> (xstrdup (arg));
+ c->var.set<std::string> (std::string (arg));
option_changed = 1;
}
@@ -403,17 +398,14 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
else
val = xstrdup ("");
- char * const var = c->var.get<char *> ();
- if (var == nullptr
- || strcmp (var, val) != 0)
+ const std::string var = c->var.get<std::string> ();
+ if (var != std::string (val))
{
- xfree (var);
- c->var.set<char *> (val);
+ c->var.set<std::string> (std::string (val));
option_changed = 1;
}
- else
- xfree (val);
+ xfree (val);
}
break;
case var_boolean:
@@ -590,15 +582,12 @@ 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.get<std::string> ().c_str ());
+ break;
case var_enum:
- {
- 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);
- }
+ gdb::observers::command_param_changed.notify
+ (name, c->var.get<const char *> ());
break;
case var_boolean:
{
@@ -650,21 +639,19 @@ get_setshow_command_value_string (const cmd_list_element *c)
{
case var_string:
{
- char * var = c->var.get<char *> ();
- if (var != nullptr)
- stb.putstr (var, '"');
+ std::string var = c->var.get<std::string> ();
+ if (!var.empty ())
+ stb.putstr (var.c_str (), '"');
}
break;
case var_string_noescape:
case var_optional_filename:
case var_filename:
+ stb.puts (c->var.get<std::string> ().c_str ());
+ break;
case var_enum:
{
- const char *var;
- if (c->var.type () == var_enum)
- var = c->var.get<const char *> ();
- else
- var = c->var.get<char *> ();
+ const char * var = c->var.get<const char *> ();
if (var != nullptr)
stb.puts (var);
}
diff --git a/gdb/command.h b/gdb/command.h
index 644812c4d46..4e4583dcfa8 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -97,16 +97,15 @@ typedef enum var_types
/* String which the user enters with escapes (e.g. the user types
\n and it is a real newline in the stored string).
- *VAR is a malloc'd string, or NULL if the string is empty. */
+ *VAR is a std::string, "" if the string is empty. */
var_string,
/* String which stores what the user types verbatim.
- *VAR is a malloc'd string, or NULL if the string is empty. */
+ *VAR is std::string, "" if the string is empty. */
var_string_noescape,
- /* String which stores a filename. (*VAR) is a malloc'd string,
- or "" if the string was empty. */
+ /* String which stores a filename. (*VAR) is a std::string,
+ "" if the string was empty. */
var_optional_filename,
- /* String which stores a filename. (*VAR) is a malloc'd
- string. */
+ /* String which stores a filename. (*VAR) is a std::string. */
var_filename,
/* ZeroableInteger. *VAR is an int. Like var_integer except
that zero really means zero. */
@@ -166,9 +165,9 @@ inline bool var_type_uses<int> (var_types t)
|| t == var_zuinteger_unlimited);
}
-/* Return true if a setting of type T is backed by a char * variable. */
+/* Return true if a setting of type T is backed by a std::string variable. */
template<>
-inline bool var_type_uses<char *> (var_types t)
+inline bool var_type_uses<std::string> (var_types t)
{
return (t == var_string || t == var_string_noescape
|| t == var_optional_filename || t == var_filename);
@@ -577,25 +576,25 @@ extern set_show_commands add_setshow_boolean_cmd
cmd_list_element **show_list);
extern set_show_commands add_setshow_filename_cmd
- (const char *name, command_class theclass, char **var, const char *set_doc,
+ (const char *name, command_class theclass, std::string *var, const char *set_doc,
const char *show_doc, const char *help_doc, cmd_func_ftype *set_func,
show_value_ftype *show_func, cmd_list_element **set_list,
cmd_list_element **show_list);
extern set_show_commands add_setshow_string_cmd
- (const char *name, command_class theclass, char **var, const char *set_doc,
+ (const char *name, command_class theclass, std::string *var, const char *set_doc,
const char *show_doc, const char *help_doc, cmd_func_ftype *set_func,
show_value_ftype *show_func, cmd_list_element **set_list,
cmd_list_element **show_list);
extern set_show_commands add_setshow_string_noescape_cmd
- (const char *name, command_class theclass, char **var, const char *set_doc,
+ (const char *name, command_class theclass, std::string *var, const char *set_doc,
const char *show_doc, const char *help_doc, cmd_func_ftype *set_func,
show_value_ftype *show_func, cmd_list_element **set_list,
cmd_list_element **show_list);
extern set_show_commands add_setshow_optional_filename_cmd
- (const char *name, command_class theclass, char **var, const char *set_doc,
+ (const char *name, command_class theclass, std::string *var, const char *set_doc,
const char *show_doc, const char *help_doc, cmd_func_ftype *set_func,
show_value_ftype *show_func, cmd_list_element **set_list,
cmd_list_element **show_list);
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index e815348ff07..25cfd283b33 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -495,7 +495,22 @@ get_expr_block_and_pc (CORE_ADDR *pc)
}
/* String for 'set compile-args' and 'show compile-args'. */
-static char *compile_args;
+static std::string compile_args =
+ /* Override flags possibly coming from DW_AT_producer. */
+ "-O0 -gdwarf-4"
+ /* We use -fPIE Otherwise GDB would need to reserve space large enough for
+ any object file in the inferior in advance to get the final address when
+ to link the object file to and additionally the default system linker
+ script would need to be modified so that one can specify there the
+ absolute target address.
+ -fPIC is not used at is would require from GDB to generate .got. */
+ " -fPIE"
+ /* We want warnings, except for some commonly happening for GDB commands. */
+ " -Wall "
+ " -Wno-unused-but-set-variable"
+ " -Wno-unused-variable"
+ /* Override CU's possible -fstack-protector-strong. */
+ " -fno-stack-protector";
/* Parsed form of COMPILE_ARGS. */
static gdb_argv compile_args_argv;
@@ -505,7 +520,7 @@ static gdb_argv compile_args_argv;
static void
set_compile_args (const char *args, int from_tty, struct cmd_list_element *c)
{
- compile_args_argv = gdb_argv (compile_args);
+ compile_args_argv = gdb_argv (compile_args.c_str ());
}
/* Implement 'show compile-args'. */
@@ -520,7 +535,7 @@ show_compile_args (struct ui_file *file, int from_tty,
}
/* String for 'set compile-gcc' and 'show compile-gcc'. */
-static char *compile_gcc;
+static std::string compile_gcc;
/* Implement 'show compile-gcc'. */
@@ -696,13 +711,13 @@ compile_to_object (struct command_line *cmd, const char *cmd_string,
compiler->set_verbose (compile_debug);
- if (compile_gcc[0] != 0)
+ if (!compile_gcc.empty ())
{
if (compiler->version () < GCC_FE_VERSION_1)
error (_("Command 'set compile-gcc' requires GCC version 6 or higher "
"(libcc1 interface version 1 or higher)"));
- compiler->set_driver_filename (compile_gcc);
+ compiler->set_driver_filename (compile_gcc.c_str ());
}
else
{
@@ -1029,23 +1044,9 @@ String quoting is parsed like in shell, for example:\n\
-mno-align-double \"-I/dir with a space/include\""),
set_compile_args, show_compile_args, &setlist, &showlist);
- /* Override flags possibly coming from DW_AT_producer. */
- compile_args = xstrdup ("-O0 -gdwarf-4"
- /* We use -fPIE Otherwise GDB would need to reserve space large enough for
- any object file in the inferior in advance to get the final address when
- to link the object file to and additionally the default system linker
- script would need to be modified so that one can specify there the
- absolute target address.
- -fPIC is not used at is would require from GDB to generate .got. */
- " -fPIE"
- /* We want warnings, except for some commonly happening for GDB commands. */
- " -Wall "
- " -Wno-unused-but-set-variable"
- " -Wno-unused-variable"
- /* Override CU's possible -fstack-protector-strong. */
- " -fno-stack-protector"
- );
- set_compile_args (compile_args, 0, NULL);
+
+ /* Initialize compile_args_argv. */
+ set_compile_args (compile_args.c_str (), 0, NULL);
add_setshow_optional_filename_cmd ("compile-gcc", class_support,
&compile_gcc,
@@ -1058,5 +1059,4 @@ It should be absolute filename of the gcc executable.\n\
If empty the default target triplet will be searched in $PATH."),
NULL, show_compile_gcc, &setlist,
&showlist);
- compile_gcc = xstrdup ("");
}
diff --git a/gdb/corefile.c b/gdb/corefile.c
index ff2494738b0..4b1451487e2 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -395,7 +395,7 @@ write_memory_signed_integer (CORE_ADDR addr, int len,
const char *gnutarget;
/* Same thing, except it is "auto" not NULL for the default case. */
-static char *gnutarget_string;
+static std::string gnutarget_string;
static void
show_gnutarget_string (struct ui_file *file, int from_tty,
struct cmd_list_element *c,
@@ -409,15 +409,15 @@ static void
set_gnutarget_command (const char *ignore, int from_tty,
struct cmd_list_element *c)
{
- char *gend = gnutarget_string + strlen (gnutarget_string);
+ const char *gend = gnutarget_string.c_str () + gnutarget_string.size ();
+ gend = remove_trailing_whitespace (gnutarget_string.c_str (), gend);
+ gnutarget_string
+ = gnutarget_string.substr (0, gend - gnutarget_string.data ());
- gend = remove_trailing_whitespace (gnutarget_string, gend);
- *gend = '\0';
-
- if (strcmp (gnutarget_string, "auto") == 0)
+ if (gnutarget_string == "auto")
gnutarget = NULL;
else
- gnutarget = gnutarget_string;
+ gnutarget = gnutarget_string.c_str ();
}
/* A completion function for "set gnutarget". */
@@ -449,8 +449,7 @@ complete_set_gnutarget (struct cmd_list_element *cmd,
void
set_gnutarget (const char *newtarget)
{
- xfree (gnutarget_string);
- gnutarget_string = xstrdup (newtarget);
+ gnutarget_string = newtarget;
set_gnutarget_command (NULL, 0, NULL);
}
diff --git a/gdb/defs.h b/gdb/defs.h
index 6dea4099554..f7e09eca9db 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -121,7 +121,7 @@ using RequireLongest = gdb::Requires<gdb::Or<std::is_same<T, LONGEST>,
extern int dbx_commands;
/* * System root path, used to find libraries etc. */
-extern char *gdb_sysroot;
+extern std::string gdb_sysroot;
/* * GDB datadir, used to store data files. */
extern std::string gdb_datadir;
@@ -131,7 +131,7 @@ extern std::string gdb_datadir;
extern std::string python_libdir;
/* * Search path for separate debug files. */
-extern char *debug_file_directory;
+extern std::string debug_file_directory;
/* GDB's SIGINT handler basically sets a flag; code that might take a
long time before it gets back to the event loop, and which ought to
diff --git a/gdb/disasm.c b/gdb/disasm.c
index 7f730f68188..c788f5b618c 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -39,7 +39,7 @@
/* This variable is used to hold the prospective disassembler_options value
which is set by the "set disassembler_options" command. */
-static char *prospective_options = NULL;
+static std::string prospective_options;
/* This structure is used to store line number information for the
deprecated /m option.
@@ -928,13 +928,16 @@ get_disassembler_options (struct gdbarch *gdbarch)
}
void
-set_disassembler_options (char *prospective_options)
+set_disassembler_options (const char *prospective_options)
{
struct gdbarch *gdbarch = get_current_arch ();
char **disassembler_options = gdbarch_disassembler_options (gdbarch);
const disasm_options_and_args_t *valid_options_and_args;
const disasm_options_t *valid_options;
- char *options = remove_whitespace_and_extra_commas (prospective_options);
+ gdb::unique_xmalloc_ptr<char> prospective_options_local
+ = make_unique_xstrdup (prospective_options);
+ char *options = remove_whitespace_and_extra_commas
+ (prospective_options_local.get ());
const char *opt;
/* Allow all architectures, even ones that do not support 'set disassembler',
@@ -1003,7 +1006,7 @@ static void
set_disassembler_options_sfunc (const char *args, int from_tty,
struct cmd_list_element *c)
{
- set_disassembler_options (prospective_options);
+ set_disassembler_options (prospective_options.c_str ());
}
static void
diff --git a/gdb/disasm.h b/gdb/disasm.h
index eb82bc3cd01..6cbfdcd4f6b 100644
--- a/gdb/disasm.h
+++ b/gdb/disasm.h
@@ -164,6 +164,6 @@ extern char *get_disassembler_options (struct gdbarch *gdbarch);
/* Sets the active gdbarch's disassembler options to OPTIONS. */
-extern void set_disassembler_options (char *options);
+extern void set_disassembler_options (const char *options);
#endif
diff --git a/gdb/dwarf2/dwz.c b/gdb/dwarf2/dwz.c
index f9d5db6b48a..5e963d5b47a 100644
--- a/gdb/dwarf2/dwz.c
+++ b/gdb/dwarf2/dwz.c
@@ -118,7 +118,7 @@ dwz_search_other_debugdirs (std::string &filename, bfd_byte *buildid,
gdb_bfd_ref_ptr dwz_bfd;
std::vector<gdb::unique_xmalloc_ptr<char>> debugdir_vec
- = dirnames_to_char_ptr_vec (debug_file_directory);
+ = dirnames_to_char_ptr_vec (debug_file_directory.c_str ());
for (const gdb::unique_xmalloc_ptr<char> &debugdir : debugdir_vec)
{
diff --git a/gdb/dwarf2/index-cache.c b/gdb/dwarf2/index-cache.c
index c3decf16ead..f439b37db5a 100644
--- a/gdb/dwarf2/index-cache.c
+++ b/gdb/dwarf2/index-cache.c
@@ -37,7 +37,7 @@
static bool debug_index_cache = false;
/* The index cache directory, used for "set/show index-cache directory". */
-static char *index_cache_directory = NULL;
+static std::string index_cache_directory;
/* See dwarf-index.cache.h. */
index_cache global_index_cache;
@@ -290,9 +290,9 @@ set_index_cache_directory_command (const char *arg, int from_tty,
cmd_list_element *element)
{
/* Make sure the index cache directory is absolute and tilde-expanded. */
- gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (index_cache_directory));
- xfree (index_cache_directory);
- index_cache_directory = abs.release ();
+ gdb::unique_xmalloc_ptr<char> abs
+ = gdb_abspath (index_cache_directory.c_str ());
+ index_cache_directory = abs.get ();
global_index_cache.set_directory (index_cache_directory);
}
@@ -325,7 +325,7 @@ _initialize_index_cache ()
std::string cache_dir = get_standard_cache_dir ();
if (!cache_dir.empty ())
{
- index_cache_directory = xstrdup (cache_dir.c_str ());
+ index_cache_directory = cache_dir;
global_index_cache.set_directory (std::move (cache_dir));
}
else
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index acabee3315f..9f004a2435e 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -12032,10 +12032,10 @@ try_open_dwop_file (dwarf2_per_objfile *per_objfile,
gdb::unique_xmalloc_ptr<char> search_path_holder;
if (search_cwd)
{
- if (*debug_file_directory != '\0')
+ if (!debug_file_directory.empty ())
{
search_path_holder.reset (concat (".", dirname_separator_string,
- debug_file_directory,
+ debug_file_directory.c_str (),
(char *) NULL));
search_path = search_path_holder.get ();
}
@@ -12043,7 +12043,7 @@ try_open_dwop_file (dwarf2_per_objfile *per_objfile,
search_path = ".";
}
else
- search_path = debug_file_directory;
+ search_path = debug_file_directory.c_str ();
/* Add the path for the executable binary to the list of search paths. */
std::string objfile_dir = ldirname (objfile_name (per_objfile->objfile));
@@ -12114,7 +12114,7 @@ open_dwo_file (dwarf2_per_objfile *per_objfile,
/* That didn't work, try debug-file-directory, which, despite its name,
is a list of paths. */
- if (*debug_file_directory == '\0')
+ if (debug_file_directory.empty ())
return NULL;
return try_open_dwop_file (per_objfile, file_name,
@@ -12443,7 +12443,7 @@ open_dwp_file (dwarf2_per_objfile *per_objfile, const char *file_name)
[IWBN if the dwp file name was recorded in the executable, akin to
.gnu_debuglink, but that doesn't exist yet.]
Strip the directory from FILE_NAME and search again. */
- if (*debug_file_directory != '\0')
+ if (!debug_file_directory.empty ())
{
/* Don't implicitly search the current directory here.
If the user wants to search "." to handle this case,
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 002a7dc95e0..31e2e4dfb1a 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -428,13 +428,11 @@ display_gdb_prompt (const char *new_prompt)
static std::string
top_level_prompt (void)
{
- char *prompt;
-
/* Give observers a chance of changing the prompt. E.g., the python
`gdb.prompt_hook' is installed as an observer. */
- gdb::observers::before_prompt.notify (get_prompt ());
+ gdb::observers::before_prompt.notify (get_prompt ().c_str ());
- prompt = get_prompt ();
+ const std::string &prompt = get_prompt ();
if (annotation_level >= 2)
{
@@ -445,7 +443,7 @@ top_level_prompt (void)
beginning. */
const char suffix[] = "\n\032\032prompt\n";
- return std::string (prefix) + prompt + suffix;
+ return std::string (prefix) + prompt.c_str () + suffix;
}
return prompt;
@@ -1174,7 +1172,7 @@ handle_sigtstp (int sig)
static void
async_sigtstp_handler (gdb_client_data arg)
{
- char *prompt = get_prompt ();
+ const std::string &prompt = get_prompt ();
signal (SIGTSTP, SIG_DFL);
#if HAVE_SIGPROCMASK
@@ -1189,7 +1187,7 @@ async_sigtstp_handler (gdb_client_data arg)
#endif
raise (SIGTSTP);
signal (SIGTSTP, handle_sigtstp);
- printf_unfiltered ("%s", prompt);
+ printf_unfiltered ("%s", prompt.c_str ());
gdb_flush (gdb_stdout);
/* Forget about any previous command -- null line now will do
diff --git a/gdb/fork-child.c b/gdb/fork-child.c
index 3ce7d64b855..3e995ed6f65 100644
--- a/gdb/fork-child.c
+++ b/gdb/fork-child.c
@@ -33,14 +33,14 @@
/* The exec-wrapper, if any, that will be used when starting the
inferior. */
-static char *exec_wrapper = NULL;
+static std::string exec_wrapper;
/* See gdbsupport/common-inferior.h. */
const char *
get_exec_wrapper ()
{
- return exec_wrapper;
+ return !exec_wrapper.empty () ? exec_wrapper.c_str () : nullptr;
}
/* See nat/fork-inferior.h. */
@@ -139,8 +139,7 @@ gdb_startup_inferior (pid_t pid, int num_traps)
static void
unset_exec_wrapper_command (const char *args, int from_tty)
{
- xfree (exec_wrapper);
- exec_wrapper = NULL;
+ exec_wrapper.clear ();
}
static void
diff --git a/gdb/guile/scm-param.c b/gdb/guile/scm-param.c
index 6bf1da17203..a0f31afa081 100644
--- a/gdb/guile/scm-param.c
+++ b/gdb/guile/scm-param.c
@@ -44,7 +44,7 @@ union pascm_variable
unsigned int uintval;
/* Hold a string, for the various string types. */
- char *stringval;
+ std::string *stringval;
/* Hold a string, for enums. */
const char *cstringval;
@@ -57,10 +57,7 @@ union pascm_variable
2) Call register-parameter! to add the parameter to gdb.
It is done this way so that the constructor, make-parameter, doesn't have
any side-effects. This means that the smob needs to store everything
- that was passed to make-parameter.
-
- N.B. There is no free function for this smob.
- All objects pointed to by this smob must live in GC space. */
+ that was passed to make-parameter. */
struct param_smob
{
@@ -121,7 +118,10 @@ struct setting_wrapper final : public base_setting
explicit setting_wrapper (param_smob *s)
{
m_var_type = s->type;
- m_var = &s->value;
+ if (var_type_uses<std::string> (m_var_type))
+ m_var = s->value.stringval;
+ else
+ m_var = &s->value;
}
};
@@ -420,14 +420,14 @@ add_setshow_generic (enum var_types param_type, enum command_class cmd_class,
case var_string:
commands = add_setshow_string_cmd (cmd_name, cmd_class,
- &self->value.stringval, set_doc,
+ self->value.stringval, set_doc,
show_doc, help_doc, set_func,
show_func, set_list, show_list);
break;
case var_string_noescape:
commands = add_setshow_string_noescape_cmd (cmd_name, cmd_class,
- &self->value.stringval,
+ self->value.stringval,
set_doc, show_doc, help_doc,
set_func, show_func, set_list,
show_list);
@@ -436,7 +436,7 @@ add_setshow_generic (enum var_types param_type, enum command_class cmd_class,
case var_optional_filename:
commands = add_setshow_optional_filename_cmd (cmd_name, cmd_class,
- &self->value.stringval,
+ self->value.stringval,
set_doc, show_doc, help_doc,
set_func, show_func,
set_list, show_list);
@@ -444,7 +444,7 @@ add_setshow_generic (enum var_types param_type, enum command_class cmd_class,
case var_filename:
commands = add_setshow_filename_cmd (cmd_name, cmd_class,
- &self->value.stringval, set_doc,
+ self->value.stringval, set_doc,
show_doc, help_doc, set_func,
show_func, set_list, show_list);
break;
@@ -589,14 +589,14 @@ pascm_param_value (base_setting const &var, int arg_pos, const char *func_name)
case var_string_noescape:
case var_optional_filename:
case var_filename:
- case var_enum:
{
- const char *str;
- if (var.type () == var_enum)
- str = var.get<const char *> ();
- else
- str = var.get<char *> ();
+ const std::string str = var.get<std::string> ();
+ return gdbscm_scm_from_host_string (str.c_str (), str.length ());
+ }
+ case var_enum:
+ {
+ const char *str = var.get<const char *> ();
if (str == nullptr)
str = "";
return gdbscm_scm_from_host_string (str, strlen (str));
@@ -669,13 +669,7 @@ pascm_set_param_value_x (param_smob *p_smob,
value, arg_pos, func_name,
_("string or #f for non-PARAM_FILENAME parameters"));
if (gdbscm_is_false (value))
- {
- xfree (var.get<char *> ());
- if (var.type () == var_optional_filename)
- var.set<char *> (xstrdup (""));
- else
- var.set<char *> (nullptr);
- }
+ var.set<std::string> ("");
else
{
SCM exception;
@@ -684,8 +678,7 @@ pascm_set_param_value_x (param_smob *p_smob,
= gdbscm_scm_to_host_string (value, nullptr, &exception);
if (string == nullptr)
gdbscm_throw (exception);
- xfree (var.get<char *> ());
- var.set<char *> (string.release ());
+ var.set<std::string> (string.release ());
}
break;
@@ -785,6 +778,21 @@ pascm_set_param_value_x (param_smob *p_smob,
gdb_assert_not_reached ("bad parameter type");
}
}
+
+/* Free function for a param_smob. */
+static size_t
+pascm_free_parameter_smob (SCM self)
+{
+ param_smob *p_smob = (param_smob *) SCM_SMOB_DATA (self);
+
+ if (var_type_uses<std::string> (p_smob->type))
+ {
+ delete p_smob->value.stringval;
+ p_smob->value.stringval = nullptr;
+ }
+
+ return 0;
+}
\f
/* Parameter Scheme functions. */
@@ -941,6 +949,10 @@ gdbscm_make_parameter (SCM name_scm, SCM rest)
p_smob->set_func = set_func;
p_smob->show_func = show_func;
+ scm_set_smob_free (parameter_smob_tag, pascm_free_parameter_smob);
+ if (var_type_uses<std::string> (p_smob->type))
+ p_smob->value.stringval = new std::string;
+
if (initial_value_arg_pos > 0)
{
if (gdbscm_is_procedure (initial_value_scm))
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index c183b60e81a..35c97d0296f 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -71,16 +71,16 @@ static void step_1 (int, int, const char *);
Arguments are separated by spaces. Empty string (pointer to '\0')
means no args. */
-static char *inferior_args_scratch;
+static std::string inferior_args_scratch;
/* Scratch area where the new cwd will be stored by 'set cwd'. */
-static char *inferior_cwd_scratch;
+static std::string inferior_cwd_scratch;
/* Scratch area where 'set inferior-tty' will store user-provided value.
We'll immediate copy it into per-inferior storage. */
-static char *inferior_io_terminal_scratch;
+static std::string inferior_io_terminal_scratch;
/* Pid of our debugged inferior, or 0 if no inferior now.
Since various parts of infrun.c test this to see whether there is a program
@@ -2021,7 +2021,6 @@ path_info (const char *args, int from_tty)
static void
path_command (const char *dirname, int from_tty)
{
- char *exec_path;
const char *env;
dont_repeat ();
@@ -2029,10 +2028,9 @@ path_command (const char *dirname, int from_tty)
/* Can be null if path is not set. */
if (!env)
env = "";
- exec_path = xstrdup (env);
- mod_path (dirname, &exec_path);
- current_inferior ()->environment.set (path_var_name, exec_path);
- xfree (exec_path);
+ std::string exec_path = env;
+ mod_path (dirname, exec_path);
+ current_inferior ()->environment.set (path_var_name, exec_path.c_str ());
if (from_tty)
path_info (NULL, from_tty);
}
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index eb2aa5ac409..03fc943c093 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -110,7 +110,7 @@ class thread_db_target final : public target_ops
gdb::byte_vector thread_info_to_thread_handle (struct thread_info *) override;
};
-static char *libthread_db_search_path;
+static std::string libthread_db_search_path = LIBTHREAD_DB_SEARCH_PATH;
/* Set to true if thread_db auto-loading is enabled
by the "set auto-load libthread-db" command. */
@@ -135,11 +135,8 @@ static void
set_libthread_db_search_path (const char *ignored, int from_tty,
struct cmd_list_element *c)
{
- if (*libthread_db_search_path == '\0')
- {
- xfree (libthread_db_search_path);
- libthread_db_search_path = xstrdup (LIBTHREAD_DB_SEARCH_PATH);
- }
+ if (libthread_db_search_path.empty ())
+ libthread_db_search_path = LIBTHREAD_DB_SEARCH_PATH;
}
/* If non-zero, print details of libthread_db processing. */
@@ -942,7 +939,7 @@ try_thread_db_load_1 (struct thread_db_info *info)
printf_unfiltered (_("[Thread debugging using libthread_db enabled]\n"));
- if (*libthread_db_search_path || libthread_db_debug)
+ if (!libthread_db_search_path.empty () || libthread_db_debug)
{
struct ui_file *file;
const char *library;
@@ -955,7 +952,7 @@ try_thread_db_load_1 (struct thread_db_info *info)
disabled, still print it to gdb_stdout if debug output is
enabled. User visible output should not depend on debug
settings. */
- file = *libthread_db_search_path != '\0' ? gdb_stdout : gdb_stdlog;
+ file = !libthread_db_search_path.empty () ? gdb_stdout : gdb_stdlog;
fprintf_unfiltered (file,
_("Using host libthread_db library \"%ps\".\n"),
styled_string (file_name_style.style (), library));
@@ -1143,7 +1140,7 @@ thread_db_load_search (void)
bool rc = false;
std::vector<gdb::unique_xmalloc_ptr<char>> dir_vec
- = dirnames_to_char_ptr_vec (libthread_db_search_path);
+ = dirnames_to_char_ptr_vec (libthread_db_search_path.c_str ());
for (const gdb::unique_xmalloc_ptr<char> &this_dir_up : dir_vec)
{
@@ -2006,8 +2003,6 @@ _initialize_thread_db ()
and until there is a running inferior, we can't tell which
libthread_db is the correct one to load. */
- libthread_db_search_path = xstrdup (LIBTHREAD_DB_SEARCH_PATH);
-
add_setshow_optional_filename_cmd ("libthread-db-search-path",
class_support,
&libthread_db_search_path, _("\
diff --git a/gdb/main.c b/gdb/main.c
index 5761ce2bdbe..ca4ccc375dc 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -65,7 +65,7 @@ char *interpreter_p;
int dbx_commands = 0;
/* System root path, used to find libraries etc. */
-char *gdb_sysroot = 0;
+std::string gdb_sysroot;
/* GDB datadir, used to store data files. */
std::string gdb_datadir;
@@ -710,19 +710,14 @@ captured_main_1 (struct captured_main_args *context)
perror_warning_with_name (_("error finding working directory"));
/* Set the sysroot path. */
- gdb_sysroot
- = xstrdup (relocate_gdb_directory (TARGET_SYSTEM_ROOT,
- TARGET_SYSTEM_ROOT_RELOCATABLE).c_str ());
+ gdb_sysroot = relocate_gdb_directory (TARGET_SYSTEM_ROOT,
+ TARGET_SYSTEM_ROOT_RELOCATABLE);
- if (*gdb_sysroot == '\0')
- {
- xfree (gdb_sysroot);
- gdb_sysroot = xstrdup (TARGET_SYSROOT_PREFIX);
- }
+ if (gdb_sysroot.empty ())
+ gdb_sysroot = TARGET_SYSROOT_PREFIX;
debug_file_directory
- = xstrdup (relocate_gdb_directory (DEBUGDIR,
- DEBUGDIR_RELOCATABLE).c_str ());
+ = relocate_gdb_directory (DEBUGDIR, DEBUGDIR_RELOCATABLE);
gdb_datadir = relocate_gdb_directory (GDB_DATADIR,
GDB_DATADIR_RELOCATABLE);
diff --git a/gdb/maint-test-options.c b/gdb/maint-test-options.c
index b773e759bb7..77a013d3b7a 100644
--- a/gdb/maint-test-options.c
+++ b/gdb/maint-test-options.c
@@ -133,17 +133,12 @@ struct test_options_opts
const char *enum_opt = test_options_enum_values_xxx;
unsigned int uint_opt = 0;
int zuint_unl_opt = 0;
- char *string_opt = nullptr;
+ std::string string_opt;
test_options_opts () = default;
DISABLE_COPY_AND_ASSIGN (test_options_opts);
- ~test_options_opts ()
- {
- xfree (string_opt);
- }
-
/* Dump the options to FILE. ARGS is the remainder unprocessed
arguments. */
void dump (ui_file *file, const char *args) const
@@ -162,9 +157,7 @@ struct test_options_opts
(zuint_unl_opt == -1
? "unlimited"
: plongest (zuint_unl_opt)),
- (string_opt != nullptr
- ? string_opt
- : ""),
+ string_opt.c_str (),
args);
}
};
diff --git a/gdb/maint-test-settings.c b/gdb/maint-test-settings.c
index 0ce88905518..eea3ea99572 100644
--- a/gdb/maint-test-settings.c
+++ b/gdb/maint-test-settings.c
@@ -44,13 +44,13 @@ static unsigned int maintenance_test_settings_zuinteger;
static int maintenance_test_settings_zuinteger_unlimited;
-static char *maintenance_test_settings_string;
+static std::string maintenance_test_settings_string;
-static char *maintenance_test_settings_string_noescape;
+static std::string maintenance_test_settings_string_noescape;
-static char *maintenance_test_settings_optional_filename;
+static std::string maintenance_test_settings_optional_filename;
-static char *maintenance_test_settings_filename;
+static std::string maintenance_test_settings_filename;
/* Enum values for the "maintenance set/show test-settings boolean"
commands. */
diff --git a/gdb/mi/mi-cmd-env.c b/gdb/mi/mi-cmd-env.c
index 703c63251fd..f9685a515ad 100644
--- a/gdb/mi/mi-cmd-env.c
+++ b/gdb/mi/mi-cmd-env.c
@@ -93,7 +93,7 @@ mi_cmd_env_cd (const char *command, char **argv, int argc)
}
static void
-env_mod_path (const char *dirname, char **which_path)
+env_mod_path (const char *dirname, std::string &which_path)
{
if (dirname == 0 || dirname[0] == '\0')
return;
@@ -109,7 +109,6 @@ void
mi_cmd_env_path (const char *command, char **argv, int argc)
{
struct ui_out *uiout = current_uiout;
- char *exec_path;
const char *env;
int reset = 0;
int oind = 0;
@@ -152,11 +151,11 @@ mi_cmd_env_path (const char *command, char **argv, int argc)
argv += oind;
argc -= oind;
-
+ std::string exec_path;
if (reset)
{
/* Reset implies resetting to original path first. */
- exec_path = xstrdup (orig_path);
+ exec_path = orig_path;
}
else
{
@@ -166,14 +165,14 @@ mi_cmd_env_path (const char *command, char **argv, int argc)
/* Can be null if path is not set. */
if (!env)
env = "";
- exec_path = xstrdup (env);
+
+ exec_path = env;
}
for (i = argc - 1; i >= 0; --i)
- env_mod_path (argv[i], &exec_path);
+ env_mod_path (argv[i], exec_path);
- current_inferior ()->environment.set (path_var_name, exec_path);
- xfree (exec_path);
+ current_inferior ()->environment.set (path_var_name, exec_path.c_str ());
env = current_inferior ()->environment.get (path_var_name);
uiout->field_string ("path", env);
}
@@ -228,12 +227,11 @@ mi_cmd_env_dir (const char *command, char **argv, int argc)
if (reset)
{
/* Reset means setting to default path first. */
- xfree (source_path);
init_source_path ();
}
for (i = argc - 1; i >= 0; --i)
- env_mod_path (argv[i], &source_path);
+ env_mod_path (argv[i], source_path);
uiout->field_string ("source-path", source_path);
forget_cached_source_info ();
diff --git a/gdb/proc-api.c b/gdb/proc-api.c
index 5f60eeeb129..40bde7893bd 100644
--- a/gdb/proc-api.c
+++ b/gdb/proc-api.c
@@ -50,14 +50,14 @@ struct trans {
static bool procfs_trace = false;
static FILE *procfs_file = NULL;
-static char *procfs_filename;
+static std::string procfs_filename = "procfs_trace";
static void
prepare_to_trace (void)
{
if (procfs_trace) /* if procfs tracing turned on */
if (procfs_file == NULL) /* if output file not yet open */
- procfs_file = fopen (procfs_filename, "a"); /* open output file */
+ procfs_file = fopen (procfs_filename.c_str (), "a"); /* open output file */
}
static void
@@ -425,7 +425,6 @@ Show tracing for /proc api calls."), NULL,
NULL, /* FIXME: i18n: */
&setlist, &showlist);
- procfs_filename = xstrdup ("procfs_trace");
add_setshow_filename_cmd ("procfs-file", no_class, &procfs_filename, _("\
Set filename for /proc tracefile."), _("\
Show filename for /proc tracefile."), NULL,
diff --git a/gdb/python/py-param.c b/gdb/python/py-param.c
index 905e91c76dd..95137d55b3b 100644
--- a/gdb/python/py-param.c
+++ b/gdb/python/py-param.c
@@ -64,8 +64,9 @@ union parmpy_variable
/* Hold an unsigned integer value, for uinteger. */
unsigned int uintval;
- /* Hold a string, for the various string types. */
- char *stringval;
+ /* Hold a string, for the various string types. The std::string is
+ new-ed. */
+ std::string *stringval;
/* Hold a string, for enums. */
const char *cstringval;
@@ -96,7 +97,10 @@ struct setting_wrapper final: base_setting
explicit setting_wrapper (parmpy_object *s)
{
m_var_type = s->type;
- m_var = &s->value;
+ if (var_type_uses<std::string> (m_var_type))
+ m_var = s->value.stringval;
+ else
+ m_var = &s->value;
}
};
@@ -152,13 +156,7 @@ set_parameter_value (parmpy_object *self, PyObject *value)
return -1;
}
if (value == Py_None)
- {
- xfree (self->value.stringval);
- if (self->type == var_optional_filename)
- self->value.stringval = xstrdup ("");
- else
- self->value.stringval = NULL;
- }
+ self->value.stringval->clear ();
else
{
gdb::unique_xmalloc_ptr<char>
@@ -166,8 +164,7 @@ set_parameter_value (parmpy_object *self, PyObject *value)
if (string == NULL)
return -1;
- xfree (self->value.stringval);
- self->value.stringval = string.release ();
+ *self->value.stringval = string.get ();
}
break;
@@ -514,14 +511,14 @@ add_setshow_generic (int parmclass, enum command_class cmdclass,
case var_string:
commands = add_setshow_string_cmd (cmd_name.get (), cmdclass,
- &self->value.stringval, set_doc,
+ self->value.stringval, set_doc,
show_doc, help_doc, get_set_value,
get_show_value, set_list, show_list);
break;
case var_string_noescape:
commands = add_setshow_string_noescape_cmd (cmd_name.get (), cmdclass,
- &self->value.stringval,
+ self->value.stringval,
set_doc, show_doc, help_doc,
get_set_value, get_show_value,
set_list, show_list);
@@ -529,7 +526,7 @@ add_setshow_generic (int parmclass, enum command_class cmdclass,
case var_optional_filename:
commands = add_setshow_optional_filename_cmd (cmd_name.get (), cmdclass,
- &self->value.stringval,
+ self->value.stringval,
set_doc, show_doc, help_doc,
get_set_value,
get_show_value, set_list,
@@ -538,7 +535,7 @@ add_setshow_generic (int parmclass, enum command_class cmdclass,
case var_filename:
commands = add_setshow_filename_cmd (cmd_name.get (), cmdclass,
- &self->value.stringval, set_doc,
+ self->value.stringval, set_doc,
show_doc, help_doc, get_set_value,
get_show_value, set_list, show_list);
break;
@@ -721,6 +718,9 @@ parmpy_init (PyObject *self, PyObject *args, PyObject *kwds)
obj->type = (enum var_types) parmclass;
memset (&obj->value, 0, sizeof (obj->value));
+ if (var_type_uses<std::string> (obj->type))
+ obj->value.stringval = new std::string;
+
gdb::unique_xmalloc_ptr<char> cmd_name
= gdbpy_parse_command_name (name, &set_list, &setlist);
if (cmd_name == nullptr)
@@ -753,7 +753,16 @@ parmpy_init (PyObject *self, PyObject *args, PyObject *kwds)
return 0;
}
-\f
+/* Deallocate function for a gdb.Parameter. */
+
+static void
+parmpy_dealloc (PyObject *obj)
+{
+ parmpy_object *parm_obj = (parmpy_object *) obj;
+
+ if (var_type_uses<std::string> (parm_obj->type))
+ delete parm_obj->value.stringval;
+}
/* Initialize the 'parameters' module. */
int
@@ -792,7 +801,7 @@ PyTypeObject parmpy_object_type =
"gdb.Parameter", /*tp_name*/
sizeof (parmpy_object), /*tp_basicsize*/
0, /*tp_itemsize*/
- 0, /*tp_dealloc*/
+ parmpy_dealloc, /*tp_dealloc*/
0, /*tp_print*/
0, /*tp_getattr*/
0, /*tp_setattr*/
diff --git a/gdb/python/python.c b/gdb/python/python.c
index c89405c4e84..81e9a338fe8 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -458,15 +458,13 @@ gdbpy_parameter_value (base_setting const & var)
case var_filename:
case var_enum:
{
- const char *str;
- if (var.type () == var_enum)
- str = var.get<const char *> ();
- else
- str = var.get<char *> ();
-
- if (str == nullptr)
- str = "";
- return host_string_to_python_string (str).release ();
+ std::string str;
+ if (var.type () == var_enum)
+ str = var.get<const char *> ();
+ else
+ str = var.get<std::string> ();
+
+ return host_string_to_python_string (str.c_str ()).release ();
}
case var_boolean:
diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index 5bedc04011d..5bfce2a9a31 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -680,10 +680,9 @@ gdbsim_target_open (const char *args, int from_tty)
int len;
char *arg_buf;
struct sim_inferior_data *sim_data;
- const char *sysroot;
SIM_DESC gdbsim_desc;
- sysroot = gdb_sysroot;
+ const std::string &sysroot = gdb_sysroot;
if (is_target_filename (sysroot))
sysroot += strlen (TARGET_SYSROOT_PREFIX);
@@ -704,7 +703,7 @@ gdbsim_target_open (const char *args, int from_tty)
len = (7 + 1 /* gdbsim */
+ strlen (" -E little")
+ strlen (" --architecture=xxxxxxxxxx")
- + strlen (" --sysroot=") + strlen (sysroot) +
+ + strlen (" --sysroot=") + sysroot.length () +
+ (args ? strlen (args) : 0)
+ 50) /* slack */ ;
arg_buf = (char *) alloca (len);
@@ -731,7 +730,7 @@ gdbsim_target_open (const char *args, int from_tty)
}
/* Pass along gdb's concept of the sysroot. */
strcat (arg_buf, " --sysroot=");
- strcat (arg_buf, sysroot);
+ strcat (arg_buf, sysroot.c_str ());
/* finally, any explicit args */
if (args)
{
diff --git a/gdb/remote.c b/gdb/remote.c
index 429a5f5e630..e9e2104867d 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1004,7 +1004,7 @@ static const struct program_space_key<char, gdb::xfree_deleter<char>>
remote exec-file commands. While the remote exec-file setting is
per-program-space, the set/show machinery uses this as the
location of the remote exec-file value. */
-static char *remote_exec_file_var;
+static std::string remote_exec_file_var;
/* The size to align memory write packets, when practical. The protocol
does not guarantee any alignment, and gdb will generate short
@@ -1355,8 +1355,8 @@ static void
set_remote_exec_file (const char *ignored, int from_tty,
struct cmd_list_element *c)
{
- gdb_assert (remote_exec_file_var != NULL);
- set_pspace_remote_exec_file (current_program_space, remote_exec_file_var);
+ set_pspace_remote_exec_file (current_program_space,
+ remote_exec_file_var.c_str ());
}
/* The "set/show remote exec-file" show command hook. */
@@ -12618,7 +12618,7 @@ remote_target::filesystem_is_local ()
this case we treat the remote filesystem as local if the
sysroot is exactly TARGET_SYSROOT_PREFIX and if the stub
does not support vFile:open. */
- if (strcmp (gdb_sysroot, TARGET_SYSROOT_PREFIX) == 0)
+ if (gdb_sysroot == TARGET_SYSROOT_PREFIX)
{
enum packet_support ps = packet_support (PACKET_vFile_open);
@@ -13246,7 +13246,7 @@ remote_target::download_tracepoint (struct bp_location *loc)
"ignoring tp %d cond"), b->number);
}
- if (b->commands || *default_collect)
+ if (b->commands || !default_collect.empty ())
{
size_left = buf.size () - strlen (buf.data ());
diff --git a/gdb/serial.c b/gdb/serial.c
index 7d1bc535ed9..5ec79a11aac 100644
--- a/gdb/serial.c
+++ b/gdb/serial.c
@@ -38,7 +38,7 @@ static struct serial *scb_base;
/* Non-NULL gives filename which contains a recording of the remote session,
suitable for playback by gdbserver. */
-static char *serial_logfile = NULL;
+static std::string serial_logfile;
static struct ui_file *serial_logfp = NULL;
static const struct serial_ops *serial_interface_lookup (const char *);
@@ -251,12 +251,12 @@ serial_open_ops_1 (const struct serial_ops *ops, const char *open_name)
scb->next = scb_base;
scb_base = scb;
- if (serial_logfile != NULL)
+ if (!serial_logfile.empty ())
{
stdio_file_up file (new stdio_file ());
- if (!file->open (serial_logfile, "w"))
- perror_with_name (serial_logfile);
+ if (!file->open (serial_logfile.c_str (), "w"))
+ perror_with_name (serial_logfile.c_str ());
serial_logfp = file.release ();
}
diff --git a/gdb/solib.c b/gdb/solib.c
index 317f7eb485e..3e7ad60d258 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -96,7 +96,7 @@ struct target_so_ops *current_target_so_ops;
/* If non-empty, this is a search path for loading non-absolute shared library
symbol files. This takes precedence over the environment variables PATH
and LD_LIBRARY_PATH. */
-static char *solib_search_path = NULL;
+static std::string solib_search_path;
static void
show_solib_search_path (struct ui_file *file, int from_tty,
struct cmd_list_element *c, const char *value)
@@ -155,7 +155,7 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
int found_file = -1;
gdb::unique_xmalloc_ptr<char> temp_pathname;
const char *fskind = effective_target_file_system_kind ();
- const char *sysroot = gdb_sysroot;
+ const char *sysroot = gdb_sysroot.c_str ();
int prefix_len, orig_prefix_len;
/* If the absolute prefix starts with "target:" but the filesystem
@@ -319,8 +319,8 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
/* If not found, and we're looking for a solib, search the
solib_search_path (if any). */
- if (is_solib && found_file < 0 && solib_search_path != NULL)
- found_file = openp (solib_search_path,
+ if (is_solib && found_file < 0 && !solib_search_path.empty ())
+ found_file = openp (solib_search_path.c_str (),
OPF_TRY_CWD_FIRST | OPF_RETURN_REALPATH,
in_pathname, O_RDONLY | O_BINARY, &temp_pathname);
@@ -328,8 +328,8 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
solib_search_path (if any) for the basename only (ignoring the
path). This is to allow reading solibs from a path that differs
from the opened path. */
- if (is_solib && found_file < 0 && solib_search_path != NULL)
- found_file = openp (solib_search_path,
+ if (is_solib && found_file < 0 && !solib_search_path.empty ())
+ found_file = openp (solib_search_path.c_str (),
OPF_TRY_CWD_FIRST | OPF_RETURN_REALPATH,
target_lbasename (fskind, in_pathname),
O_RDONLY | O_BINARY, &temp_pathname);
@@ -378,7 +378,7 @@ exec_file_find (const char *in_pathname, int *fd)
if (in_pathname == NULL)
return NULL;
- if (*gdb_sysroot != '\0' && IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname))
+ if (!gdb_sysroot.empty () && IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname))
{
result = solib_find_1 (in_pathname, fd, false);
@@ -1394,18 +1394,18 @@ gdb_sysroot_changed (const char *ignored, int from_tty,
const char *old_prefix = "remote:";
const char *new_prefix = TARGET_SYSROOT_PREFIX;
- if (startswith (gdb_sysroot, old_prefix))
+ if (startswith (gdb_sysroot.c_str (), old_prefix))
{
static bool warning_issued = false;
gdb_assert (strlen (old_prefix) == strlen (new_prefix));
- memcpy (gdb_sysroot, new_prefix, strlen (new_prefix));
+ gdb_sysroot = new_prefix + gdb_sysroot.substr (strlen (old_prefix));
if (!warning_issued)
{
warning (_("\"%s\" is deprecated, use \"%s\" instead."),
old_prefix, new_prefix);
- warning (_("sysroot set to \"%s\"."), gdb_sysroot);
+ warning (_("sysroot set to \"%s\"."), gdb_sysroot.c_str ());
warning_issued = true;
}
diff --git a/gdb/source.c b/gdb/source.c
index 7d1934bd6c9..f1854c1ec4a 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -57,7 +57,7 @@
/* Path of directories to search for source files.
Same format as the PATH environment variable's value. */
-char *source_path;
+std::string source_path;
/* Support for source path substitution commands. */
@@ -365,17 +365,15 @@ set_directories_command (const char *args,
{
/* This is the value that was set.
It needs to be processed to maintain $cdir:$cwd and remove dups. */
- char *set_path = source_path;
+ std::string set_path = source_path;
/* We preserve the invariant that $cdir:$cwd begins life at the end of
the list by calling init_source_path. If they appear earlier in
SET_PATH then mod_path will move them appropriately.
mod_path will also remove duplicates. */
init_source_path ();
- if (*set_path != '\0')
- mod_path (set_path, &source_path);
-
- xfree (set_path);
+ if (!set_path.empty ())
+ mod_path (set_path.c_str (), source_path);
}
/* Print the list of source directories.
@@ -386,7 +384,7 @@ static void
show_directories_1 (char *ignore, int from_tty)
{
puts_filtered ("Source directories searched: ");
- puts_filtered (source_path);
+ puts_filtered (source_path.c_str ());
puts_filtered ("\n");
}
@@ -437,10 +435,7 @@ forget_cached_source_info (void)
void
init_source_path (void)
{
- char buf[20];
-
- xsnprintf (buf, sizeof (buf), "$cdir%c$cwd", DIRNAME_SEPARATOR);
- source_path = xstrdup (buf);
+ source_path = string_printf ("$cdir%c$cwd", DIRNAME_SEPARATOR);
forget_cached_source_info ();
}
@@ -456,20 +451,20 @@ directory_command (const char *dirname, int from_tty)
{
if (!from_tty || query (_("Reinitialize source path to empty? ")))
{
- xfree (source_path);
init_source_path ();
value_changed = true;
}
}
else
{
- mod_path (dirname, &source_path);
+ mod_path (dirname, source_path);
forget_cached_source_info ();
value_changed = true;
}
if (value_changed)
{
- gdb::observers::command_param_changed.notify ("directories", source_path);
+ gdb::observers::command_param_changed.notify ("directories",
+ source_path.c_str ());
if (from_tty)
show_directories_1 ((char *) 0, from_tty);
}
@@ -481,13 +476,13 @@ directory_command (const char *dirname, int from_tty)
void
directory_switch (const char *dirname, int from_tty)
{
- add_path (dirname, &source_path, 0);
+ add_path (dirname, source_path, 0);
}
/* Add zero or more directories to the front of an arbitrary path. */
void
-mod_path (const char *dirname, char **which_path)
+mod_path (const char *dirname, std::string &which_path)
{
add_path (dirname, which_path, 1);
}
@@ -675,6 +670,17 @@ add_path (const char *dirname, char **which_path, int parse_separators)
}
}
+/* add_path would need to be re-written to work on an std::string, but this is
+ not trivial. Hence this overload which copies to a `char *` and back. */
+
+void
+add_path (const char *dirname, std::string &which_path, int parse_separators)
+{
+ char *which_path_copy = xstrdup (which_path.data ());
+ add_path (dirname, &which_path_copy, parse_separators);
+ which_path = which_path_copy;
+ xfree (which_path_copy);
+}
static void
info_source_command (const char *ignore, int from_tty)
@@ -953,7 +959,7 @@ source_full_path_of (const char *filename,
{
int fd;
- fd = openp (source_path,
+ fd = openp (source_path.c_str (),
OPF_TRY_CWD_FIRST | OPF_SEARCH_IN_PATH | OPF_RETURN_REALPATH,
filename, O_RDONLY, full_pathname);
if (fd < 0)
@@ -1044,7 +1050,8 @@ find_and_open_source (const char *filename,
const char *dirname,
gdb::unique_xmalloc_ptr<char> *fullname)
{
- char *path = source_path;
+ const char *path = source_path.c_str ();
+ std::string expanded_path_holder;
const char *p;
int result;
@@ -1086,19 +1093,22 @@ find_and_open_source (const char *filename,
/* Replace a path entry of $cdir with the compilation directory
name. */
#define cdir_len 5
- p = strstr (source_path, "$cdir");
+ p = strstr (source_path.c_str (), "$cdir");
if (p && (p == path || p[-1] == DIRNAME_SEPARATOR)
&& (p[cdir_len] == DIRNAME_SEPARATOR || p[cdir_len] == '\0'))
{
- int len;
-
- path = (char *)
- alloca (strlen (source_path) + 1 + strlen (dirname) + 1);
- len = p - source_path;
- strncpy (path, source_path, len); /* Before $cdir */
- strcpy (path + len, dirname); /* new stuff */
- strcat (path + len, source_path + len + cdir_len); /* After
- $cdir */
+ int len = p - source_path.c_str ();
+
+ /* Before $cdir */
+ expanded_path_holder = source_path.substr (0, len);
+
+ /* new stuff */
+ expanded_path_holder += dirname;
+
+ /* After $cdir */
+ expanded_path_holder += source_path.c_str () + len + cdir_len;
+
+ path = expanded_path_holder.c_str ();
}
}
diff --git a/gdb/source.h b/gdb/source.h
index 6c383efbbb2..2b9e8f38c03 100644
--- a/gdb/source.h
+++ b/gdb/source.h
@@ -39,13 +39,14 @@ extern int openp (const char *, openp_flags, const char *, int,
extern int source_full_path_of (const char *, gdb::unique_xmalloc_ptr<char> *);
-extern void mod_path (const char *, char **);
+extern void mod_path (const char *, std::string &);
extern void add_path (const char *, char **, int);
+extern void add_path (const char *, std::string &, int);
extern void directory_switch (const char *, int);
-extern char *source_path;
+extern std::string source_path;
extern void init_source_path (void);
diff --git a/gdb/stack.c b/gdb/stack.c
index 516e4d45696..653dc391507 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -2424,12 +2424,7 @@ print_frame_local_vars (struct frame_info *frame,
struct info_print_options
{
bool quiet = false;
- char *type_regexp = nullptr;
-
- ~info_print_options ()
- {
- xfree (type_regexp);
- }
+ std::string type_regexp;
};
/* The options used by the 'info locals' and 'info args' commands. */
@@ -2488,9 +2483,11 @@ info_locals_command (const char *args, int from_tty)
if (args != nullptr && *args == '\0')
args = nullptr;
- print_frame_local_vars (get_selected_frame (_("No frame selected.")),
- opts.quiet, args, opts.type_regexp,
- 0, gdb_stdout);
+ print_frame_local_vars
+ (get_selected_frame (_("No frame selected.")),
+ opts.quiet, args,
+ opts.type_regexp.empty () ? nullptr : opts.type_regexp.c_str (),
+ 0, gdb_stdout);
}
/* Iterate over all the argument variables in block B. */
@@ -2599,8 +2596,11 @@ info_args_command (const char *args, int from_tty)
if (args != nullptr && *args == '\0')
args = nullptr;
- print_frame_arg_vars (get_selected_frame (_("No frame selected.")),
- opts.quiet, args, opts.type_regexp, gdb_stdout);
+ print_frame_arg_vars
+ (get_selected_frame (_("No frame selected.")),
+ opts.quiet, args,
+ opts.type_regexp.empty () ? nullptr : opts.type_regexp.c_str (),
+ gdb_stdout);
}
\f
/* Return the symbol-block in which the selected frame is executing.
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 0eb48d04d5b..9e5c2d48881 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1349,7 +1349,7 @@ separate_debug_file_exists (const std::string &name, unsigned long crc,
return 1;
}
-char *debug_file_directory = NULL;
+std::string debug_file_directory;
static void
show_debug_file_directory (struct ui_file *file, int from_tty,
struct cmd_list_element *c, const char *value)
@@ -1406,8 +1406,9 @@ find_separate_debug_file (const char *dir,
bool target_prefix = startswith (dir, "target:");
const char *dir_notarget = target_prefix ? dir + strlen ("target:") : dir;
std::vector<gdb::unique_xmalloc_ptr<char>> debugdir_vec
- = dirnames_to_char_ptr_vec (debug_file_directory);
- gdb::unique_xmalloc_ptr<char> canon_sysroot = gdb_realpath (gdb_sysroot);
+ = dirnames_to_char_ptr_vec (debug_file_directory.c_str ());
+ gdb::unique_xmalloc_ptr<char> canon_sysroot
+ = gdb_realpath (gdb_sysroot.c_str ());
/* MS-Windows/MS-DOS don't allow colons in file names; we must
convert the drive letter into a one-letter directory, so that the
@@ -1448,7 +1449,7 @@ find_separate_debug_file (const char *dir,
if (canon_sysroot.get () != NULL)
base_path = child_path (canon_sysroot.get (), canon_dir);
else
- base_path = child_path (gdb_sysroot, canon_dir);
+ base_path = child_path (gdb_sysroot.c_str (), canon_dir);
}
if (base_path != NULL)
{
@@ -2654,7 +2655,7 @@ add_filename_language (const char *ext, enum language lang)
filename_language_table.emplace_back (ext, lang);
}
-static char *ext_args;
+static std::string ext_args;
static void
show_ext_args (struct ui_file *file, int from_tty,
struct cmd_list_element *c, const char *value)
@@ -2669,48 +2670,48 @@ static void
set_ext_lang_command (const char *args,
int from_tty, struct cmd_list_element *e)
{
- char *cp = ext_args;
- enum language lang;
+ const char *begin = ext_args.c_str ();
+ const char *end = ext_args.c_str ();
/* First arg is filename extension, starting with '.' */
- if (*cp != '.')
- error (_("'%s': Filename extension must begin with '.'"), ext_args);
+ if (*end != '.')
+ error (_("'%s': Filename extension must begin with '.'"), ext_args.c_str ());
/* Find end of first arg. */
- while (*cp && !isspace (*cp))
- cp++;
+ while (*end != '\0' && !isspace (*end))
+ end++;
- if (*cp == '\0')
+ if (*end == '\0')
error (_("'%s': two arguments required -- "
"filename extension and language"),
- ext_args);
+ ext_args.c_str ());
- /* Null-terminate first arg. */
- *cp++ = '\0';
+ /* Extract first arg, the extension. */
+ std::string extension = ext_args.substr (0, end - begin);
/* Find beginning of second arg, which should be a source language. */
- cp = skip_spaces (cp);
+ begin = skip_spaces (end);
- if (*cp == '\0')
+ if (*begin == '\0')
error (_("'%s': two arguments required -- "
"filename extension and language"),
- ext_args);
+ ext_args.c_str ());
/* Lookup the language from among those we know. */
- lang = language_enum (cp);
+ language lang = language_enum (begin);
auto it = filename_language_table.begin ();
/* Now lookup the filename extension: do we already know it? */
for (; it != filename_language_table.end (); it++)
{
- if (it->ext == ext_args)
+ if (it->ext == extension)
break;
}
if (it == filename_language_table.end ())
{
/* New file extension. */
- add_filename_language (ext_args, lang);
+ add_filename_language (extension.data (), lang);
}
else
{
@@ -3784,8 +3785,7 @@ test_set_ext_lang_command ()
SELF_CHECK (lang == language_unknown);
/* Test adding a new extension using the CLI command. */
- auto args_holder = make_unique_xstrdup (".hello rust");
- ext_args = args_holder.get ();
+ ext_args = ".hello rust";
set_ext_lang_command (NULL, 1, NULL);
lang = deduce_language_from_filename ("cake.hello");
@@ -3793,8 +3793,7 @@ test_set_ext_lang_command ()
/* Test overriding an existing extension using the CLI command. */
int size_before = filename_language_table.size ();
- args_holder.reset (xstrdup (".hello pascal"));
- ext_args = args_holder.get ();
+ ext_args = ".hello pascal";
set_ext_lang_command (NULL, 1, NULL);
int size_after = filename_language_table.size ();
diff --git a/gdb/symtab.c b/gdb/symtab.c
index fa3f42207ec..df7ff1e0c6c 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -5146,12 +5146,7 @@ struct info_vars_funcs_options
{
bool quiet = false;
bool exclude_minsyms = false;
- char *type_regexp = nullptr;
-
- ~info_vars_funcs_options ()
- {
- xfree (type_regexp);
- }
+ std::string type_regexp;
};
/* The options used by the 'info variables' and 'info functions'
@@ -5174,8 +5169,7 @@ static const gdb::option::option_def info_vars_funcs_options_defs[] = {
gdb::option::string_option_def<info_vars_funcs_options> {
"t",
- [] (info_vars_funcs_options *opt) { return &opt->type_regexp;
- },
+ [] (info_vars_funcs_options *opt) { return &opt->type_regexp; },
nullptr, /* show_cmd_cb */
nullptr /* set_doc */
}
@@ -5219,8 +5213,10 @@ info_variables_command (const char *args, int from_tty)
if (args != nullptr && *args == '\0')
args = nullptr;
- symtab_symbol_info (opts.quiet, opts.exclude_minsyms, args, VARIABLES_DOMAIN,
- opts.type_regexp, from_tty);
+ symtab_symbol_info
+ (opts.quiet, opts.exclude_minsyms, args, VARIABLES_DOMAIN,
+ opts.type_regexp.empty () ? nullptr : opts.type_regexp.c_str (),
+ from_tty);
}
/* Implement the 'info functions' command. */
@@ -5236,8 +5232,10 @@ info_functions_command (const char *args, int from_tty)
if (args != nullptr && *args == '\0')
args = nullptr;
- symtab_symbol_info (opts.quiet, opts.exclude_minsyms, args,
- FUNCTIONS_DOMAIN, opts.type_regexp, from_tty);
+ symtab_symbol_info
+ (opts.quiet, opts.exclude_minsyms, args, FUNCTIONS_DOMAIN,
+ opts.type_regexp.empty () ? nullptr : opts.type_regexp.c_str (),
+ from_tty);
}
/* Holds the -q option for the 'info types' command. */
@@ -6749,14 +6747,8 @@ info_module_subcommand (bool quiet, const char *module_regexp,
struct info_modules_var_func_options
{
bool quiet = false;
- char *type_regexp = nullptr;
- char *module_regexp = nullptr;
-
- ~info_modules_var_func_options ()
- {
- xfree (type_regexp);
- xfree (module_regexp);
- }
+ std::string type_regexp;
+ std::string module_regexp;
};
/* The options used by 'info module variables' and 'info module functions'
@@ -6806,8 +6798,11 @@ info_module_functions_command (const char *args, int from_tty)
if (args != nullptr && *args == '\0')
args = nullptr;
- info_module_subcommand (opts.quiet, opts.module_regexp, args,
- opts.type_regexp, FUNCTIONS_DOMAIN);
+ info_module_subcommand
+ (opts.quiet,
+ opts.module_regexp.empty () ? nullptr : opts.module_regexp.c_str (), args,
+ opts.type_regexp.empty () ? nullptr : opts.type_regexp.c_str (),
+ FUNCTIONS_DOMAIN);
}
/* Implements the 'info module variables' command. */
@@ -6822,8 +6817,11 @@ info_module_variables_command (const char *args, int from_tty)
if (args != nullptr && *args == '\0')
args = nullptr;
- info_module_subcommand (opts.quiet, opts.module_regexp, args,
- opts.type_regexp, VARIABLES_DOMAIN);
+ info_module_subcommand
+ (opts.quiet,
+ opts.module_regexp.empty () ? nullptr : opts.module_regexp.c_str (), args,
+ opts.type_regexp.empty () ? nullptr : opts.type_regexp.c_str (),
+ VARIABLES_DOMAIN);
}
/* Command completer for 'info module ...' sub-commands. */
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index f94ace756d4..f5ec617bb2e 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -510,7 +510,7 @@ target_desc_info_free (struct target_desc_info *tdesc_info)
/* The string manipulated by the "set tdesc filename ..." command. */
-static char *tdesc_filename_cmd_string;
+static std::string tdesc_filename_cmd_string;
/* Fetch the current target's description, and switch the current
architecture to one which incorporates that description. */
diff --git a/gdb/top.c b/gdb/top.c
index 6e0f43d2fd9..908a6d5ee54 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -84,8 +84,6 @@
extern void initialize_all_files (void);
-static bool history_filename_empty (void);
-
#define PROMPT(X) the_prompts.prompt_stack[the_prompts.top + X].prompt
#define PREFIX(X) the_prompts.prompt_stack[the_prompts.top + X].prefix
#define SUFFIX(X) the_prompts.prompt_stack[the_prompts.top + X].suffix
@@ -908,12 +906,16 @@ static bool command_editing_p;
variable must be set to something sensible. */
static bool write_history_p;
+/* The name of the file in which GDB history will be written. If this is
+ set to NULL, of the empty string then history will not be written. */
+static std::string history_filename;
+
/* Implement 'show history save'. */
static void
show_write_history_p (struct ui_file *file, int from_tty,
struct cmd_list_element *c, const char *value)
{
- if (!write_history_p || !history_filename_empty ())
+ if (!write_history_p || !history_filename.empty ())
fprintf_filtered (file, _("Saving of the history record on exit is %s.\n"),
value);
else
@@ -947,24 +949,12 @@ show_history_remove_duplicates (struct ui_file *file, int from_tty,
value);
}
-/* The name of the file in which GDB history will be written. If this is
- set to NULL, of the empty string then history will not be written. */
-static char *history_filename;
-
-/* Return true if the history_filename is either NULL or the empty string,
- indicating that we should not try to read, nor write out the history. */
-static bool
-history_filename_empty (void)
-{
- return (history_filename == nullptr || *history_filename == '\0');
-}
-
/* Implement 'show history filename'. */
static void
show_history_filename (struct ui_file *file, int from_tty,
struct cmd_list_element *c, const char *value)
{
- if (!history_filename_empty ())
+ if (!history_filename.empty ())
fprintf_filtered (file, _("The filename in which to record "
"the command history is \"%ps\".\n"),
styled_string (file_name_style.style (), value));
@@ -1231,14 +1221,15 @@ gdb_safe_append_history (void)
int ret, saved_errno;
std::string local_history_filename
- = string_printf ("%s-gdb%ld~", history_filename, (long) getpid ());
+ = string_printf ("%s-gdb%ld~", history_filename.c_str (), (long) getpid ());
- ret = rename (history_filename, local_history_filename.c_str ());
+ ret = rename (history_filename.c_str (), local_history_filename.c_str ());
saved_errno = errno;
if (ret < 0 && saved_errno != ENOENT)
{
warning (_("Could not rename %ps to %ps: %s"),
- styled_string (file_name_style.style (), history_filename),
+ styled_string (file_name_style.style (),
+ history_filename.c_str ()),
styled_string (file_name_style.style (),
local_history_filename.c_str ()),
safe_strerror (saved_errno));
@@ -1266,11 +1257,11 @@ gdb_safe_append_history (void)
history_max_entries);
}
- ret = rename (local_history_filename.c_str (), history_filename);
+ ret = rename (local_history_filename.c_str (), history_filename.c_str ());
saved_errno = errno;
if (ret < 0 && saved_errno != EEXIST)
warning (_("Could not rename %s to %s: %s"),
- local_history_filename.c_str (), history_filename,
+ local_history_filename.c_str (), history_filename.c_str (),
safe_strerror (saved_errno));
}
}
@@ -1643,12 +1634,12 @@ tree, and GDB will still find it.)\n\
/* The current top level prompt, settable with "set prompt", and/or
with the python `gdb.prompt_hook' hook. */
-static char *top_prompt;
+static std::string top_prompt;
/* Access method for the GDB prompt string. */
-char *
-get_prompt (void)
+const std::string &
+get_prompt ()
{
return top_prompt;
}
@@ -1658,10 +1649,7 @@ get_prompt (void)
void
set_prompt (const char *s)
{
- char *p = xstrdup (s);
-
- xfree (top_prompt);
- top_prompt = p;
+ top_prompt = s;
}
\f
@@ -1801,7 +1789,7 @@ quit_force (int *exit_arg, int from_tty)
/* Save the history information if it is appropriate to do so. */
try
{
- if (write_history_p && history_filename)
+ if (write_history_p && !history_filename.empty ())
{
int save = 0;
@@ -2049,27 +2037,8 @@ init_history (void)
set_readline_history_size (history_size_setshow_var);
- tmpenv = getenv ("GDBHISTFILE");
- if (tmpenv != nullptr)
- history_filename = xstrdup (tmpenv);
- else if (history_filename == nullptr)
- {
- /* We include the current directory so that if the user changes
- directories the file written will be the same as the one
- that was read. */
-#ifdef __MSDOS__
- /* No leading dots in file names are allowed on MSDOS. */
- const char *fname = "_gdb_history";
-#else
- const char *fname = ".gdb_history";
-#endif
-
- gdb::unique_xmalloc_ptr<char> temp (gdb_abspath (fname));
- history_filename = temp.release ();
- }
-
- if (!history_filename_empty ())
- read_history (history_filename);
+ if (!history_filename.empty ())
+ read_history (history_filename.c_str ());
}
static void
@@ -2119,21 +2088,20 @@ show_exec_done_display_p (struct ui_file *file, int from_tty,
Extension languages, for example Python's gdb.parameter API, will read
the value directory from this variable, so we must ensure that this
always contains the correct value. */
-static char *staged_gdb_datadir;
+static std::string staged_gdb_datadir;
/* "set" command for the gdb_datadir configuration variable. */
static void
set_gdb_datadir (const char *args, int from_tty, struct cmd_list_element *c)
{
- set_gdb_data_directory (staged_gdb_datadir);
+ set_gdb_data_directory (staged_gdb_datadir.c_str ());
/* SET_GDB_DATA_DIRECTORY will resolve relative paths in
STAGED_GDB_DATADIR, so we now copy the value from GDB_DATADIR
back into STAGED_GDB_DATADIR so the extension languages can read the
correct value. */
- free (staged_gdb_datadir);
- staged_gdb_datadir = strdup (gdb_datadir.c_str ());
+ staged_gdb_datadir = gdb_datadir;
gdb::observers::gdb_datadir_changed.notify ();
}
@@ -2158,12 +2126,13 @@ set_history_filename (const char *args,
/* We include the current directory so that if the user changes
directories the file written will be the same as the one
that was read. */
- if (!history_filename_empty () && !IS_ABSOLUTE_PATH (history_filename))
+ if (!history_filename.empty ()
+ && !IS_ABSOLUTE_PATH (history_filename.c_str ()))
{
- gdb::unique_xmalloc_ptr<char> temp (gdb_abspath (history_filename));
+ gdb::unique_xmalloc_ptr<char> temp
+ (gdb_abspath (history_filename.c_str ()));
- xfree (history_filename);
- history_filename = temp.release ();
+ history_filename = temp.get ();
}
}
@@ -2329,7 +2298,7 @@ When set, GDB uses the specified path to search for data files."),
&setlist,
&showlist);
/* Prime the initial value for data-directory. */
- staged_gdb_datadir = strdup (gdb_datadir.c_str ());
+ staged_gdb_datadir = gdb_datadir;
add_setshow_auto_boolean_cmd ("interactive-mode", class_support,
&interactive_mode, _("\
@@ -2415,3 +2384,28 @@ gdb_init ()
/* Create $_gdb_major and $_gdb_minor convenience variables. */
init_gdb_version_vars ();
}
+
+void _initialize_top ();
+void
+_initialize_top ()
+{
+ /* Determine a default value for the history filename. */
+ const char *tmpenv = getenv ("GDBHISTFILE");
+ if (tmpenv != nullptr)
+ history_filename = tmpenv;
+ else
+ {
+ /* We include the current directory so that if the user changes
+ directories the file written will be the same as the one
+ that was read. */
+#ifdef __MSDOS__
+ /* No leading dots in file names are allowed on MSDOS. */
+ const char *fname = "_gdb_history";
+#else
+ const char *fname = ".gdb_history";
+#endif
+
+ gdb::unique_xmalloc_ptr<char> temp (gdb_abspath (fname));
+ history_filename = temp.get ();
+ }
+}
diff --git a/gdb/top.h b/gdb/top.h
index ef88ca024e4..f8a5c39bbdf 100644
--- a/gdb/top.h
+++ b/gdb/top.h
@@ -261,7 +261,7 @@ extern scoped_value_mark prepare_execute_command (void);
/* This function returns a pointer to the string that is used
by gdb for its command prompt. */
-extern char *get_prompt (void);
+extern const std::string &get_prompt ();
/* This function returns a pointer to the string that is used
by gdb for its command prompt. */
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 06cf9cad29a..3997d211182 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -130,7 +130,7 @@ static traceframe_info_up current_traceframe_info;
static struct cmd_list_element *tfindlist;
/* List of expressions to collect by default at each tracepoint hit. */
-char *default_collect;
+std::string default_collect;
static bool disconnected_tracing;
@@ -146,15 +146,15 @@ static int trace_buffer_size = -1;
/* Textual notes applying to the current and/or future trace runs. */
-static char *trace_user = NULL;
+static std::string trace_user;
/* Textual notes applying to the current and/or future trace runs. */
-static char *trace_notes = NULL;
+static std::string trace_notes;
/* Textual notes applying to the stopping of a trace. */
-static char *trace_stop_notes = NULL;
+static std::string trace_stop_notes;
/* support routines */
@@ -1688,10 +1688,11 @@ start_tracing (const char *notes)
target_set_trace_buffer_size (trace_buffer_size);
if (!notes)
- notes = trace_notes;
- ret = target_set_trace_notes (trace_user, notes, NULL);
+ notes = trace_notes.c_str ();
- if (!ret && (trace_user || notes))
+ ret = target_set_trace_notes (trace_user.c_str (), notes, NULL);
+
+ if (!ret && (!trace_user.empty () || notes))
warning (_("Target does not support trace user/notes, info ignored"));
/* Now insert traps and begin collecting data. */
@@ -1764,7 +1765,8 @@ stop_tracing (const char *note)
}
if (!note)
- note = trace_stop_notes;
+ note = trace_stop_notes.c_str ();
+
ret = target_set_trace_notes (NULL, NULL, note);
if (!ret && note)
@@ -2804,10 +2806,10 @@ all_tracepoint_actions (struct breakpoint *t)
validation is per-tracepoint (local var "xyz" might be valid for
one tracepoint and not another, etc), we make up the action on
the fly, and don't cache it. */
- if (*default_collect)
+ if (!default_collect.empty ())
{
gdb::unique_xmalloc_ptr<char> default_collect_line
- (xstrprintf ("collect %s", default_collect));
+ (xstrprintf ("collect %s", default_collect.c_str ()));
validate_actionline (default_collect_line.get (), t);
actions.reset (new struct command_line (simple_control,
@@ -2896,7 +2898,7 @@ set_trace_user (const char *args, int from_tty,
{
int ret;
- ret = target_set_trace_notes (trace_user, NULL, NULL);
+ ret = target_set_trace_notes (trace_user.c_str (), NULL, NULL);
if (!ret)
warning (_("Target does not support trace notes, user ignored"));
@@ -2908,7 +2910,7 @@ set_trace_notes (const char *args, int from_tty,
{
int ret;
- ret = target_set_trace_notes (NULL, trace_notes, NULL);
+ ret = target_set_trace_notes (NULL, trace_notes.c_str (), NULL);
if (!ret)
warning (_("Target does not support trace notes, note ignored"));
@@ -2920,7 +2922,7 @@ set_trace_stop_notes (const char *args, int from_tty,
{
int ret;
- ret = target_set_trace_notes (NULL, NULL, trace_stop_notes);
+ ret = target_set_trace_notes (NULL, NULL, trace_stop_notes.c_str ());
if (!ret)
warning (_("Target does not support trace notes, stop note ignored"));
@@ -4137,7 +4139,6 @@ Tracepoint actions may include collecting of specified data,\n\
single-stepping, or enabling/disabling other tracepoints,\n\
depending on target's capabilities."));
- default_collect = xstrdup ("");
add_setshow_string_cmd ("default-collect", class_trace,
&default_collect, _("\
Set the list of expressions to collect by default."), _("\
diff --git a/gdb/tracepoint.h b/gdb/tracepoint.h
index b32dd61fdfc..a66226f93f1 100644
--- a/gdb/tracepoint.h
+++ b/gdb/tracepoint.h
@@ -159,7 +159,7 @@ struct trace_status
struct trace_status *current_trace_status (void);
-extern char *default_collect;
+extern std::string default_collect;
extern int trace_regblock_size;
--
2.31.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 3/4] gdb: Have setter and getter callbacks for settings
2021-08-08 19:22 [PATCH v2 0/4] gdb: Refactor cmd_list_element.var Lancelot SIX
2021-08-08 19:22 ` [PATCH v2 1/4] gdb: Add typesafe getter/setter for cmd_list_element.var Lancelot SIX
2021-08-08 19:23 ` [PATCH v2 2/4] gdb: make string-like set show commands use std::string variable Lancelot SIX
@ 2021-08-08 19:23 ` Lancelot SIX
2021-08-10 3:01 ` Simon Marchi
2021-08-08 19:23 ` [PATCH v2 4/4] gdb: Setting setter return a bool to tell if the value changed Lancelot SIX
3 siblings, 1 reply; 15+ messages in thread
From: Lancelot SIX @ 2021-08-08 19:23 UTC (permalink / raw)
To: gdb-patches; +Cc: Lancelot SIX
The main motivation behind this improvement is to help the
implementation of a patch Simon Marchi is preparing to fix a bug when
MI or Python try to access parameters that are inferior dependent (see
PR/28085).
This commit extends the previous one, which introduces the setting
object to represent a static variable that can be set or shown with the
appropriate commands. This patch proposes that a setting can either
contain a pointer to a static variable holding a setting, or pointers to
a pair of setter and getter callback functions.
The callbacks functions can be used to generate values on the fly when
the setting is accessed (or set the value in a context dependent way).
This is useful when the source of truth is not contained in the
variable pointed to by the setting instance.
Given that the callback function call is hidden within the setting
abstraction introduced earlier, none of the sites accessing the setting
needs to be updated. The registered getter or setter is used whatever
the way to access it is (through MI, Python, Guild, the "with" command
and the $_gdb_setting / $_gdb_setting_str convenience functions).
All the add_setshow_*_cmd are given a new overload that will accept the
pair of function pointers (set / get functions) instead of the pointer
to a global variable.
Tested on GNU/Linux x86_64 with no regression observed.
---
gdb/cli/cli-decode.c | 366 +++++++++++++++++++++++++++++++++++++++----
gdb/command.h | 324 +++++++++++++++++++++++++++++++++++++-
2 files changed, 650 insertions(+), 40 deletions(-)
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 865e1331c8f..3b08bda6493 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -460,6 +460,10 @@ empty_func (const char *args, int from_tty, cmd_list_element *c)
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_SETTING_FUNC is a pointer to an optional function callback used to set
+ the setting value.
+ GET_SETTING_FUNC is a pointer to an optional function callback used to get
+ the setting value.
DOC is the documentation string. */
template<typename T>
@@ -469,6 +473,8 @@ add_set_or_show_cmd (const char *name,
enum command_class theclass,
var_types var_type,
T *var,
+ setting_setter_ftype<T> set_setting_func,
+ setting_getter_ftype<T> get_setting_func,
const char *doc,
struct cmd_list_element **list)
{
@@ -476,7 +482,14 @@ add_set_or_show_cmd (const char *name,
gdb_assert (type == set_cmd || type == show_cmd);
c->type = type;
- c->var.set_p<T> (var_type, var);
+
+ gdb_assert ((var != nullptr)
+ != (set_setting_func != nullptr && get_setting_func != nullptr));
+ if (var != nullptr)
+ c->var.set_p<T> (var_type, var);
+ else
+ c->var.set_accessors<T> (var_type, set_setting_func, get_setting_func);
+
/* This needs to be something besides NULL so that this isn't
treated as a help class. */
c->func = empty_func;
@@ -486,9 +499,11 @@ 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
- non-NULL). SET_DOC, SHOW_DOC and HELP_DOC are the documentation
- strings.
+ command. If nullptr is given as VAR, then both SET_SETTING_FUNC and
+ GET_SETTING_FUNC must be provided. SET_SETTING_FUNC and GET_SETTING_FUNC are
+ callbacks used to access and modify the underlying property, whatever its
+ storage is. SET_FUNC and SHOW_FUNC are the callback functions (if non-NULL).
+ SET_DOC, SHOW_DOC and HELP_DOC are the documentation strings.
Return the newly created set and show commands. */
@@ -499,6 +514,8 @@ add_setshow_cmd_full (const char *name,
var_types var_type, T *var,
const char *set_doc, const char *show_doc,
const char *help_doc,
+ setting_setter_ftype<T> set_setting_func,
+ setting_getter_ftype<T> get_setting_func,
cmd_func_ftype *set_func,
show_value_ftype *show_func,
struct cmd_list_element **set_list,
@@ -520,6 +537,7 @@ add_setshow_cmd_full (const char *name,
full_show_doc = xstrdup (show_doc);
}
set = add_set_or_show_cmd<T> (name, set_cmd, theclass, var_type, var,
+ set_setting_func, get_setting_func,
full_set_doc, set_list);
set->doc_allocated = 1;
@@ -527,6 +545,7 @@ add_setshow_cmd_full (const char *name,
set->func = set_func;
show = add_set_or_show_cmd<T> (name, show_cmd, theclass, var_type, var,
+ set_setting_func, get_setting_func,
full_show_doc, show_list);
show->doc_allocated = 1;
show->show_value_func = show_func;
@@ -558,12 +577,36 @@ add_setshow_enum_cmd (const char *name,
set_show_commands commands
= 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);
+ nullptr, nullptr, set_func,
+ show_func, set_list, show_list);
commands.set->enums = enumlist;
return commands;
}
+/* Same as above but using a getter and a setter function instead of a pointer
+ to a global storage buffer. */
+
+set_show_commands
+add_setshow_enum_cmd (const char *name, command_class theclass,
+ const char *const *enumlist, const char *set_doc,
+ const char *show_doc, const char *help_doc,
+ setting_setter_ftype<const char *> set_func,
+ setting_getter_ftype<const char *> get_func,
+ show_value_ftype *show_func,
+ cmd_list_element **set_list,
+ cmd_list_element **show_list)
+{
+ auto cmds = add_setshow_cmd_full<const char *> (name, theclass, var_enum,
+ nullptr, set_doc, show_doc,
+ help_doc, set_func, get_func,
+ nullptr, show_func, set_list,
+ show_list);
+
+ cmds.set->enums = enumlist;
+
+ return cmds;
+}
+
/* See cli-decode.h. */
const char * const auto_boolean_enums[] = { "on", "off", "auto", NULL };
@@ -584,17 +627,42 @@ add_setshow_auto_boolean_cmd (const char *name,
struct cmd_list_element **show_list)
{
set_show_commands commands
- = 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);
+ = add_setshow_cmd_full<enum auto_boolean> (name, theclass, var_auto_boolean,
+ var, set_doc, show_doc, help_doc,
+ nullptr, nullptr, set_func,
+ show_func, set_list, show_list);
commands.set->enums = auto_boolean_enums;
return commands;
}
+/* Same as above but using a getter and a setter function instead of a pointer
+ to a global storage buffer. */
+
+set_show_commands
+add_setshow_auto_boolean_cmd (const char *name, command_class theclass,
+ const char *set_doc, const char *show_doc,
+ const char *help_doc,
+ setting_setter_ftype<enum auto_boolean> set_func,
+ setting_getter_ftype<enum auto_boolean> get_func,
+ show_value_ftype *show_func,
+ cmd_list_element **set_list,
+ cmd_list_element **show_list)
+{
+ auto cmds = add_setshow_cmd_full<enum auto_boolean> (name, theclass,
+ var_auto_boolean,
+ nullptr, set_doc,
+ show_doc, help_doc,
+ set_func, get_func,
+ nullptr, show_func,
+ set_list, show_list);
+
+ cmds.set->enums = auto_boolean_enums;
+
+ return cmds;
+}
+
/* See cli-decode.h. */
const char * const boolean_enums[] = { "on", "off", NULL };
@@ -616,7 +684,7 @@ add_setshow_boolean_cmd (const char *name, enum command_class theclass, bool *va
set_show_commands commands
= add_setshow_cmd_full<bool> (name, theclass, var_boolean, var,
set_doc, show_doc, help_doc,
- set_func, show_func,
+ nullptr, nullptr, set_func, show_func,
set_list, show_list);
commands.set->enums = boolean_enums;
@@ -624,6 +692,29 @@ add_setshow_boolean_cmd (const char *name, enum command_class theclass, bool *va
return commands;
}
+/* Same as above but using a getter and a setter function instead of a pointer
+ to a global storage buffer. */
+
+set_show_commands
+add_setshow_boolean_cmd (const char *name, command_class theclass,
+ const char *set_doc, const char *show_doc,
+ const char *help_doc,
+ setting_setter_ftype<bool> set_func,
+ setting_getter_ftype<bool> get_func,
+ show_value_ftype *show_func,
+ cmd_list_element **set_list,
+ cmd_list_element **show_list)
+{
+ auto cmds = add_setshow_cmd_full<bool> (name, theclass, var_boolean, nullptr,
+ set_doc, show_doc, help_doc,
+ set_func, get_func, nullptr,
+ show_func, set_list, show_list);
+
+ cmds.set->enums = boolean_enums;
+
+ return cmds;
+}
+
/* Add element named NAME to both the set and show command LISTs (the
list for set/show or some sublist thereof). */
@@ -640,14 +731,38 @@ add_setshow_filename_cmd (const char *name, enum command_class theclass,
set_show_commands commands
= add_setshow_cmd_full<std::string> (name, theclass, var_filename, var,
set_doc, show_doc, help_doc,
- set_func, show_func,
- set_list, show_list);
+ nullptr, nullptr, set_func,
+ show_func, set_list, show_list);
set_cmd_completer (commands.set, filename_completer);
return commands;
}
+/* Same as above but using a getter and a setter function instead of a pointer
+ to a global storage buffer. */
+
+set_show_commands
+add_setshow_filename_cmd (const char *name, command_class theclass,
+ const char *set_doc, const char *show_doc,
+ const char *help_doc,
+ setting_setter_ftype<std::string> set_func,
+ setting_getter_ftype<std::string> get_func,
+ show_value_ftype *show_func,
+ cmd_list_element **set_list,
+ cmd_list_element **show_list)
+{
+ auto cmds = add_setshow_cmd_full<std::string> (name, theclass, var_filename,
+ nullptr, set_doc, show_doc,
+ help_doc, set_func, get_func,
+ nullptr, show_func, set_list,
+ show_list);
+
+ set_cmd_completer (cmds.set, filename_completer);
+
+ return cmds;
+}
+
/* Add element named NAME to both the set and show command LISTs (the
list for set/show or some sublist thereof). */
@@ -663,9 +778,9 @@ add_setshow_string_cmd (const char *name, enum command_class theclass,
{
set_show_commands commands
= add_setshow_cmd_full<std::string> (name, theclass, var_string, var,
- set_doc, show_doc, help_doc,
- set_func, show_func,
- set_list, show_list);
+ set_doc, show_doc, help_doc,
+ nullptr, nullptr, set_func,
+ show_func, set_list, show_list);
/* Disable the default symbol completer. */
set_cmd_completer (commands.set, nullptr);
@@ -673,6 +788,31 @@ add_setshow_string_cmd (const char *name, enum command_class theclass,
return commands;
}
+/* Same as above but using a getter and a setter function instead of a pointer
+ to a global storage buffer. */
+
+set_show_commands
+add_setshow_string_cmd (const char *name, command_class theclass,
+ const char *set_doc, const char *show_doc,
+ const char *help_doc,
+ setting_setter_ftype<std::string> set_func,
+ setting_getter_ftype<std::string> get_func,
+ show_value_ftype *show_func,
+ cmd_list_element **set_list,
+ cmd_list_element **show_list)
+{
+ auto cmds = add_setshow_cmd_full<std::string> (name, theclass, var_string,
+ nullptr, set_doc, show_doc,
+ help_doc, set_func, get_func,
+ nullptr, show_func, set_list,
+ show_list);
+
+ /* Disable the default symbol completer. */
+ set_cmd_completer (cmds.set, nullptr);
+
+ return cmds;
+}
+
/* Add element named NAME to both the set and show command LISTs (the
list for set/show or some sublist thereof). */
@@ -689,8 +829,8 @@ add_setshow_string_noescape_cmd (const char *name, enum command_class theclass,
set_show_commands commands
= add_setshow_cmd_full<std::string> (name, theclass, var_string_noescape,
var, set_doc, show_doc, help_doc,
- set_func, show_func, set_list,
- show_list);
+ nullptr, nullptr, set_func, show_func,
+ set_list, show_list);
/* Disable the default symbol completer. */
set_cmd_completer (commands.set, nullptr);
@@ -698,6 +838,32 @@ add_setshow_string_noescape_cmd (const char *name, enum command_class theclass,
return commands;
}
+/* Same as above but using a getter and a setter function instead of a pointer
+ to a global storage buffer. */
+
+set_show_commands
+add_setshow_string_noescape_cmd (const char *name, command_class theclass,
+ const char *set_doc, const char *show_doc,
+ const char *help_doc,
+ setting_setter_ftype<std::string> set_func,
+ setting_getter_ftype<std::string> get_func,
+ show_value_ftype *show_func,
+ cmd_list_element **set_list,
+ cmd_list_element **show_list)
+{
+ auto cmds = add_setshow_cmd_full<std::string> (name, theclass,
+ var_string_noescape, nullptr,
+ set_doc, show_doc, help_doc,
+ set_func, get_func,
+ nullptr, show_func, set_list,
+ show_list);
+
+ /* Disable the default symbol completer. */
+ set_cmd_completer (cmds.set, nullptr);
+
+ return cmds;
+}
+
/* Add element named NAME to both the set and show command LISTs (the
list for set/show or some sublist thereof). */
@@ -714,14 +880,38 @@ add_setshow_optional_filename_cmd (const char *name, enum command_class theclass
set_show_commands commands
= add_setshow_cmd_full<std::string> (name, theclass, var_optional_filename,
var, set_doc, show_doc, help_doc,
- set_func, show_func, set_list,
- show_list);
-
+ nullptr, nullptr, set_func, show_func,
+ set_list, show_list);
+
set_cmd_completer (commands.set, filename_completer);
return commands;
}
+/* Same as above but using a getter and a setter function instead of a pointer
+ to a global storage buffer. */
+
+set_show_commands
+add_setshow_optional_filename_cmd (const char *name, command_class theclass,
+ const char *set_doc, const char *show_doc,
+ const char *help_doc,
+ setting_setter_ftype<std::string> set_func,
+ setting_getter_ftype<std::string> get_func,
+ show_value_ftype *show_func,
+ cmd_list_element **set_list,
+ cmd_list_element **show_list)
+{
+ auto cmds =
+ add_setshow_cmd_full<std::string> (name, theclass, var_optional_filename,
+ nullptr, set_doc, show_doc, help_doc,
+ set_func, get_func, nullptr, show_func,
+ set_list,show_list);
+
+ set_cmd_completer (cmds.set, filename_completer);
+
+ return cmds;
+}
+
/* Completes on literal "unlimited". Used by integer commands that
support a special "unlimited" value. */
@@ -756,15 +946,39 @@ 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<int> (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,
+ nullptr, nullptr, set_func,
+ show_func, set_list, show_list);
set_cmd_completer (commands.set, integer_unlimited_completer);
return commands;
}
+/* Same as above but using a getter and a setter function instead of a pointer
+ to a global storage buffer. */
+
+set_show_commands
+add_setshow_integer_cmd (const char *name, command_class theclass,
+ const char *set_doc, const char *show_doc,
+ const char *help_doc,
+ setting_setter_ftype<int> set_func,
+ setting_getter_ftype<int> get_func,
+ show_value_ftype *show_func,
+ cmd_list_element **set_list,
+ cmd_list_element **show_list)
+{
+ auto cmds = add_setshow_cmd_full<int> (name, theclass, var_integer, nullptr,
+ set_doc, show_doc, help_doc, set_func,
+ get_func, nullptr, show_func, set_list,
+ show_list);
+
+ set_cmd_completer (cmds.set, integer_unlimited_completer);
+
+ return cmds;
+}
+
/* Add element named NAME to both the set and show command LISTs (the
list for set/show or some sublist thereof). CLASS is as in
add_cmd. VAR is address of the variable which will contain the
@@ -783,14 +997,38 @@ add_setshow_uinteger_cmd (const char *name, enum command_class theclass,
set_show_commands commands
= 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);
+ nullptr, nullptr, set_func,
+ show_func, set_list, show_list);
set_cmd_completer (commands.set, integer_unlimited_completer);
return commands;
}
+/* Same as above but using a getter and a setter function instead of a pointer
+ to a global storage buffer. */
+
+set_show_commands
+add_setshow_uinteger_cmd (const char *name, command_class theclass,
+ const char *set_doc, const char *show_doc,
+ const char *help_doc,
+ setting_setter_ftype<unsigned int> set_func,
+ setting_getter_ftype<unsigned int> get_func,
+ show_value_ftype *show_func,
+ cmd_list_element **set_list,
+ cmd_list_element **show_list)
+{
+ auto cmds = add_setshow_cmd_full<unsigned int> (name, theclass, var_uinteger,
+ nullptr, set_doc, show_doc,
+ help_doc, set_func, get_func,
+ nullptr, show_func, set_list,
+ show_list);
+
+ set_cmd_completer (cmds.set, integer_unlimited_completer);
+
+ return cmds;
+}
+
/* Add element named NAME to both the set and show command LISTs (the
list for set/show or some sublist thereof). CLASS is as in
add_cmd. VAR is address of the variable which will contain the
@@ -808,8 +1046,27 @@ add_setshow_zinteger_cmd (const char *name, enum command_class theclass,
{
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);
+ nullptr, nullptr, set_func,
+ show_func, set_list, show_list);
+}
+
+/* Same as above but using a getter and a setter function instead of a pointer
+ to a global storage buffer. */
+
+set_show_commands
+add_setshow_zinteger_cmd (const char *name, command_class theclass,
+ const char *set_doc, const char *show_doc,
+ const char *help_doc,
+ setting_setter_ftype<int> set_func,
+ setting_getter_ftype<int> get_func,
+ show_value_ftype *show_func,
+ cmd_list_element **set_list,
+ cmd_list_element **show_list)
+{
+ return add_setshow_cmd_full<int> (name, theclass, var_zinteger, nullptr,
+ set_doc, show_doc, help_doc, set_func,
+ get_func, nullptr, show_func, set_list,
+ show_list);
}
set_show_commands
@@ -826,14 +1083,39 @@ add_setshow_zuinteger_unlimited_cmd (const char *name,
{
set_show_commands commands
= 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_doc, show_doc, help_doc, nullptr,
+ nullptr, set_func, show_func, set_list,
+ show_list);
set_cmd_completer (commands.set, integer_unlimited_completer);
return commands;
}
+/* Same as above but using a getter and a setter function instead of a pointer
+ to a global storage buffer. */
+
+set_show_commands
+add_setshow_zuinteger_unlimited_cmd (const char *name, command_class theclass,
+ const char *set_doc, const char *show_doc,
+ const char *help_doc,
+ setting_setter_ftype<int> set_func,
+ setting_getter_ftype<int> get_func,
+ show_value_ftype *show_func,
+ cmd_list_element **set_list,
+ cmd_list_element **show_list)
+{
+ auto cmds
+ = add_setshow_cmd_full<int> (name, theclass, var_zuinteger_unlimited,
+ nullptr, set_doc, show_doc, help_doc, set_func,
+ get_func, nullptr, show_func, set_list,
+ show_list);
+
+ set_cmd_completer (cmds.set, integer_unlimited_completer);
+
+ return cmds;
+}
+
/* Add element named NAME to both the set and show command LISTs (the
list for set/show or some sublist thereof). CLASS is as in
add_cmd. VAR is address of the variable which will contain the
@@ -851,7 +1133,27 @@ add_setshow_zuinteger_cmd (const char *name, enum command_class theclass,
{
return add_setshow_cmd_full<unsigned int> (name, theclass, var_zuinteger,
var, set_doc, show_doc, help_doc,
- set_func, show_func, set_list,
+ nullptr, nullptr, set_func,
+ show_func, set_list, show_list);
+}
+
+/* Same as above but using a getter and a setter function instead of a pointer
+ to a global storage buffer. */
+
+set_show_commands
+add_setshow_zuinteger_cmd (const char *name, command_class theclass,
+ const char *set_doc, const char *show_doc,
+ const char *help_doc,
+ setting_setter_ftype<unsigned int> set_func,
+ setting_getter_ftype<unsigned int> get_func,
+ show_value_ftype *show_func,
+ cmd_list_element **set_list,
+ cmd_list_element **show_list)
+{
+ return add_setshow_cmd_full<unsigned int> (name, theclass, var_zuinteger,
+ nullptr, set_doc, show_doc,
+ help_doc, set_func, get_func,
+ nullptr, show_func, set_list,
show_list);
}
diff --git a/gdb/command.h b/gdb/command.h
index 4e4583dcfa8..7b8fe5ea9f0 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -181,6 +181,194 @@ inline bool var_type_uses<const char *> (var_types t)
return t == var_enum;
}
+/* Function signature for a callback used to get a value from a setting. */
+
+template<typename T>
+using setting_getter_ftype = T (*) ();
+
+/* Contains a function to access a parameter of a setting. */
+
+union setting_getter
+{
+ setting_getter_ftype<bool> get_bool;
+ setting_getter_ftype<int> get_int;
+ setting_getter_ftype<unsigned int> get_uint;
+ setting_getter_ftype<auto_boolean> get_auto_boolean;
+ setting_getter_ftype<std::string> get_string;
+ setting_getter_ftype<const char *> get_const_string;
+};
+
+
+/* Get the function from GETTERS that is appropriate for type T (const and non
+ const versions). */
+
+template<typename T>
+inline const setting_getter_ftype<T> &
+get_setting_getter (const setting_getter &getters);
+
+template<typename T>
+inline setting_getter_ftype<T> &
+get_setting_getter (setting_getter &getters);
+
+/* Extract the setting_getter_ftype<bool> field from GETTERS. */
+template<>
+inline const setting_getter_ftype<bool> &
+get_setting_getter<bool> (const setting_getter &getters)
+{ return getters.get_bool; }
+
+template<>
+inline setting_getter_ftype<bool> &
+get_setting_getter<bool> (setting_getter &getters)
+{ return getters.get_bool; }
+
+/* Extract the setting_getter_ftype<int> field from GETTERS. */
+template<>
+inline const setting_getter_ftype<int> &
+get_setting_getter<int> (const setting_getter &getters)
+{ return getters.get_int; }
+
+template<>
+inline setting_getter_ftype<int> &
+get_setting_getter<int> (setting_getter &getters)
+{ return getters.get_int; }
+
+/* Extract the setting_getter_ftype<unsigned int> field from GETTERS. */
+template<>
+inline const setting_getter_ftype<unsigned int> &
+get_setting_getter<unsigned int> (const setting_getter &getters)
+{ return getters.get_uint; }
+
+template<>
+inline setting_getter_ftype<unsigned int> &
+get_setting_getter<unsigned int> (setting_getter &getters)
+{ return getters.get_uint; }
+
+/* Extract the setting_getter_ftype<auto_boolean> field from GETTERS. */
+template<>
+inline const setting_getter_ftype<auto_boolean> &
+get_setting_getter<auto_boolean> (const setting_getter &getters)
+{ return getters.get_auto_boolean; }
+
+template<>
+inline setting_getter_ftype<auto_boolean> &
+get_setting_getter<auto_boolean> (setting_getter &getters)
+{ return getters.get_auto_boolean; }
+
+/* Extract the setting_getter_ftype<std::string> field from GETTERS. */
+template<>
+inline const setting_getter_ftype<std::string> &
+get_setting_getter<std::string> (const setting_getter &getters)
+{ return getters.get_string; }
+
+template<>
+inline setting_getter_ftype<std::string> &
+get_setting_getter<std::string> (setting_getter &getters)
+{ return getters.get_string; }
+
+/* Extract the setting_getter_ftype<const char *> field from GETTERS. */
+template<>
+inline const setting_getter_ftype<const char *> &
+get_setting_getter<const char *> (const setting_getter &getters)
+{ return getters.get_const_string; }
+
+template<>
+inline setting_getter_ftype<const char *> &
+get_setting_getter<const char *> (setting_getter &getters)
+{ return getters.get_const_string; }
+
+/* Function signature for a callback used to set a value to a setting. */
+
+template<typename T>
+using setting_setter_ftype = void (*) (T);
+
+/* Contains a function to set a value of a setting. */
+union setting_setter
+{
+ setting_setter_ftype<bool> set_bool;
+ setting_setter_ftype<int> set_int;
+ setting_setter_ftype<unsigned int> set_uint;
+ setting_setter_ftype<auto_boolean> set_auto_boolean;
+ setting_setter_ftype<std::string> set_string;
+ setting_setter_ftype<const char *> set_const_string;
+};
+
+/* Get the function from SETTERS that is appropriate for type T (const and non
+ const versions). */
+
+template<typename T>
+inline const setting_setter_ftype<T> &
+get_setting_setter (const setting_setter &setters);
+
+template<typename T>
+inline setting_setter_ftype<T> &
+get_setting_setter (setting_setter &setters);
+
+/* Extract the setting_setter_ftype<bool> field from SETTERS. */
+template<>
+inline const setting_setter_ftype<bool> &
+get_setting_setter<bool> (const setting_setter &setters)
+{ return setters.set_bool; }
+
+template<>
+inline setting_setter_ftype<bool> &
+get_setting_setter<bool> (setting_setter &setters)
+{ return setters.set_bool; }
+
+/* Extract the setting_setter_ftype<int> field from SETTERS. */
+template<>
+inline const setting_setter_ftype<int> &
+get_setting_setter<int> (const setting_setter &setters)
+{ return setters.set_int; }
+
+template<>
+inline setting_setter_ftype<int> &
+get_setting_setter<int> (setting_setter &setters)
+{ return setters.set_int; }
+
+/* Extract the setting_setter_ftype<unsigned int> field from SETTERS. */
+template<>
+inline const setting_setter_ftype<unsigned int> &
+get_setting_setter<unsigned int> (const setting_setter &setters)
+{ return setters.set_uint; }
+
+template<>
+inline setting_setter_ftype<unsigned int> &
+get_setting_setter<unsigned int> (setting_setter &setters)
+{ return setters.set_uint; }
+
+/* Extract the setting_setter_ftype<auto_boolean> field from SETTERS. */
+template<>
+inline const setting_setter_ftype<auto_boolean> &
+get_setting_setter<auto_boolean> (const setting_setter &setters)
+{ return setters.set_auto_boolean; }
+
+template<>
+inline setting_setter_ftype<auto_boolean> &
+get_setting_setter<auto_boolean> (setting_setter &setters)
+{ return setters.set_auto_boolean; }
+
+/* Extract the setting_setter_ftype<std::string> field from SETTERS. */
+template<>
+inline const setting_setter_ftype<std::string> &
+get_setting_setter<std::string> (const setting_setter &setters)
+{ return setters.set_string; }
+
+template<>
+inline setting_setter_ftype<std::string> &
+get_setting_setter<std::string> (setting_setter &setters)
+{ return setters.set_string; }
+
+/* Extract the setting_setter_ftype<const char *> field from SETTERS. */
+template<>
+inline const setting_setter_ftype<const char *> &
+get_setting_setter<const char *> (const setting_setter &setters)
+{ return setters.set_const_string; }
+
+template<>
+inline setting_setter_ftype<const char *> &
+get_setting_setter<const char *> (setting_setter &setters)
+{ return setters.set_const_string; }
+
/* Abstraction that contains access to data that can be set or shown.
The underlying data can be of any VAR_TYPES type. */
@@ -214,10 +402,18 @@ struct base_setting
template<typename T>
T get () const
{
- return *get_p<T> ();
+ gdb_assert (var_type_uses<T> (this->m_var_type));
+ auto getter = get_setting_getter<T> (this->m_getter);
+
+ if (getter != nullptr)
+ return (*getter) ();
+ else
+ return *get_p<T> ();
}
- /* Sets the value V to the underlying buffer.
+ /* Sets the value referenced by the setting to V. If we have a user-provided
+ setter, use it to set the setting. Otherwise copy the value V to the
+ internally referenced buffer.
The template parameter T indicates the type of the variable used to store
the setting.
@@ -231,25 +427,50 @@ struct base_setting
gdb_assert (var_type_uses<T> (this->m_var_type));
gdb_assert (!this->empty ());
- *static_cast<T *> (this->m_var) = v;
+ auto setter = get_setting_setter<T> (this->m_setter);
+
+ if (setter != nullptr)
+ (*setter) (v);
+ else
+ *static_cast<T *> (this->m_var) = v;
+ }
+
+ /* Set the user provided setter and getter functions. */
+ template<typename T>
+ void
+ set_accessors (var_types var_type,
+ setting_setter_ftype<T> setter,
+ setting_getter_ftype<T> getter)
+ {
+ this->m_var_type = var_type;
+ get_setting_setter<T> (this->m_setter) = setter;
+ get_setting_getter<T> (this->m_getter) = getter;
}
/* A setting is valid (can be evaluated to true) if it contains a valid
- reference to a memory buffer. */
- explicit operator bool () const
+ reference to a memory buffer, or if it has getter and setter callbacks
+ set. */
+ explicit operator bool() const
{
- return !this->empty ();
+ 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. */
+ /* The type of the variable M_VAR is pointing to. If M_VAR is nullptr or if
+ m_getter and m_setter are 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
{
@@ -563,72 +784,159 @@ extern set_show_commands add_setshow_enum_cmd
show_value_ftype *show_func, cmd_list_element **set_list,
cmd_list_element **show_list);
+extern set_show_commands add_setshow_enum_cmd
+ (const char *name, command_class theclass, const char *const *enumlist,
+ const char *set_doc, const char *show_doc,
+ const char *help_doc, setting_setter_ftype<const char *> set_func,
+ setting_getter_ftype<const char *> get_func, show_value_ftype *show_func,
+ cmd_list_element **set_list, cmd_list_element **show_list);
+
extern set_show_commands add_setshow_auto_boolean_cmd
(const char *name, command_class theclass, auto_boolean *var,
const char *set_doc, const char *show_doc, const char *help_doc,
cmd_func_ftype *set_func, show_value_ftype *show_func,
cmd_list_element **set_list, cmd_list_element **show_list);
+extern set_show_commands add_setshow_auto_boolean_cmd
+ (const char *name, command_class theclass, const char *set_doc,
+ const char *show_doc, const char *help_doc,
+ setting_setter_ftype<enum auto_boolean> set_func,
+ setting_getter_ftype<enum auto_boolean> get_func,
+ show_value_ftype *show_func, cmd_list_element **set_list,
+ cmd_list_element **show_list);
+
extern set_show_commands add_setshow_boolean_cmd
(const char *name, command_class theclass, bool *var, const char *set_doc,
const char *show_doc, const char *help_doc, cmd_func_ftype *set_func,
show_value_ftype *show_func, cmd_list_element **set_list,
cmd_list_element **show_list);
+extern set_show_commands add_setshow_boolean_cmd
+ (const char *name, command_class theclass, const char *set_doc,
+ const char *show_doc, const char *help_doc,
+ setting_setter_ftype<bool> set_func,
+ setting_getter_ftype<bool> get_func, show_value_ftype *show_func,
+ cmd_list_element **set_list, cmd_list_element **show_list);
+
extern set_show_commands add_setshow_filename_cmd
(const char *name, command_class theclass, std::string *var, const char *set_doc,
const char *show_doc, const char *help_doc, cmd_func_ftype *set_func,
show_value_ftype *show_func, cmd_list_element **set_list,
cmd_list_element **show_list);
+extern set_show_commands add_setshow_filename_cmd
+ (const char *name, command_class theclass, const char *set_doc,
+ const char *show_doc, const char *help_doc,
+ setting_setter_ftype<std::string> set_func,
+ setting_getter_ftype<std::string> get_func, show_value_ftype *show_func,
+ cmd_list_element **set_list, cmd_list_element **show_list);
+
extern set_show_commands add_setshow_string_cmd
(const char *name, command_class theclass, std::string *var, const char *set_doc,
const char *show_doc, const char *help_doc, cmd_func_ftype *set_func,
show_value_ftype *show_func, cmd_list_element **set_list,
cmd_list_element **show_list);
+extern set_show_commands add_setshow_string_cmd
+ (const char *name, command_class theclass, const char *set_doc,
+ const char *show_doc, const char *help_doc,
+ setting_setter_ftype<std::string> set_func,
+ setting_getter_ftype<std::string> get_func,
+ show_value_ftype *show_func, cmd_list_element **set_list,
+ cmd_list_element **show_list);
+
extern set_show_commands add_setshow_string_noescape_cmd
(const char *name, command_class theclass, std::string *var, const char *set_doc,
const char *show_doc, const char *help_doc, cmd_func_ftype *set_func,
show_value_ftype *show_func, cmd_list_element **set_list,
cmd_list_element **show_list);
+extern set_show_commands add_setshow_string_noescape_cmd
+ (const char *name, command_class theclass, const char *set_doc,
+ const char *show_doc, const char *help_doc,
+ setting_setter_ftype<std::string> set_func,
+ setting_getter_ftype<std::string> get_func, show_value_ftype *show_func,
+ cmd_list_element **set_list, cmd_list_element **show_list);
+
extern set_show_commands add_setshow_optional_filename_cmd
(const char *name, command_class theclass, std::string *var, const char *set_doc,
const char *show_doc, const char *help_doc, cmd_func_ftype *set_func,
show_value_ftype *show_func, cmd_list_element **set_list,
cmd_list_element **show_list);
+extern set_show_commands add_setshow_optional_filename_cmd
+ (const char *name, command_class theclass, const char *set_doc,
+ const char *show_doc, const char *help_doc,
+ setting_setter_ftype<std::string> set_func,
+ setting_getter_ftype<std::string> get_func,
+ show_value_ftype *show_func, cmd_list_element **set_list,
+ cmd_list_element **show_list);
+
extern set_show_commands add_setshow_integer_cmd
(const char *name, command_class theclass, int *var, const char *set_doc,
const char *show_doc, const char *help_doc, cmd_func_ftype *set_func,
show_value_ftype *show_func, cmd_list_element **set_list,
cmd_list_element **show_list);
+extern set_show_commands add_setshow_integer_cmd
+ (const char *name, command_class theclass, const char *set_doc,
+ const char *show_doc, const char *help_doc,
+ setting_setter_ftype<int> set_func,
+ setting_getter_ftype<int> get_func, show_value_ftype *show_func,
+ cmd_list_element **set_list, cmd_list_element **show_list);
+
extern set_show_commands add_setshow_uinteger_cmd
(const char *name, command_class theclass, unsigned int *var,
const char *set_doc, const char *show_doc, const char *help_doc,
cmd_func_ftype *set_func, show_value_ftype *show_func,
cmd_list_element **set_list, cmd_list_element **show_list);
+extern set_show_commands add_setshow_uinteger_cmd
+ (const char *name, command_class theclass, const char *set_doc,
+ const char *show_doc, const char *help_doc,
+ setting_setter_ftype<unsigned int> set_func,
+ setting_getter_ftype<unsigned int> get_func, show_value_ftype *show_func,
+ cmd_list_element **set_list, cmd_list_element **show_list);
+
extern set_show_commands add_setshow_zinteger_cmd
(const char *name, command_class theclass, int *var, const char *set_doc,
const char *show_doc, const char *help_doc, cmd_func_ftype *set_func,
show_value_ftype *show_func, cmd_list_element **set_list,
cmd_list_element **show_list);
+extern set_show_commands add_setshow_zinteger_cmd
+ (const char *name, command_class theclass, const char *set_doc,
+ const char *show_doc, const char *help_doc,
+ setting_setter_ftype<int> set_func,
+ setting_getter_ftype<int> get_func, show_value_ftype *show_func,
+ cmd_list_element **set_list, cmd_list_element **show_list);
+
extern set_show_commands add_setshow_zuinteger_cmd
(const char *name, command_class theclass, unsigned int *var,
const char *set_doc, const char *show_doc, const char *help_doc,
cmd_func_ftype *set_func, show_value_ftype *show_func,
cmd_list_element **set_list, cmd_list_element **show_list);
+extern set_show_commands add_setshow_zuinteger_cmd
+ (const char *name, command_class theclass, const char *set_doc,
+ const char *show_doc, const char *help_doc,
+ setting_setter_ftype<unsigned int> set_func,
+ setting_getter_ftype<unsigned int> get_func, show_value_ftype *show_func,
+ cmd_list_element **set_list, cmd_list_element **show_list);
+
extern set_show_commands add_setshow_zuinteger_unlimited_cmd
(const char *name, command_class theclass, int *var, const char *set_doc,
const char *show_doc, const char *help_doc, cmd_func_ftype *set_func,
show_value_ftype *show_func, cmd_list_element **set_list,
cmd_list_element **show_list);
+extern set_show_commands add_setshow_zuinteger_unlimited_cmd
+ (const char *name, command_class theclass, const char *set_doc,
+ const char *show_doc, const char *help_doc,
+ setting_setter_ftype<int> set_func, setting_getter_ftype<int> get_func,
+ show_value_ftype *show_func, cmd_list_element **set_list,
+ cmd_list_element **show_list);
+
/* Do a "show" command for each thing on a command list. */
extern void cmd_show_list (struct cmd_list_element *, int);
--
2.31.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 4/4] gdb: Setting setter return a bool to tell if the value changed
2021-08-08 19:22 [PATCH v2 0/4] gdb: Refactor cmd_list_element.var Lancelot SIX
` (2 preceding siblings ...)
2021-08-08 19:23 ` [PATCH v2 3/4] gdb: Have setter and getter callbacks for settings Lancelot SIX
@ 2021-08-08 19:23 ` Lancelot SIX
3 siblings, 0 replies; 15+ messages in thread
From: Lancelot SIX @ 2021-08-08 19:23 UTC (permalink / raw)
To: gdb-patches; +Cc: Lancelot SIX
GDB can notify observers when a parameter is changed.
To do that, do_set_command (in gdb/cli/cli-setshow.c) compares the new
value against the old one before updating it, and based on that notifies
observers. This looks like something like:
int valuechanged = 0;
switch (cmd->var.type ())
{
case var_integer:
{
LONGEST new_val = parse_and_eval_long (arg)
if (new_val != cmd->var.get<int> ())
{
cmd->var.get<int> (new_val);
value_changes = 1;
}
}
break;
case var_string:
{
std::string val = std::string (arg);
if (new_val != cmd->var.get<std::string> ())
{
cmd->var.get<std::string> (new_val);
value_changes = 1;
}
}
break;
case...
/* And so on for all possible var_types. */
}
This comparison is done for each possible var_type, which leads to
unnecessary code duplication.
In this patch I propose to move all those checks in one place within the
setting setter method. This limits the code duplication and simplifies
the do_set_command implementation.
This patch also changes slightly the way a value change is detected.
Instead of comparing the user provided value against the current value
of the setting, we compare the value of the setting before and after the
set operation. This is meant to handle edge cases where trying to set
an unrecognized value would be equivalent to a noop (the actual value
remains unchanged).
There should be no user visible change introduced by this commit.
Tested on x86_64 GNU/Linux.
[1] https://review.lttng.org/c/binutils-gdb/+/5831/41
---
gdb/cli/cli-setshow.c | 96 +++++++++----------------------------------
gdb/command.h | 6 ++-
2 files changed, 25 insertions(+), 77 deletions(-)
diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c
index 3af99a5e917..591db466784 100644
--- a/gdb/cli/cli-setshow.c
+++ b/gdb/cli/cli-setshow.c
@@ -31,17 +31,17 @@
/* Return true if the change of command parameter should be notified. */
-static int
-notify_command_param_changed_p (int param_changed, struct cmd_list_element *c)
+static bool
+notify_command_param_changed_p (bool param_changed, struct cmd_list_element *c)
{
- if (param_changed == 0)
- return 0;
+ if (!param_changed)
+ return false;
if (c->theclass == class_maintenance || c->theclass == class_deprecated
|| c->theclass == class_obscure)
- return 0;
+ return false;
- return 1;
+ return true;
}
\f
@@ -305,7 +305,7 @@ void
do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
{
/* A flag to indicate the option is changed or not. */
- int option_changed = 0;
+ bool option_changed = false;
gdb_assert (c->type == set_cmd);
@@ -353,26 +353,12 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
*q++ = '\0';
newobj = (char *) xrealloc (newobj, q - newobj);
- const std::string var = c->var.get<std::string> ();
- if (var != std::string (newobj))
- {
- c->var.set<std::string> (std::string (newobj));
-
- option_changed = 1;
- }
+ option_changed = c->var.set<std::string> (std::string (newobj));
xfree (newobj);
}
break;
case var_string_noescape:
- {
- const std::string var = c->var.get<std::string> ();
- if (var != arg)
- {
- c->var.set<std::string> (std::string (arg));
-
- option_changed = 1;
- }
- }
+ option_changed = c->var.set<std::string> (std::string (arg));
break;
case var_filename:
if (*arg == '\0')
@@ -398,13 +384,8 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
else
val = xstrdup ("");
- const std::string var = c->var.get<std::string> ();
- if (var != std::string (val))
- {
- c->var.set<std::string> (std::string (val));
-
- option_changed = 1;
- }
+ option_changed
+ = c->var.set<std::string> (std::string (val));
xfree (val);
}
break;
@@ -414,38 +395,18 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
if (val < 0)
error (_("\"on\" or \"off\" expected."));
- if (val != c->var.get<bool> ())
- {
- c->var.set<bool> (val);
- option_changed = 1;
- }
+ option_changed = c->var.set<bool> (val);
}
break;
case var_auto_boolean:
- {
- enum auto_boolean val = parse_auto_binary_operation (arg);
-
- if (c->var.get<enum auto_boolean> () != val)
- {
- c->var.set<enum auto_boolean> (val);
-
- option_changed = 1;
- }
- }
+ option_changed = c->var.set<enum auto_boolean> (parse_auto_binary_operation (arg));
break;
case var_uinteger:
case var_zuinteger:
- {
- unsigned int val = parse_cli_var_uinteger (c->var.type (), &arg, true);
-
- if (c->var.get<unsigned int> () != val)
- {
- c->var.set<unsigned int> (val);
-
- option_changed = 1;
- }
- }
+ option_changed
+ = c->var.set<unsigned int> (parse_cli_var_uinteger (c->var.type (),
+ &arg, true));
break;
case var_integer:
case var_zinteger:
@@ -475,12 +436,7 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
|| (c->var.type () == var_zinteger && val > INT_MAX))
error (_("integer %s out of range"), plongest (val));
- if (c->var.get<int> () != val)
- {
- c->var.set<int> (val);
-
- option_changed = 1;
- }
+ option_changed = c->var.set<int> (val);
}
break;
case var_enum:
@@ -493,24 +449,12 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
if (*after != '\0')
error (_("Junk after item \"%.*s\": %s"), len, arg, after);
- if (c->var.get<const char *> () != match)
- {
- c->var.set<const char *> (match);
-
- option_changed = 1;
- }
+ option_changed = c->var.set<const char *> (match);
}
break;
case var_zuinteger_unlimited:
- {
- int val = parse_cli_var_zuinteger_unlimited (&arg, true);
-
- if (c->var.get<int> () != val)
- {
- c->var.set<int> (val);
- option_changed = 1;
- }
- }
+ option_changed = c->var.set<int>
+ (parse_cli_var_zuinteger_unlimited (&arg, true));
break;
default:
error (_("gdb internal error: bad var_type in do_setshow_command"));
diff --git a/gdb/command.h b/gdb/command.h
index 7b8fe5ea9f0..b30a6917211 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -420,19 +420,23 @@ struct base_setting
The var_type of the setting must match T. */
template<typename T>
- void set (T v)
+ bool set (T v)
{
/* Check that the current instance is of one of the supported types for
this instantiation. */
gdb_assert (var_type_uses<T> (this->m_var_type));
gdb_assert (!this->empty ());
+ const T old_value = this->get<T> ();
+
auto setter = get_setting_setter<T> (this->m_setter);
if (setter != nullptr)
(*setter) (v);
else
*static_cast<T *> (this->m_var) = v;
+
+ return old_value != this->get<T> ();
}
/* Set the user provided setter and getter functions. */
--
2.31.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] gdb: Add typesafe getter/setter for cmd_list_element.var
2021-08-08 19:22 ` [PATCH v2 1/4] gdb: Add typesafe getter/setter for cmd_list_element.var Lancelot SIX
@ 2021-08-10 1:29 ` Simon Marchi
2021-08-10 12:22 ` Simon Marchi
1 sibling, 0 replies; 15+ messages in thread
From: Simon Marchi @ 2021-08-10 1:29 UTC (permalink / raw)
To: Lancelot SIX, gdb-patches
On 2021-08-08 3:22 p.m., Lancelot SIX via Gdb-patches wrote:
> 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.
Thanks, this is OK, with the following nits fixed.
> diff --git a/gdb/command.h b/gdb/command.h
> index baf34401a07..644812c4d46 100644
> --- a/gdb/command.h
> +++ b/gdb/command.h
> @@ -125,6 +125,164 @@ 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
Double space.
> + 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 t);
> +
> +/* 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->empty ());
> +
> + 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->empty ());
> +
> + *static_cast<T *> (this->m_var) = v;
> + }
> +
> + /* A setting is valid (can be evaluated to true) if it contains a valid
> + reference to a memory buffer. */
> + explicit operator bool () const
> + {
> + return !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 };
> +
> + /* Indicates if the current instance has a underlying buffer. */
> + bool empty () const
> + {
> + return m_var == nullptr;
> + }
> +
> +};
> +
> +
> +/* A base_setting with additional methods to set the underlying buffer and
Remove extra empty line above.
> @@ -566,14 +577,13 @@ pascm_param_type_name (enum var_types param_type)
> If TYPE 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)
The comment "If TYPE is not supported..." should be updated.
Simon
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/4] gdb: Have setter and getter callbacks for settings
2021-08-08 19:23 ` [PATCH v2 3/4] gdb: Have setter and getter callbacks for settings Lancelot SIX
@ 2021-08-10 3:01 ` Simon Marchi
2021-08-10 22:18 ` Lancelot SIX
0 siblings, 1 reply; 15+ messages in thread
From: Simon Marchi @ 2021-08-10 3:01 UTC (permalink / raw)
To: Lancelot SIX, gdb-patches
On 2021-08-08 3:23 p.m., Lancelot SIX via Gdb-patches wrote:
> The main motivation behind this improvement is to help the
> implementation of a patch Simon Marchi is preparing to fix a bug when
> MI or Python try to access parameters that are inferior dependent (see
> PR/28085).
>
> This commit extends the previous one, which introduces the setting
> object to represent a static variable that can be set or shown with the
> appropriate commands. This patch proposes that a setting can either
> contain a pointer to a static variable holding a setting, or pointers to
> a pair of setter and getter callback functions.
>
> The callbacks functions can be used to generate values on the fly when
> the setting is accessed (or set the value in a context dependent way).
> This is useful when the source of truth is not contained in the
> variable pointed to by the setting instance.
>
> Given that the callback function call is hidden within the setting
> abstraction introduced earlier, none of the sites accessing the setting
> needs to be updated. The registered getter or setter is used whatever
> the way to access it is (through MI, Python, Guild, the "with" command
> and the $_gdb_setting / $_gdb_setting_str convenience functions).
>
> All the add_setshow_*_cmd are given a new overload that will accept the
> pair of function pointers (set / get functions) instead of the pointer
> to a global variable.
LGTM, except a small issue I found while rebasing my patch on top of
your series:
> @@ -231,25 +427,50 @@ struct base_setting
> gdb_assert (var_type_uses<T> (this->m_var_type));
> gdb_assert (!this->empty ());
I think this needs to call operator bool, like so:
gdb_assert (*this);
Otherwise, the assert fails when using a getter/setter (empty only
checks for m_var to be non-NULL, which is false when using
getter/setter).
I don't find this notation super clear, it might be clearer if we could
call a named method instead of operator bool. Maybe "empty" could mean
"does not have a buffer nor getter/setter"?
Simon
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] gdb: Add typesafe getter/setter for cmd_list_element.var
2021-08-08 19:22 ` [PATCH v2 1/4] gdb: Add typesafe getter/setter for cmd_list_element.var Lancelot SIX
2021-08-10 1:29 ` Simon Marchi
@ 2021-08-10 12:22 ` Simon Marchi
2021-08-10 21:58 ` Lancelot SIX
1 sibling, 1 reply; 15+ messages in thread
From: Simon Marchi @ 2021-08-10 12:22 UTC (permalink / raw)
To: Lancelot SIX, gdb-patches
I'm forwarding some comments from Tony Tye, from AMD, who looked at the
patch, and had some minor comments.
> @@ -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;
The comment should probably be updated, this is no longer a pointer.
Well, for now it contains a pointer, but later in the series it can
become a getter/setter too. So maybe just say "Setting affected by
...".
> diff --git a/gdb/command.h b/gdb/command.h
> index baf34401a07..644812c4d46 100644
> --- a/gdb/command.h
> +++ b/gdb/command.h
> @@ -125,6 +125,164 @@ 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 t);
So what will happen if you try to use var_type_uses with an invalid
template type, link error? Tony suggested using a static_assert to have
a compile-time error instead, with an explanation. I suppose you would
define a body for this function and put a static_assert (false) in
there?
Simon
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] gdb: Add typesafe getter/setter for cmd_list_element.var
2021-08-10 12:22 ` Simon Marchi
@ 2021-08-10 21:58 ` Lancelot SIX
2021-08-11 0:56 ` Simon Marchi
0 siblings, 1 reply; 15+ messages in thread
From: Lancelot SIX @ 2021-08-10 21:58 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
On Tue, Aug 10, 2021 at 08:22:31AM -0400, Simon Marchi wrote:
> I'm forwarding some comments from Tony Tye, from AMD, who looked at the
> patch, and had some minor comments.
>
> > @@ -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;
>
> The comment should probably be updated, this is no longer a pointer.
> Well, for now it contains a pointer, but later in the series it can
> become a getter/setter too. So maybe just say "Setting affected by
> ...".
Indeed. I’ll update this and incorporate the other remarks you sent in
your previous email.
>
> > diff --git a/gdb/command.h b/gdb/command.h
> > index baf34401a07..644812c4d46 100644
> > --- a/gdb/command.h
> > +++ b/gdb/command.h
> > @@ -125,6 +125,164 @@ 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 t);
>
> So what will happen if you try to use var_type_uses with an invalid
> template type, link error? Tony suggested using a static_assert to have
> a compile-time error instead, with an explanation. I suppose you would
> define a body for this function and put a static_assert (false) in
> there?
With a 'standard' (development) build, this translate to compile-time
error. I did not pay enough attention, but I implicitly relied on the
fact that the configure script adds '-Werror' by default. Something
should be in place to fail as early as possible (i.e. compile-time)
whatever compiler options are in use.
We cannot use a plain 'static_assert (false)' here, it will fail even if
this template is never instantiated. All versions of g++/clang++ I just
tried do trigger consistently:
$CXX -c -x c++ - <<EOT
template<typename T>
void foo ()
{ static_assert (false, "Invalid instantiation."); }
EOT
<stdin>:3:4: error: static_assert failed "Invalid instantiation."
{ static_assert (false, "Invalid instantiation."); }
^ ~~~~~
1 error generated.
I could use something similar to this:
template<typename T>
inline bool var_type_uses (var_types var_type ATTRIBUTE_UNUSED)
{
static_assert (sizeof(T) == 0,
"Invalid type used to instantiate 'var_type_uses'. "
"Use one of the types associated with enum var_types.");
return false;
}
It is not the prettiest way of writing an assertion, but it is the only
one I am aware of that can work in this situation.
Would this be OK?
Lancelot.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/4] gdb: Have setter and getter callbacks for settings
2021-08-10 3:01 ` Simon Marchi
@ 2021-08-10 22:18 ` Lancelot SIX
2021-08-11 1:01 ` Simon Marchi
0 siblings, 1 reply; 15+ messages in thread
From: Lancelot SIX @ 2021-08-10 22:18 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
> > @@ -231,25 +427,50 @@ struct base_setting
> > gdb_assert (var_type_uses<T> (this->m_var_type));
> > gdb_assert (!this->empty ());
>
> I think this needs to call operator bool, like so:
>
> gdb_assert (*this);
>
> Otherwise, the assert fails when using a getter/setter (empty only
> checks for m_var to be non-NULL, which is false when using
> getter/setter).
>
Thanks for spotting this. The second assertion should only be on the
branch that does not use the setter callback.
I also realized that the 'set' method should use 'get_p<T> () = v', which
includes this assertion making the one you ran into redundant.
> I don't find this notation super clear, it might be clearer if we could
> call a named method instead of operator bool. Maybe "empty" could mean
> "does not have a buffer nor getter/setter"?
I think I got confused about what empty means when I rebased the patch
and handled the conflicts from the previous iteration. This is a good
indication that the naming is not as good as it could be.
'empty' refers to the fact that there is an underlying buffer register.
I could just check 'm_var != nullptr' in 'get_p' and 'operator bool()'.
This is a protected method not meant to be used outside of the class
anyway.
The 'bool()' operator is intended to check if a setting is
valid, i.e. 'get' and 'set' can be called without a guarantied error.
We could use something like 'valid ()' instead (or 'good ()' if we want
to mimic iostream). Not that 'empty' would not work, I think I would
prefer to read
if (foo.valid ())
use (foo);
over
if (!foo.empty ())
use (foo);
I tend to prefer the affirmative version over the double negation.
I’ll change that for the next iteration.
Lancelot.
>
> Simon
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] gdb: Add typesafe getter/setter for cmd_list_element.var
2021-08-10 21:58 ` Lancelot SIX
@ 2021-08-11 0:56 ` Simon Marchi
0 siblings, 0 replies; 15+ messages in thread
From: Simon Marchi @ 2021-08-11 0:56 UTC (permalink / raw)
To: Lancelot SIX; +Cc: gdb-patches
>>> diff --git a/gdb/command.h b/gdb/command.h
>>> index baf34401a07..644812c4d46 100644
>>> --- a/gdb/command.h
>>> +++ b/gdb/command.h
>>> @@ -125,6 +125,164 @@ 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 t);
>>
>> So what will happen if you try to use var_type_uses with an invalid
>> template type, link error? Tony suggested using a static_assert to have
>> a compile-time error instead, with an explanation. I suppose you would
>> define a body for this function and put a static_assert (false) in
>> there?
>
> With a 'standard' (development) build, this translate to compile-time
> error. I did not pay enough attention, but I implicitly relied on the
> fact that the configure script adds '-Werror' by default. Something
> should be in place to fail as early as possible (i.e. compile-time)
> whatever compiler options are in use.
>
> We cannot use a plain 'static_assert (false)' here, it will fail even if
> this template is never instantiated. All versions of g++/clang++ I just
> tried do trigger consistently:
>
> $CXX -c -x c++ - <<EOT
> template<typename T>
> void foo ()
> { static_assert (false, "Invalid instantiation."); }
> EOT
>
> <stdin>:3:4: error: static_assert failed "Invalid instantiation."
> { static_assert (false, "Invalid instantiation."); }
> ^ ~~~~~
> 1 error generated.
>
> I could use something similar to this:
>
> template<typename T>
> inline bool var_type_uses (var_types var_type ATTRIBUTE_UNUSED)
> {
> static_assert (sizeof(T) == 0,
> "Invalid type used to instantiate 'var_type_uses'. "
> "Use one of the types associated with enum var_types.");
> return false;
> }
>
>
> It is not the prettiest way of writing an assertion, but it is the only
> one I am aware of that can work in this situation.
>
> Would this be OK?
I think what you have already is fine. It produces a diagnostic, which
is typically error in development builds. And it's not like
var_type_uses is meant to be used everywhere, it's used in the internal
command code and that's it. I think it's sufficiently clear like that.
Simon
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/4] gdb: Have setter and getter callbacks for settings
2021-08-10 22:18 ` Lancelot SIX
@ 2021-08-11 1:01 ` Simon Marchi
2021-08-11 20:00 ` Lancelot SIX
0 siblings, 1 reply; 15+ messages in thread
From: Simon Marchi @ 2021-08-11 1:01 UTC (permalink / raw)
To: Lancelot SIX; +Cc: gdb-patches
On 2021-08-10 6:18 p.m., Lancelot SIX wrote:
>>> @@ -231,25 +427,50 @@ struct base_setting
>>> gdb_assert (var_type_uses<T> (this->m_var_type));
>>> gdb_assert (!this->empty ());
>>
>> I think this needs to call operator bool, like so:
>>
>> gdb_assert (*this);
>>
>> Otherwise, the assert fails when using a getter/setter (empty only
>> checks for m_var to be non-NULL, which is false when using
>> getter/setter).
>>
>
> Thanks for spotting this. The second assertion should only be on the
> branch that does not use the setter callback.
>
> I also realized that the 'set' method should use 'get_p<T> () = v', which
> includes this assertion making the one you ran into redundant.
>
>
>> I don't find this notation super clear, it might be clearer if we could
>> call a named method instead of operator bool. Maybe "empty" could mean
>> "does not have a buffer nor getter/setter"?
>
> I think I got confused about what empty means when I rebased the patch
> and handled the conflicts from the previous iteration. This is a good
> indication that the naming is not as good as it could be.
>
> 'empty' refers to the fact that there is an underlying buffer register.
> I could just check 'm_var != nullptr' in 'get_p' and 'operator bool()'.
> This is a protected method not meant to be used outside of the class
> anyway.
>
> The 'bool()' operator is intended to check if a setting is
> valid, i.e. 'get' and 'set' can be called without a guarantied error.
> We could use something like 'valid ()' instead (or 'good ()' if we want
> to mimic iostream). Not that 'empty' would not work, I think I would
> prefer to read
>
> if (foo.valid ())
> use (foo);
>
> over
>
> if (!foo.empty ())
> use (foo);
>
> I tend to prefer the affirmative version over the double negation.
>
> I’ll change that for the next iteration.
Another option would be to use
gdb::optional<setting> var;
in cmd_list_element, and make it such that a setting can never exist in
an invalid state. Two constructor would exist, one to construct a
setting with a pointer to a buffer and another with the setter/getter.
There would be no need (I think) for set_p and set_accessors.
But otherwise, `valid` sounds good to me as well.
Simon
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/4] gdb: Have setter and getter callbacks for settings
2021-08-11 1:01 ` Simon Marchi
@ 2021-08-11 20:00 ` Lancelot SIX
2021-08-19 21:03 ` Simon Marchi
0 siblings, 1 reply; 15+ messages in thread
From: Lancelot SIX @ 2021-08-11 20:00 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
> Another option would be to use
>
> gdb::optional<setting> var;
>
> in cmd_list_element, and make it such that a setting can never exist in
> an invalid state. Two constructor would exist, one to construct a
> setting with a pointer to a buffer and another with the setter/getter.
> There would be no need (I think) for set_p and set_accessors.
This approach would be a problem within param_smob
(gdb/guile/scm-param.c). This struct is memset initialized, so we cannot
have one of its member which is not a PODType (see the redefinition of
memset in gdbsupport/poison.h). We could have a pointer to a properly
constructed setting within a param_smob plus some manual memory
management, but I am not sure it is worth it.
>
> But otherwise, `valid` sounds good to me as well.
I’ll go for that.
Lancelot.
>
> Simon
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/4] gdb: Have setter and getter callbacks for settings
2021-08-11 20:00 ` Lancelot SIX
@ 2021-08-19 21:03 ` Simon Marchi
2021-08-20 7:04 ` Lancelot SIX
0 siblings, 1 reply; 15+ messages in thread
From: Simon Marchi @ 2021-08-19 21:03 UTC (permalink / raw)
To: Lancelot SIX; +Cc: gdb-patches
On 2021-08-11 4:00 p.m., Lancelot SIX wrote:
>> Another option would be to use
>>
>> gdb::optional<setting> var;
>>
>> in cmd_list_element, and make it such that a setting can never exist in
>> an invalid state. Two constructor would exist, one to construct a
>> setting with a pointer to a buffer and another with the setter/getter.
>> There would be no need (I think) for set_p and set_accessors.
>
> This approach would be a problem within param_smob
> (gdb/guile/scm-param.c). This struct is memset initialized, so we cannot
> have one of its member which is not a PODType (see the redefinition of
> memset in gdbsupport/poison.h). We could have a pointer to a properly
> constructed setting within a param_smob plus some manual memory
> management, but I am not sure it is worth it.
Tony Tye sent me some comments about this patch, which made me
reconsider the approach a bit. I don't see the problem with
Guile / param_smob. In your patch, setting_wrapper is just a transient
object, it exists just for a moment to make the glue between a setting
(which can be handled by the common command code) and a param_smob. But
it never needs to be stored anywhere (in a POD type or otherwise).
I implemented this approach of using gdb::optional and constructors, and
it seems to work pretty well. I prefer it, because, as mentioned
before, the setting object can't exist in an invalid state. And that
is one less thing to worry about.
The other comment was that the unions setting_getter and setting_setter
plus all the get_setting_setter / get_setting_getter implementations
were not really needed but add a lot of code. Tony suggested to save
the getter and setter function pointers to type-erased variables, just
like we do for m_var. And I don't think we lose any safety, as
var_type_uses already validates that we don't pass some invalid type T,
and that T matches var_type. That removes a lot of boilerplate. I
implemented this, and stored the getters / setters as `void (*) ()`.
Storing them as `void *` wouldn't work, because you can't freely convert
function pointers to data pointers and vice versa.
I started from your users/lsix/refactor-typesafe-var, made changes to
the various patches, and uploaded it to
users/simark/refactor-typesafe-var.
Let me know what you think.
Simon
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/4] gdb: Have setter and getter callbacks for settings
2021-08-19 21:03 ` Simon Marchi
@ 2021-08-20 7:04 ` Lancelot SIX
0 siblings, 0 replies; 15+ messages in thread
From: Lancelot SIX @ 2021-08-20 7:04 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
On Thu, Aug 19, 2021 at 05:03:32PM -0400, Simon Marchi wrote:
> On 2021-08-11 4:00 p.m., Lancelot SIX wrote:
> >> Another option would be to use
> >>
> >> gdb::optional<setting> var;
> >>
> >> in cmd_list_element, and make it such that a setting can never exist in
> >> an invalid state. Two constructor would exist, one to construct a
> >> setting with a pointer to a buffer and another with the setter/getter.
> >> There would be no need (I think) for set_p and set_accessors.
> >
> > This approach would be a problem within param_smob
> > (gdb/guile/scm-param.c). This struct is memset initialized, so we cannot
> > have one of its member which is not a PODType (see the redefinition of
> > memset in gdbsupport/poison.h). We could have a pointer to a properly
> > constructed setting within a param_smob plus some manual memory
> > management, but I am not sure it is worth it.
>
> Tony Tye sent me some comments about this patch, which made me
> reconsider the approach a bit. I don't see the problem with
> Guile / param_smob. In your patch, setting_wrapper is just a transient
> object, it exists just for a moment to make the glue between a setting
> (which can be handled by the common command code) and a param_smob. But
> it never needs to be stored anywhere (in a POD type or otherwise).
>
> I implemented this approach of using gdb::optional and constructors, and
> it seems to work pretty well. I prefer it, because, as mentioned
> before, the setting object can't exist in an invalid state. And that
> is one less thing to worry about.
Hi,
I think that in my first iteration on this series I had a non trivial
ctor and it triggered a problem around the guile code. I did not dig
too much in this direction and did not reconsider later. If
gdb::optional can be used, I also prefer the option where we only use
non trivial constructors and ensure that the object can only exist in a
valid state.
>
> The other comment was that the unions setting_getter and setting_setter
> plus all the get_setting_setter / get_setting_getter implementations
> were not really needed but add a lot of code. Tony suggested to save
> the getter and setter function pointers to type-erased variables, just
> like we do for m_var. And I don't think we lose any safety, as
> var_type_uses already validates that we don't pass some invalid type T,
> and that T matches var_type. That removes a lot of boilerplate. I
> implemented this, and stored the getters / setters as `void (*) ()`.
> Storing them as `void *` wouldn't work, because you can't freely convert
> function pointers to data pointers and vice versa.
>
I did use a union there because I could not use 'void *'. I did not
consider to cast to 'void (*) ()' but indeed, it will be a lot less
code, more consistent with the type erasure done with m_var and as safe
as the union option. Only good reasons to go for this approach.
> I started from your users/lsix/refactor-typesafe-var, made changes to
> the various patches, and uploaded it to
> users/simark/refactor-typesafe-var.
>
I’ll have a look as soon as I can. Thanks for doing this!
> Let me know what you think.
>
I like both of the changes you have suggested. They are improvements
over my patch set. Thanks a lot to you and Tony for the reviews,
suggestions and improvements.
Lancelot.
> Simon
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-08-20 7:04 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-08 19:22 [PATCH v2 0/4] gdb: Refactor cmd_list_element.var Lancelot SIX
2021-08-08 19:22 ` [PATCH v2 1/4] gdb: Add typesafe getter/setter for cmd_list_element.var Lancelot SIX
2021-08-10 1:29 ` Simon Marchi
2021-08-10 12:22 ` Simon Marchi
2021-08-10 21:58 ` Lancelot SIX
2021-08-11 0:56 ` Simon Marchi
2021-08-08 19:23 ` [PATCH v2 2/4] gdb: make string-like set show commands use std::string variable Lancelot SIX
2021-08-08 19:23 ` [PATCH v2 3/4] gdb: Have setter and getter callbacks for settings Lancelot SIX
2021-08-10 3:01 ` Simon Marchi
2021-08-10 22:18 ` Lancelot SIX
2021-08-11 1:01 ` Simon Marchi
2021-08-11 20:00 ` Lancelot SIX
2021-08-19 21:03 ` Simon Marchi
2021-08-20 7:04 ` Lancelot SIX
2021-08-08 19:23 ` [PATCH v2 4/4] gdb: Setting setter return a bool to tell if the value changed Lancelot SIX
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).