[PATCH] Fix bug 15181 Missing continue on dprintf that defaults to "gdb" style from "agent"

[PATCH] Fix bug 15181 Missing continue on dprintf that defaults to "gdb" style from "agent"

[Hui Zhu]

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

Hi,

This patch is from Marc to fix
http://sourceware.org/bugzilla/show_bug.cgi?id=15181
I add a test for it.

Please help me review it.

Thanks,
Hui

2013-04-21  Marc Khouzam  <marc.khouzam@ericsson.com>

	PR gdb/15181

	* breakpoint.c (update_dprintf_command_list): Add "continue" to
	command list if target cannot run dprintf commands.

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

	PR gdb/15181

	* gdb.base/dprintf.exp: Test dprintf commands if target cannot
	run dprintf commands.

[-- Attachment #2: dprintf-miss-continue.txt --]
[-- Type: text/plain, Size: 883 bytes --]

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -8910,6 +8910,7 @@ update_dprintf_command_list (struct brea
 {
   char *dprintf_args = b->extra_string;
   char *printf_line = NULL;
+  int need_continue = 0;
 
   if (!dprintf_args)
     return;
@@ -8950,6 +8951,7 @@ update_dprintf_command_list (struct brea
 	{
 	  warning (_("Target cannot run dprintf commands, falling back to GDB printf"));
 	  printf_line = xstrprintf ("printf %s", dprintf_args);
+	  need_continue = 1;
 	}
     }
   else
@@ -8961,7 +8963,8 @@ update_dprintf_command_list (struct brea
   {
     struct command_line *printf_cmd_line, *cont_cmd_line = NULL;
 
-    if (strcmp (dprintf_style, dprintf_style_agent) != 0)
+    if (strcmp (dprintf_style, dprintf_style_agent) != 0
+	|| need_continue)
       {
 	cont_cmd_line = xmalloc (sizeof (struct command_line));
 	cont_cmd_line->control_type = simple_control;

[-- Attachment #3: dprintf-miss-continue-test.txt --]
[-- Type: text/plain, Size: 636 bytes --]

--- a/gdb/testsuite/gdb.base/dprintf.exp
+++ b/gdb/testsuite/gdb.base/dprintf.exp
@@ -129,6 +129,17 @@ if $target_can_dprintf {
 	"\[\r\n\]\tbreakpoint already hit 2 times"
 	"\[\r\n\]        agent-printf \"arg=%d, g=%d\\\\n\", arg, g"
     }
+} else {
+    delete_breakpoints
+
+    gdb_test "dprintf foo,\"Hello\\n\"" \
+	"Dprintf .*"
+    gdb_test_sequence "info breakpoints" "dprintf info 3" {
+	"\[\r\n\]Num     Type           Disp Enb Address +What"
+	"\[\r\n\]6       dprintf"
+	"\[\r\n\]        printf \"Hello\\\\n\""
+	"\[\r\n\]        continue"
+    }
 }
 
 gdb_test "set dprintf-style foobar" "Undefined item: \"foobar\"." \

Re: [PATCH] Fix bug 15181 Missing continue on dprintf that defaults to "gdb" style from "agent"

[Yao Qi]

On 04/21/2013 07:06 PM, Hui Zhu wrote:
> This patch is from Marc to fix
> http://sourceware.org/bugzilla/show_bug.cgi?id=15181
> I add a test for it.

Hui,
I am afraid that it is not correct to append "continue" command to 
dprintf commands, considering PR 15075 dprintf inteferes with "next". 
This patch <http://sourceware.org/ml/gdb-patches/2013-02/msg00736.html> 
removes "continue" command from dprintf commands, and it is reasonable 
to remove "continue", IMO.  The patch is not perfect, as it disallows 
users setting commands to dprintf, and I prefer to support commands in 
dprintf, personally.

Since you are actively fixing dprintf bugs, feel free to take PR 15075 
over, and fix it first.  Then, PR 15181 may not be a bug at all.

-- 
Yao (齐尧)

Re: [PATCH] Fix bug 15181 Missing continue on dprintf that defaults to "gdb" style from "agent"

[Hui Zhu]

On Mon, Apr 22, 2013 at 8:24 AM, Yao Qi <yao@codesourcery.com> wrote:
> On 04/21/2013 07:06 PM, Hui Zhu wrote:
>>
>> This patch is from Marc to fix
>> http://sourceware.org/bugzilla/show_bug.cgi?id=15181
>> I add a test for it.
>
>
> Hui,
> I am afraid that it is not correct to append "continue" command to dprintf
> commands, considering PR 15075 dprintf inteferes with "next". This patch
> <http://sourceware.org/ml/gdb-patches/2013-02/msg00736.html> removes
> "continue" command from dprintf commands, and it is reasonable to remove
> "continue", IMO.  The patch is not perfect, as it disallows users setting
> commands to dprintf, and I prefer to support commands in dprintf,
> personally.
>
> Since you are actively fixing dprintf bugs, feel free to take PR 15075 over,
> and fix it first.  Then, PR 15181 may not be a bug at all.

The patch for remove "continue" is still in discussion, right?

So I think the patch from Marc is more easy to handle 15181.  And it
will not affect new patches.

Thanks,
Hui

And
>
> --
> Yao (齐尧)

Re: [PATCH] Fix bug 15181 Missing continue on dprintf that defaults to "gdb" style from "agent"

[Yao Qi]

On 04/23/2013 11:41 AM, Hui Zhu wrote:
> The patch for remove "continue" is still in discussion, right?
>

Yes.

> So I think the patch from Marc is more easy to handle 15181.  And it
> will not affect new patches.

We need patch to fix problem if it is a bug.  Whether PR 15181 is a bug 
or not depends on how PR 15075 is fixed.  As I said, I suggest that we'd 
better fix PR 15075 first.  If you insist, I don't mind.  Other 
maintainers can review this patch.

-- 
Yao (齐尧)