[PATCH] not trigger pagination with dprintf

[PATCH] not trigger pagination with dprintf

[Hui Zhu]

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

Hi,

This is the patch for http://sourceware.org/bugzilla/show_bug.cgi?id=15182
I agree with what Marc said in the bug report.

So this patch make dprintf not trigger pagination.

Please help me review it.

Thanks,
Hui

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

	PR gdb/15182

	* printcmd.c (printf_c_string): Add argument filter and handle it.
	(printf_wide_c_string): Ditto.
	(printf_decfloat): Ditto.
	(printf_pointer): Ditto.
	(ui_printf): Ditto.
	(printf_command): Update argument of ui_printf.
	(eval_command): Ditto.

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

--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -2001,7 +2001,7 @@ print_variable_and_value (const char *na
 
 static void
 printf_c_string (struct ui_file *stream, const char *format,
-		 struct value *value)
+		 struct value *value, int filter)
 {
   gdb_byte *str;
   CORE_ADDR tem;
@@ -2026,7 +2026,10 @@ printf_c_string (struct ui_file *stream,
     read_memory (tem, str, j);
   str[j] = 0;
 
-  fprintf_filtered (stream, format, (char *) str);
+  if (filter)
+    fprintf_filtered (stream, format, (char *) str);
+  else
+    fprintf_unfiltered (stream, format, (char *) str);
 }
 
 /* Subroutine of ui_printf to simplify it.
@@ -2035,7 +2038,7 @@ printf_c_string (struct ui_file *stream,
 
 static void
 printf_wide_c_string (struct ui_file *stream, const char *format,
-		      struct value *value)
+		      struct value *value, int filter)
 {
   gdb_byte *str;
   CORE_ADDR tem;
@@ -2075,7 +2078,10 @@ printf_wide_c_string (struct ui_file *st
 			     &output, translit_char);
   obstack_grow_str0 (&output, "");
 
-  fprintf_filtered (stream, format, obstack_base (&output));
+  if (filter)
+    fprintf_filtered (stream, format, obstack_base (&output));
+  else
+    fprintf_unfiltered (stream, format, obstack_base (&output));
   do_cleanups (inner_cleanup);
 }
 
@@ -2084,14 +2090,17 @@ printf_wide_c_string (struct ui_file *st
 
 static void
 printf_decfloat (struct ui_file *stream, const char *format,
-		 struct value *value)
+		 struct value *value, int filter)
 {
   const gdb_byte *param_ptr = value_contents (value);
 
 #if defined (PRINTF_HAS_DECFLOAT)
   /* If we have native support for Decimal floating
      printing, handle it here.  */
-  fprintf_filtered (stream, format, param_ptr);
+  if (filter)
+    fprintf_filtered (stream, format, param_ptr);
+  else
+    fprintf_unfiltered (stream, format, param_ptr);
 #else
   /* As a workaround until vasprintf has native support for DFP
      we convert the DFP values to string and print them using
@@ -2160,7 +2169,10 @@ printf_decfloat (struct ui_file *stream,
   decimal_to_string (dfp_ptr, dfp_len, byte_order, decstr);
 
   /* Print the DFP value.  */
-  fprintf_filtered (stream, "%s", decstr);
+  if (filter)
+    fprintf_filtered (stream, "%s", decstr);
+  else
+    fprintf_unfiltered (stream, "%s", decstr);
 #endif
 }
 
