[RFA] Add set/show debug unwind command

[RFA] Add set/show debug unwind command

[Tristan Gingold]

Hi,

while working on the Win64 SEH unwinder (my next patch), I have found useful to have a debug flag for unwinder.

Tested by building gdb.

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.

diff --git a/gdb/frame-unwind.c b/gdb/frame-unwind.c
index c90144f..e6035cc 100644
--- a/gdb/frame-unwind.c
+++ b/gdb/frame-unwind.c
@@ -27,6 +27,8 @@
 #include "exceptions.h"
 #include "gdb_assert.h"
 #include "gdb_obstack.h"
+#include "command.h"
+#include "gdbcmd.h"
 
 static struct gdbarch_data *frame_unwind_data;
 
@@ -237,6 +239,17 @@ frame_unwind_got_address (struct frame_info *frame, int regnum,
   return reg_val;
 }
 
+/* Flag to control debugging.  */
+
+int unwind_debug;
+static void
+show_unwind_debug (struct ui_file *file, int from_tty,
+		  struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("unwind debugging is %s.\n"), value);
+}
+
+
 /* -Wmissing-prototypes */
 extern initialize_file_ftype _initialize_frame_unwind;
 
@@ -244,4 +257,14 @@ void
 _initialize_frame_unwind (void)
 {
   frame_unwind_data = gdbarch_data_register_pre_init (frame_unwind_init);
+
+  /* Debug this files internals.  */
+  add_setshow_zinteger_cmd ("unwind", class_maintenance, &unwind_debug,  _("\
+Set unwind debugging."), _("\
+Show unwind debugging."), _("\
+When non-zero, frame specific internal debugging is enabled."),
+			    NULL,
+			    show_unwind_debug,
+			    &setdebuglist, &showdebuglist);
+
 }
diff --git a/gdb/frame-unwind.h b/gdb/frame-unwind.h
index f82d763..64405b9 100644
--- a/gdb/frame-unwind.h
+++ b/gdb/frame-unwind.h
@@ -217,4 +217,8 @@ struct value *frame_unwind_got_bytes (struct frame_info *frame, int regnum,
 struct value *frame_unwind_got_address (struct frame_info *frame, int regnum,
 					CORE_ADDR addr);
 
+/* Flag to control debugging.  */
+
+extern int unwind_debug;
+
 #endif

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

[Joel Brobecker]

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

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

[Joel Brobecker]

> +show_unwind_debug (struct ui_file *file, int from_tty,
> +		  struct cmd_list_element *c, const char *value)

I forgot something: There is a formatting issue on the second line
(code not properly aligned with the first parameter on the line
above).

-- 
Joel

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

[Pedro Alves]

Hi Tristan,

On 06/15/2012 08:51 AM, Tristan Gingold wrote:

> 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.


We also document debug and maintenance commands in the manual.

-- 
Pedro Alves

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

[Joel Brobecker]

> We also document debug and maintenance commands in the manual.

Thanks for reminding us! And NEWS as well, in fact.

-- 
Joel

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

[Tristan Gingold]

On Jun 15, 2012, at 8:22 PM, Joel Brobecker wrote:

>> We also document debug and maintenance commands in the manual.
> 
> Thanks for reminding us! And NEWS as well, in fact.

Thanks for the comments.  Here is the new version.

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.
	* doc/gdb.texinfo (Debugging Output): Document set/show debug
	unwind.
	* NEWS: Mention it.


diff --git a/gdb/NEWS b/gdb/NEWS
index 476e5a0..2384489 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -503,6 +503,10 @@ show trace-stop-notes
   instance as an explanation, if you are stopping a trace run that was
   started by someone else.
 
+set debug unwind
+show debug unwind
+  Control display of debugging info for some unwinders.
+
 * New remote packets
 
 QTEnable
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 014cfd8..30cd053 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -21850,6 +21850,11 @@ message.
 @item show debug timestamp
 Displays the current state of displaying timestamps with @value{GDBN}
 debugging info.
+@item set debug unwind
+@cindex unwinding debugging info
+Turns on or off display debugging info from some unwinders.
+@item show debug unwind
+Display the current state of displaying unwind debugging info.
 @item set debugvarobj
 @cindex variable object debugging info
 Turns on or off display of @value{GDBN} variable object debugging
diff --git a/gdb/frame-unwind.c b/gdb/frame-unwind.c
index c90144f..ab67a76 100644
--- a/gdb/frame-unwind.c
+++ b/gdb/frame-unwind.c
@@ -27,6 +27,8 @@
 #include "exceptions.h"
 #include "gdb_assert.h"
 #include "gdb_obstack.h"
+#include "command.h"
+#include "gdbcmd.h"
 
 static struct gdbarch_data *frame_unwind_data;
 
@@ -237,6 +239,20 @@ frame_unwind_got_address (struct frame_info *frame, int regnum,
   return reg_val;
 }
 
+/* Flag to control debugging.  */
+
+int unwind_debug;
+
+/* Implements show debug unwind.  */
+
+static void
+show_unwind_debug (struct ui_file *file, int from_tty,
+		   struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("unwind debugging is %s.\n"), value);
+}
+
+
 /* -Wmissing-prototypes */
 extern initialize_file_ftype _initialize_frame_unwind;
 
@@ -244,4 +260,15 @@ void
 _initialize_frame_unwind (void)
 {
   frame_unwind_data = gdbarch_data_register_pre_init (frame_unwind_init);
+
+  /* Debug this files internals.  */
+  add_setshow_zinteger_cmd ("unwind", class_maintenance, &unwind_debug,
+			    _("Set unwind debugging."),
+			    _("Show unwind debugging."),
+			    _("When non-zero, frame specific internal "
+			      "debugging is enabled."),
+			    NULL,
+			    show_unwind_debug,
+			    &setdebuglist, &showdebuglist);
+
 }
diff --git a/gdb/frame-unwind.h b/gdb/frame-unwind.h
index f82d763..64405b9 100644
--- a/gdb/frame-unwind.h
+++ b/gdb/frame-unwind.h
@@ -217,4 +217,8 @@ struct value *frame_unwind_got_bytes (struct frame_info *frame, int regnum,
 struct value *frame_unwind_got_address (struct frame_info *frame, int regnum,
 					CORE_ADDR addr);
 
+/* Flag to control debugging.  */
+
+extern int unwind_debug;
+
 #endif

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

[Joel Brobecker]

> 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.
> 	* doc/gdb.texinfo (Debugging Output): Document set/show debug
> 	unwind.
> 	* NEWS: Mention it.

Looks good for the code part, just one tiny comment:

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

-- 
Joel

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

[Eli Zaretskii]

> From: Tristan Gingold <gingold@adacore.com>
> Date: Mon, 18 Jun 2012 11:12:31 +0200
> Cc: Pedro Alves <palves@redhat.com>, "gdb-patches@sourceware.org ml" <gdb-patches@sourceware.org>
> 
> +set debug unwind
> +show debug unwind
> +  Control display of debugging info for some unwinders.

Can we be more specific (especially in the manual) about which
unwinders are affected?

Otherwise, the documentation parts are OK.  Thanks.