Re: [PATCH v4 1/4] gdb: Introduce setting construct within cmd_list_element

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2021-09-30 08:12, Simon Marchi via Gdb-patches wrote:
>> @@ -667,26 +701,35 @@ 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 value = var.get<unsigned int> ();
> 
> As Pedro mentioned, we prefer "const unsigned int" (here and below).
> 
> LGTM with that fixed.

In fact, I tried to build it and there's a build failure due to a recent
change:

      CXX    bt-utils.o
    In file included from /home/simark/src/binutils-gdb/gdb/../gdbsupport/common-defs.h:201,
                     from /home/simark/src/binutils-gdb/gdb/defs.h:28,
                     from /home/simark/src/binutils-gdb/gdb/bt-utils.c:18:
    /home/simark/src/binutils-gdb/gdb/bt-utils.c: In function ‘void gdb_internal_backtrace_set_cmd(const char*, int, cmd_list_element*)’:
    /home/simark/src/binutils-gdb/gdb/bt-utils.c:32:18: error: ‘struct cmd_list_element’ has no member named ‘var_type’
       32 |   gdb_assert (c->var_type == var_boolean);
          |                  ^~~~~~~~
    /home/simark/src/binutils-gdb/gdb/../gdbsupport/gdb_assert.h:35:13: note: in definition of macro ‘gdb_assert’
       35 |   ((void) ((expr) ? 0 :                                                       \
          |             ^~~~

I think the change below should fix it.

Since it's one of those
settings that reverts the value if the new value is invalid, it could
eventually be changed to use a getter/setter, so that the setter simply
rejects the new value if it's not valid.


From 96df63a390451e0bbdfc74784ea09a3aa360656d Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Thu, 30 Sep 2021 08:26:41 -0400
Subject: [PATCH] fix

Change-Id: Id78157cacf2de9cfacba1f19dea17b0f7b65aa0f
---
 gdb/bt-utils.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/gdb/bt-utils.c b/gdb/bt-utils.c
index 79e6e090d428..2054e68c6e5a 100644
--- a/gdb/bt-utils.c
+++ b/gdb/bt-utils.c
@@ -29,15 +29,15 @@ gdb_internal_backtrace_set_cmd (const char *args, int from_tty,
 				cmd_list_element *c)
 {
   gdb_assert (c->type == set_cmd);
-  gdb_assert (c->var_type == var_boolean);
-  gdb_assert (c->var != nullptr);
+  gdb_assert (c->var->type () == var_boolean);
+  gdb_assert (c->var.has_value ());
 
 #ifndef GDB_PRINT_INTERNAL_BACKTRACE
-  bool *var_ptr = (bool *) c->var;
+  bool val = c->var->get<bool> ();
 
-  if (*var_ptr)
+  if (val)
     {
-      *var_ptr = false;
+      c->var->set<bool> (false);
       error (_("support for this feature is not compiled into GDB"));
     }
 #endif
-- 
2.33.0