* [PATCH 0/4] Small cmd_list_element-related cleanups
@ 2021-06-25 20:03 Simon Marchi
2021-06-25 20:03 ` [PATCH 1/4] gdb: add context getter/setter to cmd_list_element Simon Marchi
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Simon Marchi @ 2021-06-25 20:03 UTC (permalink / raw)
To: gdb-patches
While fixing a bug related to cmd_list_element, I made these patches
that I thought would be good on their own.
Simon Marchi (4):
gdb: add context getter/setter to cmd_list_element
gdb: add assert in cmd_list_element::set_context
gdb: remove context parameter from add_setshow_enum_cmd
gdb/guile: use return values of add_setshow functions in
add_setshow_generic
gdb/ada-lang.c | 7 +-
gdb/break-catch-sig.c | 3 +-
gdb/break-catch-syscall.c | 3 +-
gdb/break-catch-throw.c | 7 +-
gdb/breakpoint.c | 11 +--
gdb/cli/cli-decode.c | 22 +-----
gdb/cli/cli-decode.h | 16 +++-
gdb/cli/cli-dump.c | 6 +-
gdb/cli/cli-style.c | 67 +++++++++-------
gdb/command.h | 8 +-
gdb/guile/scm-cmd.c | 6 +-
gdb/guile/scm-param.c | 158 ++++++++++++++++----------------------
gdb/python/py-cmd.c | 6 +-
gdb/python/py-param.c | 8 +-
gdb/target.c | 7 +-
gdb/tui/tui-layout.c | 5 +-
16 files changed, 163 insertions(+), 177 deletions(-)
--
2.32.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/4] gdb: add context getter/setter to cmd_list_element
2021-06-25 20:03 [PATCH 0/4] Small cmd_list_element-related cleanups Simon Marchi
@ 2021-06-25 20:03 ` Simon Marchi
2021-06-25 21:03 ` Andrew Burgess
2021-06-25 20:03 ` [PATCH 2/4] gdb: add assert in cmd_list_element::set_context Simon Marchi
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Simon Marchi @ 2021-06-25 20:03 UTC (permalink / raw)
To: gdb-patches
Straightforward replacement of get_cmd_context / set_cmd_context with
cmd_list_element methods.
gdb/ChangeLog:
* cli/cli-decode.h (struct cmd_list_element) <set_context,
context>: New.
<context>: Rename to...
<m_context>: ... this.
* cli/cli-decode.c (set_cmd_context, get_cmd_context): Remove.
* command.h (set_cmd_context, get_cmd_context): Remove, use
cmd_list_element::set_context and cmd_list_element::context
everywhere instead.
Change-Id: I5016b0079014e3f17d1aa449ada7954473bf2b5d
---
gdb/ada-lang.c | 7 ++++---
gdb/break-catch-sig.c | 3 ++-
gdb/break-catch-syscall.c | 3 ++-
gdb/break-catch-throw.c | 7 ++++---
gdb/breakpoint.c | 11 ++++++-----
gdb/cli/cli-decode.c | 19 ++++---------------
gdb/cli/cli-decode.h | 13 ++++++++++---
gdb/cli/cli-dump.c | 6 +++---
gdb/cli/cli-style.c | 5 +++--
gdb/command.h | 6 ------
gdb/guile/scm-cmd.c | 6 +++---
gdb/guile/scm-param.c | 8 ++++----
gdb/python/py-cmd.c | 6 +++---
gdb/python/py-param.c | 8 ++++----
gdb/target.c | 7 ++++---
gdb/tui/tui-layout.c | 5 ++---
16 files changed, 58 insertions(+), 62 deletions(-)
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 49a7d5b36b6c..9a9bad3b4d8a 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -49,6 +49,7 @@
#include "typeprint.h"
#include "namespace.h"
#include "cli/cli-style.h"
+#include "cli/cli-decode.h"
#include "value.h"
#include "mi/mi-common.h"
@@ -12229,7 +12230,7 @@ catch_ada_exception_command (const char *arg_entry, int from_tty,
std::string excep_string;
std::string cond_string;
- tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
+ tempflag = command->context () == CATCH_TEMPORARY;
if (!arg)
arg = "";
@@ -12254,7 +12255,7 @@ catch_ada_handlers_command (const char *arg_entry, int from_tty,
std::string excep_string;
std::string cond_string;
- tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
+ tempflag = command->context () == CATCH_TEMPORARY;
if (!arg)
arg = "";
@@ -12322,7 +12323,7 @@ catch_assert_command (const char *arg_entry, int from_tty,
int tempflag;
std::string cond_string;
- tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
+ tempflag = command->context () == CATCH_TEMPORARY;
if (!arg)
arg = "";
diff --git a/gdb/break-catch-sig.c b/gdb/break-catch-sig.c
index 9530dea86ba4..5c7d62fe1ecd 100644
--- a/gdb/break-catch-sig.c
+++ b/gdb/break-catch-sig.c
@@ -29,6 +29,7 @@
#include "cli/cli-utils.h"
#include "completer.h"
#include "cli/cli-style.h"
+#include "cli/cli-decode.h"
#include <string>
@@ -389,7 +390,7 @@ catch_signal_command (const char *arg, int from_tty,
bool catch_all = false;
std::vector<gdb_signal> filter;
- tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
+ tempflag = command->context () == CATCH_TEMPORARY;
arg = skip_spaces (arg);
diff --git a/gdb/break-catch-syscall.c b/gdb/break-catch-syscall.c
index 9100ac6d5444..78e7079a831b 100644
--- a/gdb/break-catch-syscall.c
+++ b/gdb/break-catch-syscall.c
@@ -30,6 +30,7 @@
#include "observable.h"
#include "xml-syscall.h"
#include "cli/cli-style.h"
+#include "cli/cli-decode.h"
/* An instance of this type is used to represent a syscall catchpoint.
A breakpoint is really of this type iff its ops pointer points to
@@ -439,7 +440,7 @@ catch_syscall_command_1 (const char *arg, int from_tty,
error (_("The feature 'catch syscall' is not supported on \
this architecture yet."));
- tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
+ tempflag = command->context () == CATCH_TEMPORARY;
arg = skip_spaces (arg);
diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c
index 7fc6953b90cd..f05c2f8a6480 100644
--- a/gdb/break-catch-throw.c
+++ b/gdb/break-catch-throw.c
@@ -36,6 +36,7 @@
#include "gdb_regex.h"
#include "cp-support.h"
#include "location.h"
+#include "cli/cli-decode.h"
/* Each spot where we may place an exception-related catchpoint has
two names: the SDT probe point and the function name. This
@@ -456,7 +457,7 @@ static void
catch_catch_command (const char *arg, int from_tty,
struct cmd_list_element *command)
{
- bool tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
+ bool tempflag = command->context () == CATCH_TEMPORARY;
catch_exception_event (EX_EVENT_CATCH, arg, tempflag, from_tty);
}
@@ -467,7 +468,7 @@ static void
catch_throw_command (const char *arg, int from_tty,
struct cmd_list_element *command)
{
- bool tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
+ bool tempflag = command->context () == CATCH_TEMPORARY;
catch_exception_event (EX_EVENT_THROW, arg, tempflag, from_tty);
}
@@ -478,7 +479,7 @@ static void
catch_rethrow_command (const char *arg, int from_tty,
struct cmd_list_element *command)
{
- bool tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
+ bool tempflag = command->context () == CATCH_TEMPORARY;
catch_exception_event (EX_EVENT_RETHROW, arg, tempflag, from_tty);
}
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 0595c6f8cbd4..dbbea6b8bff7 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -67,6 +67,7 @@
#include "thread-fsm.h"
#include "tid-parse.h"
#include "cli/cli-style.h"
+#include "cli/cli-decode.h"
/* readline include files */
#include "readline/tilde.h"
@@ -8196,7 +8197,7 @@ catch_load_or_unload (const char *arg, int from_tty, int is_load,
struct cmd_list_element *command)
{
const int enabled = 1;
- bool temp = get_cmd_context (command) == CATCH_TEMPORARY;
+ bool temp = command->context () == CATCH_TEMPORARY;
add_solib_catchpoint (arg, is_load, temp, enabled);
}
@@ -11280,7 +11281,7 @@ catch_fork_command_1 (const char *arg, int from_tty,
const char *cond_string = NULL;
catch_fork_kind fork_kind;
- fork_kind = (catch_fork_kind) (uintptr_t) get_cmd_context (command);
+ fork_kind = (catch_fork_kind) (uintptr_t) command->context ();
bool temp = (fork_kind == catch_fork_temporary
|| fork_kind == catch_vfork_temporary);
@@ -11324,7 +11325,7 @@ catch_exec_command_1 (const char *arg, int from_tty,
{
struct gdbarch *gdbarch = get_current_arch ();
const char *cond_string = NULL;
- bool temp = get_cmd_context (command) == CATCH_TEMPORARY;
+ bool temp = command->context () == CATCH_TEMPORARY;
if (!arg)
arg = "";
@@ -15214,13 +15215,13 @@ add_catch_command (const char *name, const char *docstring,
command = add_cmd (name, class_breakpoint, docstring,
&catch_cmdlist);
set_cmd_sfunc (command, sfunc);
- set_cmd_context (command, user_data_catch);
+ command->set_context (user_data_catch);
set_cmd_completer (command, completer);
command = add_cmd (name, class_breakpoint, docstring,
&tcatch_cmdlist);
set_cmd_sfunc (command, sfunc);
- set_cmd_context (command, user_data_tcatch);
+ command->set_context (user_data_tcatch);
set_cmd_completer (command, completer);
}
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 009986c9cb10..f2fdeb03a285 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -134,18 +134,6 @@ cmd_cfunc_eq (struct cmd_list_element *cmd, cmd_const_cfunc_ftype *cfunc)
return cmd->func == do_const_cfunc && cmd->function.const_cfunc == cfunc;
}
-void
-set_cmd_context (struct cmd_list_element *cmd, void *context)
-{
- cmd->context = context;
-}
-
-void *
-get_cmd_context (struct cmd_list_element *cmd)
-{
- return cmd->context;
-}
-
void
set_cmd_completer (struct cmd_list_element *cmd, completer_ftype *completer)
{
@@ -593,8 +581,8 @@ add_setshow_enum_cmd (const char *name,
set_list, show_list);
commands.set->enums = enumlist;
- set_cmd_context (commands.set, context);
- set_cmd_context (commands.show, context);
+ commands.set->set_context (context);
+ commands.show->set_context (context);
return commands;
}
@@ -920,7 +908,8 @@ delete_cmd (const char *name, struct cmd_list_element **list,
if (strcmp (iter->name, name) == 0)
{
if (iter->destroyer)
- iter->destroyer (iter, iter->context);
+ iter->destroyer (iter, iter->context ());
+
if (iter->hookee_pre)
iter->hookee_pre->hook_pre = 0;
*prehook = iter->hook_pre;
diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index 9328659775ca..1692a6e28352 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -93,6 +93,12 @@ struct cmd_list_element
bool is_command_class_help () const
{ return this->func == nullptr; }
+ void set_context (void *context)
+ { m_context = context; }
+
+ void *context () const
+ { return m_context; }
+
/* Points to next command in this list. */
struct cmd_list_element *next = nullptr;
@@ -173,9 +179,6 @@ struct cmd_list_element
}
function;
- /* Local state (context) for this command. This can be anything. */
- void *context = nullptr;
-
/* Documentation of this command (or help topic).
First line is brief documentation; remaining lines form, with it,
the full documentation. First line should end with a period.
@@ -256,6 +259,10 @@ struct cmd_list_element
when this command is being executed. It will be set back to false
when the command has been executed. */
int *suppress_notification = nullptr;
+
+private:
+ /* Local state (context) for this command. This can be anything. */
+ void *m_context = nullptr;
};
/* Functions that implement commands about CLI commands. */
diff --git a/gdb/cli/cli-dump.c b/gdb/cli/cli-dump.c
index 95ce85e1c449..efb400418042 100644
--- a/gdb/cli/cli-dump.c
+++ b/gdb/cli/cli-dump.c
@@ -333,7 +333,7 @@ struct dump_context
static void
call_dump_func (struct cmd_list_element *c, const char *args, int from_tty)
{
- struct dump_context *d = (struct dump_context *) get_cmd_context (c);
+ struct dump_context *d = (struct dump_context *) c->context ();
d->func (args, d->mode);
}
@@ -352,7 +352,7 @@ add_dump_command (const char *name,
d = XNEW (struct dump_context);
d->func = func;
d->mode = FOPEN_WB;
- set_cmd_context (c, d);
+ c->set_context (d);
c->func = call_dump_func;
c = add_cmd (name, all_commands, descr, &append_cmdlist);
@@ -360,7 +360,7 @@ add_dump_command (const char *name,
d = XNEW (struct dump_context);
d->func = func;
d->mode = FOPEN_AB;
- set_cmd_context (c, d);
+ c->set_context (d);
c->func = call_dump_func;
/* Replace "Dump " at start of docstring with "Append " (borrowed
diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
index 25a1d888f860..0b88dba56227 100644
--- a/gdb/cli/cli-style.c
+++ b/gdb/cli/cli-style.c
@@ -19,6 +19,7 @@
#include "defs.h"
#include "cli/cli-cmds.h"
+#include "cli/cli-decode.h"
#include "cli/cli-setshow.h"
#include "cli/cli-style.h"
#include "source-cache.h"
@@ -167,7 +168,7 @@ void
cli_style_option::do_set_value (const char *ignore, int from_tty,
struct cmd_list_element *cmd)
{
- cli_style_option *cso = (cli_style_option *) get_cmd_context (cmd);
+ cli_style_option *cso = (cli_style_option *) cmd->context ();
cso->changed.notify ();
}
@@ -180,7 +181,7 @@ do_show (const char *what, struct ui_file *file,
struct cmd_list_element *cmd,
const char *value)
{
- cli_style_option *cso = (cli_style_option *) get_cmd_context (cmd);
+ cli_style_option *cso = (cli_style_option *) cmd->context ();
fputs_filtered (_("The "), file);
fprintf_styled (file, cso->style (), _("\"%s\" style"), cso->name ());
fprintf_filtered (file, _(" %s is: %s\n"), what, value);
diff --git a/gdb/command.h b/gdb/command.h
index f8988e414943..9413a50c2971 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -253,12 +253,6 @@ extern void set_cmd_completer_handle_brkchars (struct cmd_list_element *,
extern int cmd_cfunc_eq (struct cmd_list_element *cmd,
cmd_const_cfunc_ftype *cfun);
-/* Each command object has a local context attached to it. */
-extern void set_cmd_context (struct cmd_list_element *cmd,
- void *context);
-extern void *get_cmd_context (struct cmd_list_element *cmd);
-
-
/* Execute CMD's pre/post hook. Throw an error if the command fails.
If already executing this pre/post hook, or there is no pre/post
hook, the call is silently ignored. */
diff --git a/gdb/guile/scm-cmd.c b/gdb/guile/scm-cmd.c
index 39c915e0ab22..ab3dad7483cc 100644
--- a/gdb/guile/scm-cmd.c
+++ b/gdb/guile/scm-cmd.c
@@ -294,7 +294,7 @@ static void
cmdscm_function (struct cmd_list_element *command,
const char *args, int from_tty)
{
- command_smob *c_smob/*obj*/ = (command_smob *) get_cmd_context (command);
+ command_smob *c_smob/*obj*/ = (command_smob *) command->context ();
SCM arg_scm, tty_scm, result;
gdb_assert (c_smob != NULL);
@@ -383,7 +383,7 @@ cmdscm_completer (struct cmd_list_element *command,
completion_tracker &tracker,
const char *text, const char *word)
{
- command_smob *c_smob/*obj*/ = (command_smob *) get_cmd_context (command);
+ command_smob *c_smob/*obj*/ = (command_smob *) command->context ();
SCM completer_result_scm;
SCM text_scm, word_scm;
@@ -788,7 +788,7 @@ gdbscm_register_command_x (SCM self)
cmd->destroyer = cmdscm_destroyer;
c_smob->command = cmd;
- set_cmd_context (cmd, c_smob);
+ cmd->set_context (c_smob);
if (gdbscm_is_true (c_smob->complete))
{
diff --git a/gdb/guile/scm-param.c b/gdb/guile/scm-param.c
index 5f02e13ae49a..86f61057a32f 100644
--- a/gdb/guile/scm-param.c
+++ b/gdb/guile/scm-param.c
@@ -274,7 +274,7 @@ pascm_signal_setshow_error (SCM exception, const char *msg)
static void
pascm_set_func (const char *args, int from_tty, struct cmd_list_element *c)
{
- param_smob *p_smob = (param_smob *) get_cmd_context (c);
+ param_smob *p_smob = (param_smob *) c->context ();
SCM self, result, exception;
gdb_assert (gdbscm_is_procedure (p_smob->set_func));
@@ -314,7 +314,7 @@ static void
pascm_show_func (struct ui_file *file, int from_tty,
struct cmd_list_element *c, const char *value)
{
- param_smob *p_smob = (param_smob *) get_cmd_context (c);
+ param_smob *p_smob = (param_smob *) c->context ();
SCM value_scm, self, result, exception;
gdb_assert (gdbscm_is_procedure (p_smob->show_func));
@@ -468,13 +468,13 @@ add_setshow_generic (enum var_types param_type, enum command_class cmd_class,
tmp_name = cmd_name;
param = lookup_cmd (&tmp_name, *show_list, "", NULL, 0, 1);
gdb_assert (param != NULL);
- set_cmd_context (param, self);
+ param->set_context (self);
*set_cmd = param;
tmp_name = cmd_name;
param = lookup_cmd (&tmp_name, *set_list, "", NULL, 0, 1);
gdb_assert (param != NULL);
- set_cmd_context (param, self);
+ param->set_context (self);
*show_cmd = param;
}
diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
index 4f01fc0b5f35..0467ebd67349 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -103,7 +103,7 @@ static void
cmdpy_function (struct cmd_list_element *command,
const char *args, int from_tty)
{
- cmdpy_object *obj = (cmdpy_object *) get_cmd_context (command);
+ cmdpy_object *obj = (cmdpy_object *) command->context ();
gdbpy_enter enter_py (get_current_arch (), current_language);
@@ -172,7 +172,7 @@ static gdbpy_ref<>
cmdpy_completer_helper (struct cmd_list_element *command,
const char *text, const char *word)
{
- cmdpy_object *obj = (cmdpy_object *) get_cmd_context (command);
+ cmdpy_object *obj = (cmdpy_object *) command->context ();
if (obj == NULL)
error (_("Invalid invocation of Python command object."));
@@ -532,7 +532,7 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
cmd->name_allocated = 1;
obj->command = cmd;
- set_cmd_context (cmd, self_ref.release ());
+ cmd->set_context (self_ref.release ());
set_cmd_completer (cmd, ((completetype == -1) ? cmdpy_completer
: completers[completetype].completer));
if (completetype == -1)
diff --git a/gdb/python/py-param.c b/gdb/python/py-param.c
index d0a4850bdc04..f9dcb076c603 100644
--- a/gdb/python/py-param.c
+++ b/gdb/python/py-param.c
@@ -376,7 +376,7 @@ static void
get_set_value (const char *args, int from_tty,
struct cmd_list_element *c)
{
- PyObject *obj = (PyObject *) get_cmd_context (c);
+ PyObject *obj = (PyObject *) c->context ();
gdb::unique_xmalloc_ptr<char> set_doc_string;
gdbpy_enter enter_py (get_current_arch (), current_language);
@@ -411,7 +411,7 @@ get_show_value (struct ui_file *file, int from_tty,
struct cmd_list_element *c,
const char *value)
{
- PyObject *obj = (PyObject *) get_cmd_context (c);
+ PyObject *obj = (PyObject *) c->context ();
gdb::unique_xmalloc_ptr<char> show_doc_string;
gdbpy_enter enter_py (get_current_arch (), current_language);
@@ -569,8 +569,8 @@ add_setshow_generic (int parmclass, enum command_class cmdclass,
}
/* Register Python objects in both commands' context. */
- set_cmd_context (commands.set, self);
- set_cmd_context (commands.show, self);
+ commands.set->set_context (self);
+ commands.show->set_context (self);
/* We (unfortunately) currently leak the command name. */
cmd_name.release ();
diff --git a/gdb/target.c b/gdb/target.c
index 6babfc562563..767685fbca39 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -52,6 +52,7 @@
#include <unordered_map>
#include "target-connection.h"
#include "valprint.h"
+#include "cli/cli-decode.h"
static void generic_tls_error (void) ATTRIBUTE_NORETURN;
@@ -837,7 +838,7 @@ target_log_command (const char *p)
static void
open_target (const char *args, int from_tty, struct cmd_list_element *command)
{
- auto *ti = static_cast<target_info *> (get_cmd_context (command));
+ auto *ti = static_cast<target_info *> (command->context ());
target_open_ftype *func = target_factories[ti];
if (targetdebug)
@@ -874,7 +875,7 @@ information on the arguments for a particular protocol, type\n\
`help target ' followed by the protocol name."),
&targetlist, 0, &cmdlist);
c = add_cmd (t.shortname, no_class, t.doc, &targetlist);
- set_cmd_context (c, (void *) &t);
+ c->set_context ((void *) &t);
set_cmd_sfunc (c, open_target);
if (completer != NULL)
set_cmd_completer (c, completer);
@@ -892,7 +893,7 @@ add_deprecated_target_alias (const target_info &tinfo, const char *alias)
see PR cli/15104. */
c = add_cmd (alias, no_class, tinfo.doc, &targetlist);
set_cmd_sfunc (c, open_target);
- set_cmd_context (c, (void *) &tinfo);
+ c->set_context ((void *) &tinfo);
alt = xstrprintf ("target %s", tinfo.shortname);
deprecate_cmd (c, alt);
}
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index b54e74819457..f9c94dea96cd 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -172,8 +172,7 @@ static void
tui_apply_layout (struct cmd_list_element *command,
const char *args, int from_tty)
{
- tui_layout_split *layout
- = (tui_layout_split *) get_cmd_context (command);
+ tui_layout_split *layout = (tui_layout_split *) command->context ();
/* Make sure the curses mode is enabled. */
tui_enable ();
@@ -858,7 +857,7 @@ This layout was created using:\n\
name, name, spec.c_str ()));
cmd = add_cmd (name, class_tui, nullptr, doc.get (), &layout_list);
- set_cmd_context (cmd, layout);
+ cmd->set_context (layout);
/* There is no API to set this. */
cmd->func = tui_apply_layout;
cmd->destroyer = destroy_layout;
--
2.32.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/4] gdb: add assert in cmd_list_element::set_context
2021-06-25 20:03 [PATCH 0/4] Small cmd_list_element-related cleanups Simon Marchi
2021-06-25 20:03 ` [PATCH 1/4] gdb: add context getter/setter to cmd_list_element Simon Marchi
@ 2021-06-25 20:03 ` Simon Marchi
2021-06-25 21:05 ` Andrew Burgess
2021-06-25 20:03 ` [PATCH 3/4] gdb: remove context parameter from add_setshow_enum_cmd Simon Marchi
2021-06-25 20:03 ` [PATCH 4/4] gdb/guile: use return values of add_setshow functions in add_setshow_generic Simon Marchi
3 siblings, 1 reply; 10+ messages in thread
From: Simon Marchi @ 2021-06-25 20:03 UTC (permalink / raw)
To: gdb-patches
If something tries to set a context pointer on a cmd_list_element and
m_context is no nullptr, it's likely that two parts of the code are
trying to set different contexts, and one will overwrite the other.
This is almost guaranteed to lead to bad behavior or a crash, as one of
the spots will not be using the data it expects. This happened to me
during development, so I think having this assert would be useful to
catch this problem earlier.
gdb/ChangeLog:
* cli/cli-decode.h (struct cmd_list_element) <set_context>: Add
assert.
Change-Id: I1f2e9fda1bf2bec1b732c9b90e7d7910a97f2ac6
---
gdb/cli/cli-decode.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index 1692a6e28352..241535ae5b50 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -94,7 +94,10 @@ struct cmd_list_element
{ return this->func == nullptr; }
void set_context (void *context)
- { m_context = context; }
+ {
+ gdb_assert (m_context == nullptr);
+ m_context = context;
+ }
void *context () const
{ return m_context; }
--
2.32.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/4] gdb: remove context parameter from add_setshow_enum_cmd
2021-06-25 20:03 [PATCH 0/4] Small cmd_list_element-related cleanups Simon Marchi
2021-06-25 20:03 ` [PATCH 1/4] gdb: add context getter/setter to cmd_list_element Simon Marchi
2021-06-25 20:03 ` [PATCH 2/4] gdb: add assert in cmd_list_element::set_context Simon Marchi
@ 2021-06-25 20:03 ` Simon Marchi
2021-06-25 21:09 ` Andrew Burgess
2021-06-25 20:03 ` [PATCH 4/4] gdb/guile: use return values of add_setshow functions in add_setshow_generic Simon Marchi
3 siblings, 1 reply; 10+ messages in thread
From: Simon Marchi @ 2021-06-25 20:03 UTC (permalink / raw)
To: gdb-patches
I propose removing the context parameter from add_setshow_enum_cmd. It
was useful before add_setshow_enum_cmd returned both created commands,
as the caller couldn't easily set the context itself. But now, I think
it's fine to just let the caller do it.
Change-Id: I377c4e6820ec9d5069492ed28f4cba342ce1336e
---
gdb/cli/cli-decode.c | 7 +----
gdb/cli/cli-style.c | 62 +++++++++++++++++++++++++++-----------------
gdb/command.h | 2 +-
3 files changed, 40 insertions(+), 31 deletions(-)
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index f2fdeb03a285..633a3ad93095 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -571,8 +571,7 @@ add_setshow_enum_cmd (const char *name,
cmd_const_sfunc_ftype *set_func,
show_value_ftype *show_func,
struct cmd_list_element **set_list,
- struct cmd_list_element **show_list,
- void *context)
+ struct cmd_list_element **show_list)
{
set_show_commands commands
= add_setshow_cmd_full (name, theclass, var_enum, var,
@@ -580,10 +579,6 @@ add_setshow_enum_cmd (const char *name,
set_func, show_func,
set_list, show_list);
commands.set->enums = enumlist;
-
- commands.set->set_context (context);
- commands.show->set_context (context);
-
return commands;
}
diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
index 0b88dba56227..aca19c51b848 100644
--- a/gdb/cli/cli-style.c
+++ b/gdb/cli/cli-style.c
@@ -230,32 +230,46 @@ cli_style_option::add_setshow_commands (enum command_class theclass,
0, set_list);
add_show_prefix_cmd (m_name, no_class, prefix_doc, &m_show_list,
0, show_list);
+ set_show_commands commands;
+
+ commands = add_setshow_enum_cmd
+ ("foreground", theclass, cli_colors,
+ &m_foreground,
+ _("Set the foreground color for this property."),
+ _("Show the foreground color for this property."),
+ nullptr,
+ do_set_value,
+ do_show_foreground,
+ &m_set_list, &m_show_list);
+ commands.set->set_context (this);
+ commands.show->set_context (this);
+
+ commands = add_setshow_enum_cmd
+ ("background", theclass, cli_colors,
+ &m_background,
+ _("Set the background color for this property."),
+ _("Show the background color for this property."),
+ nullptr,
+ do_set_value,
+ do_show_background,
+ &m_set_list, &m_show_list);
+ commands.set->set_context (this);
+ commands.show->set_context (this);
- add_setshow_enum_cmd ("foreground", theclass, cli_colors,
- &m_foreground,
- _("Set the foreground color for this property."),
- _("Show the foreground color for this property."),
- nullptr,
- do_set_value,
- do_show_foreground,
- &m_set_list, &m_show_list, (void *) this);
- add_setshow_enum_cmd ("background", theclass, cli_colors,
- &m_background,
- _("Set the background color for this property."),
- _("Show the background color for this property."),
- nullptr,
- do_set_value,
- do_show_background,
- &m_set_list, &m_show_list, (void *) this);
if (!skip_intensity)
- add_setshow_enum_cmd ("intensity", theclass, cli_intensities,
- &m_intensity,
- _("Set the display intensity for this property."),
- _("Show the display intensity for this property."),
- nullptr,
- do_set_value,
- do_show_intensity,
- &m_set_list, &m_show_list, (void *) this);
+ {
+ commands = add_setshow_enum_cmd
+ ("intensity", theclass, cli_intensities,
+ &m_intensity,
+ _("Set the display intensity for this property."),
+ _("Show the display intensity for this property."),
+ nullptr,
+ do_set_value,
+ do_show_intensity,
+ &m_set_list, &m_show_list);
+ commands.set->set_context (this);
+ commands.show->set_context (this);
+ }
}
static cmd_list_element *style_set_list;
diff --git a/gdb/command.h b/gdb/command.h
index 9413a50c2971..711cbdcf43e1 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -403,7 +403,7 @@ extern set_show_commands add_setshow_enum_cmd
const char **var, const char *set_doc, const char *show_doc,
const char *help_doc, cmd_const_sfunc_ftype *set_func,
show_value_ftype *show_func, cmd_list_element **set_list,
- cmd_list_element **show_list, void *context = nullptr);
+ cmd_list_element **show_list);
extern set_show_commands add_setshow_auto_boolean_cmd
(const char *name, command_class theclass, auto_boolean *var,
--
2.32.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/4] gdb/guile: use return values of add_setshow functions in add_setshow_generic
2021-06-25 20:03 [PATCH 0/4] Small cmd_list_element-related cleanups Simon Marchi
` (2 preceding siblings ...)
2021-06-25 20:03 ` [PATCH 3/4] gdb: remove context parameter from add_setshow_enum_cmd Simon Marchi
@ 2021-06-25 20:03 ` Simon Marchi
2021-06-25 21:17 ` Andrew Burgess
3 siblings, 1 reply; 10+ messages in thread
From: Simon Marchi @ 2021-06-25 20:03 UTC (permalink / raw)
To: gdb-patches
Use the set_show_commands objects returned by the add_setshow functions
in add_setshow_generic. This lets us avoid looking up the commands
after creating them, instead using the return objects directly.
Make add_setshow_generic return a set_show_commands object, which is a
bit nicer than returning both commands by parameter.
Finally, store using that object in param_smob.
Equivalent of 7bd22f56a3cf ("gdb/python: use return values of
add_setshow functions in add_setshow_generic"), but for guile.
gdb/ChangeLog:
* guile/scm-param.c (struct param_smob) <set_command,
show_command>: Remove.
<commands>: New.
(pascm_is_valid): Adjust.
(add_setshow_generic): Use return values of add_setshow
functions, return a set_show_commands.
(gdbscm_register_parameter_x): Adjust.
Change-Id: I18ed9e7dd5646529491c86749a5cb20763acd1f0
---
gdb/guile/scm-param.c | 154 ++++++++++++++++++------------------------
1 file changed, 66 insertions(+), 88 deletions(-)
diff --git a/gdb/guile/scm-param.c b/gdb/guile/scm-param.c
index 86f61057a32f..c052d04974ae 100644
--- a/gdb/guile/scm-param.c
+++ b/gdb/guile/scm-param.c
@@ -89,8 +89,7 @@ struct param_smob
/* The corresponding gdb command objects.
These are NULL if the parameter has not been registered yet, or
is no longer registered. */
- struct cmd_list_element *set_command;
- struct cmd_list_element *show_command;
+ set_show_commands commands;
/* The value of the parameter. */
union pascm_variable value;
@@ -232,7 +231,7 @@ pascm_get_param_smob_arg_unsafe (SCM self, int arg_pos, const char *func_name)
static int
pascm_is_valid (param_smob *p_smob)
{
- return p_smob->set_command != NULL;
+ return p_smob->commands.set != nullptr;
}
\f
/* A helper function which return the default documentation string for
@@ -350,111 +349,100 @@ pascm_show_func (struct ui_file *file, int from_tty,
/* A helper function that dispatches to the appropriate add_setshow
function. */
-static void
+static set_show_commands
add_setshow_generic (enum var_types param_type, enum command_class cmd_class,
char *cmd_name, param_smob *self,
char *set_doc, char *show_doc, char *help_doc,
cmd_const_sfunc_ftype *set_func,
show_value_ftype *show_func,
struct cmd_list_element **set_list,
- struct cmd_list_element **show_list,
- struct cmd_list_element **set_cmd,
- struct cmd_list_element **show_cmd)
+ struct cmd_list_element **show_list)
{
- struct cmd_list_element *param = NULL;
- const char *tmp_name = NULL;
+ set_show_commands commands;
switch (param_type)
{
case var_boolean:
- add_setshow_boolean_cmd (cmd_name, cmd_class,
- &self->value.boolval,
- set_doc, show_doc, help_doc,
- set_func, show_func,
- set_list, show_list);
-
+ commands = add_setshow_boolean_cmd (cmd_name, cmd_class,
+ &self->value.boolval, set_doc,
+ show_doc, help_doc, set_func,
+ show_func, set_list, show_list);
break;
case var_auto_boolean:
- add_setshow_auto_boolean_cmd (cmd_name, cmd_class,
- &self->value.autoboolval,
- set_doc, show_doc, help_doc,
- set_func, show_func,
- set_list, show_list);
+ commands = add_setshow_auto_boolean_cmd (cmd_name, cmd_class,
+ &self->value.autoboolval,
+ set_doc, show_doc, help_doc,
+ set_func, show_func, set_list,
+ show_list);
break;
case var_uinteger:
- add_setshow_uinteger_cmd (cmd_name, cmd_class,
- &self->value.uintval,
- set_doc, show_doc, help_doc,
- set_func, show_func,
- set_list, show_list);
+ commands = add_setshow_uinteger_cmd (cmd_name, cmd_class,
+ &self->value.uintval, set_doc,
+ show_doc, help_doc, set_func,
+ show_func, set_list, show_list);
break;
case var_zinteger:
- add_setshow_zinteger_cmd (cmd_name, cmd_class,
- &self->value.intval,
- set_doc, show_doc, help_doc,
- set_func, show_func,
- set_list, show_list);
+ commands = add_setshow_zinteger_cmd (cmd_name, cmd_class,
+ &self->value.intval, set_doc,
+ show_doc, help_doc, set_func,
+ show_func, set_list, show_list);
break;
case var_zuinteger:
- add_setshow_zuinteger_cmd (cmd_name, cmd_class,
- &self->value.uintval,
- set_doc, show_doc, help_doc,
- set_func, show_func,
- set_list, show_list);
+ commands = add_setshow_zuinteger_cmd (cmd_name, cmd_class,
+ &self->value.uintval, set_doc,
+ show_doc, help_doc, set_func,
+ show_func, set_list, show_list);
break;
case var_zuinteger_unlimited:
- add_setshow_zuinteger_unlimited_cmd (cmd_name, cmd_class,
- &self->value.intval,
- set_doc, show_doc, help_doc,
- set_func, show_func,
- set_list, show_list);
+ commands = add_setshow_zuinteger_unlimited_cmd (cmd_name, cmd_class,
+ &self->value.intval,
+ set_doc, show_doc,
+ help_doc, set_func,
+ show_func, set_list,
+ show_list);
break;
case var_string:
- add_setshow_string_cmd (cmd_name, cmd_class,
- &self->value.stringval,
- set_doc, show_doc, help_doc,
- set_func, show_func,
- set_list, show_list);
+ commands = add_setshow_string_cmd (cmd_name, cmd_class,
+ &self->value.stringval, set_doc,
+ show_doc, help_doc, set_func,
+ show_func, set_list, show_list);
break;
case var_string_noescape:
- add_setshow_string_noescape_cmd (cmd_name, cmd_class,
- &self->value.stringval,
- set_doc, show_doc, help_doc,
- set_func, show_func,
- set_list, show_list);
+ commands = add_setshow_string_noescape_cmd (cmd_name, cmd_class,
+ &self->value.stringval,
+ set_doc, show_doc, help_doc,
+ set_func, show_func, set_list,
+ show_list);
break;
case var_optional_filename:
- add_setshow_optional_filename_cmd (cmd_name, cmd_class,
- &self->value.stringval,
- set_doc, show_doc, help_doc,
- set_func, show_func,
- set_list, show_list);
+ commands = add_setshow_optional_filename_cmd (cmd_name, cmd_class,
+ &self->value.stringval,
+ set_doc, show_doc, help_doc,
+ set_func, show_func,
+ set_list, show_list);
break;
case var_filename:
- add_setshow_filename_cmd (cmd_name, cmd_class,
- &self->value.stringval,
- set_doc, show_doc, help_doc,
- set_func, show_func,
- set_list, show_list);
+ commands = add_setshow_filename_cmd (cmd_name, cmd_class,
+ &self->value.stringval, set_doc,
+ show_doc, help_doc, set_func,
+ show_func, set_list, show_list);
break;
case var_enum:
- add_setshow_enum_cmd (cmd_name, cmd_class,
- self->enumeration,
- &self->value.cstringval,
- set_doc, show_doc, help_doc,
- set_func, show_func,
- set_list, show_list);
+ commands = add_setshow_enum_cmd (cmd_name, cmd_class, self->enumeration,
+ &self->value.cstringval, set_doc,
+ show_doc, help_doc, set_func, show_func,
+ set_list, show_list);
/* Initialize the value, just in case. */
self->value.cstringval = self->enumeration[0];
break;
@@ -463,19 +451,12 @@ add_setshow_generic (enum var_types param_type, enum command_class cmd_class,
gdb_assert_not_reached ("bad param_type value");
}
- /* Lookup created parameter, and register Scheme object against the
- parameter context. Perform this task against both lists. */
- tmp_name = cmd_name;
- param = lookup_cmd (&tmp_name, *show_list, "", NULL, 0, 1);
- gdb_assert (param != NULL);
- param->set_context (self);
- *set_cmd = param;
-
- tmp_name = cmd_name;
- param = lookup_cmd (&tmp_name, *set_list, "", NULL, 0, 1);
- gdb_assert (param != NULL);
- param->set_context (self);
- *show_cmd = param;
+ /* Register Scheme object against the commandsparameter context. Perform this
+ task against both lists. */
+ commands.set->set_context (self);
+ commands.show->set_context (self);
+
+ return commands;
}
/* Return an array of strings corresponding to the enum values for
@@ -1012,15 +993,12 @@ gdbscm_register_parameter_x (SCM self)
gdbscm_gdb_exception exc {};
try
{
- add_setshow_generic (p_smob->type, p_smob->cmd_class,
- p_smob->cmd_name, p_smob,
- p_smob->set_doc, p_smob->show_doc, p_smob->doc,
- (gdbscm_is_procedure (p_smob->set_func)
- ? pascm_set_func : NULL),
- (gdbscm_is_procedure (p_smob->show_func)
- ? pascm_show_func : NULL),
- set_list, show_list,
- &p_smob->set_command, &p_smob->show_command);
+ p_smob->commands = add_setshow_generic
+ (p_smob->type, p_smob->cmd_class, p_smob->cmd_name, p_smob,
+ p_smob->set_doc, p_smob->show_doc, p_smob->doc,
+ (gdbscm_is_procedure (p_smob->set_func) ? pascm_set_func : NULL),
+ (gdbscm_is_procedure (p_smob->show_func) ? pascm_show_func : NULL),
+ set_list, show_list);
}
catch (const gdb_exception &except)
{
--
2.32.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] gdb: add context getter/setter to cmd_list_element
2021-06-25 20:03 ` [PATCH 1/4] gdb: add context getter/setter to cmd_list_element Simon Marchi
@ 2021-06-25 21:03 ` Andrew Burgess
0 siblings, 0 replies; 10+ messages in thread
From: Andrew Burgess @ 2021-06-25 21:03 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
* Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-06-25 16:03:19 -0400]:
> Straightforward replacement of get_cmd_context / set_cmd_context with
> cmd_list_element methods.
>
> gdb/ChangeLog:
>
> * cli/cli-decode.h (struct cmd_list_element) <set_context,
> context>: New.
> <context>: Rename to...
> <m_context>: ... this.
> * cli/cli-decode.c (set_cmd_context, get_cmd_context): Remove.
> * command.h (set_cmd_context, get_cmd_context): Remove, use
> cmd_list_element::set_context and cmd_list_element::context
> everywhere instead.
LGTM.
Thanks,
Andrew
>
> Change-Id: I5016b0079014e3f17d1aa449ada7954473bf2b5d
> ---
> gdb/ada-lang.c | 7 ++++---
> gdb/break-catch-sig.c | 3 ++-
> gdb/break-catch-syscall.c | 3 ++-
> gdb/break-catch-throw.c | 7 ++++---
> gdb/breakpoint.c | 11 ++++++-----
> gdb/cli/cli-decode.c | 19 ++++---------------
> gdb/cli/cli-decode.h | 13 ++++++++++---
> gdb/cli/cli-dump.c | 6 +++---
> gdb/cli/cli-style.c | 5 +++--
> gdb/command.h | 6 ------
> gdb/guile/scm-cmd.c | 6 +++---
> gdb/guile/scm-param.c | 8 ++++----
> gdb/python/py-cmd.c | 6 +++---
> gdb/python/py-param.c | 8 ++++----
> gdb/target.c | 7 ++++---
> gdb/tui/tui-layout.c | 5 ++---
> 16 files changed, 58 insertions(+), 62 deletions(-)
>
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index 49a7d5b36b6c..9a9bad3b4d8a 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -49,6 +49,7 @@
> #include "typeprint.h"
> #include "namespace.h"
> #include "cli/cli-style.h"
> +#include "cli/cli-decode.h"
>
> #include "value.h"
> #include "mi/mi-common.h"
> @@ -12229,7 +12230,7 @@ catch_ada_exception_command (const char *arg_entry, int from_tty,
> std::string excep_string;
> std::string cond_string;
>
> - tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
> + tempflag = command->context () == CATCH_TEMPORARY;
>
> if (!arg)
> arg = "";
> @@ -12254,7 +12255,7 @@ catch_ada_handlers_command (const char *arg_entry, int from_tty,
> std::string excep_string;
> std::string cond_string;
>
> - tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
> + tempflag = command->context () == CATCH_TEMPORARY;
>
> if (!arg)
> arg = "";
> @@ -12322,7 +12323,7 @@ catch_assert_command (const char *arg_entry, int from_tty,
> int tempflag;
> std::string cond_string;
>
> - tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
> + tempflag = command->context () == CATCH_TEMPORARY;
>
> if (!arg)
> arg = "";
> diff --git a/gdb/break-catch-sig.c b/gdb/break-catch-sig.c
> index 9530dea86ba4..5c7d62fe1ecd 100644
> --- a/gdb/break-catch-sig.c
> +++ b/gdb/break-catch-sig.c
> @@ -29,6 +29,7 @@
> #include "cli/cli-utils.h"
> #include "completer.h"
> #include "cli/cli-style.h"
> +#include "cli/cli-decode.h"
>
> #include <string>
>
> @@ -389,7 +390,7 @@ catch_signal_command (const char *arg, int from_tty,
> bool catch_all = false;
> std::vector<gdb_signal> filter;
>
> - tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
> + tempflag = command->context () == CATCH_TEMPORARY;
>
> arg = skip_spaces (arg);
>
> diff --git a/gdb/break-catch-syscall.c b/gdb/break-catch-syscall.c
> index 9100ac6d5444..78e7079a831b 100644
> --- a/gdb/break-catch-syscall.c
> +++ b/gdb/break-catch-syscall.c
> @@ -30,6 +30,7 @@
> #include "observable.h"
> #include "xml-syscall.h"
> #include "cli/cli-style.h"
> +#include "cli/cli-decode.h"
>
> /* An instance of this type is used to represent a syscall catchpoint.
> A breakpoint is really of this type iff its ops pointer points to
> @@ -439,7 +440,7 @@ catch_syscall_command_1 (const char *arg, int from_tty,
> error (_("The feature 'catch syscall' is not supported on \
> this architecture yet."));
>
> - tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
> + tempflag = command->context () == CATCH_TEMPORARY;
>
> arg = skip_spaces (arg);
>
> diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c
> index 7fc6953b90cd..f05c2f8a6480 100644
> --- a/gdb/break-catch-throw.c
> +++ b/gdb/break-catch-throw.c
> @@ -36,6 +36,7 @@
> #include "gdb_regex.h"
> #include "cp-support.h"
> #include "location.h"
> +#include "cli/cli-decode.h"
>
> /* Each spot where we may place an exception-related catchpoint has
> two names: the SDT probe point and the function name. This
> @@ -456,7 +457,7 @@ static void
> catch_catch_command (const char *arg, int from_tty,
> struct cmd_list_element *command)
> {
> - bool tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
> + bool tempflag = command->context () == CATCH_TEMPORARY;
>
> catch_exception_event (EX_EVENT_CATCH, arg, tempflag, from_tty);
> }
> @@ -467,7 +468,7 @@ static void
> catch_throw_command (const char *arg, int from_tty,
> struct cmd_list_element *command)
> {
> - bool tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
> + bool tempflag = command->context () == CATCH_TEMPORARY;
>
> catch_exception_event (EX_EVENT_THROW, arg, tempflag, from_tty);
> }
> @@ -478,7 +479,7 @@ static void
> catch_rethrow_command (const char *arg, int from_tty,
> struct cmd_list_element *command)
> {
> - bool tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
> + bool tempflag = command->context () == CATCH_TEMPORARY;
>
> catch_exception_event (EX_EVENT_RETHROW, arg, tempflag, from_tty);
> }
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 0595c6f8cbd4..dbbea6b8bff7 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -67,6 +67,7 @@
> #include "thread-fsm.h"
> #include "tid-parse.h"
> #include "cli/cli-style.h"
> +#include "cli/cli-decode.h"
>
> /* readline include files */
> #include "readline/tilde.h"
> @@ -8196,7 +8197,7 @@ catch_load_or_unload (const char *arg, int from_tty, int is_load,
> struct cmd_list_element *command)
> {
> const int enabled = 1;
> - bool temp = get_cmd_context (command) == CATCH_TEMPORARY;
> + bool temp = command->context () == CATCH_TEMPORARY;
>
> add_solib_catchpoint (arg, is_load, temp, enabled);
> }
> @@ -11280,7 +11281,7 @@ catch_fork_command_1 (const char *arg, int from_tty,
> const char *cond_string = NULL;
> catch_fork_kind fork_kind;
>
> - fork_kind = (catch_fork_kind) (uintptr_t) get_cmd_context (command);
> + fork_kind = (catch_fork_kind) (uintptr_t) command->context ();
> bool temp = (fork_kind == catch_fork_temporary
> || fork_kind == catch_vfork_temporary);
>
> @@ -11324,7 +11325,7 @@ catch_exec_command_1 (const char *arg, int from_tty,
> {
> struct gdbarch *gdbarch = get_current_arch ();
> const char *cond_string = NULL;
> - bool temp = get_cmd_context (command) == CATCH_TEMPORARY;
> + bool temp = command->context () == CATCH_TEMPORARY;
>
> if (!arg)
> arg = "";
> @@ -15214,13 +15215,13 @@ add_catch_command (const char *name, const char *docstring,
> command = add_cmd (name, class_breakpoint, docstring,
> &catch_cmdlist);
> set_cmd_sfunc (command, sfunc);
> - set_cmd_context (command, user_data_catch);
> + command->set_context (user_data_catch);
> set_cmd_completer (command, completer);
>
> command = add_cmd (name, class_breakpoint, docstring,
> &tcatch_cmdlist);
> set_cmd_sfunc (command, sfunc);
> - set_cmd_context (command, user_data_tcatch);
> + command->set_context (user_data_tcatch);
> set_cmd_completer (command, completer);
> }
>
> diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
> index 009986c9cb10..f2fdeb03a285 100644
> --- a/gdb/cli/cli-decode.c
> +++ b/gdb/cli/cli-decode.c
> @@ -134,18 +134,6 @@ cmd_cfunc_eq (struct cmd_list_element *cmd, cmd_const_cfunc_ftype *cfunc)
> return cmd->func == do_const_cfunc && cmd->function.const_cfunc == cfunc;
> }
>
> -void
> -set_cmd_context (struct cmd_list_element *cmd, void *context)
> -{
> - cmd->context = context;
> -}
> -
> -void *
> -get_cmd_context (struct cmd_list_element *cmd)
> -{
> - return cmd->context;
> -}
> -
> void
> set_cmd_completer (struct cmd_list_element *cmd, completer_ftype *completer)
> {
> @@ -593,8 +581,8 @@ add_setshow_enum_cmd (const char *name,
> set_list, show_list);
> commands.set->enums = enumlist;
>
> - set_cmd_context (commands.set, context);
> - set_cmd_context (commands.show, context);
> + commands.set->set_context (context);
> + commands.show->set_context (context);
>
> return commands;
> }
> @@ -920,7 +908,8 @@ delete_cmd (const char *name, struct cmd_list_element **list,
> if (strcmp (iter->name, name) == 0)
> {
> if (iter->destroyer)
> - iter->destroyer (iter, iter->context);
> + iter->destroyer (iter, iter->context ());
> +
> if (iter->hookee_pre)
> iter->hookee_pre->hook_pre = 0;
> *prehook = iter->hook_pre;
> diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
> index 9328659775ca..1692a6e28352 100644
> --- a/gdb/cli/cli-decode.h
> +++ b/gdb/cli/cli-decode.h
> @@ -93,6 +93,12 @@ struct cmd_list_element
> bool is_command_class_help () const
> { return this->func == nullptr; }
>
> + void set_context (void *context)
> + { m_context = context; }
> +
> + void *context () const
> + { return m_context; }
> +
> /* Points to next command in this list. */
> struct cmd_list_element *next = nullptr;
>
> @@ -173,9 +179,6 @@ struct cmd_list_element
> }
> function;
>
> - /* Local state (context) for this command. This can be anything. */
> - void *context = nullptr;
> -
> /* Documentation of this command (or help topic).
> First line is brief documentation; remaining lines form, with it,
> the full documentation. First line should end with a period.
> @@ -256,6 +259,10 @@ struct cmd_list_element
> when this command is being executed. It will be set back to false
> when the command has been executed. */
> int *suppress_notification = nullptr;
> +
> +private:
> + /* Local state (context) for this command. This can be anything. */
> + void *m_context = nullptr;
> };
>
> /* Functions that implement commands about CLI commands. */
> diff --git a/gdb/cli/cli-dump.c b/gdb/cli/cli-dump.c
> index 95ce85e1c449..efb400418042 100644
> --- a/gdb/cli/cli-dump.c
> +++ b/gdb/cli/cli-dump.c
> @@ -333,7 +333,7 @@ struct dump_context
> static void
> call_dump_func (struct cmd_list_element *c, const char *args, int from_tty)
> {
> - struct dump_context *d = (struct dump_context *) get_cmd_context (c);
> + struct dump_context *d = (struct dump_context *) c->context ();
>
> d->func (args, d->mode);
> }
> @@ -352,7 +352,7 @@ add_dump_command (const char *name,
> d = XNEW (struct dump_context);
> d->func = func;
> d->mode = FOPEN_WB;
> - set_cmd_context (c, d);
> + c->set_context (d);
> c->func = call_dump_func;
>
> c = add_cmd (name, all_commands, descr, &append_cmdlist);
> @@ -360,7 +360,7 @@ add_dump_command (const char *name,
> d = XNEW (struct dump_context);
> d->func = func;
> d->mode = FOPEN_AB;
> - set_cmd_context (c, d);
> + c->set_context (d);
> c->func = call_dump_func;
>
> /* Replace "Dump " at start of docstring with "Append " (borrowed
> diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
> index 25a1d888f860..0b88dba56227 100644
> --- a/gdb/cli/cli-style.c
> +++ b/gdb/cli/cli-style.c
> @@ -19,6 +19,7 @@
>
> #include "defs.h"
> #include "cli/cli-cmds.h"
> +#include "cli/cli-decode.h"
> #include "cli/cli-setshow.h"
> #include "cli/cli-style.h"
> #include "source-cache.h"
> @@ -167,7 +168,7 @@ void
> cli_style_option::do_set_value (const char *ignore, int from_tty,
> struct cmd_list_element *cmd)
> {
> - cli_style_option *cso = (cli_style_option *) get_cmd_context (cmd);
> + cli_style_option *cso = (cli_style_option *) cmd->context ();
> cso->changed.notify ();
> }
>
> @@ -180,7 +181,7 @@ do_show (const char *what, struct ui_file *file,
> struct cmd_list_element *cmd,
> const char *value)
> {
> - cli_style_option *cso = (cli_style_option *) get_cmd_context (cmd);
> + cli_style_option *cso = (cli_style_option *) cmd->context ();
> fputs_filtered (_("The "), file);
> fprintf_styled (file, cso->style (), _("\"%s\" style"), cso->name ());
> fprintf_filtered (file, _(" %s is: %s\n"), what, value);
> diff --git a/gdb/command.h b/gdb/command.h
> index f8988e414943..9413a50c2971 100644
> --- a/gdb/command.h
> +++ b/gdb/command.h
> @@ -253,12 +253,6 @@ extern void set_cmd_completer_handle_brkchars (struct cmd_list_element *,
> extern int cmd_cfunc_eq (struct cmd_list_element *cmd,
> cmd_const_cfunc_ftype *cfun);
>
> -/* Each command object has a local context attached to it. */
> -extern void set_cmd_context (struct cmd_list_element *cmd,
> - void *context);
> -extern void *get_cmd_context (struct cmd_list_element *cmd);
> -
> -
> /* Execute CMD's pre/post hook. Throw an error if the command fails.
> If already executing this pre/post hook, or there is no pre/post
> hook, the call is silently ignored. */
> diff --git a/gdb/guile/scm-cmd.c b/gdb/guile/scm-cmd.c
> index 39c915e0ab22..ab3dad7483cc 100644
> --- a/gdb/guile/scm-cmd.c
> +++ b/gdb/guile/scm-cmd.c
> @@ -294,7 +294,7 @@ static void
> cmdscm_function (struct cmd_list_element *command,
> const char *args, int from_tty)
> {
> - command_smob *c_smob/*obj*/ = (command_smob *) get_cmd_context (command);
> + command_smob *c_smob/*obj*/ = (command_smob *) command->context ();
> SCM arg_scm, tty_scm, result;
>
> gdb_assert (c_smob != NULL);
> @@ -383,7 +383,7 @@ cmdscm_completer (struct cmd_list_element *command,
> completion_tracker &tracker,
> const char *text, const char *word)
> {
> - command_smob *c_smob/*obj*/ = (command_smob *) get_cmd_context (command);
> + command_smob *c_smob/*obj*/ = (command_smob *) command->context ();
> SCM completer_result_scm;
> SCM text_scm, word_scm;
>
> @@ -788,7 +788,7 @@ gdbscm_register_command_x (SCM self)
> cmd->destroyer = cmdscm_destroyer;
>
> c_smob->command = cmd;
> - set_cmd_context (cmd, c_smob);
> + cmd->set_context (c_smob);
>
> if (gdbscm_is_true (c_smob->complete))
> {
> diff --git a/gdb/guile/scm-param.c b/gdb/guile/scm-param.c
> index 5f02e13ae49a..86f61057a32f 100644
> --- a/gdb/guile/scm-param.c
> +++ b/gdb/guile/scm-param.c
> @@ -274,7 +274,7 @@ pascm_signal_setshow_error (SCM exception, const char *msg)
> static void
> pascm_set_func (const char *args, int from_tty, struct cmd_list_element *c)
> {
> - param_smob *p_smob = (param_smob *) get_cmd_context (c);
> + param_smob *p_smob = (param_smob *) c->context ();
> SCM self, result, exception;
>
> gdb_assert (gdbscm_is_procedure (p_smob->set_func));
> @@ -314,7 +314,7 @@ static void
> pascm_show_func (struct ui_file *file, int from_tty,
> struct cmd_list_element *c, const char *value)
> {
> - param_smob *p_smob = (param_smob *) get_cmd_context (c);
> + param_smob *p_smob = (param_smob *) c->context ();
> SCM value_scm, self, result, exception;
>
> gdb_assert (gdbscm_is_procedure (p_smob->show_func));
> @@ -468,13 +468,13 @@ add_setshow_generic (enum var_types param_type, enum command_class cmd_class,
> tmp_name = cmd_name;
> param = lookup_cmd (&tmp_name, *show_list, "", NULL, 0, 1);
> gdb_assert (param != NULL);
> - set_cmd_context (param, self);
> + param->set_context (self);
> *set_cmd = param;
>
> tmp_name = cmd_name;
> param = lookup_cmd (&tmp_name, *set_list, "", NULL, 0, 1);
> gdb_assert (param != NULL);
> - set_cmd_context (param, self);
> + param->set_context (self);
> *show_cmd = param;
> }
>
> diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
> index 4f01fc0b5f35..0467ebd67349 100644
> --- a/gdb/python/py-cmd.c
> +++ b/gdb/python/py-cmd.c
> @@ -103,7 +103,7 @@ static void
> cmdpy_function (struct cmd_list_element *command,
> const char *args, int from_tty)
> {
> - cmdpy_object *obj = (cmdpy_object *) get_cmd_context (command);
> + cmdpy_object *obj = (cmdpy_object *) command->context ();
>
> gdbpy_enter enter_py (get_current_arch (), current_language);
>
> @@ -172,7 +172,7 @@ static gdbpy_ref<>
> cmdpy_completer_helper (struct cmd_list_element *command,
> const char *text, const char *word)
> {
> - cmdpy_object *obj = (cmdpy_object *) get_cmd_context (command);
> + cmdpy_object *obj = (cmdpy_object *) command->context ();
>
> if (obj == NULL)
> error (_("Invalid invocation of Python command object."));
> @@ -532,7 +532,7 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
> cmd->name_allocated = 1;
>
> obj->command = cmd;
> - set_cmd_context (cmd, self_ref.release ());
> + cmd->set_context (self_ref.release ());
> set_cmd_completer (cmd, ((completetype == -1) ? cmdpy_completer
> : completers[completetype].completer));
> if (completetype == -1)
> diff --git a/gdb/python/py-param.c b/gdb/python/py-param.c
> index d0a4850bdc04..f9dcb076c603 100644
> --- a/gdb/python/py-param.c
> +++ b/gdb/python/py-param.c
> @@ -376,7 +376,7 @@ static void
> get_set_value (const char *args, int from_tty,
> struct cmd_list_element *c)
> {
> - PyObject *obj = (PyObject *) get_cmd_context (c);
> + PyObject *obj = (PyObject *) c->context ();
> gdb::unique_xmalloc_ptr<char> set_doc_string;
>
> gdbpy_enter enter_py (get_current_arch (), current_language);
> @@ -411,7 +411,7 @@ get_show_value (struct ui_file *file, int from_tty,
> struct cmd_list_element *c,
> const char *value)
> {
> - PyObject *obj = (PyObject *) get_cmd_context (c);
> + PyObject *obj = (PyObject *) c->context ();
> gdb::unique_xmalloc_ptr<char> show_doc_string;
>
> gdbpy_enter enter_py (get_current_arch (), current_language);
> @@ -569,8 +569,8 @@ add_setshow_generic (int parmclass, enum command_class cmdclass,
> }
>
> /* Register Python objects in both commands' context. */
> - set_cmd_context (commands.set, self);
> - set_cmd_context (commands.show, self);
> + commands.set->set_context (self);
> + commands.show->set_context (self);
>
> /* We (unfortunately) currently leak the command name. */
> cmd_name.release ();
> diff --git a/gdb/target.c b/gdb/target.c
> index 6babfc562563..767685fbca39 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -52,6 +52,7 @@
> #include <unordered_map>
> #include "target-connection.h"
> #include "valprint.h"
> +#include "cli/cli-decode.h"
>
> static void generic_tls_error (void) ATTRIBUTE_NORETURN;
>
> @@ -837,7 +838,7 @@ target_log_command (const char *p)
> static void
> open_target (const char *args, int from_tty, struct cmd_list_element *command)
> {
> - auto *ti = static_cast<target_info *> (get_cmd_context (command));
> + auto *ti = static_cast<target_info *> (command->context ());
> target_open_ftype *func = target_factories[ti];
>
> if (targetdebug)
> @@ -874,7 +875,7 @@ information on the arguments for a particular protocol, type\n\
> `help target ' followed by the protocol name."),
> &targetlist, 0, &cmdlist);
> c = add_cmd (t.shortname, no_class, t.doc, &targetlist);
> - set_cmd_context (c, (void *) &t);
> + c->set_context ((void *) &t);
> set_cmd_sfunc (c, open_target);
> if (completer != NULL)
> set_cmd_completer (c, completer);
> @@ -892,7 +893,7 @@ add_deprecated_target_alias (const target_info &tinfo, const char *alias)
> see PR cli/15104. */
> c = add_cmd (alias, no_class, tinfo.doc, &targetlist);
> set_cmd_sfunc (c, open_target);
> - set_cmd_context (c, (void *) &tinfo);
> + c->set_context ((void *) &tinfo);
> alt = xstrprintf ("target %s", tinfo.shortname);
> deprecate_cmd (c, alt);
> }
> diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
> index b54e74819457..f9c94dea96cd 100644
> --- a/gdb/tui/tui-layout.c
> +++ b/gdb/tui/tui-layout.c
> @@ -172,8 +172,7 @@ static void
> tui_apply_layout (struct cmd_list_element *command,
> const char *args, int from_tty)
> {
> - tui_layout_split *layout
> - = (tui_layout_split *) get_cmd_context (command);
> + tui_layout_split *layout = (tui_layout_split *) command->context ();
>
> /* Make sure the curses mode is enabled. */
> tui_enable ();
> @@ -858,7 +857,7 @@ This layout was created using:\n\
> name, name, spec.c_str ()));
>
> cmd = add_cmd (name, class_tui, nullptr, doc.get (), &layout_list);
> - set_cmd_context (cmd, layout);
> + cmd->set_context (layout);
> /* There is no API to set this. */
> cmd->func = tui_apply_layout;
> cmd->destroyer = destroy_layout;
> --
> 2.32.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/4] gdb: add assert in cmd_list_element::set_context
2021-06-25 20:03 ` [PATCH 2/4] gdb: add assert in cmd_list_element::set_context Simon Marchi
@ 2021-06-25 21:05 ` Andrew Burgess
0 siblings, 0 replies; 10+ messages in thread
From: Andrew Burgess @ 2021-06-25 21:05 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
* Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-06-25 16:03:20 -0400]:
> If something tries to set a context pointer on a cmd_list_element and
> m_context is no nullptr, it's likely that two parts of the code are
Missing 't' - "is not nullptr".
> trying to set different contexts, and one will overwrite the other.
> This is almost guaranteed to lead to bad behavior or a crash, as one of
> the spots will not be using the data it expects. This happened to me
> during development, so I think having this assert would be useful to
> catch this problem earlier.
LGTM.
Thanks,
Andrew
>
> gdb/ChangeLog:
>
> * cli/cli-decode.h (struct cmd_list_element) <set_context>: Add
> assert.
>
> Change-Id: I1f2e9fda1bf2bec1b732c9b90e7d7910a97f2ac6
> ---
> gdb/cli/cli-decode.h | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
> index 1692a6e28352..241535ae5b50 100644
> --- a/gdb/cli/cli-decode.h
> +++ b/gdb/cli/cli-decode.h
> @@ -94,7 +94,10 @@ struct cmd_list_element
> { return this->func == nullptr; }
>
> void set_context (void *context)
> - { m_context = context; }
> + {
> + gdb_assert (m_context == nullptr);
> + m_context = context;
> + }
>
> void *context () const
> { return m_context; }
> --
> 2.32.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] gdb: remove context parameter from add_setshow_enum_cmd
2021-06-25 20:03 ` [PATCH 3/4] gdb: remove context parameter from add_setshow_enum_cmd Simon Marchi
@ 2021-06-25 21:09 ` Andrew Burgess
2021-06-26 1:38 ` Simon Marchi
0 siblings, 1 reply; 10+ messages in thread
From: Andrew Burgess @ 2021-06-25 21:09 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
* Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-06-25 16:03:21 -0400]:
> I propose removing the context parameter from add_setshow_enum_cmd. It
> was useful before add_setshow_enum_cmd returned both created commands,
> as the caller couldn't easily set the context itself. But now, I think
> it's fine to just let the caller do it.
ChangeLog ?
As non of the other add_setshow_* commands take a context, it seems
reasonable to me that this should not either, hence,
LGTM.
Thanks,
Andrew
>
> Change-Id: I377c4e6820ec9d5069492ed28f4cba342ce1336e
> ---
> gdb/cli/cli-decode.c | 7 +----
> gdb/cli/cli-style.c | 62 +++++++++++++++++++++++++++-----------------
> gdb/command.h | 2 +-
> 3 files changed, 40 insertions(+), 31 deletions(-)
>
> diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
> index f2fdeb03a285..633a3ad93095 100644
> --- a/gdb/cli/cli-decode.c
> +++ b/gdb/cli/cli-decode.c
> @@ -571,8 +571,7 @@ add_setshow_enum_cmd (const char *name,
> cmd_const_sfunc_ftype *set_func,
> show_value_ftype *show_func,
> struct cmd_list_element **set_list,
> - struct cmd_list_element **show_list,
> - void *context)
> + struct cmd_list_element **show_list)
> {
> set_show_commands commands
> = add_setshow_cmd_full (name, theclass, var_enum, var,
> @@ -580,10 +579,6 @@ add_setshow_enum_cmd (const char *name,
> set_func, show_func,
> set_list, show_list);
> commands.set->enums = enumlist;
> -
> - commands.set->set_context (context);
> - commands.show->set_context (context);
> -
> return commands;
> }
>
> diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
> index 0b88dba56227..aca19c51b848 100644
> --- a/gdb/cli/cli-style.c
> +++ b/gdb/cli/cli-style.c
> @@ -230,32 +230,46 @@ cli_style_option::add_setshow_commands (enum command_class theclass,
> 0, set_list);
> add_show_prefix_cmd (m_name, no_class, prefix_doc, &m_show_list,
> 0, show_list);
> + set_show_commands commands;
> +
> + commands = add_setshow_enum_cmd
> + ("foreground", theclass, cli_colors,
> + &m_foreground,
> + _("Set the foreground color for this property."),
> + _("Show the foreground color for this property."),
> + nullptr,
> + do_set_value,
> + do_show_foreground,
> + &m_set_list, &m_show_list);
> + commands.set->set_context (this);
> + commands.show->set_context (this);
> +
> + commands = add_setshow_enum_cmd
> + ("background", theclass, cli_colors,
> + &m_background,
> + _("Set the background color for this property."),
> + _("Show the background color for this property."),
> + nullptr,
> + do_set_value,
> + do_show_background,
> + &m_set_list, &m_show_list);
> + commands.set->set_context (this);
> + commands.show->set_context (this);
>
> - add_setshow_enum_cmd ("foreground", theclass, cli_colors,
> - &m_foreground,
> - _("Set the foreground color for this property."),
> - _("Show the foreground color for this property."),
> - nullptr,
> - do_set_value,
> - do_show_foreground,
> - &m_set_list, &m_show_list, (void *) this);
> - add_setshow_enum_cmd ("background", theclass, cli_colors,
> - &m_background,
> - _("Set the background color for this property."),
> - _("Show the background color for this property."),
> - nullptr,
> - do_set_value,
> - do_show_background,
> - &m_set_list, &m_show_list, (void *) this);
> if (!skip_intensity)
> - add_setshow_enum_cmd ("intensity", theclass, cli_intensities,
> - &m_intensity,
> - _("Set the display intensity for this property."),
> - _("Show the display intensity for this property."),
> - nullptr,
> - do_set_value,
> - do_show_intensity,
> - &m_set_list, &m_show_list, (void *) this);
> + {
> + commands = add_setshow_enum_cmd
> + ("intensity", theclass, cli_intensities,
> + &m_intensity,
> + _("Set the display intensity for this property."),
> + _("Show the display intensity for this property."),
> + nullptr,
> + do_set_value,
> + do_show_intensity,
> + &m_set_list, &m_show_list);
> + commands.set->set_context (this);
> + commands.show->set_context (this);
> + }
> }
>
> static cmd_list_element *style_set_list;
> diff --git a/gdb/command.h b/gdb/command.h
> index 9413a50c2971..711cbdcf43e1 100644
> --- a/gdb/command.h
> +++ b/gdb/command.h
> @@ -403,7 +403,7 @@ extern set_show_commands add_setshow_enum_cmd
> const char **var, const char *set_doc, const char *show_doc,
> const char *help_doc, cmd_const_sfunc_ftype *set_func,
> show_value_ftype *show_func, cmd_list_element **set_list,
> - cmd_list_element **show_list, void *context = nullptr);
> + cmd_list_element **show_list);
>
> extern set_show_commands add_setshow_auto_boolean_cmd
> (const char *name, command_class theclass, auto_boolean *var,
> --
> 2.32.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] gdb/guile: use return values of add_setshow functions in add_setshow_generic
2021-06-25 20:03 ` [PATCH 4/4] gdb/guile: use return values of add_setshow functions in add_setshow_generic Simon Marchi
@ 2021-06-25 21:17 ` Andrew Burgess
0 siblings, 0 replies; 10+ messages in thread
From: Andrew Burgess @ 2021-06-25 21:17 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
* Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-06-25 16:03:22 -0400]:
> Use the set_show_commands objects returned by the add_setshow functions
> in add_setshow_generic. This lets us avoid looking up the commands
> after creating them, instead using the return objects directly.
>
> Make add_setshow_generic return a set_show_commands object, which is a
> bit nicer than returning both commands by parameter.
>
> Finally, store using that object in param_smob.
>
> Equivalent of 7bd22f56a3cf ("gdb/python: use return values of
> add_setshow functions in add_setshow_generic"), but for guile.
>
> gdb/ChangeLog:
>
> * guile/scm-param.c (struct param_smob) <set_command,
> show_command>: Remove.
> <commands>: New.
> (pascm_is_valid): Adjust.
> (add_setshow_generic): Use return values of add_setshow
> functions, return a set_show_commands.
> (gdbscm_register_parameter_x): Adjust.
LGTM.
Thanks,
Andrew
>
> Change-Id: I18ed9e7dd5646529491c86749a5cb20763acd1f0
> ---
> gdb/guile/scm-param.c | 154 ++++++++++++++++++------------------------
> 1 file changed, 66 insertions(+), 88 deletions(-)
>
> diff --git a/gdb/guile/scm-param.c b/gdb/guile/scm-param.c
> index 86f61057a32f..c052d04974ae 100644
> --- a/gdb/guile/scm-param.c
> +++ b/gdb/guile/scm-param.c
> @@ -89,8 +89,7 @@ struct param_smob
> /* The corresponding gdb command objects.
> These are NULL if the parameter has not been registered yet, or
> is no longer registered. */
> - struct cmd_list_element *set_command;
> - struct cmd_list_element *show_command;
> + set_show_commands commands;
>
> /* The value of the parameter. */
> union pascm_variable value;
> @@ -232,7 +231,7 @@ pascm_get_param_smob_arg_unsafe (SCM self, int arg_pos, const char *func_name)
> static int
> pascm_is_valid (param_smob *p_smob)
> {
> - return p_smob->set_command != NULL;
> + return p_smob->commands.set != nullptr;
> }
> \f
> /* A helper function which return the default documentation string for
> @@ -350,111 +349,100 @@ pascm_show_func (struct ui_file *file, int from_tty,
> /* A helper function that dispatches to the appropriate add_setshow
> function. */
>
> -static void
> +static set_show_commands
> add_setshow_generic (enum var_types param_type, enum command_class cmd_class,
> char *cmd_name, param_smob *self,
> char *set_doc, char *show_doc, char *help_doc,
> cmd_const_sfunc_ftype *set_func,
> show_value_ftype *show_func,
> struct cmd_list_element **set_list,
> - struct cmd_list_element **show_list,
> - struct cmd_list_element **set_cmd,
> - struct cmd_list_element **show_cmd)
> + struct cmd_list_element **show_list)
> {
> - struct cmd_list_element *param = NULL;
> - const char *tmp_name = NULL;
> + set_show_commands commands;
>
> switch (param_type)
> {
> case var_boolean:
> - add_setshow_boolean_cmd (cmd_name, cmd_class,
> - &self->value.boolval,
> - set_doc, show_doc, help_doc,
> - set_func, show_func,
> - set_list, show_list);
> -
> + commands = add_setshow_boolean_cmd (cmd_name, cmd_class,
> + &self->value.boolval, set_doc,
> + show_doc, help_doc, set_func,
> + show_func, set_list, show_list);
> break;
>
> case var_auto_boolean:
> - add_setshow_auto_boolean_cmd (cmd_name, cmd_class,
> - &self->value.autoboolval,
> - set_doc, show_doc, help_doc,
> - set_func, show_func,
> - set_list, show_list);
> + commands = add_setshow_auto_boolean_cmd (cmd_name, cmd_class,
> + &self->value.autoboolval,
> + set_doc, show_doc, help_doc,
> + set_func, show_func, set_list,
> + show_list);
> break;
>
> case var_uinteger:
> - add_setshow_uinteger_cmd (cmd_name, cmd_class,
> - &self->value.uintval,
> - set_doc, show_doc, help_doc,
> - set_func, show_func,
> - set_list, show_list);
> + commands = add_setshow_uinteger_cmd (cmd_name, cmd_class,
> + &self->value.uintval, set_doc,
> + show_doc, help_doc, set_func,
> + show_func, set_list, show_list);
> break;
>
> case var_zinteger:
> - add_setshow_zinteger_cmd (cmd_name, cmd_class,
> - &self->value.intval,
> - set_doc, show_doc, help_doc,
> - set_func, show_func,
> - set_list, show_list);
> + commands = add_setshow_zinteger_cmd (cmd_name, cmd_class,
> + &self->value.intval, set_doc,
> + show_doc, help_doc, set_func,
> + show_func, set_list, show_list);
> break;
>
> case var_zuinteger:
> - add_setshow_zuinteger_cmd (cmd_name, cmd_class,
> - &self->value.uintval,
> - set_doc, show_doc, help_doc,
> - set_func, show_func,
> - set_list, show_list);
> + commands = add_setshow_zuinteger_cmd (cmd_name, cmd_class,
> + &self->value.uintval, set_doc,
> + show_doc, help_doc, set_func,
> + show_func, set_list, show_list);
> break;
>
> case var_zuinteger_unlimited:
> - add_setshow_zuinteger_unlimited_cmd (cmd_name, cmd_class,
> - &self->value.intval,
> - set_doc, show_doc, help_doc,
> - set_func, show_func,
> - set_list, show_list);
> + commands = add_setshow_zuinteger_unlimited_cmd (cmd_name, cmd_class,
> + &self->value.intval,
> + set_doc, show_doc,
> + help_doc, set_func,
> + show_func, set_list,
> + show_list);
> break;
>
> case var_string:
> - add_setshow_string_cmd (cmd_name, cmd_class,
> - &self->value.stringval,
> - set_doc, show_doc, help_doc,
> - set_func, show_func,
> - set_list, show_list);
> + commands = add_setshow_string_cmd (cmd_name, cmd_class,
> + &self->value.stringval, set_doc,
> + show_doc, help_doc, set_func,
> + show_func, set_list, show_list);
> break;
>
> case var_string_noescape:
> - add_setshow_string_noescape_cmd (cmd_name, cmd_class,
> - &self->value.stringval,
> - set_doc, show_doc, help_doc,
> - set_func, show_func,
> - set_list, show_list);
> + commands = add_setshow_string_noescape_cmd (cmd_name, cmd_class,
> + &self->value.stringval,
> + set_doc, show_doc, help_doc,
> + set_func, show_func, set_list,
> + show_list);
>
> break;
>
> case var_optional_filename:
> - add_setshow_optional_filename_cmd (cmd_name, cmd_class,
> - &self->value.stringval,
> - set_doc, show_doc, help_doc,
> - set_func, show_func,
> - set_list, show_list);
> + commands = add_setshow_optional_filename_cmd (cmd_name, cmd_class,
> + &self->value.stringval,
> + set_doc, show_doc, help_doc,
> + set_func, show_func,
> + set_list, show_list);
> break;
>
> case var_filename:
> - add_setshow_filename_cmd (cmd_name, cmd_class,
> - &self->value.stringval,
> - set_doc, show_doc, help_doc,
> - set_func, show_func,
> - set_list, show_list);
> + commands = add_setshow_filename_cmd (cmd_name, cmd_class,
> + &self->value.stringval, set_doc,
> + show_doc, help_doc, set_func,
> + show_func, set_list, show_list);
> break;
>
> case var_enum:
> - add_setshow_enum_cmd (cmd_name, cmd_class,
> - self->enumeration,
> - &self->value.cstringval,
> - set_doc, show_doc, help_doc,
> - set_func, show_func,
> - set_list, show_list);
> + commands = add_setshow_enum_cmd (cmd_name, cmd_class, self->enumeration,
> + &self->value.cstringval, set_doc,
> + show_doc, help_doc, set_func, show_func,
> + set_list, show_list);
> /* Initialize the value, just in case. */
> self->value.cstringval = self->enumeration[0];
> break;
> @@ -463,19 +451,12 @@ add_setshow_generic (enum var_types param_type, enum command_class cmd_class,
> gdb_assert_not_reached ("bad param_type value");
> }
>
> - /* Lookup created parameter, and register Scheme object against the
> - parameter context. Perform this task against both lists. */
> - tmp_name = cmd_name;
> - param = lookup_cmd (&tmp_name, *show_list, "", NULL, 0, 1);
> - gdb_assert (param != NULL);
> - param->set_context (self);
> - *set_cmd = param;
> -
> - tmp_name = cmd_name;
> - param = lookup_cmd (&tmp_name, *set_list, "", NULL, 0, 1);
> - gdb_assert (param != NULL);
> - param->set_context (self);
> - *show_cmd = param;
> + /* Register Scheme object against the commandsparameter context. Perform this
> + task against both lists. */
> + commands.set->set_context (self);
> + commands.show->set_context (self);
> +
> + return commands;
> }
>
> /* Return an array of strings corresponding to the enum values for
> @@ -1012,15 +993,12 @@ gdbscm_register_parameter_x (SCM self)
> gdbscm_gdb_exception exc {};
> try
> {
> - add_setshow_generic (p_smob->type, p_smob->cmd_class,
> - p_smob->cmd_name, p_smob,
> - p_smob->set_doc, p_smob->show_doc, p_smob->doc,
> - (gdbscm_is_procedure (p_smob->set_func)
> - ? pascm_set_func : NULL),
> - (gdbscm_is_procedure (p_smob->show_func)
> - ? pascm_show_func : NULL),
> - set_list, show_list,
> - &p_smob->set_command, &p_smob->show_command);
> + p_smob->commands = add_setshow_generic
> + (p_smob->type, p_smob->cmd_class, p_smob->cmd_name, p_smob,
> + p_smob->set_doc, p_smob->show_doc, p_smob->doc,
> + (gdbscm_is_procedure (p_smob->set_func) ? pascm_set_func : NULL),
> + (gdbscm_is_procedure (p_smob->show_func) ? pascm_show_func : NULL),
> + set_list, show_list);
> }
> catch (const gdb_exception &except)
> {
> --
> 2.32.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] gdb: remove context parameter from add_setshow_enum_cmd
2021-06-25 21:09 ` Andrew Burgess
@ 2021-06-26 1:38 ` Simon Marchi
0 siblings, 0 replies; 10+ messages in thread
From: Simon Marchi @ 2021-06-26 1:38 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
On 2021-06-25 5:09 p.m., Andrew Burgess wrote:
> * Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-06-25 16:03:21 -0400]:
>
>> I propose removing the context parameter from add_setshow_enum_cmd. It
>> was useful before add_setshow_enum_cmd returned both created commands,
>> as the caller couldn't easily set the context itself. But now, I think
>> it's fine to just let the caller do it.
>
> ChangeLog ?
>
> As non of the other add_setshow_* commands take a context, it seems
> reasonable to me that this should not either, hence,
>
> LGTM.
>
> Thanks,
> Andrew
Woops, added, thanks.
Simon
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-06-26 1:38 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-25 20:03 [PATCH 0/4] Small cmd_list_element-related cleanups Simon Marchi
2021-06-25 20:03 ` [PATCH 1/4] gdb: add context getter/setter to cmd_list_element Simon Marchi
2021-06-25 21:03 ` Andrew Burgess
2021-06-25 20:03 ` [PATCH 2/4] gdb: add assert in cmd_list_element::set_context Simon Marchi
2021-06-25 21:05 ` Andrew Burgess
2021-06-25 20:03 ` [PATCH 3/4] gdb: remove context parameter from add_setshow_enum_cmd Simon Marchi
2021-06-25 21:09 ` Andrew Burgess
2021-06-26 1:38 ` Simon Marchi
2021-06-25 20:03 ` [PATCH 4/4] gdb/guile: use return values of add_setshow functions in add_setshow_generic Simon Marchi
2021-06-25 21:17 ` Andrew Burgess
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).