Re: [PATCH 15/16] gdb: make cmd_list_element var an optional union

[Lancelot SIX <lsix@lancelotsix.com>]

Hi,

I plan on spending some more time on it this week end hopefully.

If you do not mind waiting a bit for my patch to be finalized, and want
to go with my approach, there is indeed no point merging yours now.  The
main thing I try to avoid is having both of us working at the same time
on different solutions of the same problem.  Your approach has the
advantage of being almost ready, I can wait for it to be stabilized
before I adapt my work on top on it.  If you are fine waiting for my
approach, I’ll proceed!

My plan is to:

- Improve slightly
  https://sourceware.org/pipermail/gdb-patches/2021-July/180991.html
  (use gdb::Requires instead of static assert, include the python
  binding I forgot because I did not build it, add some selftests of
  the runtime checks introduced by the getters and getters, and ensure
  the documentation is adequate).
- Adapt your patch using std::string on top on my architecture
  (https://sourceware.org/pipermail/gdb-patches/2021-July/180921.html).
- Add a new patch that allows the use of setter / getter (based on what
  was is drafted in
  https://sourceware.org/pipermail/gdb-patches/2021-July/181037.html).
  As a side note, the storage of user provided getter / setter functions
  is done using a unions (void* is not an appropriate for that).  It is
  possible to use the same approach to create getters and setters around
  the union of pointers you provide in your patch (but this would require
  some work, obviously).

Does that makes sense?

Best,
Lancelot.

On Fri, Jul 23, 2021 at 03:56:57PM -0400, Simon Marchi wrote:
> > Hi,
> > 
> > I reworked a bit my previous implementation to see to what extent it can
> > support what you are looking for.  Short answer: it can (and it is
> > completely transparent to the caller).
> > 
> > In the previous draft version[1], I introduced a struct that groups
> > together the var_type and the pointer to the actual data in order to
> > abstract a setting (i.e. a value with a type that can be SET or SHOWn).
> > This struct provides getters and setters in order to enforce that we
> > cast the pointer to the adequate type.  I have modified this data
> > structure so we can register user-provided callbacks.  If those
> > functions are provided, the getter and setters will call them to
> > generate retrieve the value instead of trying to dereference the
> > pointer.
> > 
> > For the moment, I just made sure that this compiles and I played with
> > dummy settings in my local tree.  It is not yet finalized nor fully
> > operational because do_set_command will try to free the pointer returned
> > by the getter when dealing with char* data.  With user provided
> > callbacks memory should probably not be managed at that level but by the
> > getter / setter.  Nothing worrying, you have a commit that plans to
> > change the char* with std::string, this should solve the issue quite
> > nicely.
> > 
> > As for the previous version, this adds a significant amount of templated
> > code that can make the implementation appear quite complex.
> > 
> > I think I’ll wait for your series to land before I finalize this
> > proposal (this is just a POC for the moment) unless you want to build on
> > top of this approach to introduce user provided callbacks.
> 
> Do you suggest I merge my patch that makes it a union first?  Won't that
> be counter productive if you plan to make it a void* again?  I'm a bit
> lost now, so I am not sure what's the best order for doing things.
> 
> Simon