Re: [PATCH v4 3/4] gdb: Have setter and getter callbacks for settings

[Lancelot SIX via Gdb-patches <gdb-patches@sourceware.org>]

On Tue, Oct 05, 2021 at 03:10:55PM -0400, Simon Marchi wrote:
> > @@ -241,13 +295,23 @@ struct setting
> >      gdb_assert (var_type_uses<T> (m_var_type));
> >      gdb_assert (m_var != nullptr);
> >
> > -    return *static_cast<T const *> (m_var);
> > +    if (m_var == nullptr)
> > +      {
> > +	gdb_assert (m_getter != nullptr);
> > +	auto getter = reinterpret_cast<setting_getter_ftype<T>> (m_getter);
> > +	return getter ();
> > +      }
> > +    else
> > +      return *static_cast<T *> (m_var);
> 
> While pulling these upstream changes to the ROCm GDB port, I found that
> we forgot to remove the `gdb_assert (m_var != nullptr)` above.
> 
> I can fix that when I send a patch that actually uses the
> getters/setters (which would trigger the assert).
> 
> Simon

Hi,

This is odd, I was sure I did fix this omission, but obviously not.
Thanks for spotting this, and my apologies for the inconvenience.

Here is a (trivial) patch that fixes that.

Best,
Lancelot.

---
From 4c5b728046770ef923323162ac48246b947267d2 Mon Sep 17 00:00:00 2001
From: Lancelot SIX <lsix@lancelotsix.com>
Date: Tue, 5 Oct 2021 19:55:19 +0000
Subject: [PATCH] gdb: Remove deprecated assertion in setting::get

The commit 702991711a91bd47b209289562843a11e7009396 (gdb: Have setter
and getter callbacks for settings) makes it possible for a setting not
to be backed by a memory buffer but use callback functions instead to
retrieve or set the setting's value.

An assertion was not properly updated to take into account that the
m_var member (which points to a memory buffer, if used) might be nullptr
if the setting uses callback functions.  If the setting is backed by a
memory buffer, the m_var has to be non nullptr, which is already checked
before the pointer is dereferenced.

This commit removes this assertion as it is not valid anymore.
---
 gdb/command.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/gdb/command.h b/gdb/command.h
index 7c226f193b8..0049ab6ff9e 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -288,7 +288,6 @@ struct setting
   const T &get () const
   {
     gdb_assert (var_type_uses<T> (m_var_type));
-    gdb_assert (m_var != nullptr);
 
     if (m_var == nullptr)
       {
-- 
2.30.2