This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] Add set/show debug unwind command
- From: Joel Brobecker <brobecker at adacore dot com>
- To: Tristan Gingold <gingold at adacore dot com>
- Cc: "gdb-patches at sourceware dot org ml" <gdb-patches at sourceware dot org>
- Date: Fri, 15 Jun 2012 10:31:14 -0700
- Subject: Re: [RFA] Add set/show debug unwind command
- References: <FA7F98C8-BFF4-4F68-AB6A-AC75DE73C09A@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