Re: [PATCH 4/4] GDB: introduce ability to disable frame unwinders

[Tom Tromey <tom@tromey.com>]

>>>>> Guinevere Larsen <blarsen@redhat.com> writes:

 
> +static enum frame_unwind_class
> +str_to_frame_unwind_class (const char **c_str)

Comment.

> +{
> +  std::string full_name = "FRAME_UNWIND_";
> +  const int start_length = full_name.length ();
> +  if (strncasecmp (*c_str, full_name.c_str (), start_length) == 0)
> +    full_name = *c_str;

In an earlier patch I was wondering how useful those FRAME_UNWIND_
strings were for the maint output.  Maybe just using human names here
would be overall better.

Then this could just call streq.

Anyway if you want to keep this approach, it's weird to allocate a
string and then use the C API.  Perhaps a string_view would be better.

 
> +/* Helper function to both enable and disable frame unwinders.
> +   if ENABLE is true, this call will be enabling unwinders,
> +   otherwise the unwinders will be disabled.  */
> +static void
> +enable_disable_frame_unwinders (const char *args, int from_tty, bool enable)
> +{
> +
> +  reinit_frame_cache ();

Stray blank line.

> +  if (args == nullptr)
> +    {
> +      error (_("specify which frame unwinder(s) should be %s"),
> +	     (enable)? "enabled" : "disabled");
> +    }

No braces.

> +  /* 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);
> +	}

This also looks over-braced.

> +  add_cmd ("disable", class_maintenance, maintenance_disable_frame_unwinders,
> +	   _("\
> +Disable one or more frame unwinder(s).\n\
> +Usage: maint frame-unwinder disable [OPTION] UNWINDER\n\
> +\n\
> +The meaning of UNWINDER depends on the OPTION given. These are the possibilities:\n\
> +\t-all    - UNWINDER is ignored. All available unwinders will be disabled\n\
> +\t-name   - UNWINDER is the exact name of the frame unwinder is to be disabled\n\
> +\t-class  - UNWINDER is the class of unwinders to be disabled.\n\

I guess I'd write that more like

disable [-all | -name UNWINDER | -class NAME]

or something like that, rather than spelling out that UNWINDER is
ignored in one case.


I think there's probably a bug in bugzilla about disabling unwinders, so
this should probably have a Bug: trailer.

Tom