Re: [RFA] Add set/show debug unwind command

[Joel Brobecker <brobecker@adacore.com>]

Hi Tristan,

> 2012-06-15  Tristan Gingold  <gingold@adacore.com>
> 
> 	* frame-unwind.c (show_unwind_debug): New function
> 	(unwind_debug): New variable.
> 	(_initialize_frame_unwind): Add show debug unwind command.
> 	* frame-unwind.h (unwind_debug): Declare.

Overall, I think this is OK, but since it's not used until your next
patch gets in, can you commit both at the same time?

You're missing a period at the of the first line.

A few comments...


> +/* Flag to control debugging.  */
> +
> +int unwind_debug;
> +static void

Can you add an empty line between the two?

Also, I am really campaining for every single function to be
documented, no matter how obvious what the function does.
Can you please update this patch accordingly? For the function
below, you can just say "Implements the "show debug unwind"
command, for instance.

> +  /* Debug this files internals.  */
                   file's

> +  add_setshow_zinteger_cmd ("unwind", class_maintenance, &unwind_debug,  _("\
> +Set unwind debugging."), _("\
> +Show unwind debugging."), _("\

For such short descriptions, I don't think we should use the formatting
you chose. We discussed this a while back, and the consensus was that
we should try to avoid it unless it makes sense. Here, I find it does
not help reading the code (the contrary, IMO), and generally speaking
it screws up the -p option of "diff".

> +When non-zero, frame specific internal debugging is enabled."),

Even for this one, let's not use that formatting, let's just split it
in two strings, like so:

                            _("When non-zero, frame specific internal "
                              "debugging is enabled."),

Thanks,
-- 
Joel