[PATCH] Let dprintf can be saved

[PATCH] Let dprintf can be saved

[Hui Zhu]

[-- Attachment #1: Type: text/plain, Size: 585 bytes --]

Hi,

This patch fix http://sourceware.org/bugzilla/show_bug.cgi?id=15165

But I didn't use commands because it is not current way to setup
commands of dprintf.

So I add a check in save_breakpoints to let it not save dprintf
commands..  If you think we need, I can put commands save code as a
special function to handle it.

Please help me review it.

Thanks,
Hui

2013-04-18  Hui Zhu  <hui@codesourcery.com>

	PR gdb/15165

	* breakpoint.c (dprintf_print_recreate): New.
	(save_breakpoints): Let it not save dprintf commands.
	(initialize_breakpoint_ops): Set dprintf_print_recreate.

[-- Attachment #2: dprintf-save.txt --]
[-- Type: text/plain, Size: 1104 bytes --]

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -13364,6 +13364,14 @@ dprintf_re_set (struct breakpoint *b)
     update_dprintf_command_list (b);
 }
 
+static void
+dprintf_print_recreate (struct breakpoint *tp, struct ui_file *fp)
+{
+  fprintf_unfiltered (fp, "dprintf %s%s", tp->addr_string,
+		      tp->extra_string);
+  print_recreate_thread (tp, fp);
+}
+
 /* The breakpoint_ops structure to be used on static tracepoints with
    markers (`-m').  */
 
@@ -15453,7 +15461,7 @@ save_breakpoints (char *filename, int fr
     if (tp->ignore_count)
       fprintf_unfiltered (fp, "  ignore $bpnum %d\n", tp->ignore_count);
 
-    if (tp->commands)
+    if (tp->type != bp_dprintf && tp->commands)
       {
 	volatile struct gdb_exception ex;	
 
@@ -15859,7 +15867,7 @@ initialize_breakpoint_ops (void)
   ops->resources_needed = bkpt_resources_needed;
   ops->print_it = bkpt_print_it;
   ops->print_mention = bkpt_print_mention;
-  ops->print_recreate = bkpt_print_recreate;
+  ops->print_recreate = dprintf_print_recreate;
 }
 
 /* Chain containing all defined "enable breakpoint" subcommands.  */

Re: [PATCH] Let dprintf can be saved

[Tom Tromey]

>>>>> "Hui" == Hui Zhu <teawater@gmail.com> writes:

Hui> +static void
Hui> +dprintf_print_recreate (struct breakpoint *tp, struct ui_file *fp)

This needs a short introductory comment.
See the other print_recreate methods for examples.

I think this patch could use a test case.
It could be added wherever the existing "save" tests live.

Tom

Re: [PATCH] Let dprintf can be saved

[Keith Seitz]

On 04/19/2013 09:34 AM, Tom Tromey wrote:
> I think this patch could use a test case.
> It could be added wherever the existing "save" tests live.

Those are in save-bp.exp.

If I may be so bold, I would like to request that tests are added for 
saving pending dprintfs, too, both before and after resolution.

I noticed a multitude of save breakpoint problems w.r.t. pending 
breakpoints (a number of which my explicit location patch fixes).

Keith

Re: [PATCH] Let dprintf can be saved

[Hui Zhu]

[-- Attachment #1: Type: text/plain, Size: 719 bytes --]

On Sat, Apr 20, 2013 at 12:34 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Hui" == Hui Zhu <teawater@gmail.com> writes:
>
> Hui> +static void
> Hui> +dprintf_print_recreate (struct breakpoint *tp, struct ui_file *fp)
>
> This needs a short introductory comment.
> See the other print_recreate methods for examples.

Fixed.

>
> I think this patch could use a test case.
> It could be added wherever the existing "save" tests live.

OK.  I will post a patch for it later.

Thanks,
Hui

>
> Tom

2013-04-18  Hui Zhu  <hui@codesourcery.com>

	PR gdb/15165

	* breakpoint.c (dprintf_print_recreate): New.
	(save_breakpoints): Let it not save dprintf commands.
	(initialize_breakpoint_ops): Set dprintf_print_recreate.

[-- Attachment #2: dprintf-save.txt --]
[-- Type: text/plain, Size: 1180 bytes --]

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -13364,6 +13364,16 @@ dprintf_re_set (struct breakpoint *b)
     update_dprintf_command_list (b);
 }
 
+/* Implement the "print_recreate" breakpoint_ops method for dprintf.  */
+
+static void
+dprintf_print_recreate (struct breakpoint *tp, struct ui_file *fp)
+{
+  fprintf_unfiltered (fp, "dprintf %s%s", tp->addr_string,
+		      tp->extra_string);
+  print_recreate_thread (tp, fp);
+}
+
 /* The breakpoint_ops structure to be used on static tracepoints with
    markers (`-m').  */
 
@@ -15453,7 +15463,7 @@ save_breakpoints (char *filename, int fr
     if (tp->ignore_count)
       fprintf_unfiltered (fp, "  ignore $bpnum %d\n", tp->ignore_count);
 
-    if (tp->commands)
+    if (tp->type != bp_dprintf && tp->commands)
       {
 	volatile struct gdb_exception ex;	
 
@@ -15859,7 +15869,7 @@ initialize_breakpoint_ops (void)
   ops->resources_needed = bkpt_resources_needed;
   ops->print_it = bkpt_print_it;
   ops->print_mention = bkpt_print_mention;
-  ops->print_recreate = bkpt_print_recreate;
+  ops->print_recreate = dprintf_print_recreate;
 }
 
 /* Chain containing all defined "enable breakpoint" subcommands.  */

Re: [PATCH] Let dprintf can be saved

[Hui Zhu]

[-- Attachment #1: Type: text/plain, Size: 1034 bytes --]

Hi,

This is patch add test for dprintf to save-bp.exp.

Thanks,
Hui

2013-04-20  Hui Zhu  <hui@codesourcery.com>

	PR gdb/15165

	* gdb.base/save-bp.exp: Add test for dprintf.

On Sat, Apr 20, 2013 at 3:04 PM, Hui Zhu <teawater@gmail.com> wrote:
> On Sat, Apr 20, 2013 at 12:34 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>>> "Hui" == Hui Zhu <teawater@gmail.com> writes:
>>
>> Hui> +static void
>> Hui> +dprintf_print_recreate (struct breakpoint *tp, struct ui_file *fp)
>>
>> This needs a short introductory comment.
>> See the other print_recreate methods for examples.
>
> Fixed.
>
>>
>> I think this patch could use a test case.
>> It could be added wherever the existing "save" tests live.
>
> OK.  I will post a patch for it later.
>
> Thanks,
> Hui
>
>>
>> Tom
>
> 2013-04-18  Hui Zhu  <hui@codesourcery.com>
>
>         PR gdb/15165
>
>         * breakpoint.c (dprintf_print_recreate): New.
>         (save_breakpoints): Let it not save dprintf commands.
>         (initialize_breakpoint_ops): Set dprintf_print_recreate.

[-- Attachment #2: dprintf-save-test.txt --]
[-- Type: text/plain, Size: 1566 bytes --]

--- a/gdb/testsuite/gdb.base/save-bp.exp
+++ b/gdb/testsuite/gdb.base/save-bp.exp
@@ -46,6 +46,8 @@ set loc_bp5 [gdb_get_line_number "with c
 gdb_breakpoint ${srcfile}:${loc_bp5}
 gdb_test "commands\nsilent\nend" "End with.*" "add breakpoint commands"
 
+gdb_test "dprintf ${srcfile}:${loc_bp5},\"At foo entry\\n\"" "Dprintf .*"
+
 # Now, save the breakpoints into a file...
 remote_file host delete "bps"
 gdb_test "save breakpoint bps"
@@ -65,5 +67,6 @@ gdb_test "source bps"
 
 # Now, verify that all breakpoints have been created correctly...
 set bp_row_start "\[0-9\]+ +breakpoint +keep +y +0x\[0-9a-f\]+ +in"
+set dprintf_row_start "\[0-9\]+ +dprintf +keep +y +0x\[0-9a-f\]+ +in"
 gdb_test "info break" \
-  " *Num +Type +Disp +Enb +Address +What\r\n$bp_row_start break_me at .*$srcfile:\[0-9\]+\r\n$bp_row_start main at .*$srcfile:$loc_bp2\r\n$bp_row_start main at .*$srcfile:$loc_bp3 +thread 1\r\n\[ \t]+stop only in thread 1\r\n$bp_row_start main at .*$srcfile:$loc_bp4\r\n\[ \t\]+stop only if i == 1( \\((host|target) evals\\))?\r\n$bp_row_start main at .*$srcfile:$loc_bp5\r\n\[ \t\]+silent"
+  " *Num +Type +Disp +Enb +Address +What\r\n$bp_row_start break_me at .*$srcfile:\[0-9\]+\r\n$bp_row_start main at .*$srcfile:$loc_bp2\r\n$bp_row_start main at .*$srcfile:$loc_bp3 +thread 1\r\n\[ \t]+stop only in thread 1\r\n$bp_row_start main at .*$srcfile:$loc_bp4\r\n\[ \t\]+stop only if i == 1( \\((host|target) evals\\))?\r\n$bp_row_start main at .*$srcfile:$loc_bp5\r\n\[ \t\]+silent\r\n$dprintf_row_start main at .*$srcfile:$loc_bp5\r\n\[ \t\]+printf.*"

Re: [PATCH] Let dprintf can be saved

[Tom Tromey]

>>>>> "Hui" == Hui Zhu <teawater@gmail.com> writes:

Hui> 2013-04-20  Hui Zhu  <hui@codesourcery.com>
Hui> 	PR gdb/15165
Hui> 	* gdb.base/save-bp.exp: Add test for dprintf.

This is ok.

FWIW I think it is best to merge the bug fix with the test case and
commit them as a single patch.

Tom

Re: [PATCH] Let dprintf can be saved

[Hui Zhu]

On Tue, Apr 23, 2013 at 12:01 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Hui" == Hui Zhu <teawater@gmail.com> writes:
>
> Hui> 2013-04-20  Hui Zhu  <hui@codesourcery.com>
> Hui>    PR gdb/15165
> Hui>    * gdb.base/save-bp.exp: Add test for dprintf.
>
> This is ok.
>
> FWIW I think it is best to merge the bug fix with the test case and
> commit them as a single patch.
>
> Tom

OK.  Checked in together.
http://sourceware.org/ml/gdb-cvs/2013-04/msg00221.html

Can I commit this patch to 7.6 branch?

Best,
Hui

Re: [PATCH] Let dprintf can be saved

[Joel Brobecker]

> OK.  Checked in together.
> http://sourceware.org/ml/gdb-cvs/2013-04/msg00221.html
> 
> Can I commit this patch to 7.6 branch?

OK for 7.6.

-- 
Joel

Re: [PATCH] Let dprintf can be saved

[Hui Zhu]

On Wed, Apr 24, 2013 at 2:06 PM, Joel Brobecker <brobecker@adacore.com> wrote:
>> OK.  Checked in together.
>> http://sourceware.org/ml/gdb-cvs/2013-04/msg00221.html
>>
>> Can I commit this patch to 7.6 branch?
>
> OK for 7.6.
>
> --
> Joel

Checked in http://sourceware.org/ml/gdb-cvs/2013-04/msg00226.html

Thanks,
Hui

Re: [PATCH] Let dprintf can be saved

[Tom Tromey]

>>>>> "Hui" == Hui Zhu <teawater@gmail.com> writes:

Hui> 2013-04-18  Hui Zhu  <hui@codesourcery.com>
Hui> 	PR gdb/15165
Hui> 	* breakpoint.c (dprintf_print_recreate): New.
Hui> 	(save_breakpoints): Let it not save dprintf commands.
Hui> 	(initialize_breakpoint_ops): Set dprintf_print_recreate.

This is ok.  Thanks.

Tom