@@ -2169,7 +2181,7 @@ printf_decfloat (struct ui_file *stream,
 
 static void
 printf_pointer (struct ui_file *stream, const char *format,
-		struct value *value)
+		struct value *value, int filter)
 {
   /* We avoid the host's %p because pointers are too
      likely to be the wrong size.  The only interesting
@@ -2219,20 +2231,26 @@ printf_pointer (struct ui_file *stream,
       *fmt_p++ = 'l';
       *fmt_p++ = 'x';
       *fmt_p++ = '\0';
-      fprintf_filtered (stream, fmt, val);
+      if (filter)
+	fprintf_filtered (stream, fmt, val);
+      else
+	fprintf_unfiltered (stream, fmt, val);
     }
   else
     {
       *fmt_p++ = 's';
       *fmt_p++ = '\0';
-      fprintf_filtered (stream, fmt, "(nil)");
+      if (filter)
+	fprintf_filtered (stream, fmt, "(nil)");
+      else
+	fprintf_unfiltered (stream, fmt, "(nil)");
     }
 }
 
 /* printf "printf format string" ARG to STREAM.  */
 
 static void
-ui_printf (const char *arg, struct ui_file *stream)
+ui_printf (const char *arg, struct ui_file *stream, int filter)
 {
   struct format_piece *fpieces;
   const char *s = arg;
@@ -2310,10 +2328,12 @@ ui_printf (const char *arg, struct ui_fi
 	switch (fpieces[fr].argclass)
 	  {
 	  case string_arg:
-	    printf_c_string (stream, current_substring, val_args[i]);
+	    printf_c_string (stream, current_substring, val_args[i],
+			     filter);
 	    break;
 	  case wide_string_arg:
-	    printf_wide_c_string (stream, current_substring, val_args[i]);
+	    printf_wide_c_string (stream, current_substring, val_args[i],
+				  filter);
 	    break;
 	  case wide_char_arg:
 	    {
@@ -2343,8 +2363,12 @@ ui_printf (const char *arg, struct ui_fi
 					 &output, translit_char);
 	      obstack_grow_str0 (&output, "");
 
-	      fprintf_filtered (stream, current_substring,
-                                obstack_base (&output));
+	      if (filter)
+	        fprintf_filtered (stream, current_substring,
+				  obstack_base (&output));
+	      else
+	        fprintf_unfiltered (stream, current_substring,
+				    obstack_base (&output));
 	      do_cleanups (inner_cleanup);
 	    }
 	    break;
@@ -2361,7 +2385,10 @@ ui_printf (const char *arg, struct ui_fi
 	      if (inv)
 		error (_("Invalid floating value found in program."));
 
-              fprintf_filtered (stream, current_substring, (double) val);
+	      if (filter)
+                fprintf_filtered (stream, current_substring, (double) val);
+	      else
+		fprintf_unfiltered (stream, current_substring, (double) val);
 	      break;
 	    }
 	  case long_double_arg:
@@ -2378,8 +2405,12 @@ ui_printf (const char *arg, struct ui_fi
 	      if (inv)
 		error (_("Invalid floating value found in program."));
 
-	      fprintf_filtered (stream, current_substring,
-                                (long double) val);
+	      if (filter)
+	        fprintf_filtered (stream, current_substring,
+				  (long double) val);
+	      else
+	        fprintf_unfiltered (stream, current_substring,
+				    (long double) val);
 	      break;
 	    }
 #else
@@ -2390,7 +2421,10 @@ ui_printf (const char *arg, struct ui_fi
 	    {
 	      long long val = value_as_long (val_args[i]);
 
-              fprintf_filtered (stream, current_substring, val);
+	      if (filter)
+		fprintf_filtered (stream, current_substring, val);
+	      else
+		fprintf_unfiltered (stream, current_substring, val);
 	      break;
 	    }
 #else
@@ -2400,22 +2434,28 @@ ui_printf (const char *arg, struct ui_fi
 	    {
 	      int val = value_as_long (val_args[i]);
 
-              fprintf_filtered (stream, current_substring, val);
+	      if (filter)
+		fprintf_filtered (stream, current_substring, val);
+	      else
+		fprintf_unfiltered (stream, current_substring, val);
 	      break;
 	    }
 	  case long_arg:
 	    {
 	      long val = value_as_long (val_args[i]);
 
-              fprintf_filtered (stream, current_substring, val);
+	      if (filter)
+		fprintf_filtered (stream, current_substring, val);
+	      else
+		fprintf_unfiltered (stream, current_substring, val);
 	      break;
 	    }
 	  /* Handles decimal floating values.  */
 	  case decfloat_arg:
-	    printf_decfloat (stream, current_substring, val_args[i]);
+	    printf_decfloat (stream, current_substring, val_args[i], filter);
 	    break;
 	  case ptr_arg:
-	    printf_pointer (stream, current_substring, val_args[i]);
+	    printf_pointer (stream, current_substring, val_args[i], filter);
 	    break;
 	  case literal_piece:
 	    /* Print a portion of the format string that has no
@@ -2426,7 +2466,10 @@ ui_printf (const char *arg, struct ui_fi
 	       have modified GCC to include -Wformat-security by
 	       default, which will warn here if there is no
 	       argument.  */
-	    fprintf_filtered (stream, current_substring, 0);
+	    if (filter)
+	      fprintf_filtered (stream, current_substring, 0);
+	    else
+	      fprintf_unfiltered (stream, current_substring, 0);
 	    break;
 	  default:
 	    internal_error (__FILE__, __LINE__,
@@ -2445,7 +2488,7 @@ ui_printf (const char *arg, struct ui_fi
 static void
 printf_command (char *arg, int from_tty)
 {
-  ui_printf (arg, gdb_stdout);
+  ui_printf (arg, gdb_stdout, from_tty);
 }
 
 /* Implement the "eval" command.  */
@@ -2457,7 +2500,7 @@ eval_command (char *arg, int from_tty)
   struct cleanup *cleanups = make_cleanup_ui_file_delete (ui_out);
   char *expanded;
 
-  ui_printf (arg, ui_out);
+  ui_printf (arg, ui_out, 1);
 
   expanded = ui_file_xstrdup (ui_out, NULL);
   make_cleanup (xfree, expanded);

Re: [PATCH] not trigger pagination with dprintf

[Tom Tromey]

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

Hui> This is the patch for http://sourceware.org/bugzilla/show_bug.cgi?id=15182
Hui> I agree with what Marc said in the bug report.

Quoting for clarity:

   a) when pagination is triggered, inferior execution will be
      interrupted until the user answers the pagination prompt

   b) pagination is triggered by the dprintf but not by real inferior
      output. So, as dprintf and inferior printouts appear interleaved
      on the screen, the pagination prompt will be triggered when the
      dprintfs add up to too many, which will seem random to the user,
      since the other printouts are also visible.


I agree these arguments are pretty good, but I don't see why they apply
particularly to dprintf as opposed to all gdb output.  But then the
result is to just disable all pagination -- something already easily
done.

So I tend to think this should not go in.

FWIW I have never understood why gdb provides _unfiltered variants of
the print functions.  It seems to me that a stream should either be
paginated or not -- having it work at the level of the individual print
means that some prints will provoke paging behavior and some will not;
and, worse, since they are in fact interleaved, the "paging" output may
not all be visible anyhow.

Tom

Re: [PATCH] not trigger pagination with dprintf

[Hui Zhu]

On Sat, Apr 20, 2013 at 2:22 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Hui" == Hui Zhu <teawater@gmail.com> writes:
>
> Hui> This is the patch for http://sourceware.org/bugzilla/show_bug.cgi?id=15182
> Hui> I agree with what Marc said in the bug report.
>
> Quoting for clarity:
>
>    a) when pagination is triggered, inferior execution will be
>       interrupted until the user answers the pagination prompt
>
>    b) pagination is triggered by the dprintf but not by real inferior
>       output. So, as dprintf and inferior printouts appear interleaved
>       on the screen, the pagination prompt will be triggered when the
>       dprintfs add up to too many, which will seem random to the user,
>       since the other printouts are also visible.
>
>
> I agree these arguments are pretty good, but I don't see why they apply
> particularly to dprintf as opposed to all gdb output.  But then the
> result is to just disable all pagination -- something already easily
> done.
>
> So I tend to think this should not go in.

Will you OK if I update patch to make printf_command call ui_printf
with filter is 0 just with dprintf?

Thanks,
Hui

>
> FWIW I have never understood why gdb provides _unfiltered variants of
> the print functions.  It seems to me that a stream should either be
> paginated or not -- having it work at the level of the individual print
> means that some prints will provoke paging behavior and some will not;
> and, worse, since they are in fact interleaved, the "paging" output may
> not all be visible anyhow.
>
> Tom

Re: [PATCH] not trigger pagination with dprintf

[Tom Tromey]

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

Hui> Will you OK if I update patch to make printf_command call ui_printf
Hui> with filter is 0 just with dprintf?

I just don't understand why dprintf should be special in this regard.
Nothing else is.

Tom

Re: [PATCH] not trigger pagination with dprintf

[Hui Zhu]

On Thu, Apr 25, 2013 at 4:40 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Hui" == Hui Zhu <teawater@gmail.com> writes:
>
> Hui> Will you OK if I update patch to make printf_command call ui_printf
> Hui> with filter is 0 just with dprintf?
>
> I just don't understand why dprintf should be special in this regard.
> Nothing else is.

Because as the Marc said in bugzilla, when pagination is triggered,
inferior execution will be interrupted until the user answers the
pagination prompt.
And dptintf breakpoint call printf in its commands.  So I want to make
dprintf can handle it.

Thanks,
Hui

>
> Tom

Re: [PATCH] not trigger pagination with dprintf

[Yao Qi]

On 04/26/2013 11:35 AM, Hui Zhu wrote:
> Because as the Marc said in bugzilla, when pagination is triggered,
> inferior execution will be interrupted until the user answers the
> pagination prompt.
> And dptintf breakpoint call printf in its commands.  So I want to make
> dprintf can handle it.

Instead of fixing this problem, could we document that user has to 
disable pagination when dprintf breakpoint is used?  Something similar 
is documented for non-stop here 
<http://sourceware.org/gdb/onlinedocs/gdb/Non_002dStop-Mode.html>

      # If using the CLI, pagination breaks non-stop.
      set pagination off
-- 
Yao (齐尧)

Re: [PATCH] not trigger pagination with dprintf

[Tom Tromey]

Hui> Because as the Marc said in bugzilla, when pagination is triggered,
Hui> inferior execution will be interrupted until the user answers the
Hui> pagination prompt.  And dptintf breakpoint call printf in its
Hui> commands.  So I want to make dprintf can handle it.

If pagination from a breakpoint's commands really breaks gdb, then it
should be disabled universally while in "commands", not just for
dprintf.

If it doesn't break gdb, well, then it seems like it is what the user
asked for.

Tom

Re: [PATCH] not trigger pagination with dprintf

[Hui Zhu]

On Sat, Apr 27, 2013 at 4:30 AM, Tom Tromey <tromey@redhat.com> wrote:
> Hui> Because as the Marc said in bugzilla, when pagination is triggered,
> Hui> inferior execution will be interrupted until the user answers the
> Hui> pagination prompt.  And dptintf breakpoint call printf in its
> Hui> commands.  So I want to make dprintf can handle it.
>
> If pagination from a breakpoint's commands really breaks gdb, then it
> should be disabled universally while in "commands", not just for
> dprintf.
>
> If it doesn't break gdb, well, then it seems like it is what the user
> asked for.

Hi Tom,

I post a new patch that temporarily set pagination_enabled to 0 in the
begin of commands execution function bpstat_do_actions to close
pagination.
Then all the commands of breakpoint will not trigger pagination.

If you think this patch is OK,  I will update patch for bug 15075
http://sourceware.org/ml/gdb-patches/2013-04/msg00711.html temporarily
set pagination_enabled to 0 too.

Thanks,
Hui

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

	PR gdb/15182
	* breakpoint.c (bpstat_do_actions): Temporarily set
	pagination_enabled to 0.

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -4374,7 +4374,11 @@ void
 bpstat_do_actions (void)
 {
   struct cleanup *cleanup_if_error = make_bpstat_clear_actions_cleanup ();
+  struct cleanup *back_to;
+  extern int pagination_enabled;

+  back_to = make_cleanup_restore_integer (&pagination_enabled);
+  pagination_enabled = 0;
   /* Do any commands attached to breakpoint we are stopped at.  */
   while (!ptid_equal (inferior_ptid, null_ptid)
 	 && target_has_execution
@@ -4387,6 +4391,7 @@ bpstat_do_actions (void)
     if (!bpstat_do_actions_1 (&inferior_thread ()->control.stop_bpstat))
       break;

+  do_cleanups (back_to);
   discard_cleanups (cleanup_if_error);
 }

Re: [PATCH] not trigger pagination with dprintf

[Doug Evans]

On Mon, Apr 29, 2013 at 5:57 PM, Hui Zhu <teawater@gmail.com> wrote:
> On Sat, Apr 27, 2013 at 4:30 AM, Tom Tromey <tromey@redhat.com> wrote:
>> Hui> Because as the Marc said in bugzilla, when pagination is triggered,
>> Hui> inferior execution will be interrupted until the user answers the
>> Hui> pagination prompt.  And dptintf breakpoint call printf in its
>> Hui> commands.  So I want to make dprintf can handle it.
>>
>> If pagination from a breakpoint's commands really breaks gdb, then it
>> should be disabled universally while in "commands", not just for
>> dprintf.
>>
>> If it doesn't break gdb, well, then it seems like it is what the user
>> asked for.
>
> Hi Tom,
>
> I post a new patch that temporarily set pagination_enabled to 0 in the
> begin of commands execution function bpstat_do_actions to close
> pagination.
> Then all the commands of breakpoint will not trigger pagination.
>
> If you think this patch is OK,  I will update patch for bug 15075
> http://sourceware.org/ml/gdb-patches/2013-04/msg00711.html temporarily
> set pagination_enabled to 0 too.
>
> Thanks,
> Hui
>
> 2013-04-30  Hui Zhu  <hui@codesourcery.com>
>
>         PR gdb/15182
>         * breakpoint.c (bpstat_do_actions): Temporarily set
>         pagination_enabled to 0.
>
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -4374,7 +4374,11 @@ void
>  bpstat_do_actions (void)
>  {
>    struct cleanup *cleanup_if_error = make_bpstat_clear_actions_cleanup ();
> +  struct cleanup *back_to;
> +  extern int pagination_enabled;
>
> +  back_to = make_cleanup_restore_integer (&pagination_enabled);
> +  pagination_enabled = 0;
>    /* Do any commands attached to breakpoint we are stopped at.  */
>    while (!ptid_equal (inferior_ptid, null_ptid)
>          && target_has_execution
> @@ -4387,6 +4391,7 @@ bpstat_do_actions (void)
>      if (!bpstat_do_actions_1 (&inferior_thread ()->control.stop_bpstat))
>        break;
>
> +  do_cleanups (back_to);
>    discard_cleanups (cleanup_if_error);
>  }

*If* we go this route, then this will require a NEWS item and a doc
addition to the breakpoint commands section of the manual to tell
users this is happening (and a note saying that any change in
pagination done by breakpoint commands is reverted when the commands
end).

But I wouldn't submit a revised patch until we decide this is what we
want to do.

Re: [PATCH] not trigger pagination with dprintf

[Hui Zhu]

On Sat, May 4, 2013 at 12:41 PM, Doug Evans <dje@google.com> wrote:
> On Mon, Apr 29, 2013 at 5:57 PM, Hui Zhu <teawater@gmail.com> wrote:
>> On Sat, Apr 27, 2013 at 4:30 AM, Tom Tromey <tromey@redhat.com> wrote:
>>> Hui> Because as the Marc said in bugzilla, when pagination is triggered,
>>> Hui> inferior execution will be interrupted until the user answers the
>>> Hui> pagination prompt.  And dptintf breakpoint call printf in its
>>> Hui> commands.  So I want to make dprintf can handle it.
>>>
>>> If pagination from a breakpoint's commands really breaks gdb, then it
>>> should be disabled universally while in "commands", not just for
>>> dprintf.
>>>
>>> If it doesn't break gdb, well, then it seems like it is what the user
>>> asked for.
>>
>> Hi Tom,
>>
>> I post a new patch that temporarily set pagination_enabled to 0 in the
>> begin of commands execution function bpstat_do_actions to close
>> pagination.
>> Then all the commands of breakpoint will not trigger pagination.
>>
>> If you think this patch is OK,  I will update patch for bug 15075
>> http://sourceware.org/ml/gdb-patches/2013-04/msg00711.html temporarily
>> set pagination_enabled to 0 too.
>>
>> Thanks,
>> Hui
>>
>> 2013-04-30  Hui Zhu  <hui@codesourcery.com>
>>
>>         PR gdb/15182
>>         * breakpoint.c (bpstat_do_actions): Temporarily set
>>         pagination_enabled to 0.
>>
>> --- a/gdb/breakpoint.c
>> +++ b/gdb/breakpoint.c
>> @@ -4374,7 +4374,11 @@ void
>>  bpstat_do_actions (void)
>>  {
>>    struct cleanup *cleanup_if_error = make_bpstat_clear_actions_cleanup ();
>> +  struct cleanup *back_to;
>> +  extern int pagination_enabled;
>>
>> +  back_to = make_cleanup_restore_integer (&pagination_enabled);
>> +  pagination_enabled = 0;
>>    /* Do any commands attached to breakpoint we are stopped at.  */
>>    while (!ptid_equal (inferior_ptid, null_ptid)
>>          && target_has_execution
>> @@ -4387,6 +4391,7 @@ bpstat_do_actions (void)
>>      if (!bpstat_do_actions_1 (&inferior_thread ()->control.stop_bpstat))
>>        break;
>>
>> +  do_cleanups (back_to);
>>    discard_cleanups (cleanup_if_error);
>>  }
>
> *If* we go this route, then this will require a NEWS item and a doc
> addition to the breakpoint commands section of the manual to tell
> users this is happening (and a note saying that any change in
> pagination done by breakpoint commands is reverted when the commands
> end).
>
> But I wouldn't submit a revised patch until we decide this is what we
> want to do.

Hi Doug,

Thanks for your remind.

I post new patches include patch for doc.

Thanks,
Hui

2013-05-07  Hui Zhu  <hui@codesourcery.com>

	PR gdb/15182
	* breakpoint.c (bpstat_do_actions): Temporarily set
	pagination_enabled to 0.

2013-05-07  Hui Zhu  <hui@codesourcery.com>

	PR gdb/15182
	* gdb.texinfo (Breakpoint Command Lists): Add introduce about auto
	set pagination off.

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -4374,7 +4374,11 @@ void
 bpstat_do_actions (void)
 {
   struct cleanup *cleanup_if_error = make_bpstat_clear_actions_cleanup ();
+  struct cleanup *back_to;
+  extern int pagination_enabled;

+  back_to = make_cleanup_restore_integer (&pagination_enabled);
+  pagination_enabled = 0;
   /* Do any commands attached to breakpoint we are stopped at.  */
   while (!ptid_equal (inferior_ptid, null_ptid)
 	 && target_has_execution
@@ -4387,6 +4391,7 @@ bpstat_do_actions (void)
     if (!bpstat_do_actions_1 (&inferior_thread ()->control.stop_bpstat))
       break;

+  do_cleanups (back_to);
   discard_cleanups (cleanup_if_error);
 }



--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -51,6 +51,9 @@ show remote trace-status-packet
   ** The -trace-save MI command can optionally save trace buffer in Common
      Trace Format now.

+* When GDB executes the commands of any breakpoint, the GDB output
+  pagination will be auto set to off.
+
 *** Changes in GDB 7.6

 * Target record has been renamed to record-full.
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -4619,6 +4619,9 @@ commands to execute when your program st
 example, you might want to print the values of certain expressions, or
 enable other breakpoints.

+When @value{GDBN} executes the commands of any breakpoint,
+the @value{GDBN} output pagination will be auto set to off.
+
 @table @code
 @kindex commands
 @kindex end@r{ (breakpoint commands)}

Re: [PATCH] not trigger pagination with dprintf

[Eli Zaretskii]

> From: Hui Zhu <teawater@gmail.com>
> Date: Tue, 7 May 2013 10:29:39 +0800
> Cc: Tom Tromey <tromey@redhat.com>, gdb-patches ml <gdb-patches@sourceware.org>, 
> 	Eli Zaretskii <eliz@gnu.org>
> 
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -51,6 +51,9 @@ show remote trace-status-packet
>    ** The -trace-save MI command can optionally save trace buffer in Common
>       Trace Format now.
> 
> +* When GDB executes the commands of any breakpoint, the GDB output
> +  pagination will be auto set to off.
                        ^^^^
"automatically"

> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -4619,6 +4619,9 @@ commands to execute when your program st
>  example, you might want to print the values of certain expressions, or
>  enable other breakpoints.
> 
> +When @value{GDBN} executes the commands of any breakpoint,
> +the @value{GDBN} output pagination will be auto set to off.
                                              ^^^^
Same here.

OK with these changes.

Thanks.

Re: [PATCH] not trigger pagination with dprintf

[Doug Evans]

On Tue, May 7, 2013 at 9:38 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Hui Zhu <teawater@gmail.com>
>> Date: Tue, 7 May 2013 10:29:39 +0800
>> Cc: Tom Tromey <tromey@redhat.com>, gdb-patches ml <gdb-patches@sourceware.org>,
>>       Eli Zaretskii <eliz@gnu.org>
>>
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -51,6 +51,9 @@ show remote trace-status-packet
>>    ** The -trace-save MI command can optionally save trace buffer in Common
>>       Trace Format now.
>>
>> +* When GDB executes the commands of any breakpoint, the GDB output
>> +  pagination will be auto set to off.
>                         ^^^^
> "automatically"

I would add "... set to off for the duration of the breakpoint commands."
or something like that.

>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -4619,6 +4619,9 @@ commands to execute when your program st
>>  example, you might want to print the values of certain expressions, or
>>  enable other breakpoints.
>>
>> +When @value{GDBN} executes the commands of any breakpoint,
>> +the @value{GDBN} output pagination will be auto set to off.
>                                               ^^^^
> Same here.

nit: I previously mentioned adding a note saying that any change to
pagination done by the user during breakpoint commands is reverted,
but that was left out.

> OK with these changes.
>
> Thanks.

Plus, for reference sake, we still haven't decided to go this route.
I'm kinda on the fence, and I'd like to hear what other GMs think.

[I realize you're just reviewing the doc parts (and thanks for that).]

Re: [PATCH] not trigger pagination with dprintf

[Tom Tromey]

>>>>> "Doug" == Doug Evans <dje@google.com> writes:

Doug> I'm kinda on the fence, and I'd like to hear what other GMs think.

I still don't understand what is wrong with asking the user to disable
pagination if that is what is desired.  Maybe the user actually wants
pagination sometimes.

Tom

Re: [PATCH] not trigger pagination with dprintf

[Doug Evans]

On Tue, May 7, 2013 at 11:08 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Doug" == Doug Evans <dje@google.com> writes:
>
> Doug> I'm kinda on the fence, and I'd like to hear what other GMs think.
>
> I still don't understand what is wrong with asking the user to disable
> pagination if that is what is desired.  Maybe the user actually wants
> pagination sometimes.

Me neither.
For the sake of discussion,
One could turn it around, however, and phrase the question as
"What should the default be while executing breakpoint commands?"

Re: [PATCH] not trigger pagination with dprintf

[Eli Zaretskii]

> Date: Tue, 7 May 2013 11:10:25 -0700
> From: Doug Evans <dje@google.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, Hui Zhu <teawater@gmail.com>, 
> 	gdb-patches <gdb-patches@sourceware.org>
> 
> On Tue, May 7, 2013 at 11:08 AM, Tom Tromey <tromey@redhat.com> wrote:
> >>>>>> "Doug" == Doug Evans <dje@google.com> writes:
> >
> > Doug> I'm kinda on the fence, and I'd like to hear what other GMs think.
> >
> > I still don't understand what is wrong with asking the user to disable
> > pagination if that is what is desired.  Maybe the user actually wants
> > pagination sometimes.
> 
> Me neither.
> For the sake of discussion,
> One could turn it around, however, and phrase the question as
> "What should the default be while executing breakpoint commands?"

What we do today, I guess, which means ON.

Re: [PATCH] not trigger pagination with dprintf

[Hui Zhu]

On Wed, May 8, 2013 at 2:28 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Tue, 7 May 2013 11:10:25 -0700
>> From: Doug Evans <dje@google.com>
>> Cc: Eli Zaretskii <eliz@gnu.org>, Hui Zhu <teawater@gmail.com>,
>>       gdb-patches <gdb-patches@sourceware.org>
>>
>> On Tue, May 7, 2013 at 11:08 AM, Tom Tromey <tromey@redhat.com> wrote:
>> >>>>>> "Doug" == Doug Evans <dje@google.com> writes:
>> >
>> > Doug> I'm kinda on the fence, and I'd like to hear what other GMs think.
>> >
>> > I still don't understand what is wrong with asking the user to disable
>> > pagination if that is what is desired.  Maybe the user actually wants
>> > pagination sometimes.
>>
>> Me neither.
>> For the sake of discussion,
>> One could turn it around, however, and phrase the question as
>> "What should the default be while executing breakpoint commands?"
>
> What we do today, I guess, which means ON.

What I thought is:
Plan A, post "set pagination on" to 15182 as workaround.  And update
doc add some introduce to dprintf part about that if need.

Plan B, add a switch to current patch that can switch on and off of
pagination with commands.  It also need update doc for that.

I prefer A.  What about you?

Thanks,
Hui