Re: [PATCH v5 4/5] gdb: introduce ability to disable frame unwinders

[Simon Marchi <simark@simark.ca>]

On 2024-10-09 09:32, Guinevere Larsen wrote:
>> OTOH, I don't think accepting user inputs FRAME_UNWIND_ is really
>> necessary.
> That's a fair question, but I don't think this is too costly (for performance, or maintenance) to keep it, in case someone reads the code instead of documentation (not that I do that or anything o.o'). If you disagree, I am fine with removing it honestly, especially with documenting the classes that you suggested later.

It's more that the FRAME_UNWIND_* names come from your enumerator names,
which are really an internal implementation detail.  It feels weird to
expose that in the UI, where things can't change (in theory, although
this is a maintenance command).

>>> +  struct gdbarch* gdbarch = current_inferior ()->arch ();
>>> +  std::vector<const frame_unwind *> unwinder_list
>>> +    = gdbarch_unwinder_list (gdbarch);
>>> +
>>> +  /* First see if the user wants to change all unwinders.  */
>>> +  if (check_for_argument (&args, "-all"))
>>> +    {
>>> +      for (const frame_unwind *u : unwinder_list)
>>> +    u->set_enabled (enable);
>>> +      return;
>>> +    }
>> Did you consider using the gdb::option framework for this (found in
>> cli/cli-option.h)?  It's been a while since I touched it, I don't recall
>> if it would be a good fit for this, but it would be worth checking.
> 
> I hadn't looked into using this framework when I first implemented it. However, looking at it now, it looks like overkill for what I need. The gdb::option path need the option array (with all the stuff that goes along with it), to end up with a very similar setup in the function itself.
> 
> Now, if you think I'm overselling the complexity or that we are likely to add more options to this command, I can add it for the next version.

I don't recall how it works exactly, I would have to try it for myself.

Simon