[patch] Change trace buffer size

[patch] Change trace buffer size

[Abid, Hafiz]

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


Hi All,
This patch adds support for QTBuffer:size in gdbserver and adds  
commands in gdb to use this packet to change the trace buffer size.   
The default value of the buffer size in gdb is -1 which means that  
target will use whatever size it likes.

I notice that there are some tests that are using "maint packet" to  
send QTBuffer:size. I was not sure what to do with them so I have left  
them alone. If reviewers suggest, I can update/remove them.

Regards,
Abid

2012-02-18  Stan Shebs  <stan@codesourcery.com>
	    Hafiz Abid Qadeer  <abidh@codesourcery.com>

	gdb/
	* NEWS: Mention set and show trace-buffer-size commands.
	* target.h (struct target_ops): New method
	to_set_trace_buffer_size.
	(target_set_trace_buffer_size): New macro.
	* target.c (update_current_target): Set up new method.
	* tracepoint.c (trace_buffer_size): New global.
	(start_tracing): Send it to the target.
	(set_trace_buffer_size): New function.
	(_initialize_tracepoint): Add new setshow for trace-buffer-size.
	* remote.c (remote_set_trace_buffer_size): New function.
	(_initialize_remote): Use it.
	(remote_set_trace_notes): Handle no-op case better.

	gdb/gdbserver/
	* tracepoint.c (trace_buffer_size): New global.
	(init_trace_buffer): Change to one-argument function, add
	realloc option.
	(handle_tracepoint_general_set): Call cmd_bigqtbuffer_size to
	handle QTBuffer:size packet.
	(cmd_bigqtbuffer_size): New function.
	(initialize_tracepoint): Default to a smaller buffer.

	gdb/doc/
	* gdb.texinfo (Starting and Stopping Trace Experiments):  
Document
	trace-buffer-size set and show commands.
	(Tracepoint Packets): Document QTBuffer:size .

	gdb/testsuite/
	* gdb.trace/trace-buffer-size.exp: New file.
	* gdb.trace/trace-buffer-size.c: New file.

[-- Attachment #2: trace-buffer-size.patch --]
[-- Type: text/x-patch, Size: 13677 bytes --]

diff --git a/gdb/NEWS b/gdb/NEWS
index 0877aa2..ea565ea 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -104,6 +104,10 @@ set debug notification
 show debug notification
   Control display of debugging info for async remote notification.
 
+set trace-buffer-size
+show trace-buffer-size
+  Request target to change the size of trace buffer.
+
 * Removed commands
 
   ** For the Renesas Super-H architecture, the "regs" command has been removed
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index e8ac8c5..2a924e7 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -11729,6 +11729,25 @@ for instance if you are looking at frames from a trace file.
 @end table
 
 @table @code
+@item set trace-buffer-size @var{n}
+@kindex set trace-buffer-size
+Request that the target use a trace buffer of @var{n} bytes.  Not all
+targets will honor the request; they may have a compiled-in size for
+the trace buffer, or some other limitation.  Set to a value of
+@code{-1} to let the target use whatever size it likes.  This is also
+the default.
+
+@item show trace-buffer-size
+@kindex show trace-buffer-size
+Show the current requested size for the trace buffer.  Note that this
+will only match the actual size if the target supports size-setting,
+and was able to handle the requested size.  For instance, if the
+target can only change buffer size between runs, this variable will
+not reflect until the next run starts.  Use @code{tstatus} to get a
+report of the actual buffer size.
+@end table
+
+@table @code
 @item set trace-user @var{text}
 @kindex set trace-user
 
@@ -38461,6 +38480,10 @@ available.
 This packet directs the target to use a circular trace buffer if
 @var{value} is 1, or a linear buffer if the value is 0.
 
+@item QTBuffer:size:@var{size}
+This packet directs the target to make the trace buffer be of size
+@var{size} if possible.
+
 @item QTNotes:@r{[}@var{type}:@var{text}@r{]}@r{[};@var{type}:@var{text}@r{]}@dots{}
 @cindex @samp{QTNotes} packet
 This packet adds optional textual notes to the trace run.  Allowable
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 95c55ad..5f88264 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -992,6 +992,8 @@ int current_traceframe = -1;
 static int circular_trace_buffer;
 #endif
 
+static int trace_buffer_size;
+
 /* Pointer to the block of memory that traceframes all go into.  */
 
 static unsigned char *trace_buffer_lo;
@@ -1478,9 +1480,16 @@ clear_inferior_trace_buffer (void)
 #endif
 
 static void
-init_trace_buffer (unsigned char *buf, int bufsize)
+init_trace_buffer (int bufsize)
 {
-  trace_buffer_lo = buf;
+  trace_buffer_size = bufsize;
+
+  /* If we already have a trace buffer, try realloc'ing.  */
+  if (trace_buffer_lo)
+    trace_buffer_lo = xrealloc (trace_buffer_lo, bufsize);
+  else
+    trace_buffer_lo = xmalloc (bufsize);
+
   trace_buffer_hi = trace_buffer_lo + bufsize;
 
   clear_trace_buffer ();
@@ -4021,6 +4030,36 @@ cmd_bigqtbuffer_circular (char *own_buf)
 }
 
 static void
+cmd_bigqtbuffer_size (char *own_buf)
+{
+  ULONGEST val;
+  LONGEST sval;
+  char *packet = own_buf;
+
+  packet += strlen ("QTBuffer:size:");
+
+  unpack_varlen_hex (packet, &val);
+  sval = (LONGEST)val;
+  /* If we don't care about the size, or the size is unchanged,
+     all is happy and nothing to do.  */
+  if (sval < 0 || sval == trace_buffer_size)
+    {
+      write_ok (own_buf);
+      return;
+    }
+  /* Right now we can't change the size during a tracing run.  */
+  if (tracing)
+    {
+       write_enn (own_buf);
+       return;
+    }
+  init_trace_buffer (sval);
+  trace_debug ("Trace buffer is now %s bytes",
+                plongest (trace_buffer_size));
+  write_ok (own_buf);
+}
+
+static void
 cmd_qtnotes (char *own_buf)
 {
   size_t nbytes;
@@ -4144,6 +4183,11 @@ handle_tracepoint_general_set (char *packet)
       cmd_bigqtbuffer_circular (packet);
       return 1;
     }
+  else if (strncmp ("QTBuffer:size:", packet, strlen ("QTBuffer:size:")) == 0)
+    {
+      cmd_bigqtbuffer_size (packet);
+      return 1;
+    }
   else if (strncmp ("QTNotes:", packet, strlen ("QTNotes:")) == 0)
     {
       cmd_qtnotes (packet);
@@ -7229,10 +7273,9 @@ get_timestamp (void)
 void
 initialize_tracepoint (void)
 {
-  /* There currently no way to change the buffer size.  */
-  const int sizeOfBuffer = 5 * 1024 * 1024;
-  unsigned char *buf = xmalloc (sizeOfBuffer);
-  init_trace_buffer (buf, sizeOfBuffer);
+  /* Start with a size that's not too large.  */
+  const int sizeOfBuffer = 1024 * 1024;
+  init_trace_buffer (sizeOfBuffer);
 
   /* Wire trace state variable 1 to be the timestamp.  This will be
      uploaded to GDB upon connection and become one of its trace state
diff --git a/gdb/remote.c b/gdb/remote.c
index 88a57c8..e0ddaf9 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -11042,6 +11042,23 @@ remote_get_min_fast_tracepoint_insn_len (void)
     }
 }
 
+static void
+remote_set_trace_buffer_size (LONGEST val)
+{
+  struct remote_state *rs = get_remote_state ();
+  char *p, *reply;
+
+  sprintf (rs->buf, "QTBuffer:size:");
+  p = rs->buf + strlen (rs->buf);
+  p += hexnumstr (p, (ULONGEST) val);
+  putpkt (rs->buf);
+  reply = remote_get_noisy_reply (&target_buf, &target_buf_size);
+  if (*reply == '\0')
+    error (_("Target does not support setting trace buffer size."));
+  if (strcmp (reply, "OK") != 0)
+    error (_("Bogus reply from target: %s"), reply);
+}
+
 static int
 remote_set_trace_notes (char *user, char *notes, char *stop_notes)
 {
@@ -11050,6 +11067,7 @@ remote_set_trace_notes (char *user, char *notes, char *stop_notes)
   char *buf = rs->buf;
   char *endbuf = rs->buf + get_remote_packet_size ();
   int nbytes;
+  int any = 0;
 
   buf += xsnprintf (buf, endbuf - buf, "QTNotes:");
   if (user)
@@ -11058,6 +11076,7 @@ remote_set_trace_notes (char *user, char *notes, char *stop_notes)
       nbytes = bin2hex (user, buf, 0);
       buf += 2 * nbytes;
       *buf++ = ';';
+      any = 1;
     }
   if (notes)
     {
@@ -11065,6 +11084,7 @@ remote_set_trace_notes (char *user, char *notes, char *stop_notes)
       nbytes = bin2hex (notes, buf, 0);
       buf += 2 * nbytes;
       *buf++ = ';';
+      any = 1;
     }
   if (stop_notes)
     {
@@ -11072,7 +11092,10 @@ remote_set_trace_notes (char *user, char *notes, char *stop_notes)
       nbytes = bin2hex (stop_notes, buf, 0);
       buf += 2 * nbytes;
       *buf++ = ';';
+      any = 1;
     }
+    if (any == 0)
+      return 0;
   /* Ensure the buffer is terminated.  */
   *buf = '\0';
 
@@ -11219,6 +11242,7 @@ Specify the serial device it is connected to\n\
   remote_ops.to_get_min_fast_tracepoint_insn_len = remote_get_min_fast_tracepoint_insn_len;
   remote_ops.to_set_disconnected_tracing = remote_set_disconnected_tracing;
   remote_ops.to_set_circular_trace_buffer = remote_set_circular_trace_buffer;
+  remote_ops.to_set_trace_buffer_size = remote_set_trace_buffer_size;
   remote_ops.to_set_trace_notes = remote_set_trace_notes;
   remote_ops.to_core_of_thread = remote_core_of_thread;
   remote_ops.to_verify_memory = remote_verify_memory;
diff --git a/gdb/target.c b/gdb/target.c
index 9d8bf6e..2e507e0 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -693,6 +693,7 @@ update_current_target (void)
       INHERIT (to_get_min_fast_tracepoint_insn_len, t);
       INHERIT (to_set_disconnected_tracing, t);
       INHERIT (to_set_circular_trace_buffer, t);
+      INHERIT (to_set_trace_buffer_size, t);
       INHERIT (to_set_trace_notes, t);
       INHERIT (to_get_tib_address, t);
       INHERIT (to_set_permissions, t);
@@ -912,6 +913,9 @@ update_current_target (void)
   de_fault (to_set_circular_trace_buffer,
 	    (void (*) (int))
 	    target_ignore);
+  de_fault (to_set_trace_buffer_size,
+	    (void (*) (LONGEST))
+	    target_ignore);
   de_fault (to_set_trace_notes,
 	    (int (*) (char *, char *, char *))
 	    return_zero);
diff --git a/gdb/target.h b/gdb/target.h
index 1971265..03b3d78 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -806,6 +806,7 @@ struct target_ops
        disconnection - set VAL to 1 to keep tracing, 0 to stop.  */
     void (*to_set_disconnected_tracing) (int val);
     void (*to_set_circular_trace_buffer) (int val);
+    void (*to_set_trace_buffer_size) (LONGEST val);
 
     /* Add/change textual notes about the trace run, returning 1 if
        successful, 0 otherwise.  */
@@ -1700,6 +1701,9 @@ extern char *target_fileio_read_stralloc (const char *filename);
 #define	target_set_circular_trace_buffer(val)	\
   (*current_target.to_set_circular_trace_buffer) (val)
 
+#define	target_set_trace_buffer_size(val)	\
+  (*current_target.to_set_trace_buffer_size) (val)
+
 #define	target_set_trace_notes(user,notes,stopnotes)		\
   (*current_target.to_set_trace_notes) ((user), (notes), (stopnotes))
 
diff --git a/gdb/testsuite/gdb.trace/trace-buffer-size.c b/gdb/testsuite/gdb.trace/trace-buffer-size.c
new file mode 100644
index 0000000..79a9ed1
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/trace-buffer-size.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011-2013 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+main ()
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.trace/trace-buffer-size.exp b/gdb/testsuite/gdb.trace/trace-buffer-size.exp
new file mode 100644
index 0000000..5c6c989
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/trace-buffer-size.exp
@@ -0,0 +1,40 @@
+# Copyright 1998-2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib "trace-support.exp"
+
+standard_testfile
+
+if [prepare_for_testing ${testfile}.exp $testfile $srcfile \
+        {debug nowarnings}] {
+    untested "failed to prepare for trace tests"
+    return -1
+}
+
+if ![runto_main] {
+    fail "can't run to main to check for trace support"
+    return -1
+}
+
+if ![gdb_target_supports_trace] {
+    unsupported "target does not support trace"
+    return -1;
+}
+
+set BUFFER_SIZE 1024
+gdb_test_no_output "set trace-buffer-size $BUFFER_SIZE" "Set Trace Buffer Size"
+gdb_test "show trace-buffer-size $BUFFER_SIZE" "Requested size of trace buffer is $BUFFER_SIZE.*" \
+"Show Trace Buffer Size"
+
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index ca104aa..f9320a3 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -173,6 +173,11 @@ static int disconnected_tracing;
 
 static int circular_trace_buffer;
 
+/* This variable is the requested trace buffer size, or -1 to indicate
+   that we don't care and leave it up to the target to set a size.  */
+
+static int trace_buffer_size = -1;
+
 /* Textual notes applying to the current and/or future trace runs.  */
 
 char *trace_user = NULL;
@@ -1829,6 +1834,7 @@ start_tracing (char *notes)
   /* Set some mode flags.  */
   target_set_disconnected_tracing (disconnected_tracing);
   target_set_circular_trace_buffer (circular_trace_buffer);
+  target_set_trace_buffer_size (trace_buffer_size);
 
   if (!notes)
     notes = trace_notes;
@@ -3231,6 +3237,13 @@ set_circular_trace_buffer (char *args, int from_tty,
 }
 
 static void
+set_trace_buffer_size (char *args, int from_tty,
+			   struct cmd_list_element *c)
+{
+  target_set_trace_buffer_size (trace_buffer_size);
+}
+
+static void
 set_trace_user (char *args, int from_tty,
 		struct cmd_list_element *c)
 {
@@ -5405,6 +5418,16 @@ up and stopping the trace run."),
 			   &setlist,
 			   &showlist);
 
+  add_setshow_zinteger_cmd ("trace-buffer-size", no_class,
+			    &trace_buffer_size, _("\
+Set requested size of trace buffer."), _("\
+Show requested size of trace buffer."), _("\
+Use this to choose a size for the trace buffer.  Some targets\n\
+may have fixed or limited buffer sizes.  A value of -1 disables\n\
+any attempt to set the buffer size and lets the target choose."),
+			    set_trace_buffer_size, NULL,
+			    &setlist, &showlist);
+
   add_setshow_string_cmd ("trace-user", class_trace,
 			  &trace_user, _("\
 Set the user name to use for current and future trace runs"), _("\

Re: [patch] Change trace buffer size

[Eli Zaretskii]

> Date: Mon, 18 Feb 2013 18:13:36 +0000
> From: "Abid, Hafiz" <hafiz_abid@mentor.com>
> CC: <stan@codesourcery.com>
> 
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -104,6 +104,10 @@ set debug notification
>  show debug notification
>    Control display of debugging info for async remote notification.
>  
> +set trace-buffer-size
> +show trace-buffer-size
> +  Request target to change the size of trace buffer.
> +

OK for this part.

> +@item show trace-buffer-size
> +@kindex show trace-buffer-size
> +Show the current requested size for the trace buffer.  Note that this
> +will only match the actual size if the target supports size-setting,
> +and was able to handle the requested size.  For instance, if the
> +target can only change buffer size between runs, this variable will
> +not reflect until the next run starts.  Use @code{tstatus} to get a
   ^^^^^^^^^^^^^^^^^
"not reflect the change until ..."

The documentation parts are OK with this fixed.

Re: [patch] Change trace buffer size

[Yao Qi]

On 02/19/2013 02:13 AM, Abid, Hafiz wrote:

Hi Abid,

> diff --git a/gdb/NEWS b/gdb/NEWS
> index 0877aa2..ea565ea 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -104,6 +104,10 @@ set debug notification
>   show debug notification
>     Control display of debugging info for async remote notification.
>
> +set trace-buffer-size
> +show trace-buffer-size
> +  Request target to change the size of trace buffer.
> +
>   * Removed commands

We may mention the new remote packet in NEWS.

> diff --git a/gdb/remote.c b/gdb/remote.c
> index 88a57c8..e0ddaf9 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -11042,6 +11042,23 @@ remote_get_min_fast_tracepoint_insn_len (void)
>       }
>   }
>
> +static void
> +remote_set_trace_buffer_size (LONGEST val)
> +{
> +  struct remote_state *rs = get_remote_state ();
> +  char *p, *reply;
> +
> +  sprintf (rs->buf, "QTBuffer:size:");
> +  p = rs->buf + strlen (rs->buf);
> +  p += hexnumstr (p, (ULONGEST) val);
> +  putpkt (rs->buf);
> +  reply = remote_get_noisy_reply (&target_buf, &target_buf_size);
> +  if (*reply == '\0')
> +    error (_("Target does not support setting trace buffer size."));

We may need a warning instead of an error here.

> +  if (strcmp (reply, "OK") != 0)
> +    error (_("Bogus reply from target: %s"), reply);
> +}
> +
>   static int
>   remote_set_trace_notes (char *user, char *notes, char *stop_notes)
>   {
> @@ -11050,6 +11067,7 @@ remote_set_trace_notes (char *user, char *notes, char *stop_notes)
>     char *buf = rs->buf;
>     char *endbuf = rs->buf + get_remote_packet_size ();
>     int nbytes;
> +  int any = 0;
>
>     buf += xsnprintf (buf, endbuf - buf, "QTNotes:");
>     if (user)
> @@ -11058,6 +11076,7 @@ remote_set_trace_notes (char *user, char *notes, char *stop_notes)
>         nbytes = bin2hex (user, buf, 0);
>         buf += 2 * nbytes;
>         *buf++ = ';';
> +      any = 1;
>       }
>     if (notes)
>       {
> @@ -11065,6 +11084,7 @@ remote_set_trace_notes (char *user, char *notes, char *stop_notes)
>         nbytes = bin2hex (notes, buf, 0);
>         buf += 2 * nbytes;
>         *buf++ = ';';
> +      any = 1;
>       }
>     if (stop_notes)
>       {
> @@ -11072,7 +11092,10 @@ remote_set_trace_notes (char *user, char *notes, char *stop_notes)
>         nbytes = bin2hex (stop_notes, buf, 0);
>         buf += 2 * nbytes;
>         *buf++ = ';';
> +      any = 1;
>       }
> +    if (any == 0)
> +      return 0;
>     /* Ensure the buffer is terminated.  */
>     *buf = '\0';
>

I think this chunk is correct, but is it really related to this patch?

> diff --git a/gdb/target.c b/gdb/target.c
> index 9d8bf6e..2e507e0 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -693,6 +693,7 @@ update_current_target (void)
>         INHERIT (to_get_min_fast_tracepoint_insn_len, t);
>         INHERIT (to_set_disconnected_tracing, t);
>         INHERIT (to_set_circular_trace_buffer, t);
> +      INHERIT (to_set_trace_buffer_size, t);
>         INHERIT (to_set_trace_notes, t);
>         INHERIT (to_get_tib_address, t);
>         INHERIT (to_set_permissions, t);
> @@ -912,6 +913,9 @@ update_current_target (void)
>     de_fault (to_set_circular_trace_buffer,
>   	    (void (*) (int))
>   	    target_ignore);
> +  de_fault (to_set_trace_buffer_size,
> +	    (void (*) (LONGEST))
> +	    target_ignore);

I am wondering 'tcomplain' may be better than 'target_ignore'.

>     de_fault (to_set_trace_notes,
>   	    (int (*) (char *, char *, char *))
>   	    return_zero);
> diff --git a/gdb/target.h b/gdb/target.h
> index 1971265..03b3d78 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -806,6 +806,7 @@ struct target_ops
>          disconnection - set VAL to 1 to keep tracing, 0 to stop.  */
>       void (*to_set_disconnected_tracing) (int val);
>       void (*to_set_circular_trace_buffer) (int val);
> +    void (*to_set_trace_buffer_size) (LONGEST val);

Comments on this field are needed, IMO.

> diff --git a/gdb/testsuite/gdb.trace/trace-buffer-size.c b/gdb/testsuite/gdb.trace/trace-buffer-size.c
> new file mode 100644
> index 0000000..79a9ed1
> --- /dev/null
> +++ b/gdb/testsuite/gdb.trace/trace-buffer-size.c
> @@ -0,0 +1,22 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2011-2013 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see<http://www.gnu.org/licenses/>.  */
> +
> +int
> +main ()
> +{
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.trace/trace-buffer-size.exp b/gdb/testsuite/gdb.trace/trace-buffer-size.exp
> new file mode 100644
> index 0000000..5c6c989
> --- /dev/null
> +++ b/gdb/testsuite/gdb.trace/trace-buffer-size.exp
> @@ -0,0 +1,40 @@
> +# Copyright 1998-2013 Free Software Foundation, Inc.
                ^^^^ 2011? to align with trace-buffer-size.c
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see<http://www.gnu.org/licenses/>.
> +
> +load_lib "trace-support.exp"
> +
> +standard_testfile
> +
> +if [prepare_for_testing ${testfile}.exp $testfile $srcfile \
> +        {debug nowarnings}] {
> +    untested "failed to prepare for trace tests"
> +    return -1
> +}
> +
> +if ![runto_main] {
> +    fail "can't run to main to check for trace support"
> +    return -1
> +}
> +
> +if ![gdb_target_supports_trace] {
> +    unsupported "target does not support trace"
> +    return -1;
> +}
> +
> +set BUFFER_SIZE 1024
> +gdb_test_no_output "set trace-buffer-size $BUFFER_SIZE" "Set Trace Buffer Size"
> +gdb_test "show trace-buffer-size $BUFFER_SIZE" "Requested size of trace buffer is $BUFFER_SIZE.*" \
> +"Show Trace Buffer Size"
> +

This only tests the commands on setting and showing the trace buffer 
size.  Do you think we need also test the affected behaviour of tracing? 
  I mean, setting the trace buffer size to a really small number, and 
see tracing is stopped quickly as trace buffer becomes full?

-- 
Yao (齐尧)

Re: [patch] Change trace buffer size

[Pedro Alves]

On 02/18/2013 06:13 PM, Abid, Hafiz wrote:
>
> Hi All, This patch adds support for QTBuffer:size in gdbserver
> and adds commands in gdb to use this packet to change the
> trace buffer size.  The default value of the buffer size in
> gdb is -1 which means that target will use whatever size it likes.
>
> I notice that there are some tests that are using "maint packet"
> to send QTBuffer:size. I was not sure what to do with them so
> I have left them alone. If reviewers suggest, I can update/remove them.

Yeah, http://sourceware.org/ml/gdb-patches/2012-02/msg00303.html

So, previously, because gdbserver didn't implement this, some
tests were skipped.  What about after the patch?

On 02/18/2013 06:13 PM, Abid, Hafiz wrote:
> +@item QTBuffer:size:@var{size}
> +This packet directs the target to make the trace buffer be of size
> +@var{size} if possible.
> +

This misses explaining the "-1" magic.

> +static int trace_buffer_size;

Should be a ssize_t, or LONGEST.  The latter is probably
better/simpler in this case.

> +init_trace_buffer (int bufsize)

This "int" too.

>  {
> -  trace_buffer_lo = buf;
> +  trace_buffer_size = bufsize;
> +
> +  /* If we already have a trace buffer, try realloc'ing.  */
> +  if (trace_buffer_lo)
> +    trace_buffer_lo = xrealloc (trace_buffer_lo, bufsize);
> +  else
> +    trace_buffer_lo = xmalloc (bufsize);

xrealloc(NULL, bufsize) is equivalent to xmalloc(bufsize),
so the if is unnecessary.  Write only:

    trace_buffer_lo = xrealloc (trace_buffer_lo, bufsize);


> +static void
> +remote_set_trace_buffer_size (LONGEST val)
> +{
> +  struct remote_state *rs = get_remote_state ();
> +  char *p, *reply;
> +
> +  sprintf (rs->buf, "QTBuffer:size:");
> +  p = rs->buf + strlen (rs->buf);
> +  p += hexnumstr (p, (ULONGEST) val);
> +  putpkt (rs->buf);

This should send -1 really as literal "-1",
instead of ffff..ff, which depends on the size
of ULONGEST, which may not be the same as the target's.
IOW, it's host dependent.

> +  reply = remote_get_noisy_reply (&target_buf, &target_buf_size);

We shouldn't be adding more uses of target_buf, target_buf_size.
Use:
  remote_get_noisy_reply (&rs->buf, &rs->buf_size);

and:

  if (*rs->buf == '\0')

but, OTOH, new packets should really be using packet_ok.

This:

>  static int
>  remote_set_trace_notes (char *user, char *notes, char *stop_notes)
>  {
> @@ -11050,6 +11067,7 @@ remote_set_trace_notes (char *user, char *notes, char *stop_notes)
>    char *buf = rs->buf;
>    char *endbuf = rs->buf + get_remote_packet_size ();
>    int nbytes;
> +  int any = 0;
>
>    buf += xsnprintf (buf, endbuf - buf, "QTNotes:");
>    if (user)
> @@ -11058,6 +11076,7 @@ remote_set_trace_notes (char *user, char *notes, char *stop_notes)
>        nbytes = bin2hex (user, buf, 0);
>        buf += 2 * nbytes;
>        *buf++ = ';';
> +      any = 1;
>      }
>    if (notes)
>      {
> @@ -11065,6 +11084,7 @@ remote_set_trace_notes (char *user, char *notes, char *stop_notes)
>        nbytes = bin2hex (notes, buf, 0);
>        buf += 2 * nbytes;
>        *buf++ = ';';
> +      any = 1;
>      }
>    if (stop_notes)
>      {
> @@ -11072,7 +11092,10 @@ remote_set_trace_notes (char *user, char *notes, char *stop_notes)
>        nbytes = bin2hex (stop_notes, buf, 0);
>        buf += 2 * nbytes;
>        *buf++ = ';';
> +      any = 1;
>      }
> +    if (any == 0)
> +      return 0;
>    /* Ensure the buffer is terminated.  */
>    *buf = '\0';

looks unrelated to trace buffer size.  Please split it
into a separate patch.

> +set BUFFER_SIZE 1024
> +gdb_test_no_output "set trace-buffer-size $BUFFER_SIZE" "Set Trace Buffer Size"
> +gdb_test "show trace-buffer-size $BUFFER_SIZE" "Requested size of trace buffer is $BUFFER_SIZE.*" \
> +"Show Trace Buffer Size"
> +

This test doesn't seem to really test that much.  I wonder if
you have ideas on expanding it?

> +/* This variable is the requested trace buffer size, or -1 to indicate
> +   that we don't care and leave it up to the target to set a size.  */
> +
> +static int trace_buffer_size = -1;

Should probably be a LONGEST too.  There's to reason to
actually limit to 31-bits, right?

> +  add_setshow_zinteger_cmd ("trace-buffer-size", no_class,
> +			    &trace_buffer_size, _("\

Oh, I see...  add_setshow_zinteger_cmd works with int...

-- 
Pedro Alves

Re: [patch] Change trace buffer size

[Pedro Alves]

On 02/19/2013 02:51 AM, Yao Qi wrote:
>>
> 
> We may mention the new remote packet in NEWS.

Indeed.  s/may/should/.

>>
>> @@ -912,6 +913,9 @@ update_current_target (void)
>>     de_fault (to_set_circular_trace_buffer,
>>           (void (*) (int))
>>           target_ignore);
>> +  de_fault (to_set_trace_buffer_size,
>> +        (void (*) (LONGEST))
>> +        target_ignore);
>
> I am wondering 'tcomplain' may be better than 'target_ignore'.

I think it's better to have the command behave the
same as the other related "set circular-trace-buffer"
set disconnected-tracing.  We've discussed this behavior in
the context of the patch that adds one of these other
commands (or both?).
Do you see a reason this command should behave different?
tcomplain would trigger if you used the "set ..." command
before being connected to any target (or after disconnecting).

-- 
Pedro Alves

Re: [patch] Change trace buffer size

[Yao Qi]

On 02/28/2013 12:54 AM, Pedro Alves wrote:
> I think it's better to have the command behave the
> same as the other related "set circular-trace-buffer"
> set disconnected-tracing.  We've discussed this behavior in
> the context of the patch that adds one of these other
> commands (or both?).
> Do you see a reason this command should behave different?

No, these commands should behave the same.

> tcomplain would trigger if you used the "set ..." command
> before being connected to any target (or after disconnecting).

then 'target_ignore' is fine to me.

-- 
Yao (齐尧)

Re: [patch] Change trace buffer size(v2)

[Abid, Hafiz]

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

Hi All,
Thanks for the review. I am attaching a new version with the changes.  
How does it look?

On 27/02/13 16:39:07, Pedro Alves wrote:
> On 02/18/2013 06:13 PM, Abid, Hafiz wrote:
> >
> > Hi All, This patch adds support for QTBuffer:size in gdbserver
> > and adds commands in gdb to use this packet to change the
> > trace buffer size.  The default value of the buffer size in
> > gdb is -1 which means that target will use whatever size it likes.
> >
> > I notice that there are some tests that are using "maint packet"
> > to send QTBuffer:size. I was not sure what to do with them so
> > I have left them alone. If reviewers suggest, I can update/remove  
> them.
> 
> Yeah, http://sourceware.org/ml/gdb-patches/2012-02/msg00303.html
> 
> So, previously, because gdbserver didn't implement this, some
> tests were skipped.  What about after the patch?
I run the circ.exp with it. The qtBuffer:size test work ok. I have to  
modify circ.exp a little to send literal '-1'. Also I observed that  
most of the tracing tests in circ.exp were being skipped. I had to add  
a runto_main before gdb_target_supports_trace to execute those tests. I  
will send another patch that will take care of circ.exp.
> 
> On 02/18/2013 06:13 PM, Abid, Hafiz wrote:
> > +@item QTBuffer:size:@var{size}
> > +This packet directs the target to make the trace buffer be of size
> > +@var{size} if possible.
> > +
Fixed.
> 
> This misses explaining the "-1" magic.
> 
> > +static int trace_buffer_size;
> 
> Should be a ssize_t, or LONGEST.  The latter is probably
> better/simpler in this case.
Fixed.

> 
> > +init_trace_buffer (int bufsize)
> 
> This "int" too.
Fixed.
> 
> xrealloc(NULL, bufsize) is equivalent to xmalloc(bufsize),
> so the if is unnecessary.  Write only:
> 
>     trace_buffer_lo = xrealloc (trace_buffer_lo, bufsize);
Done.

> > +static void
> > +remote_set_trace_buffer_size (LONGEST val)
> > +{
> > +  struct remote_state *rs = get_remote_state ();
> > +  char *p, *reply;
> > +
> > +  sprintf (rs->buf, "QTBuffer:size:");
> > +  p = rs->buf + strlen (rs->buf);
> > +  p += hexnumstr (p, (ULONGEST) val);
> > +  putpkt (rs->buf);
> 
> This should send -1 really as literal "-1",
> instead of ffff..ff, which depends on the size
> of ULONGEST, which may not be the same as the target's.
> IOW, it's host dependent.
Fixed.

> 
> > +  reply = remote_get_noisy_reply (&target_buf, &target_buf_size);
> 
> We shouldn't be adding more uses of target_buf, target_buf_size.
> Use:
>   remote_get_noisy_reply (&rs->buf, &rs->buf_size);
Done.
> 
> and:
> 
>   if (*rs->buf == '\0')
> 
> but, OTOH, new packets should really be using packet_ok.
> 
> This:
> looks unrelated to trace buffer size.  Please split it
> into a separate patch.
Removed from this patch. Will send in a separate patch.

> 
> > +set BUFFER_SIZE 1024
> > +gdb_test_no_output "set trace-buffer-size $BUFFER_SIZE" "Set Trace  
> Buffer Size"
> > +gdb_test "show trace-buffer-size $BUFFER_SIZE" "Requested size of  
> trace buffer is $BUFFER_SIZE.*" \
> > +"Show Trace Buffer Size"
> > +
> 
> This test doesn't seem to really test that much.  I wonder if
> you have ideas on expanding it?
I have tried to expand the test by setting the buffer size to be very  
small and then testing the buffer to be full.

> 
> > +/* This variable is the requested trace buffer size, or -1 to  
> indicate
> > +   that we don't care and leave it up to the target to set a  
> size.  */
> > +
> > +static int trace_buffer_size = -1;
> 
> Should probably be a LONGEST too.  There's to reason to
> actually limit to 31-bits, right?
> 
> > +  add_setshow_zinteger_cmd ("trace-buffer-size", no_class,
> > +			    &trace_buffer_size, _("\
> 
> Oh, I see...  add_setshow_zinteger_cmd works with int...
Observed that commands added with add_setshow_zinteger_cmd have problem  
with -1. For var_zinteger, do_set_command uses unsigned int and it ends  
up comparing 0xffffffff to INT_MAX. If it sounds like an oversight to  
you then I will send a patch. Here are those lines for a quick look.
	unsigned int val;
...
	val = parse_and_eval_long (arg);
...
	else if (val >= INT_MAX)
	  error (_("integer %u out of range"), val);

> 
> --
> Pedro Alves
> 
> 

Following changes were suggested by Yao.
> We may mention the new remote packet in NEWS.
Done.

>> +    error (_("Target does not support setting trace buffer size."));
> We may need a warning instead of an error here.
Fixed.

>> +    void (*to_set_trace_buffer_size) (LONGEST val);
> Comments on this field are needed, IMO.
Done.

Also fixed the documentation as suggested by Eli.

Regards,
Abid

2012-03-01  Stan Shebs  <stan@codesourcery.com>
	    Hafiz Abid Qadeer <abidh@codesourcery.com>

	gdb/
	* NEWS: Mention set and show trace-buffer-size commands.
	Mention new packet.
	* target.h (struct target_ops): New method
	to_set_trace_buffer_size.
	(target_set_trace_buffer_size): New macro.
	* target.c (update_current_target): Set up new method.
	* tracepoint.c (trace_buffer_size): New global.
	(start_tracing): Send it to the target.
	(set_trace_buffer_size): New function.
	(_initialize_tracepoint): Add new setshow for trace-buffer-size.
	* remote.c (remote_set_trace_buffer_size): New function.
	(_initialize_remote): Use it.

	gdb/gdbserver/
	* tracepoint.c (trace_buffer_size): New global.
	(init_trace_buffer): Change to one-argument function, add
	realloc option.
	(handle_tracepoint_general_set): Call cmd_bigqtbuffer_size to
	handle QTBuffer:size packet.
	(cmd_bigqtbuffer_size): New function.
	(initialize_tracepoint): Default to a smaller buffer.

	gdb/doc/
	* gdb.texinfo (Starting and Stopping Trace Experiments):  
Document
	trace-buffer-size set and show commands.
	(Tracepoint Packets): Document QTBuffer:size .

	gdb/testsuite/
	* gdb.trace/trace-buffer-size.exp: New file.
	* gdb.trace/trace-buffer-size.c: New file.




[-- Attachment #2: trace-buffer-size_v2.patch --]
[-- Type: text/x-patch, Size: 14095 bytes --]

diff --git a/gdb/NEWS b/gdb/NEWS
index 0877aa2..8fc433b 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -104,6 +104,10 @@ set debug notification
 show debug notification
   Control display of debugging info for async remote notification.
 
+set trace-buffer-size
+show trace-buffer-size
+  Request target to change the size of trace buffer.
+
 * Removed commands
 
   ** For the Renesas Super-H architecture, the "regs" command has been removed
@@ -159,6 +163,9 @@ show filename-display
   feature to be enabled.  For more information, see:
       http://fedoraproject.org/wiki/Features/MiniDebugInfo
 
+* New remote packets
+  A new packet has been defined to set the trace buffer size.
+
 *** Changes in GDB 7.5
 
 * GDB now supports x32 ABI.  Visit <http://sites.google.com/site/x32abi/>
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 5f39d2e..3cb24d5 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -11728,6 +11728,25 @@ for instance if you are looking at frames from a trace file.
 @end table
 
 @table @code
+@item set trace-buffer-size @var{n}
+@kindex set trace-buffer-size
+Request that the target use a trace buffer of @var{n} bytes.  Not all
+targets will honor the request; they may have a compiled-in size for
+the trace buffer, or some other limitation.  Set to a value of
+@code{-1} to let the target use whatever size it likes.  This is also
+the default.
+
+@item show trace-buffer-size
+@kindex show trace-buffer-size
+Show the current requested size for the trace buffer.  Note that this
+will only match the actual size if the target supports size-setting,
+and was able to handle the requested size.  For instance, if the
+target can only change buffer size between runs, this variable will
+not reflect the change until the next run starts.  Use @code{tstatus}
+to get a report of the actual buffer size.
+@end table
+
+@table @code
 @item set trace-user @var{text}
 @kindex set trace-user
 
@@ -38450,6 +38469,11 @@ available.
 This packet directs the target to use a circular trace buffer if
 @var{value} is 1, or a linear buffer if the value is 0.
 
+@item QTBuffer:size:@var{size}
+This packet directs the target to make the trace buffer be of size
+@var{size} if possible. A value of @code{-1} tells the target to
+use whatever size it likes.
+
 @item QTNotes:@r{[}@var{type}:@var{text}@r{]}@r{[};@var{type}:@var{text}@r{]}@dots{}
 @cindex @samp{QTNotes} packet
 This packet adds optional textual notes to the trace run.  Allowable
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 0f83ae6..c0b3bd7 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -992,6 +992,8 @@ int current_traceframe = -1;
 static int circular_trace_buffer;
 #endif
 
+static LONGEST trace_buffer_size;
+
 /* Pointer to the block of memory that traceframes all go into.  */
 
 static unsigned char *trace_buffer_lo;
@@ -1478,9 +1480,13 @@ clear_inferior_trace_buffer (void)
 #endif
 
 static void
-init_trace_buffer (unsigned char *buf, int bufsize)
+init_trace_buffer (LONGEST bufsize)
 {
-  trace_buffer_lo = buf;
+  trace_buffer_size = bufsize;
+
+  /* If we already have a trace buffer, try realloc'ing.  */
+  trace_buffer_lo = xrealloc (trace_buffer_lo, bufsize);
+
   trace_buffer_hi = trace_buffer_lo + bufsize;
 
   clear_trace_buffer ();
@@ -4020,6 +4026,42 @@ cmd_bigqtbuffer_circular (char *own_buf)
 }
 
 static void
+cmd_bigqtbuffer_size (char *own_buf)
+{
+  ULONGEST val;
+  LONGEST sval;
+  char *packet = own_buf;
+
+  packet += strlen ("QTBuffer:size:");
+
+  /* -1 is sent as literal "-1".  */
+  if (strcmp (packet, "-1") == 0)
+    sval = -1;
+  else
+    {
+      unpack_varlen_hex (packet, &val);
+      sval = (LONGEST)val;
+    }
+  /* If we don't care about the size, or the size is unchanged,
+     all is happy and nothing to do.  */
+  if (sval < 0 || sval == trace_buffer_size)
+    {
+      write_ok (own_buf);
+      return;
+    }
+  /* Right now we can't change the size during a tracing run.  */
+  if (tracing)
+    {
+       write_enn (own_buf);
+       return;
+    }
+  init_trace_buffer (sval);
+  trace_debug ("Trace buffer is now %s bytes",
+                plongest (trace_buffer_size));
+  write_ok (own_buf);
+}
+
+static void
 cmd_qtnotes (char *own_buf)
 {
   size_t nbytes;
@@ -4143,6 +4185,11 @@ handle_tracepoint_general_set (char *packet)
       cmd_bigqtbuffer_circular (packet);
       return 1;
     }
+  else if (strncmp ("QTBuffer:size:", packet, strlen ("QTBuffer:size:")) == 0)
+    {
+      cmd_bigqtbuffer_size (packet);
+      return 1;
+    }
   else if (strncmp ("QTNotes:", packet, strlen ("QTNotes:")) == 0)
     {
       cmd_qtnotes (packet);
@@ -7228,10 +7275,9 @@ get_timestamp (void)
 void
 initialize_tracepoint (void)
 {
-  /* There currently no way to change the buffer size.  */
-  const int sizeOfBuffer = 5 * 1024 * 1024;
-  unsigned char *buf = xmalloc (sizeOfBuffer);
-  init_trace_buffer (buf, sizeOfBuffer);
+  /* Start with a size that's not too large.  */
+  const int sizeOfBuffer = 1024 * 1024;
+  init_trace_buffer (sizeOfBuffer);
 
   /* Wire trace state variable 1 to be the timestamp.  This will be
      uploaded to GDB upon connection and become one of its trace state
diff --git a/gdb/remote.c b/gdb/remote.c
index 88a57c8..8352e80 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -11042,6 +11042,29 @@ remote_get_min_fast_tracepoint_insn_len (void)
     }
 }
 
+static void
+remote_set_trace_buffer_size (LONGEST val)
+{
+  struct remote_state *rs = get_remote_state ();
+  char *reply;
+  char *buf = rs->buf;
+  char *endbuf = rs->buf + get_remote_packet_size ();
+
+  buf += xsnprintf (buf, endbuf - buf, "QTBuffer:size:");
+  /* Send -1 as literal "-1" to avoid host size dependency.  */
+  if (val == -1)
+    buf += xsnprintf (buf, endbuf - buf, "-1");
+  else
+    buf += hexnumstr (buf, (ULONGEST) val);
+
+  putpkt (rs->buf);
+  reply = remote_get_noisy_reply (&rs->buf, &rs->buf_size);
+  if (*rs->buf == '\0')
+    warning (_("Target does not support setting trace buffer size."));
+  if (strcmp (reply, "OK") != 0)
+    warning (_("Bogus reply from target: %s"), reply);
+}
+
 static int
 remote_set_trace_notes (char *user, char *notes, char *stop_notes)
 {
@@ -11219,6 +11242,7 @@ Specify the serial device it is connected to\n\
   remote_ops.to_get_min_fast_tracepoint_insn_len = remote_get_min_fast_tracepoint_insn_len;
   remote_ops.to_set_disconnected_tracing = remote_set_disconnected_tracing;
   remote_ops.to_set_circular_trace_buffer = remote_set_circular_trace_buffer;
+  remote_ops.to_set_trace_buffer_size = remote_set_trace_buffer_size;
   remote_ops.to_set_trace_notes = remote_set_trace_notes;
   remote_ops.to_core_of_thread = remote_core_of_thread;
   remote_ops.to_verify_memory = remote_verify_memory;
diff --git a/gdb/target.c b/gdb/target.c
index 9d8bf6e..2e507e0 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -693,6 +693,7 @@ update_current_target (void)
       INHERIT (to_get_min_fast_tracepoint_insn_len, t);
       INHERIT (to_set_disconnected_tracing, t);
       INHERIT (to_set_circular_trace_buffer, t);
+      INHERIT (to_set_trace_buffer_size, t);
       INHERIT (to_set_trace_notes, t);
       INHERIT (to_get_tib_address, t);
       INHERIT (to_set_permissions, t);
@@ -912,6 +913,9 @@ update_current_target (void)
   de_fault (to_set_circular_trace_buffer,
 	    (void (*) (int))
 	    target_ignore);
+  de_fault (to_set_trace_buffer_size,
+	    (void (*) (LONGEST))
+	    target_ignore);
   de_fault (to_set_trace_notes,
 	    (int (*) (char *, char *, char *))
 	    return_zero);
diff --git a/gdb/target.h b/gdb/target.h
index 1971265..c99642d 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -806,6 +806,8 @@ struct target_ops
        disconnection - set VAL to 1 to keep tracing, 0 to stop.  */
     void (*to_set_disconnected_tracing) (int val);
     void (*to_set_circular_trace_buffer) (int val);
+    /* Set the size of trace buffer in the target.  */
+    void (*to_set_trace_buffer_size) (LONGEST val);
 
     /* Add/change textual notes about the trace run, returning 1 if
        successful, 0 otherwise.  */
@@ -1700,6 +1702,9 @@ extern char *target_fileio_read_stralloc (const char *filename);
 #define	target_set_circular_trace_buffer(val)	\
   (*current_target.to_set_circular_trace_buffer) (val)
 
+#define	target_set_trace_buffer_size(val)	\
+  (*current_target.to_set_trace_buffer_size) (val)
+
 #define	target_set_trace_notes(user,notes,stopnotes)		\
   (*current_target.to_set_trace_notes) ((user), (notes), (stopnotes))
 
diff --git a/gdb/testsuite/gdb.trace/trace-buffer-size.c b/gdb/testsuite/gdb.trace/trace-buffer-size.c
new file mode 100644
index 0000000..e32b756
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/trace-buffer-size.c
@@ -0,0 +1,31 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011-2013 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int var = 10;
+
+int
+test_function ()
+{
+  return 0;
+}
+
+int
+main ()
+{
+  test_function (); 
+  return 0; /*breakpoint1*/
+}
diff --git a/gdb/testsuite/gdb.trace/trace-buffer-size.exp b/gdb/testsuite/gdb.trace/trace-buffer-size.exp
new file mode 100644
index 0000000..4533f77
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/trace-buffer-size.exp
@@ -0,0 +1,54 @@
+# Copyright 1998-2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib "trace-support.exp"
+
+standard_testfile
+
+if [prepare_for_testing ${testfile}.exp $testfile $srcfile \
+        {debug nowarnings}] {
+    untested "failed to prepare for trace tests"
+    return -1
+}
+
+if ![runto_main] {
+    fail "can't run to main to check for trace support"
+    return -1
+}
+
+if ![gdb_target_supports_trace] {
+    unsupported "target does not support trace"
+    return -1;
+}
+
+set BUFFER_SIZE 4
+gdb_test_no_output "set trace-buffer-size $BUFFER_SIZE" "Set Trace Buffer Size"
+gdb_test "show trace-buffer-size $BUFFER_SIZE" "Requested size of trace buffer is $BUFFER_SIZE.*" \
+"Show Trace Buffer Size"
+
+# We set trace buffer to very small size. Then after running trace, we check
+# if it is full. This will show if setting trace buffer size really worked.
+gdb_breakpoint ${srcfile}:[gdb_get_line_number "breakpoint1"]
+gdb_test "trace test_function" \
+    "Tracepoint \[0-9\]+ at .*" \
+    "set tracepoint at test_function"
+gdb_trace_setactions "1. Set action for trace point" "" \
+    "collect var" "^$"
+gdb_test "tstart"     ".*" ""
+gdb_test "continue"   ".*" ""
+gdb_test "tstatus"    ".*Trace stopped because the buffer was full.*" \
+"Buffer full check"
+gdb_test "tstop"    ".*" ""
+
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 9a80aa3..bd4f2c7 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -173,6 +173,11 @@ static int disconnected_tracing;
 
 static int circular_trace_buffer;
 
+/* This variable is the requested trace buffer size, or -1 to indicate
+   that we don't care and leave it up to the target to set a size.  */
+
+static int trace_buffer_size = -1;
+
 /* Textual notes applying to the current and/or future trace runs.  */
 
 char *trace_user = NULL;
@@ -1829,6 +1834,7 @@ start_tracing (char *notes)
   /* Set some mode flags.  */
   target_set_disconnected_tracing (disconnected_tracing);
   target_set_circular_trace_buffer (circular_trace_buffer);
+  target_set_trace_buffer_size (trace_buffer_size);
 
   if (!notes)
     notes = trace_notes;
@@ -3231,6 +3237,13 @@ set_circular_trace_buffer (char *args, int from_tty,
 }
 
 static void
+set_trace_buffer_size (char *args, int from_tty,
+			   struct cmd_list_element *c)
+{
+  target_set_trace_buffer_size (trace_buffer_size);
+}
+
+static void
 set_trace_user (char *args, int from_tty,
 		struct cmd_list_element *c)
 {
@@ -5416,6 +5429,16 @@ up and stopping the trace run."),
 			   &setlist,
 			   &showlist);
 
+  add_setshow_zinteger_cmd ("trace-buffer-size", no_class,
+			    &trace_buffer_size, _("\
+Set requested size of trace buffer."), _("\
+Show requested size of trace buffer."), _("\
+Use this to choose a size for the trace buffer.  Some targets\n\
+may have fixed or limited buffer sizes.  A value of -1 disables\n\
+any attempt to set the buffer size and lets the target choose."),
+			    set_trace_buffer_size, NULL,
+			    &setlist, &showlist);
+
   add_setshow_string_cmd ("trace-user", class_trace,
 			  &trace_user, _("\
 Set the user name to use for current and future trace runs"), _("\

zinteger setshow commands broken

[Pedro Alves]

On 03/01/2013 05:07 PM, Abid, Hafiz wrote:
>>
>>
>> Should probably be a LONGEST too.  There's to reason to
>> actually limit to 31-bits, right?
>>
>> > +  add_setshow_zinteger_cmd ("trace-buffer-size", no_class,
>> > +                &trace_buffer_size, _("\
>>
>> Oh, I see...  add_setshow_zinteger_cmd works with int...
> Observed that commands added with add_setshow_zinteger_cmd have problem with -1. For var_zinteger, do_set_command uses unsigned int and it ends up comparing 0xffffffff to INT_MAX. If it sounds like an oversight to you then I will send a patch. Here are those lines for a quick look.
>     unsigned int val;
> ...
>     val = parse_and_eval_long (arg);
> ...
>     else if (val >= INT_MAX)
>       error (_("integer %u out of range"), val);
> 

Eh, indeed.  Sounds like a regression.  We have other
commands where -1 is treated specially:

(gdb) help set remote hardware-watchpoint-limit
Set the maximum number of target hardware watchpoints.
Specify a negative limit for unlimited.
(gdb) set remote hardware-watchpoint-limit -1
integer 4294967295 out of range

This surely worked at some point.

BTW, I think the new var_zuinteger_unlimited would be more
suitable.  The difference is that "show" really shows "unlimited"
instead of -1.  BTW2, IMO, "set" should accept literal "unlimited"
string as well too.

-- 
Pedro Alves

Re: [patch] Change trace buffer size(v2)

[Pedro Alves]

On 03/01/2013 05:07 PM, Abid, Hafiz wrote:

>>   if (*rs->buf == '\0')
>>
>> but, OTOH, new packets should really be using packet_ok.

I think you missed this note.

>> This test doesn't seem to really test that much.  I wonder if
>> you have ideas on expanding it?
> I have tried to expand the test by setting the buffer size to be very small and then testing the buffer to be full.

Good idea.

I think it'd be even better to run the test twice, once with
a larger buffer that fits the same trace frames, and make sure
the buffer doesn't end up full, and then another time with the
small buffer.  That'd cover the case of the target's buffer being
small by default, and the buffer size packet failing to work.

> 


> +* New remote packets
> +  A new packet has been defined to set the trace buffer size.
> +

Please model from previous examples:

* New remote packets
...
QTBuffer:circular
   Set the trace buffer to be linear or circular.

It's good to actually show the packet here, so the user
can quite find it in the manual, and, makes it easier to
answer the question in a few years "when was packet FOO added?".

> +@item QTBuffer:size:@var{size}
> +This packet directs the target to make the trace buffer be of size
> +@var{size} if possible. A value of @code{-1} tells the target to
> +use whatever size it likes.

Double space after period.  I think "prefers" is more natural in the
manual.

>  static void
> +cmd_bigqtbuffer_size (char *own_buf)
> +{
> +  ULONGEST val;
> +  LONGEST sval;
> +  char *packet = own_buf;
> +
> +  packet += strlen ("QTBuffer:size:");
> +
> +  /* -1 is sent as literal "-1".  */
> +  if (strcmp (packet, "-1") == 0)
> +    sval = -1;
> +  else
> +    {
> +      unpack_varlen_hex (packet, &val);
> +      sval = (LONGEST)val;
> +    }
> +  /* If we don't care about the size, or the size is unchanged,
> +     all is happy and nothing to do.  */

     s/all is happy and/there's/

> +  if (sval < 0 || sval == trace_buffer_size)
> +    {
> +      write_ok (own_buf);
> +      return;
> +    }
> +  /* Right now we can't change the size during a tracing run.  */

Let's drop distracting speculation:

  s/Right now we can't/Can't/

> +  if (tracing)
> +    {
> +       write_enn (own_buf);
> +       return;
> +    }

Better do this check before anything else, avoid
unnecessary special cases.

Also, if sval is < 0, this is not checking for "tracing" (fixed
by doing the check earlier), but also,

> +  init_trace_buffer (sval);

... nor does init_trace_buffer have any logic to
set the trace buffer size to whatever the "target likes":

 static void
-init_trace_buffer (unsigned char *buf, int bufsize)
+init_trace_buffer (LONGEST bufsize)
 {
-  trace_buffer_lo = buf;
+  trace_buffer_size = bufsize;
+
+  /* If we already have a trace buffer, try realloc'ing.  */
+  trace_buffer_lo = xrealloc (trace_buffer_lo, bufsize);
+
   trace_buffer_hi = trace_buffer_lo + bufsize;

   clear_trace_buffer ();


Please make sure to test that at least manually, though
it'd be better if the testsuite also exercised this.


> +static void
> +remote_set_trace_buffer_size (LONGEST val)
> +{
> +  struct remote_state *rs = get_remote_state ();
> +  char *reply;
> +  char *buf = rs->buf;
> +  char *endbuf = rs->buf + get_remote_packet_size ();
> +
> +  buf += xsnprintf (buf, endbuf - buf, "QTBuffer:size:");
> +  /* Send -1 as literal "-1" to avoid host size dependency.  */
> +  if (val == -1)
> +    buf += xsnprintf (buf, endbuf - buf, "-1");
> +  else
> +    buf += hexnumstr (buf, (ULONGEST) val);

This would mishandle other negative numbers (though
something else should be preventing those.
var_zuinteger_unlimited would be one way, though it'd
probably be a good idea to check for val<0 && val != -1 here too.

FYI, there's no need to special case -1 actually:

  if (val < 0)
    buf += hexnumstr (buf, (ULONGEST) -val);
  else
    buf += hexnumstr (buf, (ULONGEST) val);


> +
> +  putpkt (rs->buf);
> +  reply = remote_get_noisy_reply (&rs->buf, &rs->buf_size);
> +  if (*rs->buf == '\0')

Did you try packet_ok?  Really new commands/packets should be
using it.

> +    warning (_("Target does not support setting trace buffer size."));
> +  if (strcmp (reply, "OK") != 0)
> +    warning (_("Bogus reply from target: %s"), reply);
> +}
> +


> +
> +set BUFFER_SIZE 4
> +gdb_test_no_output "set trace-buffer-size $BUFFER_SIZE" "Set Trace Buffer Size"
> +gdb_test "show trace-buffer-size $BUFFER_SIZE" "Requested size of trace buffer is $BUFFER_SIZE.*" \
> +"Show Trace Buffer Size"

I think this overflows 80 columns.  Since you're breaking the line, could
you break it so it doesn't overflow 80 columns, please?

I'd write as:

gdb_test_no_output \
   "set trace-buffer-size $BUFFER_SIZE"
   "Set Trace Buffer Size"

gdb_test \
  "show trace-buffer-size $BUFFER_SIZE" \
  "Requested size of trace buffer is $BUFFER_SIZE.*" \
  "Show Trace Buffer Size"

> +
> +# We set trace buffer to very small size. Then after running trace, we check
> +# if it is full. This will show if setting trace buffer size really worked.

Double space after period.

> +gdb_breakpoint ${srcfile}:[gdb_get_line_number "breakpoint1"]
> +gdb_test "trace test_function" \
> +    "Tracepoint \[0-9\]+ at .*" \
> +    "set tracepoint at test_function"
> +gdb_trace_setactions "1. Set action for trace point" "" \
> +    "collect var" "^$"
> +gdb_test "tstart"     ".*" ""

gdb_test_no_output "tstart"

> +gdb_test "continue"   ".*" ""

Better to match something rather than ".*".

Look for "run trace experiment" on other tests.

> +gdb_test "tstatus"    ".*Trace stopped because the buffer was full.*" \
> +"Buffer full check"

Column overflow again, I think.

> +gdb_test "tstop"    ".*" ""

gdb_test_test_no_output.


>  
> +  add_setshow_zinteger_cmd ("trace-buffer-size", no_class,
> +			    &trace_buffer_size, _("\
> +Set requested size of trace buffer."), _("\
> +Show requested size of trace buffer."), _("\
> +Use this to choose a size for the trace buffer.  Some targets\n\
> +may have fixed or limited buffer sizes.  A value of -1 disables\n\
> +any attempt to set the buffer size and lets the target choose."),
> +			    set_trace_buffer_size, NULL,
> +			    &setlist, &showlist);
> +

As I mentioned elsewhere, I think it'd be better to use the
new var_zuinteger_unlimited.

We should really fix the broken -1 handling too, before 7.6.

>    add_setshow_string_cmd ("trace-user", class_trace,
>  			  &trace_user, _("\
>  Set the user name to use for current and future trace runs"), _("\

-- 
Pedro Alves

Re: zinteger setshow commands broken

[Abid, Hafiz]

On 01/03/13 18:13:21, Pedro Alves wrote:
> 
> BTW, I think the new var_zuinteger_unlimited would be more
> suitable.  The difference is that "show" really shows "unlimited"
> instead of -1.  BTW2, IMO, "set" should accept literal "unlimited"
> string as well too.
To make sure that I understand it right, you are suggesting that  
commands which accepts -1 as special value should be implemented using  
var_zuinteger_unlimited? One is the new command that I am working on to  
change the trace buffer size. Other is "remote  
hardware-watchpoint-limit".

Regards,
Abid

Re: zinteger setshow commands broken

[Yao Qi]

On 03/04/2013 06:10 PM, Abid, Hafiz wrote:
> One is the new command that I am working on to
> change the trace buffer size. Other is "remote
> hardware-watchpoint-limit".

Abid,
My patches <http://sourceware.org/ml/gdb-patches/2013-02/msg00388.html> 
change commands "set remote XXX-limit" to zuinteger_unlimited.  They are 
still pending for review.  I should send a ping again :)

-- 
Yao (齐尧)

Re: zinteger setshow commands broken

[Pedro Alves]

On 03/04/2013 10:10 AM, Abid, Hafiz wrote:
> On 01/03/13 18:13:21, Pedro Alves wrote:
>>
>> BTW, I think the new var_zuinteger_unlimited would be more
>> suitable.  The difference is that "show" really shows "unlimited"
>> instead of -1.  BTW2, IMO, "set" should accept literal "unlimited"
>> string as well too.
> To make sure that I understand it right, you are suggesting that commands which accepts -1 as special value should be implemented using var_zuinteger_unlimited? 

I'm suggesting that your new command, that accepts -1 as special value
as meaning "unlimited", would be better implemented using
var_zuinteger_unlimited.

> One is the new command that I am working on to change the trace buffer size. Other is "remote hardware-watchpoint-limit".

-- 
Pedro Alves

Re: [patch] Change trace buffer size(v3)

[Abid, Hafiz]

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

Hi Pedro,
Thanks for review. Here is an updated patch which tries to addresses  
your comments.

On 01/03/13 20:01:58, Pedro Alves wrote:
> but, OTOH, new packets should really be using packet_ok.
> I think you missed this note.
Fixed.
> 
> Good idea.
> 
> I think it'd be even better to run the test twice, once with
> a larger buffer that fits the same trace frames, and make sure
> the buffer doesn't end up full, and then another time with the
> small buffer.  That'd cover the case of the target's buffer being
> small by default, and the buffer size packet failing to work.
Done.
> 
> Please model from previous examples:
> 
> * New remote packets
> ...
> QTBuffer:circular
>    Set the trace buffer to be linear or circular.
Done.
> 
> > +@item QTBuffer:size:@var{size}
> > +This packet directs the target to make the trace buffer be of size
> > +@var{size} if possible. A value of @code{-1} tells the target to
> > +use whatever size it likes.
> 
> Double space after period.  I think "prefers" is more natural in the
> manual.
Fixed.
> 

> > +  /* If we don't care about the size, or the size is unchanged,
> > +     all is happy and nothing to do.  */
> 
>      s/all is happy and/there's/
Fixed.
> 
> > +  /* Right now we can't change the size during a tracing run.  */
> 
> Let's drop distracting speculation:
>   s/Right now we can't/Can't/
Fixed.
> 
> > +  if (tracing)
> > +    {
> > +       write_enn (own_buf);
> > +       return;
> > +    }
> 
> Better do this check before anything else, avoid
> unnecessary special cases.
Moved to start of function.
> 
> Also, if sval is < 0, this is not checking for "tracing" (fixed
> by doing the check earlier), but also,
> 
> > +  init_trace_buffer (sval);
> 
> ... nor does init_trace_buffer have any logic to
> set the trace buffer size to whatever the "target likes":
I have now added a default size. On -1, that deafult size is used.
> 
>  static void
> -init_trace_buffer (unsigned char *buf, int bufsize)
> +init_trace_buffer (LONGEST bufsize)
>  {
> -  trace_buffer_lo = buf;
> +  trace_buffer_size = bufsize;
> +
> +  /* If we already have a trace buffer, try realloc'ing.  */
> +  trace_buffer_lo = xrealloc (trace_buffer_lo, bufsize);
> +
>    trace_buffer_hi = trace_buffer_lo + bufsize;
> 
>    clear_trace_buffer ();
> 
> 
> Please make sure to test that at least manually, though
> it'd be better if the testsuite also exercised this.
I tested this manualy by running trace experiment to fill the trace  
buffer.
Then changed the size to a larger value and run trace again. It worked  
ok.
> 
> 
> > +static void
> > +remote_set_trace_buffer_size (LONGEST val)
> > +{
> > +  struct remote_state *rs = get_remote_state ();
> > +  char *reply;
> > +  char *buf = rs->buf;
> > +  char *endbuf = rs->buf + get_remote_packet_size ();
> > +
> > +  buf += xsnprintf (buf, endbuf - buf, "QTBuffer:size:");
> > +  /* Send -1 as literal "-1" to avoid host size dependency.  */
> > +  if (val == -1)
> > +    buf += xsnprintf (buf, endbuf - buf, "-1");
> > +  else
> > +    buf += hexnumstr (buf, (ULONGEST) val);
> 
> This would mishandle other negative numbers (though
> something else should be preventing those.
> var_zuinteger_unlimited would be one way, though it'd
> probably be a good idea to check for val<0 && val != -1 here too.
Added check for val<0 && val != -1.
> 
> FYI, there's no need to special case -1 actually:
> 
>   if (val < 0)
>     buf += hexnumstr (buf, (ULONGEST) -val);
>   else
>     buf += hexnumstr (buf, (ULONGEST) val);
I did not clearly understand how it works. Because -val will end up  
sending 1. So I kept the
literal -1 as you suggested in previous email. Although I modified the  
check from == -1 to < 0.
> 
> 
> > +
> > +  putpkt (rs->buf);
> > +  reply = remote_get_noisy_reply (&rs->buf, &rs->buf_size);
> > +  if (*rs->buf == '\0')
> 
> Did you try packet_ok?  Really new commands/packets should be
> using it.
I have used packet_ok and related machinery so that gdbserver informs  
about the support of QTBuffer:size packet now.
> 
> > +set BUFFER_SIZE 4
> > +gdb_test_no_output "set trace-buffer-size $BUFFER_SIZE" "Set Trace  
> Buffer Size"
> > +gdb_test "show trace-buffer-size $BUFFER_SIZE" "Requested size of  
> trace buffer is $BUFFER_SIZE.*" \
> > +"Show Trace Buffer Size"
> 
> I think this overflows 80 columns.  Since you're breaking the line,  
> could
> you break it so it doesn't overflow 80 columns, please?
Fixed

> > +
> > +# We set trace buffer to very small size. Then after running  
> trace, we check
> > +# if it is full. This will show if setting trace buffer size  
> really worked.
> 
> Double space after period.
Fixed
> 
> > +gdb_test "tstart"     ".*" ""
> 
> gdb_test_no_output "tstart"
Done
> 
> > +gdb_test "continue"   ".*" ""
> Better to match something rather than ".*".
> Look for "run trace experiment" on other tests.
Fixed.
> 
> > +gdb_test "tstatus"    ".*Trace stopped because the buffer was  
> full.*" \
> > +"Buffer full check"
> 
> Column overflow again, I think.
Fixed.
> 
> gdb_test_test_no_output.
Removed this command as it was not really needed.
> 
> 
> As I mentioned elsewhere, I think it'd be better to use the
> new var_zuinteger_unlimited.
Done.
> 
> We should really fix the broken -1 handling too, before 7.6.
> 
How do you suggest we do that?

Regards,
Abid

2012-03-04  Stan Shebs  <stan@codesourcery.com>
	    Hafiz Abid Qadeer  <abidh@codesourcery.com>

	gdb/
	* NEWS: Mention set and show trace-buffer-size commands.
	Mention new packet.
	* target.h (struct target_ops): New method
	to_set_trace_buffer_size.
	(target_set_trace_buffer_size): New macro.
	* target.c (update_current_target): Set up new method.
	* tracepoint.c (trace_buffer_size): New global.
	(start_tracing): Send it to the target.
	(set_trace_buffer_size): New function.
	(_initialize_tracepoint): Add new setshow for trace-buffer-size.
	* remote.c (remote_set_trace_buffer_size): New function.
	(_initialize_remote): Use it.
	(PACKET_QTBuffer_size): New enum.
	(remote_protocol_features): Added an entry for
	PACKET_QTBuffer_size.

	gdb/gdbserver/
	* tracepoint.c (trace_buffer_size): New global.
	(DEFAULT_TRACE_BUFFER_SIZE): New define.
	(init_trace_buffer): Change to one-argument function, add
	realloc option.
	(handle_tracepoint_general_set): Call cmd_bigqtbuffer_size to
	handle QTBuffer:size packet.
	(cmd_bigqtbuffer_size): New function.
	(initialize_tracepoint): Call init_trace_buffer with
	DEFAULT_TRACE_BUFFER_SIZE.
	* server.c (handle_query): Added QTBuffer:size in the
	supported packets.

	gdb/doc/
	* gdb.texinfo (Starting and Stopping Trace Experiments):  
Document
	trace-buffer-size set and show commands.
	(Tracepoint Packets): Document QTBuffer:size .

	gdb/testsuite/
	* gdb.trace/trace-buffer-size.exp: New file.
	* gdb.trace/trace-buffer-size.c: New file.




[-- Attachment #2: trace-buffer-size_v3.patch --]
[-- Type: text/x-patch, Size: 16884 bytes --]

diff --git a/gdb/NEWS b/gdb/NEWS
index 0877aa2..3fb68eb 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -104,6 +104,10 @@ set debug notification
 show debug notification
   Control display of debugging info for async remote notification.
 
+set trace-buffer-size
+show trace-buffer-size
+  Request target to change the size of trace buffer.
+
 * Removed commands
 
   ** For the Renesas Super-H architecture, the "regs" command has been removed
@@ -159,6 +163,11 @@ show filename-display
   feature to be enabled.  For more information, see:
       http://fedoraproject.org/wiki/Features/MiniDebugInfo
 
+* New remote packets
+
+QTBuffer:size
+   Set the size of trace buffer.
+
 *** Changes in GDB 7.5
 
 * GDB now supports x32 ABI.  Visit <http://sites.google.com/site/x32abi/>
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 5f39d2e..99745f1 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -11728,6 +11728,25 @@ for instance if you are looking at frames from a trace file.
 @end table
 
 @table @code
+@item set trace-buffer-size @var{n}
+@kindex set trace-buffer-size
+Request that the target use a trace buffer of @var{n} bytes.  Not all
+targets will honor the request; they may have a compiled-in size for
+the trace buffer, or some other limitation.  Set to a value of
+@code{-1} to let the target use whatever size it likes.  This is also
+the default.
+
+@item show trace-buffer-size
+@kindex show trace-buffer-size
+Show the current requested size for the trace buffer.  Note that this
+will only match the actual size if the target supports size-setting,
+and was able to handle the requested size.  For instance, if the
+target can only change buffer size between runs, this variable will
+not reflect the change until the next run starts.  Use @code{tstatus}
+to get a report of the actual buffer size.
+@end table
+
+@table @code
 @item set trace-user @var{text}
 @kindex set trace-user
 
@@ -38450,6 +38469,11 @@ available.
 This packet directs the target to use a circular trace buffer if
 @var{value} is 1, or a linear buffer if the value is 0.
 
+@item QTBuffer:size:@var{size}
+This packet directs the target to make the trace buffer be of size
+@var{size} if possible.  A value of @code{-1} tells the target to
+use whatever size it prefers.
+
 @item QTNotes:@r{[}@var{type}:@var{text}@r{]}@r{[};@var{type}:@var{text}@r{]}@dots{}
 @cindex @samp{QTNotes} packet
 This packet adds optional textual notes to the trace run.  Allowable
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 768eae7..922a5bf 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1637,6 +1637,7 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 	  strcat (own_buf, ";qXfer:statictrace:read+");
 	  strcat (own_buf, ";qXfer:traceframe-info:read+");
 	  strcat (own_buf, ";EnableDisableTracepoints+");
+	  strcat (own_buf, ";QTBuffer:size+");
 	  strcat (own_buf, ";tracenz+");
 	}
 
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 0f83ae6..61d09c0 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -30,6 +30,8 @@
 
 #include "ax.h"
 
+#define DEFAULT_TRACE_BUFFER_SIZE 1048576 /* 1024*1024 */
+
 /* This file is built for both GDBserver, and the in-process
    agent (IPA), a shared library that includes a tracing agent that is
    loaded by the inferior to support fast tracepoints.  Fast
@@ -992,6 +994,8 @@ int current_traceframe = -1;
 static int circular_trace_buffer;
 #endif
 
+static LONGEST trace_buffer_size;
+
 /* Pointer to the block of memory that traceframes all go into.  */
 
 static unsigned char *trace_buffer_lo;
@@ -1478,9 +1482,13 @@ clear_inferior_trace_buffer (void)
 #endif
 
 static void
-init_trace_buffer (unsigned char *buf, int bufsize)
+init_trace_buffer (LONGEST bufsize)
 {
-  trace_buffer_lo = buf;
+  trace_buffer_size = bufsize;
+
+  /* If we already have a trace buffer, try realloc'ing.  */
+  trace_buffer_lo = xrealloc (trace_buffer_lo, bufsize);
+
   trace_buffer_hi = trace_buffer_lo + bufsize;
 
   clear_trace_buffer ();
@@ -4020,6 +4028,43 @@ cmd_bigqtbuffer_circular (char *own_buf)
 }
 
 static void
+cmd_bigqtbuffer_size (char *own_buf)
+{
+  ULONGEST val;
+  LONGEST sval;
+  char *packet = own_buf;
+
+  /* Can't change the size during a tracing run.  */
+  if (tracing)
+    {
+      write_enn (own_buf);
+      return;
+    }
+
+  packet += strlen ("QTBuffer:size:");
+
+  /* -1 is sent as literal "-1".  */
+  if (strcmp (packet, "-1") == 0)
+    sval = DEFAULT_TRACE_BUFFER_SIZE;
+  else
+    {
+      unpack_varlen_hex (packet, &val);
+      sval = (LONGEST)val;
+    }
+  /* If the size is unchanged, there's nothing to do.  */
+  if (sval == trace_buffer_size)
+    {
+      write_ok (own_buf);
+      return;
+    }
+
+  init_trace_buffer (sval);
+  trace_debug ("Trace buffer is now %s bytes",
+	       plongest (trace_buffer_size));
+  write_ok (own_buf);
+}
+
+static void
 cmd_qtnotes (char *own_buf)
 {
   size_t nbytes;
@@ -4143,6 +4188,11 @@ handle_tracepoint_general_set (char *packet)
       cmd_bigqtbuffer_circular (packet);
       return 1;
     }
+  else if (strncmp ("QTBuffer:size:", packet, strlen ("QTBuffer:size:")) == 0)
+    {
+      cmd_bigqtbuffer_size (packet);
+      return 1;
+    }
   else if (strncmp ("QTNotes:", packet, strlen ("QTNotes:")) == 0)
     {
       cmd_qtnotes (packet);
@@ -7228,10 +7278,8 @@ get_timestamp (void)
 void
 initialize_tracepoint (void)
 {
-  /* There currently no way to change the buffer size.  */
-  const int sizeOfBuffer = 5 * 1024 * 1024;
-  unsigned char *buf = xmalloc (sizeOfBuffer);
-  init_trace_buffer (buf, sizeOfBuffer);
+  /* Start with the default size.  */
+  init_trace_buffer (DEFAULT_TRACE_BUFFER_SIZE);
 
   /* Wire trace state variable 1 to be the timestamp.  This will be
      uploaded to GDB upon connection and become one of its trace state
diff --git a/gdb/remote.c b/gdb/remote.c
index 88a57c8..6e9a59b 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1284,6 +1284,7 @@ enum {
   PACKET_qXfer_fdpic,
   PACKET_QDisableRandomization,
   PACKET_QAgent,
+  PACKET_QTBuffer_size,
   PACKET_MAX
 };
 
@@ -3993,6 +3994,8 @@ static struct protocol_feature remote_protocol_features[] = {
   { "QDisableRandomization", PACKET_DISABLE, remote_supported_packet,
     PACKET_QDisableRandomization },
   { "QAgent", PACKET_DISABLE, remote_supported_packet, PACKET_QAgent},
+  { "QTBuffer:size", PACKET_DISABLE,
+    remote_supported_packet, PACKET_QTBuffer_size},
   { "tracenz", PACKET_DISABLE,
     remote_string_tracing_feature, -1 },
 };
@@ -11042,6 +11045,41 @@ remote_get_min_fast_tracepoint_insn_len (void)
     }
 }
 
+static void
+remote_set_trace_buffer_size (LONGEST val)
+{
+  if (remote_protocol_packets[PACKET_QTBuffer_size].support !=
+      PACKET_DISABLE)
+    {
+      struct remote_state *rs = get_remote_state ();
+      char *buf = rs->buf;
+      char *endbuf = rs->buf + get_remote_packet_size ();
+      enum packet_result result;
+
+      if (val < 0 && val != -1)
+	{
+	  warning (_("Invalid value %s for the size of trace buffer."),
+		   plongest (val) );
+	  return;
+	}
+
+      buf += xsnprintf (buf, endbuf - buf, "QTBuffer:size:");
+      /* Send -1 as literal "-1" to avoid host size dependency.  */
+      if (val < 0)
+	buf += xsnprintf (buf, endbuf - buf, "-1");
+      else
+	buf += hexnumstr (buf, (ULONGEST) val);
+
+      putpkt (rs->buf);
+      getpkt (&rs->buf, &rs->buf_size, 0);
+      result = packet_ok (rs->buf,
+		  &remote_protocol_packets[PACKET_QTBuffer_size]);
+
+      if (result != PACKET_OK)
+	warning (_("Bogus reply from target: %s"), rs->buf);
+    }
+}
+
 static int
 remote_set_trace_notes (char *user, char *notes, char *stop_notes)
 {
@@ -11219,6 +11257,7 @@ Specify the serial device it is connected to\n\
   remote_ops.to_get_min_fast_tracepoint_insn_len = remote_get_min_fast_tracepoint_insn_len;
   remote_ops.to_set_disconnected_tracing = remote_set_disconnected_tracing;
   remote_ops.to_set_circular_trace_buffer = remote_set_circular_trace_buffer;
+  remote_ops.to_set_trace_buffer_size = remote_set_trace_buffer_size;
   remote_ops.to_set_trace_notes = remote_set_trace_notes;
   remote_ops.to_core_of_thread = remote_core_of_thread;
   remote_ops.to_verify_memory = remote_verify_memory;
@@ -11755,6 +11794,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
   add_packet_config_cmd (&remote_protocol_packets[PACKET_QAgent],
 			 "QAgent", "agent", 0);
 
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_QTBuffer_size],
+			 "QTBuffer:size", "trace-buffer-size", 0);
+
   /* Keep the old ``set remote Z-packet ...'' working.  Each individual
      Z sub-packet has its own set and show commands, but users may
      have sets to this variable in their .gdbinit files (or in their
diff --git a/gdb/target.c b/gdb/target.c
index 9d8bf6e..2e507e0 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -693,6 +693,7 @@ update_current_target (void)
       INHERIT (to_get_min_fast_tracepoint_insn_len, t);
       INHERIT (to_set_disconnected_tracing, t);
       INHERIT (to_set_circular_trace_buffer, t);
+      INHERIT (to_set_trace_buffer_size, t);
       INHERIT (to_set_trace_notes, t);
       INHERIT (to_get_tib_address, t);
       INHERIT (to_set_permissions, t);
@@ -912,6 +913,9 @@ update_current_target (void)
   de_fault (to_set_circular_trace_buffer,
 	    (void (*) (int))
 	    target_ignore);
+  de_fault (to_set_trace_buffer_size,
+	    (void (*) (LONGEST))
+	    target_ignore);
   de_fault (to_set_trace_notes,
 	    (int (*) (char *, char *, char *))
 	    return_zero);
diff --git a/gdb/target.h b/gdb/target.h
index 1971265..c99642d 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -806,6 +806,8 @@ struct target_ops
        disconnection - set VAL to 1 to keep tracing, 0 to stop.  */
     void (*to_set_disconnected_tracing) (int val);
     void (*to_set_circular_trace_buffer) (int val);
+    /* Set the size of trace buffer in the target.  */
+    void (*to_set_trace_buffer_size) (LONGEST val);
 
     /* Add/change textual notes about the trace run, returning 1 if
        successful, 0 otherwise.  */
@@ -1700,6 +1702,9 @@ extern char *target_fileio_read_stralloc (const char *filename);
 #define	target_set_circular_trace_buffer(val)	\
   (*current_target.to_set_circular_trace_buffer) (val)
 
+#define	target_set_trace_buffer_size(val)	\
+  (*current_target.to_set_trace_buffer_size) (val)
+
 #define	target_set_trace_notes(user,notes,stopnotes)		\
   (*current_target.to_set_trace_notes) ((user), (notes), (stopnotes))
 
diff --git a/gdb/testsuite/gdb.trace/trace-buffer-size.c b/gdb/testsuite/gdb.trace/trace-buffer-size.c
new file mode 100644
index 0000000..e32b756
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/trace-buffer-size.c
@@ -0,0 +1,31 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011-2013 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int var = 10;
+
+int
+test_function ()
+{
+  return 0;
+}
+
+int
+main ()
+{
+  test_function (); 
+  return 0; /*breakpoint1*/
+}
diff --git a/gdb/testsuite/gdb.trace/trace-buffer-size.exp b/gdb/testsuite/gdb.trace/trace-buffer-size.exp
new file mode 100644
index 0000000..7891cc6
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/trace-buffer-size.exp
@@ -0,0 +1,79 @@
+# Copyright 1998-2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib "trace-support.exp"
+
+standard_testfile
+
+if [prepare_for_testing ${testfile}.exp $testfile $srcfile \
+        {debug nowarnings}] {
+    untested "failed to prepare for trace tests"
+    return -1
+}
+
+if ![runto_main] {
+    fail "can't run to main to check for trace support"
+    return -1
+}
+
+if ![gdb_target_supports_trace] {
+    unsupported "target does not support trace"
+    return -1;
+}
+
+set BUFFER_SIZE 4
+gdb_test_no_output \
+"set trace-buffer-size $BUFFER_SIZE" \
+"Set Trace Buffer Size"
+
+gdb_test "show trace-buffer-size $BUFFER_SIZE" \
+"Requested size of trace buffer is $BUFFER_SIZE.*" \
+"Show Trace Buffer Size"
+
+# We set trace buffer to very small size.  Then after running trace,
+# we check if it is full.  This will show if setting trace buffer
+# size really worked.
+gdb_breakpoint ${srcfile}:[gdb_get_line_number "breakpoint1"]
+gdb_test "trace test_function" \
+  "Tracepoint \[0-9\]+ at .*" \
+  "set tracepoint at test_function"
+gdb_trace_setactions "Set action for trace point 1" "" \
+  "collect var" "^$"
+gdb_test_no_output "tstart" 
+gdb_test "continue" \
+  "Continuing.*Breakpoint $decimal.*" \
+  "run trace experiment 1"
+gdb_test "tstatus" \
+  ".*Trace stopped because the buffer was full.*" \
+  "Buffer full check 1"
+
+# Use the default size  and trace buffer should not be
+# full this time.
+clean_restart ${testfile}
+runto_main
+gdb_breakpoint ${srcfile}:[gdb_get_line_number "breakpoint1"]
+gdb_test "trace test_function" \
+  "Tracepoint \[0-9\]+ at .*" \
+  "set tracepoint at test_function"
+gdb_trace_setactions "Set action for trace point 2" "" \
+  "collect var" "^$"
+gdb_test_no_output "tstart" 
+gdb_test "continue" \
+  "Continuing.*Breakpoint $decimal.*" \
+  "run trace experiment 2"
+gdb_test "tstatus" \
+  ".*Trace is running on the target.*" \
+  "Buffer full check 2"
+gdb_test_no_output "tstop"
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 9a80aa3..b8945bb 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -173,6 +173,11 @@ static int disconnected_tracing;
 
 static int circular_trace_buffer;
 
+/* This variable is the requested trace buffer size, or -1 to indicate
+   that we don't care and leave it up to the target to set a size.  */
+
+static int trace_buffer_size = -1;
+
 /* Textual notes applying to the current and/or future trace runs.  */
 
 char *trace_user = NULL;
@@ -1829,6 +1834,7 @@ start_tracing (char *notes)
   /* Set some mode flags.  */
   target_set_disconnected_tracing (disconnected_tracing);
   target_set_circular_trace_buffer (circular_trace_buffer);
+  target_set_trace_buffer_size (trace_buffer_size);
 
   if (!notes)
     notes = trace_notes;
@@ -3231,6 +3237,13 @@ set_circular_trace_buffer (char *args, int from_tty,
 }
 
 static void
+set_trace_buffer_size (char *args, int from_tty,
+			   struct cmd_list_element *c)
+{
+  target_set_trace_buffer_size (trace_buffer_size);
+}
+
+static void
 set_trace_user (char *args, int from_tty,
 		struct cmd_list_element *c)
 {
@@ -5416,6 +5429,16 @@ up and stopping the trace run."),
 			   &setlist,
 			   &showlist);
 
+  add_setshow_zuinteger_unlimited_cmd ("trace-buffer-size", no_class,
+			    &trace_buffer_size, _("\
+Set requested size of trace buffer."), _("\
+Show requested size of trace buffer."), _("\
+Use this to choose a size for the trace buffer.  Some targets\n\
+may have fixed or limited buffer sizes.  A value of -1 disables\n\
+any attempt to set the buffer size and lets the target choose."),
+			    set_trace_buffer_size, NULL,
+			    &setlist, &showlist);
+
   add_setshow_string_cmd ("trace-user", class_trace,
 			  &trace_user, _("\
 Set the user name to use for current and future trace runs"), _("\

Re: [patch] Change trace buffer size(v3)

[Pedro Alves]

On 03/04/2013 07:02 PM, Abid, Hafiz wrote:

>>  static void
>> -init_trace_buffer (unsigned char *buf, int bufsize)
>> +init_trace_buffer (LONGEST bufsize)
>>  {
>> -  trace_buffer_lo = buf;
>> +  trace_buffer_size = bufsize;
>> +
>> +  /* If we already have a trace buffer, try realloc'ing.  */
>> +  trace_buffer_lo = xrealloc (trace_buffer_lo, bufsize);
>> +
>>    trace_buffer_hi = trace_buffer_lo + bufsize;
>>
>>    clear_trace_buffer ();
>>
>>
>> Please make sure to test that at least manually, though
>> it'd be better if the testsuite also exercised this.
> I tested this manualy by running trace experiment to fill the trace buffer.
> Then changed the size to a larger value and run trace again. It worked ok.

One idea to make the testsuite check this would be to leverage "tstatus",
which prints the trace buffer size:

 (gdb) tstatus
 No trace has been run on the target.
 Collected 0 trace frames.
 Trace buffer has 1048576 bytes of 1048576 bytes free (0% full).
                                   ^^^^^^^
 Trace will stop if GDB disconnects.
 Not looking at any trace frame.

So, you'd read the default buffer size, change the trace buffer
size to something not the default, and then change it back
to the default with -1, and check that really worked.

>>
>>
>> > +static void
>> > +remote_set_trace_buffer_size (LONGEST val)
>> > +{
>> > +  struct remote_state *rs = get_remote_state ();
>> > +  char *reply;
>> > +  char *buf = rs->buf;
>> > +  char *endbuf = rs->buf + get_remote_packet_size ();
>> > +
>> > +  buf += xsnprintf (buf, endbuf - buf, "QTBuffer:size:");
>> > +  /* Send -1 as literal "-1" to avoid host size dependency.  */
>> > +  if (val == -1)
>> > +    buf += xsnprintf (buf, endbuf - buf, "-1");
>> > +  else
>> > +    buf += hexnumstr (buf, (ULONGEST) val);
>>
>> This would mishandle other negative numbers (though
>> something else should be preventing those.
>> var_zuinteger_unlimited would be one way, though it'd
>> probably be a good idea to check for val<0 && val != -1 here too.
> Added check for val<0 && val != -1.
>>
>> FYI, there's no need to special case -1 actually:
>>
>>   if (val < 0)
>>     buf += hexnumstr (buf, (ULONGEST) -val);
>>   else
>>     buf += hexnumstr (buf, (ULONGEST) val);
> I did not clearly understand how it works. Because -val will end up sending 1. So I kept the
> literal -1 as you suggested in previous email. Although I modified the check from == -1 to < 0.

The minus sign was missing:

   if (val < 0)
     {
       *buf++ = '-';
       buf += hexnumstr (buf, (ULONGEST) -val);
     }
   else
     buf += hexnumstr (buf, (ULONGEST) val);

>>
>>
>> > +
>> > +  putpkt (rs->buf);
>> > +  reply = remote_get_noisy_reply (&rs->buf, &rs->buf_size);
>> > +  if (*rs->buf == '\0')
>>
>> Did you try packet_ok?  Really new commands/packets should be
>> using it.
> I have used packet_ok and related machinery so that gdbserver informs about the support of QTBuffer:size packet now.

Thanks.  The qSupported feature need to be documented as well.

>>
>> As I mentioned elsewhere, I think it'd be better to use the
>> new var_zuinteger_unlimited.
> Done.
>>
>> We should really fix the broken -1 handling too, before 7.6.
>>
> How do you suggest we do that?

Dunno, haven't looked in detail but I'm hoping it to
be obvious to whoever debugs it.  ;-)

> 2012-03-04  Stan Shebs  <stan@codesourcery.com>
>         Hafiz Abid Qadeer  <abidh@codesourcery.com>
> 
>     gdb/
>     * NEWS: Mention set and show trace-buffer-size commands.
>     Mention new packet.
>     * target.h (struct target_ops): New method
>     to_set_trace_buffer_size.
>     (target_set_trace_buffer_size): New macro.
>     * target.c (update_current_target): Set up new method.
>     * tracepoint.c (trace_buffer_size): New global.
>     (start_tracing): Send it to the target.
>     (set_trace_buffer_size): New function.
>     (_initialize_tracepoint): Add new setshow for trace-buffer-size.
>     * remote.c (remote_set_trace_buffer_size): New function.
>     (_initialize_remote): Use it.
>     (PACKET_QTBuffer_size): New enum.
>     (remote_protocol_features): Added an entry for
>     PACKET_QTBuffer_size.

s/Added/Add/

> 
>     gdb/gdbserver/
>     * tracepoint.c (trace_buffer_size): New global.
>     (DEFAULT_TRACE_BUFFER_SIZE): New define.
>     (init_trace_buffer): Change to one-argument function, add
>     realloc option.

Use '.', not ',' to separate changes.  "add realloc option" doesn't
really make sense.  I can guess it referred to the "if" in a previous
revision?

>     (handle_tracepoint_general_set): Call cmd_bigqtbuffer_size to
>     handle QTBuffer:size packet.
>     (cmd_bigqtbuffer_size): New function.
>     (initialize_tracepoint): Call init_trace_buffer with
>     DEFAULT_TRACE_BUFFER_SIZE.
>     * server.c (handle_query): Added QTBuffer:size in the
>     supported packets.

s/Added/Add/

> 
>     gdb/doc/
>     * gdb.texinfo (Starting and Stopping Trace Experiments): Document
>     trace-buffer-size set and show commands.
>     (Tracepoint Packets): Document QTBuffer:size .

Spurious space after that last '.'.

> index 768eae7..922a5bf 100644
> --- a/gdb/gdbserver/server.c
> +++ b/gdb/gdbserver/server.c
> @@ -1637,6 +1637,7 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
>  	  strcat (own_buf, ";qXfer:statictrace:read+");
>  	  strcat (own_buf, ";qXfer:traceframe-info:read+");
>  	  strcat (own_buf, ";EnableDisableTracepoints+");
> +	  strcat (own_buf, ";QTBuffer:size+");

This new feature needs to be documented as well.

>  	  strcat (own_buf, ";tracenz+");
>  	}
>  
> diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
> index 0f83ae6..61d09c0 100644
> --- a/gdb/gdbserver/tracepoint.c
> +++ b/gdb/gdbserver/tracepoint.c
> @@ -30,6 +30,8 @@
>  
>  #include "ax.h"
>  
> +#define DEFAULT_TRACE_BUFFER_SIZE 1048576 /* 1024*1024 */

This actually changes the default:

-  /* There currently no way to change the buffer size.  */
-  const int sizeOfBuffer = 5 * 1024 * 1024;

Not sure if that was intended.  If it was, please do that
as a separate change.

> +
>  /* This file is built for both GDBserver, and the in-process
>     agent (IPA), a shared library that includes a tracing agent that is
>     loaded by the inferior to support fast tracepoints.  Fast
> @@ -992,6 +994,8 @@ int current_traceframe = -1;
>  static int circular_trace_buffer;
>  #endif
>  
> +static LONGEST trace_buffer_size;
> +

Misses leading comment.

>  static void
> +cmd_bigqtbuffer_size (char *own_buf)
> +{
> +  ULONGEST val;
> +  LONGEST sval;
> +  char *packet = own_buf;
> +
> +  /* Can't change the size during a tracing run.  */
> +  if (tracing)
> +    {
> +      write_enn (own_buf);
> +      return;
> +    }
> +
> +  packet += strlen ("QTBuffer:size:");
> +
> +  /* -1 is sent as literal "-1".  */
> +  if (strcmp (packet, "-1") == 0)
> +    sval = DEFAULT_TRACE_BUFFER_SIZE;
> +  else
> +    {
> +      unpack_varlen_hex (packet, &val);
> +      sval = (LONGEST)val;
> +    }
> +  /* If the size is unchanged, there's nothing to do.  */
> +  if (sval == trace_buffer_size)
> +    {
> +      write_ok (own_buf);
> +      return;
> +    }
> +
> +  init_trace_buffer (sval);

Oh, I just realized this does nothing to update
the trace buffer of the in-process agent.  :-/

> +  trace_debug ("Trace buffer is now %s bytes",
> +	       plongest (trace_buffer_size));
> +  write_ok (own_buf);

Might as well write the more concise:

    if (sval == trace_buffer_size)
      {
        init_trace_buffer (sval);
        trace_debug ("Trace buffer is now %s bytes",
	       plongest (trace_buffer_size));
      }
    write_ok (own_buf);

> +}



> +static void
> +remote_set_trace_buffer_size (LONGEST val)
> +{
> +  if (remote_protocol_packets[PACKET_QTBuffer_size].support !=
> +      PACKET_DISABLE)
> +    {
> +      struct remote_state *rs = get_remote_state ();
> +      char *buf = rs->buf;
> +      char *endbuf = rs->buf + get_remote_packet_size ();
> +      enum packet_result result;
> +
> +      if (val < 0 && val != -1)
> +	{
> +	  warning (_("Invalid value %s for the size of trace buffer."),
> +		   plongest (val) );

Should really be an assertion.  There should be no way to
trigger this, unless there's a bug elsewhere, right?

> +	  return;
> +	}
> +
> +      buf += xsnprintf (buf, endbuf - buf, "QTBuffer:size:");
> +      /* Send -1 as literal "-1" to avoid host size dependency.  */
> +      if (val < 0)
> +	buf += xsnprintf (buf, endbuf - buf, "-1");
> +      else
> +	buf += hexnumstr (buf, (ULONGEST) val);
> +
> +      putpkt (rs->buf);
> +      getpkt (&rs->buf, &rs->buf_size, 0);

Any reason this version lost remote_get_noisy_reply?

> +      result = packet_ok (rs->buf,
> +		  &remote_protocol_packets[PACKET_QTBuffer_size]);
> +
> +      if (result != PACKET_OK)
> +	warning (_("Bogus reply from target: %s"), rs->buf);
> +    }
> +}


> diff --git a/gdb/testsuite/gdb.trace/trace-buffer-size.exp b/gdb/testsuite/gdb.trace/trace-buffer-size.exp

> +set BUFFER_SIZE 4
> +gdb_test_no_output \
> +"set trace-buffer-size $BUFFER_SIZE" \
> +"Set Trace Buffer Size"
> +
> +gdb_test "show trace-buffer-size $BUFFER_SIZE" \
> +"Requested size of trace buffer is $BUFFER_SIZE.*" \
> +"Show Trace Buffer Size"

Please indent the continuation lines, like you do below.

> +
> +# We set trace buffer to very small size.  Then after running trace,
> +# we check if it is full.  This will show if setting trace buffer
> +# size really worked.
> +gdb_breakpoint ${srcfile}:[gdb_get_line_number "breakpoint1"]
> +gdb_test "trace test_function" \
> +  "Tracepoint \[0-9\]+ at .*" \
> +  "set tracepoint at test_function"
> +gdb_trace_setactions "Set action for trace point 1" "" \
> +  "collect var" "^$"
> +gdb_test_no_output "tstart" 
> +gdb_test "continue" \
> +  "Continuing.*Breakpoint $decimal.*" \
> +  "run trace experiment 1"
> +gdb_test "tstatus" \
> +  ".*Trace stopped because the buffer was full.*" \
> +  "Buffer full check 1"

s/Buffer/buffer/

> +
> +# Use the default size  and trace buffer should not be
> +# full this time.

Spurious double space after size.  I think connecting
the independent clauses with "and" reads a bit confusingly.
I suggest:

  # Use the default size -- the trace buffer should not end up
  # full this time.

> +clean_restart ${testfile}
> +runto_main
> +gdb_breakpoint ${srcfile}:[gdb_get_line_number "breakpoint1"]
> +gdb_test "trace test_function" \
> +  "Tracepoint \[0-9\]+ at .*" \
> +  "set tracepoint at test_function"
> +gdb_trace_setactions "Set action for trace point 2" "" \
> +  "collect var" "^$"
> +gdb_test_no_output "tstart" 
> +gdb_test "continue" \
> +  "Continuing.*Breakpoint $decimal.*" \
> +  "run trace experiment 2"
> +gdb_test "tstatus" \
> +  ".*Trace is running on the target.*" \
> +  "Buffer full check 2"

s/Buffer/buffer/

> +gdb_test_no_output "tstop"
> diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
> index 9a80aa3..b8945bb 100644
> --- a/gdb/tracepoint.c
> +++ b/gdb/tracepoint.c
> @@ -173,6 +173,11 @@ static int disconnected_tracing;
>  
>  static int circular_trace_buffer;
>  
> +/* This variable is the requested trace buffer size, or -1 to indicate
> +   that we don't care and leave it up to the target to set a size.  */
> +
> +static int trace_buffer_size = -1;
> +

This is used with:

extern void
  add_setshow_zuinteger_unlimited_cmd (char *name,
				       enum command_class class,
				       unsigned int *var,
                                       ^^^^^^^^^^^^

So, ... we were discussing this in the other thread.
Not sure what to tell you to do here.  :-)  My inclination
is to change the function to take a signed integer, and
use -1 instead of UINT_MAX.

>  /* Textual notes applying to the current and/or future trace runs.  */
>  
>  char *trace_user = NULL;
> @@ -1829,6 +1834,7 @@ start_tracing (char *notes)
>    /* Set some mode flags.  */
>    target_set_disconnected_tracing (disconnected_tracing);
>    target_set_circular_trace_buffer (circular_trace_buffer);
> +  target_set_trace_buffer_size (trace_buffer_size);
>  
>    if (!notes)
>      notes = trace_notes;
> @@ -3231,6 +3237,13 @@ set_circular_trace_buffer (char *args, int from_tty,
>  }

-- 
Pedro Alves

Re: [patch] Change trace buffer size(v4)

[Abid, Hafiz]

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

Hi Pedro,
Thanks for review. Here is an updated patch.

On 04/03/13 20:43:20, Pedro Alves wrote:
> One idea to make the testsuite check this would be to leverage  
> "tstatus",
> which prints the trace buffer size:
Done.

> The minus sign was missing:
> if (val < 0)
>     {
>       *buf++ = '-';
>       buf += hexnumstr (buf, (ULONGEST) -val);
>     }​
>   else
>     buf += hexnumstr (buf, (ULONGEST) val);
Done.

> Thanks.  The qSupported feature need to be documented as well.
Done.

> Dunno, haven't looked in detail but I'm hoping it to
> be obvious to whoever debugs it.  ;-)
I will look into it.

> 
> s/Added/Add/
Fixed. Sorry for making this mistake twice.

> Use '.', not ',' to separate changes.  "add realloc option" doesn't
> really make sense.  I can guess it referred to the "if" in a previous
> revision?
Fixed.

> s/Added/Add/
Fixed.

> 
> >     (Tracepoint Packets): Document QTBuffer:size .
> Spurious space after that last '.'.
Fixed.

> 
> This new feature needs to be documented as well.
Done.

> 
> -  /* There currently no way to change the buffer size.  */
> -  const int sizeOfBuffer = 5 * 1024 * 1024;
> 
> Not sure if that was intended.  If it was, please do that
> as a separate change.
Changed it back to 5*1024*1024;

> > +static LONGEST trace_buffer_size;
> Misses leading comment.
Fixed.
> > +  init_trace_buffer (sval);
> 
> Oh, I just realized this does nothing to update
> the trace buffer of the in-process agent.  :-/
Changing IPA trace buffer was not supported in this patch.
Do you think that it must be done before this patch can go in?

> > +  trace_debug ("Trace buffer is now %s bytes",
> > +	       plongest (trace_buffer_size));
> > +  write_ok (own_buf);
> 
> Might as well write the more concise:
> 
>     if (sval == trace_buffer_size)
>       {
>         init_trace_buffer (sval);
>         trace_debug ("Trace buffer is now %s bytes",
> 	       plongest (trace_buffer_size));
>       }
>     write_ok (own_buf);
I think you mean to say that there is no harm in calling  
init_trace_buffer
when sval == trace_buffer_size, as xrealloc will not do anything. So I
removed the check.

> 
> > +      if (val < 0 && val != -1)
> > +	{
> > +	  warning (_("Invalid value %s for the size of trace buffer."),
> > +		   plongest (val) );
> 
> Should really be an assertion.  There should be no way to
> trigger this, unless there's a bug elsewhere, right?
Yes. Only -1 in negative number should come here. So added an assertion.

> 
> > +      putpkt (rs->buf);
> > +      getpkt (&rs->buf, &rs->buf_size, 0);
> 
> Any reason this version lost remote_get_noisy_reply?
I noticed that packet_ok () is used with getpkt thoughout the file. So I
removed remote_get_noisy_reply.

> 
> > +      result = packet_ok (rs->buf,
> > +		  &remote_protocol_packets[PACKET_QTBuffer_size]);

> > +gdb_test "show trace-buffer-size $BUFFER_SIZE" \
> > +"Requested size of trace buffer is $BUFFER_SIZE.*" \
> > +"Show Trace Buffer Size"
> Please indent the continuation lines, like you do below.
Fixed. Sorry again on this. I should have been more careful.

> > +  "Buffer full check 1"
> s/Buffer/buffer/
Fixed.
> 
> > +
> > +# Use the default size  and trace buffer should not be
> > +# full this time.
> 
> Spurious double space after size.  I think connecting
> the independent clauses with "and" reads a bit confusingly.
> I suggest:
> 
>   # Use the default size -- the trace buffer should not end up
>   # full this time.
Fixed

> 
> > +  ".*Trace is running on the target.*" \
> > +  "Buffer full check 2"
> 
> s/Buffer/buffer/
Fixed.

> > +static int trace_buffer_size = -1;
> > +
> 
> This is used with:
> 
> extern void
>   add_setshow_zuinteger_unlimited_cmd (char *name,
> 				       enum command_class class,
> 				       unsigned int *var,
>                                        ^^^^^^^^^^^^
> 
> So, ... we were discussing this in the other thread.
> Not sure what to tell you to do here.  :-)  My inclination
> is to change the function to take a signed integer, and
> use -1 instead of UINT_MAX.
I see that Yao has already sent a patch for it. So probably I don't  
need any change
here.

Thanks,
Abid

2012-03-05  Stan Shebs  <stan@codesourcery.com>
	    Hafiz Abid Qadeer  <abidh@codesourcery.com>

	gdb/
	* NEWS: Mention set and show trace-buffer-size commands.
	Mention new packet.
	* target.h (struct target_ops): New method
	to_set_trace_buffer_size.
	(target_set_trace_buffer_size): New macro.
	* target.c (update_current_target): Set up new method.
	* tracepoint.c (trace_buffer_size): New global.
	(start_tracing): Send it to the target.
	(set_trace_buffer_size): New function.
	(_initialize_tracepoint): Add new setshow for trace-buffer-size.
	* remote.c (remote_set_trace_buffer_size): New function.
	(_initialize_remote): Use it.
	(PACKET_QTBuffer_size): New enum.
	(remote_protocol_features): Add an entry for
	PACKET_QTBuffer_size.

	gdb/gdbserver/
	* tracepoint.c (trace_buffer_size): New global.
	(DEFAULT_TRACE_BUFFER_SIZE): New define.
	(init_trace_buffer): Change to one-argument function. Allocate
	trace buffer memory.
	(handle_tracepoint_general_set): Call cmd_bigqtbuffer_size to
	handle QTBuffer:size packet.
	(cmd_bigqtbuffer_size): New function.
	(initialize_tracepoint): Call init_trace_buffer with
	DEFAULT_TRACE_BUFFER_SIZE.
	* server.c (handle_query): Add QTBuffer:size in the
	supported packets.

	gdb/doc/
	* gdb.texinfo (Starting and Stopping Trace Experiments):  
Document
	trace-buffer-size set and show commands.
	(Tracepoint Packets): Document QTBuffer:size.
	(General Query Packets): Document QTBuffer:size.

	gdb/testsuite/
	* gdb.trace/trace-buffer-size.exp: New file.
	* gdb.trace/trace-buffer-size.c: New file.

[-- Attachment #2: trace-buffer-size_v4.patch --]
[-- Type: text/x-patch, Size: 18257 bytes --]

diff --git a/gdb/NEWS b/gdb/NEWS
index 0877aa2..3fb68eb 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -104,6 +104,10 @@ set debug notification
 show debug notification
   Control display of debugging info for async remote notification.
 
+set trace-buffer-size
+show trace-buffer-size
+  Request target to change the size of trace buffer.
+
 * Removed commands
 
   ** For the Renesas Super-H architecture, the "regs" command has been removed
@@ -159,6 +163,11 @@ show filename-display
   feature to be enabled.  For more information, see:
       http://fedoraproject.org/wiki/Features/MiniDebugInfo
 
+* New remote packets
+
+QTBuffer:size
+   Set the size of trace buffer.
+
 *** Changes in GDB 7.5
 
 * GDB now supports x32 ABI.  Visit <http://sites.google.com/site/x32abi/>
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 5f39d2e..34226ce 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -11728,6 +11728,25 @@ for instance if you are looking at frames from a trace file.
 @end table
 
 @table @code
+@item set trace-buffer-size @var{n}
+@kindex set trace-buffer-size
+Request that the target use a trace buffer of @var{n} bytes.  Not all
+targets will honor the request; they may have a compiled-in size for
+the trace buffer, or some other limitation.  Set to a value of
+@code{-1} to let the target use whatever size it likes.  This is also
+the default.
+
+@item show trace-buffer-size
+@kindex show trace-buffer-size
+Show the current requested size for the trace buffer.  Note that this
+will only match the actual size if the target supports size-setting,
+and was able to handle the requested size.  For instance, if the
+target can only change buffer size between runs, this variable will
+not reflect the change until the next run starts.  Use @code{tstatus}
+to get a report of the actual buffer size.
+@end table
+
+@table @code
 @item set trace-user @var{text}
 @kindex set trace-user
 
@@ -37373,6 +37392,11 @@ These are the currently defined stub features and their properties:
 @tab @samp{-}
 @tab No
 
+@item @samp{QTBuffer:size}
+@tab No
+@tab @samp{-}
+@tab No
+
 @item @samp{tracenz}
 @tab No
 @tab @samp{-}
@@ -37527,6 +37551,10 @@ The remote stub supports the @samp{QTEnable} (@pxref{QTEnable}) and
 @samp{QTDisable} (@pxref{QTDisable}) packets that allow tracepoints
 to be enabled and disabled while a trace experiment is running.
 
+@item QTBuffer:size
+The remote stub supports the @samp{QTBuffer:size} (@pxref{QTBuffer:size})
+packet that allows to change the size of trace buffer.
+
 @item tracenz
 @cindex string tracing, in remote protocol
 The remote stub supports the @samp{tracenz} bytecode for collecting strings.
@@ -38450,6 +38478,11 @@ available.
 This packet directs the target to use a circular trace buffer if
 @var{value} is 1, or a linear buffer if the value is 0.
 
+@item QTBuffer:size:@var{size}
+This packet directs the target to make the trace buffer be of size
+@var{size} if possible.  A value of @code{-1} tells the target to
+use whatever size it prefers.
+
 @item QTNotes:@r{[}@var{type}:@var{text}@r{]}@r{[};@var{type}:@var{text}@r{]}@dots{}
 @cindex @samp{QTNotes} packet
 This packet adds optional textual notes to the trace run.  Allowable
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 768eae7..922a5bf 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1637,6 +1637,7 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 	  strcat (own_buf, ";qXfer:statictrace:read+");
 	  strcat (own_buf, ";qXfer:traceframe-info:read+");
 	  strcat (own_buf, ";EnableDisableTracepoints+");
+	  strcat (own_buf, ";QTBuffer:size+");
 	  strcat (own_buf, ";tracenz+");
 	}
 
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 0f83ae6..cb5ef15 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -30,6 +30,8 @@
 
 #include "ax.h"
 
+#define DEFAULT_TRACE_BUFFER_SIZE 5242880 /* 5*1024*1024 */
+
 /* This file is built for both GDBserver, and the in-process
    agent (IPA), a shared library that includes a tracing agent that is
    loaded by the inferior to support fast tracepoints.  Fast
@@ -992,6 +994,10 @@ int current_traceframe = -1;
 static int circular_trace_buffer;
 #endif
 
+/* Size of the trace buffer.  */
+
+static LONGEST trace_buffer_size;
+
 /* Pointer to the block of memory that traceframes all go into.  */
 
 static unsigned char *trace_buffer_lo;
@@ -1478,9 +1484,13 @@ clear_inferior_trace_buffer (void)
 #endif
 
 static void
-init_trace_buffer (unsigned char *buf, int bufsize)
+init_trace_buffer (LONGEST bufsize)
 {
-  trace_buffer_lo = buf;
+  trace_buffer_size = bufsize;
+
+  /* If we already have a trace buffer, try realloc'ing.  */
+  trace_buffer_lo = xrealloc (trace_buffer_lo, bufsize);
+
   trace_buffer_hi = trace_buffer_lo + bufsize;
 
   clear_trace_buffer ();
@@ -4020,6 +4030,37 @@ cmd_bigqtbuffer_circular (char *own_buf)
 }
 
 static void
+cmd_bigqtbuffer_size (char *own_buf)
+{
+  ULONGEST val;
+  LONGEST sval;
+  char *packet = own_buf;
+
+  /* Can't change the size during a tracing run.  */
+  if (tracing)
+    {
+      write_enn (own_buf);
+      return;
+    }
+
+  packet += strlen ("QTBuffer:size:");
+
+  /* -1 is sent as literal "-1".  */
+  if (strcmp (packet, "-1") == 0)
+    sval = DEFAULT_TRACE_BUFFER_SIZE;
+  else
+    {
+      unpack_varlen_hex (packet, &val);
+      sval = (LONGEST)val;
+    }
+ 
+  init_trace_buffer (sval);
+  trace_debug ("Trace buffer is now %s bytes",
+	       plongest (trace_buffer_size));
+  write_ok (own_buf);
+}
+
+static void
 cmd_qtnotes (char *own_buf)
 {
   size_t nbytes;
@@ -4143,6 +4184,11 @@ handle_tracepoint_general_set (char *packet)
       cmd_bigqtbuffer_circular (packet);
       return 1;
     }
+  else if (strncmp ("QTBuffer:size:", packet, strlen ("QTBuffer:size:")) == 0)
+    {
+      cmd_bigqtbuffer_size (packet);
+      return 1;
+    }
   else if (strncmp ("QTNotes:", packet, strlen ("QTNotes:")) == 0)
     {
       cmd_qtnotes (packet);
@@ -7228,10 +7274,8 @@ get_timestamp (void)
 void
 initialize_tracepoint (void)
 {
-  /* There currently no way to change the buffer size.  */
-  const int sizeOfBuffer = 5 * 1024 * 1024;
-  unsigned char *buf = xmalloc (sizeOfBuffer);
-  init_trace_buffer (buf, sizeOfBuffer);
+  /* Start with the default size.  */
+  init_trace_buffer (DEFAULT_TRACE_BUFFER_SIZE);
 
   /* Wire trace state variable 1 to be the timestamp.  This will be
      uploaded to GDB upon connection and become one of its trace state
diff --git a/gdb/remote.c b/gdb/remote.c
index 88a57c8..628419c 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1284,6 +1284,7 @@ enum {
   PACKET_qXfer_fdpic,
   PACKET_QDisableRandomization,
   PACKET_QAgent,
+  PACKET_QTBuffer_size,
   PACKET_MAX
 };
 
@@ -3993,6 +3994,8 @@ static struct protocol_feature remote_protocol_features[] = {
   { "QDisableRandomization", PACKET_DISABLE, remote_supported_packet,
     PACKET_QDisableRandomization },
   { "QAgent", PACKET_DISABLE, remote_supported_packet, PACKET_QAgent},
+  { "QTBuffer:size", PACKET_DISABLE,
+    remote_supported_packet, PACKET_QTBuffer_size},
   { "tracenz", PACKET_DISABLE,
     remote_string_tracing_feature, -1 },
 };
@@ -11042,6 +11045,38 @@ remote_get_min_fast_tracepoint_insn_len (void)
     }
 }
 
+static void
+remote_set_trace_buffer_size (LONGEST val)
+{
+  if (remote_protocol_packets[PACKET_QTBuffer_size].support !=
+      PACKET_DISABLE)
+    {
+      struct remote_state *rs = get_remote_state ();
+      char *buf = rs->buf;
+      char *endbuf = rs->buf + get_remote_packet_size ();
+      enum packet_result result;
+
+      gdb_assert (val >= 0 || val == -1);
+      buf += xsnprintf (buf, endbuf - buf, "QTBuffer:size:");
+      /* Send -1 as literal "-1" to avoid host size dependency.  */
+      if (val < 0)
+	{
+	  *buf++ = '-';
+          buf += hexnumstr (buf, (ULONGEST) -val);
+	}
+      else
+	buf += hexnumstr (buf, (ULONGEST) val);
+
+      putpkt (rs->buf);
+      getpkt (&rs->buf, &rs->buf_size, 0);
+      result = packet_ok (rs->buf,
+		  &remote_protocol_packets[PACKET_QTBuffer_size]);
+
+      if (result != PACKET_OK)
+	warning (_("Bogus reply from target: %s"), rs->buf);
+    }
+}
+
 static int
 remote_set_trace_notes (char *user, char *notes, char *stop_notes)
 {
@@ -11219,6 +11254,7 @@ Specify the serial device it is connected to\n\
   remote_ops.to_get_min_fast_tracepoint_insn_len = remote_get_min_fast_tracepoint_insn_len;
   remote_ops.to_set_disconnected_tracing = remote_set_disconnected_tracing;
   remote_ops.to_set_circular_trace_buffer = remote_set_circular_trace_buffer;
+  remote_ops.to_set_trace_buffer_size = remote_set_trace_buffer_size;
   remote_ops.to_set_trace_notes = remote_set_trace_notes;
   remote_ops.to_core_of_thread = remote_core_of_thread;
   remote_ops.to_verify_memory = remote_verify_memory;
@@ -11755,6 +11791,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
   add_packet_config_cmd (&remote_protocol_packets[PACKET_QAgent],
 			 "QAgent", "agent", 0);
 
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_QTBuffer_size],
+			 "QTBuffer:size", "trace-buffer-size", 0);
+
   /* Keep the old ``set remote Z-packet ...'' working.  Each individual
      Z sub-packet has its own set and show commands, but users may
      have sets to this variable in their .gdbinit files (or in their
diff --git a/gdb/target.c b/gdb/target.c
index 9d8bf6e..2e507e0 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -693,6 +693,7 @@ update_current_target (void)
       INHERIT (to_get_min_fast_tracepoint_insn_len, t);
       INHERIT (to_set_disconnected_tracing, t);
       INHERIT (to_set_circular_trace_buffer, t);
+      INHERIT (to_set_trace_buffer_size, t);
       INHERIT (to_set_trace_notes, t);
       INHERIT (to_get_tib_address, t);
       INHERIT (to_set_permissions, t);
@@ -912,6 +913,9 @@ update_current_target (void)
   de_fault (to_set_circular_trace_buffer,
 	    (void (*) (int))
 	    target_ignore);
+  de_fault (to_set_trace_buffer_size,
+	    (void (*) (LONGEST))
+	    target_ignore);
   de_fault (to_set_trace_notes,
 	    (int (*) (char *, char *, char *))
 	    return_zero);
diff --git a/gdb/target.h b/gdb/target.h
index 1971265..c99642d 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -806,6 +806,8 @@ struct target_ops
        disconnection - set VAL to 1 to keep tracing, 0 to stop.  */
     void (*to_set_disconnected_tracing) (int val);
     void (*to_set_circular_trace_buffer) (int val);
+    /* Set the size of trace buffer in the target.  */
+    void (*to_set_trace_buffer_size) (LONGEST val);
 
     /* Add/change textual notes about the trace run, returning 1 if
        successful, 0 otherwise.  */
@@ -1700,6 +1702,9 @@ extern char *target_fileio_read_stralloc (const char *filename);
 #define	target_set_circular_trace_buffer(val)	\
   (*current_target.to_set_circular_trace_buffer) (val)
 
+#define	target_set_trace_buffer_size(val)	\
+  (*current_target.to_set_trace_buffer_size) (val)
+
 #define	target_set_trace_notes(user,notes,stopnotes)		\
   (*current_target.to_set_trace_notes) ((user), (notes), (stopnotes))
 
diff --git a/gdb/testsuite/gdb.trace/trace-buffer-size.c b/gdb/testsuite/gdb.trace/trace-buffer-size.c
new file mode 100644
index 0000000..e32b756
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/trace-buffer-size.c
@@ -0,0 +1,31 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011-2013 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int var = 10;
+
+int
+test_function ()
+{
+  return 0;
+}
+
+int
+main ()
+{
+  test_function (); 
+  return 0; /*breakpoint1*/
+}
diff --git a/gdb/testsuite/gdb.trace/trace-buffer-size.exp b/gdb/testsuite/gdb.trace/trace-buffer-size.exp
new file mode 100644
index 0000000..9528fb3
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/trace-buffer-size.exp
@@ -0,0 +1,105 @@
+# Copyright 1998-2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib "trace-support.exp"
+
+standard_testfile
+
+if [prepare_for_testing ${testfile}.exp $testfile $srcfile \
+        {debug nowarnings}] {
+    untested "failed to prepare for trace tests"
+    return -1
+}
+
+if ![runto_main] {
+    fail "can't run to main to check for trace support"
+    return -1
+}
+
+if ![gdb_target_supports_trace] {
+    unsupported "target does not support trace"
+    return -1;
+}
+
+set BUFFER_SIZE 4
+
+# Save default trace buffer size in 'default_size'.
+gdb_test_multiple "tstatus" "tstatus check 1" {
+    -re ".*Trace buffer has ($decimal) bytes of ($decimal) bytes free.*" {
+        set default_size $expect_out(2,string)
+    }
+}
+
+# Change buffer size to 'BUFFER_SIZE'.
+gdb_test_no_output \
+  "set trace-buffer-size $BUFFER_SIZE" \
+  "set trace buffer size 1"
+
+gdb_test "tstatus" \
+  ".*Trace buffer has $decimal bytes of $BUFFER_SIZE bytes free.*" \
+  "tstatus check 2"
+
+gdb_test "show trace-buffer-size $BUFFER_SIZE" \
+  "Requested size of trace buffer is $BUFFER_SIZE.*" \
+  "show trace buffer size"
+
+gdb_test_no_output \
+  "set trace-buffer-size -1" \
+  "set trace buffer size 2"
+
+# Test that tstatus gives us default buffer size now.
+gdb_test "tstatus" \
+  ".*Trace buffer has $decimal bytes of $default_size bytes free.*" \
+  "tstatus check 3"
+
+gdb_test_no_output \
+  "set trace-buffer-size $BUFFER_SIZE" \
+  "set trace buffer size 3"
+
+# We set trace buffer to very small size.  Then after running trace,
+# we check if it is full.  This will show if setting trace buffer
+# size really worked.
+gdb_breakpoint ${srcfile}:[gdb_get_line_number "breakpoint1"]
+gdb_test "trace test_function" \
+  "Tracepoint \[0-9\]+ at .*" \
+  "set tracepoint at test_function"
+gdb_trace_setactions "Set action for trace point 1" "" \
+  "collect var" "^$"
+gdb_test_no_output "tstart" 
+gdb_test "continue" \
+  "Continuing.*Breakpoint $decimal.*" \
+  "run trace experiment 1"
+gdb_test "tstatus" \
+  ".*Trace stopped because the buffer was full.*" \
+  "buffer full check 1"
+
+# Use the default size -- the trace buffer should not end up
+# full this time
+clean_restart ${testfile}
+runto_main
+gdb_breakpoint ${srcfile}:[gdb_get_line_number "breakpoint1"]
+gdb_test "trace test_function" \
+  "Tracepoint \[0-9\]+ at .*" \
+  "set tracepoint at test_function"
+gdb_trace_setactions "Set action for trace point 2" "" \
+  "collect var" "^$"
+gdb_test_no_output "tstart" 
+gdb_test "continue" \
+  "Continuing.*Breakpoint $decimal.*" \
+  "run trace experiment 2"
+gdb_test "tstatus" \
+  ".*Trace is running on the target.*" \
+  "buffer full check 2"
+gdb_test_no_output "tstop"
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 9a80aa3..b8945bb 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -173,6 +173,11 @@ static int disconnected_tracing;
 
 static int circular_trace_buffer;
 
+/* This variable is the requested trace buffer size, or -1 to indicate
+   that we don't care and leave it up to the target to set a size.  */
+
+static int trace_buffer_size = -1;
+
 /* Textual notes applying to the current and/or future trace runs.  */
 
 char *trace_user = NULL;
@@ -1829,6 +1834,7 @@ start_tracing (char *notes)
   /* Set some mode flags.  */
   target_set_disconnected_tracing (disconnected_tracing);
   target_set_circular_trace_buffer (circular_trace_buffer);
+  target_set_trace_buffer_size (trace_buffer_size);
 
   if (!notes)
     notes = trace_notes;
@@ -3231,6 +3237,13 @@ set_circular_trace_buffer (char *args, int from_tty,
 }
 
 static void
+set_trace_buffer_size (char *args, int from_tty,
+			   struct cmd_list_element *c)
+{
+  target_set_trace_buffer_size (trace_buffer_size);
+}
+
+static void
 set_trace_user (char *args, int from_tty,
 		struct cmd_list_element *c)
 {
@@ -5416,6 +5429,16 @@ up and stopping the trace run."),
 			   &setlist,
 			   &showlist);
 
+  add_setshow_zuinteger_unlimited_cmd ("trace-buffer-size", no_class,
+			    &trace_buffer_size, _("\
+Set requested size of trace buffer."), _("\
+Show requested size of trace buffer."), _("\
+Use this to choose a size for the trace buffer.  Some targets\n\
+may have fixed or limited buffer sizes.  A value of -1 disables\n\
+any attempt to set the buffer size and lets the target choose."),
+			    set_trace_buffer_size, NULL,
+			    &setlist, &showlist);
+
   add_setshow_string_cmd ("trace-user", class_trace,
 			  &trace_user, _("\
 Set the user name to use for current and future trace runs"), _("\

Re: [patch] Change trace buffer size(v4)

[Pedro Alves]

On 03/05/2013 02:12 PM, Abid, Hafiz wrote:

>> Oh, I just realized this does nothing to update
>> the trace buffer of the in-process agent.  :-/
> Changing IPA trace buffer was not supported in this patch.
> Do you think that it must be done before this patch can go in?

It's always good for the submission to be explicit in these
things, and good to consider these issues upfront.  I'm interested
in hearing your thoughts on the subject upfront, and the plans
you had, if any.

I gave it some thought, and looked at the code to refresh a bit,
and I think things will still work correctly if the IPA buffer is
either smaller or larger than gdbserver's.

Running the whole set of gdb.trace tests (--directory=gdb.trace)
with a gdbserver and IPA hacked to default to different
buffer sizes once would provide some assurance, though
not that much.

>> > +  trace_debug ("Trace buffer is now %s bytes",
>> > +           plongest (trace_buffer_size));
>> > +  write_ok (own_buf);
>>
>> Might as well write the more concise:
>>
>>     if (sval == trace_buffer_size)
>>       {
>>         init_trace_buffer (sval);
>>         trace_debug ("Trace buffer is now %s bytes",
>>            plongest (trace_buffer_size));
>>       }
>>     write_ok (own_buf);
> I think you mean to say that there is no harm in calling init_trace_buffer
> when sval == trace_buffer_size, as xrealloc will not do anything. So I
> removed the check.

I had actually meant as I wrote it, to preserve your original
intention.  It's was functionally the same as the original, but
with only one write_ok call, and one return point, with the same
number of ifs, hence, more concise.  But removing the "if" is of
course fine too, and perhaps really better for being simpler
and for always displaying the debug message.

>> > +      putpkt (rs->buf);
>> > +      getpkt (&rs->buf, &rs->buf_size, 0);
>>
>> Any reason this version lost remote_get_noisy_reply?
> I noticed that packet_ok () is used with getpkt thoughout the file. So I
> removed remote_get_noisy_reply.

OTOH, tracepoint packets call remote_get_noisy_reply, for an
ancient reason, but nowadays to so that we assist the target
with relocation if necessary (qRelocInsn; gdbserver uses it).

> 
>>
>> > +      result = packet_ok (rs->buf,
>> > +          &remote_protocol_packets[PACKET_QTBuffer_size]);
> 
>> > +gdb_test "show trace-buffer-size $BUFFER_SIZE" \
>> > +"Requested size of trace buffer is $BUFFER_SIZE.*" \
>> > +"Show Trace Buffer Size"
>> Please indent the continuation lines, like you do below.
> Fixed. Sorry again on this. I should have been more careful.

Not a problem.  After a while these things start sticking
out.  It'll come to you too.  ;-)

> 2012-03-05  Stan Shebs  <stan@codesourcery.com>
>         Hafiz Abid Qadeer  <abidh@codesourcery.com>
> 
>     gdb/
>     * NEWS: Mention set and show trace-buffer-size commands.
>     Mention new packet.
>     * target.h (struct target_ops): New method
>     to_set_trace_buffer_size.
>     (target_set_trace_buffer_size): New macro.
>     * target.c (update_current_target): Set up new method.
>     * tracepoint.c (trace_buffer_size): New global.
>     (start_tracing): Send it to the target.
>     (set_trace_buffer_size): New function.
>     (_initialize_tracepoint): Add new setshow for trace-buffer-size.
>     * remote.c (remote_set_trace_buffer_size): New function.
>     (_initialize_remote): Use it.

Missed mentioning the new remote command.

> +@item QTBuffer:size
> +The remote stub supports the @samp{QTBuffer:size} (@pxref{QTBuffer:size})
> +packet that allows to change the size of trace buffer.
> +

"of the trace buffer" I think.

> +# Save default trace buffer size in 'default_size'.
> +gdb_test_multiple "tstatus" "tstatus check 1" {
> +    -re ".*Trace buffer has ($decimal) bytes of ($decimal) bytes free.*" {
> +        set default_size $expect_out(2,string)
> +    }
> +}

If this fails, then $default_size is left uninitialized,
and then further below, we'll get tcl errors for using
an unknown variable.  The pattern we usually follow around
such gdb_test_multiple cases, is to preinitialize the variable:

   set default_size -1
   gdb_test_multiple "tstatus" "tstatus check 1" {
      -re ".*Trace buffer has ($decimal) bytes of ($decimal) bytes free.*" {
          set default_size $expect_out(2,string)
      }
   }

and then something like,

 if { $default_size < 0 } {
   ...
 }

> +* New remote packets
> +
> +QTBuffer:size
> +   Set the size of trace buffer.

Add "The remote stub reports support for this packet
to gdb's qSupported query."

This looks to be almost ready now.  Thanks,
-- 
Pedro Alves

Re: [patch] Change trace buffer size(v5)

[Abid, Hafiz]

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

Hi Pedro,
Thanks for the review. Here is an updated patch.

> It's always good for the submission to be explicit in these
> things, and good to consider these issues upfront.  I'm interested
> in hearing your thoughts on the subject upfront, and the plans
> you had, if any.
> 
> I gave it some thought, and looked at the code to refresh a bit,
> and I think things will still work correctly if the IPA buffer is
> either smaller or larger than gdbserver's.
> 
> Running the whole set of gdb.trace tests (--directory=gdb.trace)
> with a gdbserver and IPA hacked to default to different
> buffer sizes once would provide some assurance, though
> not that much.
I have run the test cases as you mentioned and did not see any
regression. I have a plan for a future patch to handle IPA buffer
size too.

> > I noticed that packet_ok () is used with getpkt thoughout the file.  
> So I
> > removed remote_get_noisy_reply.
> 
> OTOH, tracepoint packets call remote_get_noisy_reply, for an
> ancient reason, but nowadays to so that we assist the target
> with relocation if necessary (qRelocInsn; gdbserver uses it).
This patch uses remote_get_noisy_reply now.

> >     * remote.c (remote_set_trace_buffer_size): New function.
> >     (_initialize_remote): Use it.
> 
> Missed mentioning the new remote command.
Added.

> > +@item QTBuffer:size
> > +The remote stub supports the @samp{QTBuffer:size}  
> (@pxref{QTBuffer:size})
> > +packet that allows to change the size of trace buffer.
> > +
> 
> "of the trace buffer" I think.
Fixed.

> If this fails, then $default_size is left uninitialized,
> and then further below, we'll get tcl errors for using
> an unknown variable.  The pattern we usually follow around
> such gdb_test_multiple cases, is to preinitialize the variable:
> 
>    set default_size -1
>    gdb_test_multiple "tstatus" "tstatus check 1" {
>       -re ".*Trace buffer has ($decimal) bytes of ($decimal) bytes  
> free.*" {
>           set default_size $expect_out(2,string)
>       }
>    }
> 
> and then something like,
> 
>  if { $default_size < 0 } {
>    ...
>  }
Fixed.

> 
> > +* New remote packets
> > +
> > +QTBuffer:size
> > +   Set the size of trace buffer.
> 
> Add "The remote stub reports support for this packet
> to gdb's qSupported query."
Fixed.

Thanks,
Abid

2012-03-08  Stan Shebs  <stan@codesourcery.com>
	    Hafiz Abid Qadeer  <abidh@codesourcery.com>

	gdb/
	* NEWS: Mention set and show trace-buffer-size commands.
	Mention new packet.
	* target.h (struct target_ops): New method
	to_set_trace_buffer_size.
	(target_set_trace_buffer_size): New macro.
	* target.c (update_current_target): Set up new method.
	* tracepoint.c (trace_buffer_size): New global.
	(start_tracing): Send it to the target.
	(set_trace_buffer_size): New function.
	(_initialize_tracepoint): Add new setshow for trace-buffer-size.
	* remote.c (remote_set_trace_buffer_size): New function.
	(_initialize_remote): Use it.
	(QTBuffer:size) New remote command.
	(PACKET_QTBuffer_size): New enum.
	(remote_protocol_features): Add an entry for
	PACKET_QTBuffer_size.

	gdb/gdbserver/
	* tracepoint.c (trace_buffer_size): New global.
	(DEFAULT_TRACE_BUFFER_SIZE): New define.
	(init_trace_buffer): Change to one-argument function. Allocate
	trace buffer memory.
	(handle_tracepoint_general_set): Call cmd_bigqtbuffer_size to
	handle QTBuffer:size packet.
	(cmd_bigqtbuffer_size): New function.
	(initialize_tracepoint): Call init_trace_buffer with
	DEFAULT_TRACE_BUFFER_SIZE.
	* server.c (handle_query): Add QTBuffer:size in the
	supported packets.

	gdb/doc/
	* gdb.texinfo (Starting and Stopping Trace Experiments):  
Document
	trace-buffer-size set and show commands.
	(Tracepoint Packets): Document QTBuffer:size.
	(General Query Packets): Document QTBuffer:size.

	gdb/testsuite/
	* gdb.trace/trace-buffer-size.exp: New file.
	* gdb.trace/trace-buffer-size.c: New file.


[-- Attachment #2: trace-buffer-size_v5.patch --]
[-- Type: text/x-patch, Size: 18554 bytes --]

diff --git a/gdb/NEWS b/gdb/NEWS
index 05fa498..99b8add 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -105,6 +105,10 @@ set debug notification
 show debug notification
   Control display of debugging info for async remote notification.
 
+set trace-buffer-size
+show trace-buffer-size
+  Request target to change the size of trace buffer.
+
 * Removed commands
 
   ** For the Renesas Super-H architecture, the "regs" command has been removed
@@ -160,6 +164,12 @@ show filename-display
   feature to be enabled.  For more information, see:
       http://fedoraproject.org/wiki/Features/MiniDebugInfo
 
+* New remote packets
+
+QTBuffer:size
+   Set the size of trace buffer.  The remote stub reports support for this
+   packet to gdb's qSupported query.
+
 *** Changes in GDB 7.5
 
 * GDB now supports x32 ABI.  Visit <http://sites.google.com/site/x32abi/>
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 5f39d2e..f0caef1 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -11728,6 +11728,25 @@ for instance if you are looking at frames from a trace file.
 @end table
 
 @table @code
+@item set trace-buffer-size @var{n}
+@kindex set trace-buffer-size
+Request that the target use a trace buffer of @var{n} bytes.  Not all
+targets will honor the request; they may have a compiled-in size for
+the trace buffer, or some other limitation.  Set to a value of
+@code{-1} to let the target use whatever size it likes.  This is also
+the default.
+
+@item show trace-buffer-size
+@kindex show trace-buffer-size
+Show the current requested size for the trace buffer.  Note that this
+will only match the actual size if the target supports size-setting,
+and was able to handle the requested size.  For instance, if the
+target can only change buffer size between runs, this variable will
+not reflect the change until the next run starts.  Use @code{tstatus}
+to get a report of the actual buffer size.
+@end table
+
+@table @code
 @item set trace-user @var{text}
 @kindex set trace-user
 
@@ -37373,6 +37392,11 @@ These are the currently defined stub features and their properties:
 @tab @samp{-}
 @tab No
 
+@item @samp{QTBuffer:size}
+@tab No
+@tab @samp{-}
+@tab No
+
 @item @samp{tracenz}
 @tab No
 @tab @samp{-}
@@ -37527,6 +37551,10 @@ The remote stub supports the @samp{QTEnable} (@pxref{QTEnable}) and
 @samp{QTDisable} (@pxref{QTDisable}) packets that allow tracepoints
 to be enabled and disabled while a trace experiment is running.
 
+@item QTBuffer:size
+The remote stub supports the @samp{QTBuffer:size} (@pxref{QTBuffer:size})
+packet that allows to change the size of the trace buffer.
+
 @item tracenz
 @cindex string tracing, in remote protocol
 The remote stub supports the @samp{tracenz} bytecode for collecting strings.
@@ -38450,6 +38478,11 @@ available.
 This packet directs the target to use a circular trace buffer if
 @var{value} is 1, or a linear buffer if the value is 0.
 
+@item QTBuffer:size:@var{size}
+This packet directs the target to make the trace buffer be of size
+@var{size} if possible.  A value of @code{-1} tells the target to
+use whatever size it prefers.
+
 @item QTNotes:@r{[}@var{type}:@var{text}@r{]}@r{[};@var{type}:@var{text}@r{]}@dots{}
 @cindex @samp{QTNotes} packet
 This packet adds optional textual notes to the trace run.  Allowable
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 768eae7..922a5bf 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1637,6 +1637,7 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 	  strcat (own_buf, ";qXfer:statictrace:read+");
 	  strcat (own_buf, ";qXfer:traceframe-info:read+");
 	  strcat (own_buf, ";EnableDisableTracepoints+");
+	  strcat (own_buf, ";QTBuffer:size+");
 	  strcat (own_buf, ";tracenz+");
 	}
 
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 632ee14..8c46483 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -30,6 +30,8 @@
 
 #include "ax.h"
 
+#define DEFAULT_TRACE_BUFFER_SIZE 5242880 /* 5*1024*1024 */
+
 /* This file is built for both GDBserver, and the in-process
    agent (IPA), a shared library that includes a tracing agent that is
    loaded by the inferior to support fast tracepoints.  Fast
@@ -992,6 +994,10 @@ int current_traceframe = -1;
 static int circular_trace_buffer;
 #endif
 
+/* Size of the trace buffer.  */
+
+static LONGEST trace_buffer_size;
+
 /* Pointer to the block of memory that traceframes all go into.  */
 
 static unsigned char *trace_buffer_lo;
@@ -1478,9 +1484,13 @@ clear_inferior_trace_buffer (void)
 #endif
 
 static void
-init_trace_buffer (unsigned char *buf, int bufsize)
+init_trace_buffer (LONGEST bufsize)
 {
-  trace_buffer_lo = buf;
+  trace_buffer_size = bufsize;
+
+  /* If we already have a trace buffer, try realloc'ing.  */
+  trace_buffer_lo = xrealloc (trace_buffer_lo, bufsize);
+
   trace_buffer_hi = trace_buffer_lo + bufsize;
 
   clear_trace_buffer ();
@@ -4020,6 +4030,37 @@ cmd_bigqtbuffer_circular (char *own_buf)
 }
 
 static void
+cmd_bigqtbuffer_size (char *own_buf)
+{
+  ULONGEST val;
+  LONGEST sval;
+  char *packet = own_buf;
+
+  /* Can't change the size during a tracing run.  */
+  if (tracing)
+    {
+      write_enn (own_buf);
+      return;
+    }
+
+  packet += strlen ("QTBuffer:size:");
+
+  /* -1 is sent as literal "-1".  */
+  if (strcmp (packet, "-1") == 0)
+    sval = DEFAULT_TRACE_BUFFER_SIZE;
+  else
+    {
+      unpack_varlen_hex (packet, &val);
+      sval = (LONGEST)val;
+    }
+
+  init_trace_buffer (sval);
+  trace_debug ("Trace buffer is now %s bytes",
+	       plongest (trace_buffer_size));
+  write_ok (own_buf);
+}
+
+static void
 cmd_qtnotes (char *own_buf)
 {
   size_t nbytes;
@@ -4143,6 +4184,11 @@ handle_tracepoint_general_set (char *packet)
       cmd_bigqtbuffer_circular (packet);
       return 1;
     }
+  else if (strncmp ("QTBuffer:size:", packet, strlen ("QTBuffer:size:")) == 0)
+    {
+      cmd_bigqtbuffer_size (packet);
+      return 1;
+    }
   else if (strncmp ("QTNotes:", packet, strlen ("QTNotes:")) == 0)
     {
       cmd_qtnotes (packet);
@@ -7228,10 +7274,8 @@ get_timestamp (void)
 void
 initialize_tracepoint (void)
 {
-  /* There currently no way to change the buffer size.  */
-  const int sizeOfBuffer = 5 * 1024 * 1024;
-  unsigned char *buf = xmalloc (sizeOfBuffer);
-  init_trace_buffer (buf, sizeOfBuffer);
+  /* Start with the default size.  */
+  init_trace_buffer (DEFAULT_TRACE_BUFFER_SIZE);
 
   /* Wire trace state variable 1 to be the timestamp.  This will be
      uploaded to GDB upon connection and become one of its trace state
diff --git a/gdb/remote.c b/gdb/remote.c
index b69c8a8..4978fca 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1280,6 +1280,7 @@ enum {
   PACKET_qXfer_fdpic,
   PACKET_QDisableRandomization,
   PACKET_QAgent,
+  PACKET_QTBuffer_size,
   PACKET_MAX
 };
 
@@ -3989,6 +3990,8 @@ static struct protocol_feature remote_protocol_features[] = {
   { "QDisableRandomization", PACKET_DISABLE, remote_supported_packet,
     PACKET_QDisableRandomization },
   { "QAgent", PACKET_DISABLE, remote_supported_packet, PACKET_QAgent},
+  { "QTBuffer:size", PACKET_DISABLE,
+    remote_supported_packet, PACKET_QTBuffer_size},
   { "tracenz", PACKET_DISABLE,
     remote_string_tracing_feature, -1 },
 };
@@ -11038,6 +11041,38 @@ remote_get_min_fast_tracepoint_insn_len (void)
     }
 }
 
+static void
+remote_set_trace_buffer_size (LONGEST val)
+{
+  if (remote_protocol_packets[PACKET_QTBuffer_size].support !=
+      PACKET_DISABLE)
+    {
+      struct remote_state *rs = get_remote_state ();
+      char *buf = rs->buf;
+      char *endbuf = rs->buf + get_remote_packet_size ();
+      enum packet_result result;
+
+      gdb_assert (val >= 0 || val == -1);
+      buf += xsnprintf (buf, endbuf - buf, "QTBuffer:size:");
+      /* Send -1 as literal "-1" to avoid host size dependency.  */
+      if (val < 0)
+	{
+	  *buf++ = '-';
+          buf += hexnumstr (buf, (ULONGEST) -val);
+	}
+      else
+	buf += hexnumstr (buf, (ULONGEST) val);
+
+      putpkt (rs->buf);
+      remote_get_noisy_reply (&rs->buf, &rs->buf_size);
+      result = packet_ok (rs->buf,
+		  &remote_protocol_packets[PACKET_QTBuffer_size]);
+
+      if (result != PACKET_OK)
+	warning (_("Bogus reply from target: %s"), rs->buf);
+    }
+}
+
 static int
 remote_set_trace_notes (char *user, char *notes, char *stop_notes)
 {
@@ -11215,6 +11250,7 @@ Specify the serial device it is connected to\n\
   remote_ops.to_get_min_fast_tracepoint_insn_len = remote_get_min_fast_tracepoint_insn_len;
   remote_ops.to_set_disconnected_tracing = remote_set_disconnected_tracing;
   remote_ops.to_set_circular_trace_buffer = remote_set_circular_trace_buffer;
+  remote_ops.to_set_trace_buffer_size = remote_set_trace_buffer_size;
   remote_ops.to_set_trace_notes = remote_set_trace_notes;
   remote_ops.to_core_of_thread = remote_core_of_thread;
   remote_ops.to_verify_memory = remote_verify_memory;
@@ -11751,6 +11787,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
   add_packet_config_cmd (&remote_protocol_packets[PACKET_QAgent],
 			 "QAgent", "agent", 0);
 
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_QTBuffer_size],
+			 "QTBuffer:size", "trace-buffer-size", 0);
+
   /* Keep the old ``set remote Z-packet ...'' working.  Each individual
      Z sub-packet has its own set and show commands, but users may
      have sets to this variable in their .gdbinit files (or in their
diff --git a/gdb/target.c b/gdb/target.c
index eaf8b31..0b4f39d 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -693,6 +693,7 @@ update_current_target (void)
       INHERIT (to_get_min_fast_tracepoint_insn_len, t);
       INHERIT (to_set_disconnected_tracing, t);
       INHERIT (to_set_circular_trace_buffer, t);
+      INHERIT (to_set_trace_buffer_size, t);
       INHERIT (to_set_trace_notes, t);
       INHERIT (to_get_tib_address, t);
       INHERIT (to_set_permissions, t);
@@ -912,6 +913,9 @@ update_current_target (void)
   de_fault (to_set_circular_trace_buffer,
 	    (void (*) (int))
 	    target_ignore);
+  de_fault (to_set_trace_buffer_size,
+	    (void (*) (LONGEST))
+	    target_ignore);
   de_fault (to_set_trace_notes,
 	    (int (*) (char *, char *, char *))
 	    return_zero);
diff --git a/gdb/target.h b/gdb/target.h
index 1971265..c99642d 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -806,6 +806,8 @@ struct target_ops
        disconnection - set VAL to 1 to keep tracing, 0 to stop.  */
     void (*to_set_disconnected_tracing) (int val);
     void (*to_set_circular_trace_buffer) (int val);
+    /* Set the size of trace buffer in the target.  */
+    void (*to_set_trace_buffer_size) (LONGEST val);
 
     /* Add/change textual notes about the trace run, returning 1 if
        successful, 0 otherwise.  */
@@ -1700,6 +1702,9 @@ extern char *target_fileio_read_stralloc (const char *filename);
 #define	target_set_circular_trace_buffer(val)	\
   (*current_target.to_set_circular_trace_buffer) (val)
 
+#define	target_set_trace_buffer_size(val)	\
+  (*current_target.to_set_trace_buffer_size) (val)
+
 #define	target_set_trace_notes(user,notes,stopnotes)		\
   (*current_target.to_set_trace_notes) ((user), (notes), (stopnotes))
 
diff --git a/gdb/testsuite/gdb.trace/trace-buffer-size.c b/gdb/testsuite/gdb.trace/trace-buffer-size.c
new file mode 100644
index 0000000..56159c3
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/trace-buffer-size.c
@@ -0,0 +1,31 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011-2013 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int var = 10;
+
+int
+test_function ()
+{
+  return 0;
+}
+
+int
+main ()
+{
+  test_function ();
+  return 0; /*breakpoint1*/
+}
diff --git a/gdb/testsuite/gdb.trace/trace-buffer-size.exp b/gdb/testsuite/gdb.trace/trace-buffer-size.exp
new file mode 100644
index 0000000..832b635
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/trace-buffer-size.exp
@@ -0,0 +1,113 @@
+# Copyright 1998-2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib "trace-support.exp"
+
+standard_testfile
+
+if [prepare_for_testing ${testfile}.exp $testfile $srcfile \
+        {debug nowarnings}] {
+    untested "failed to prepare for trace tests"
+    return -1
+}
+
+if ![runto_main] {
+    fail "can't run to main to check for trace support"
+    return -1
+}
+
+if ![gdb_target_supports_trace] {
+    unsupported "target does not support trace"
+    return -1;
+}
+
+set BUFFER_SIZE 4
+set default_size -1
+
+# Save default trace buffer size in 'default_size'.
+gdb_test_multiple "tstatus" "tstatus check 1" {
+    -re ".*Trace buffer has ($decimal) bytes of ($decimal) bytes free.*" {
+        set default_size $expect_out(2,string)
+    }
+}
+
+# If we did not get the default size then there is no point in running the
+# tests below.
+if { $default_size < 0 } {
+  fail "Get default buffer size."
+  return -1;
+}
+
+# Change buffer size to 'BUFFER_SIZE'.
+gdb_test_no_output \
+  "set trace-buffer-size $BUFFER_SIZE" \
+  "set trace buffer size 1"
+
+gdb_test "tstatus" \
+  ".*Trace buffer has $decimal bytes of $BUFFER_SIZE bytes free.*" \
+  "tstatus check 2"
+
+gdb_test "show trace-buffer-size $BUFFER_SIZE" \
+  "Requested size of trace buffer is $BUFFER_SIZE.*" \
+  "show trace buffer size"
+
+gdb_test_no_output \
+  "set trace-buffer-size -1" \
+  "set trace buffer size 2"
+
+# Test that tstatus gives us default buffer size now.
+gdb_test "tstatus" \
+  ".*Trace buffer has $decimal bytes of $default_size bytes free.*" \
+  "tstatus check 3"
+
+gdb_test_no_output \
+  "set trace-buffer-size $BUFFER_SIZE" \
+  "set trace buffer size 3"
+
+# We set trace buffer to very small size.  Then after running trace,
+# we check if it is full.  This will show if setting trace buffer
+# size really worked.
+gdb_breakpoint ${srcfile}:[gdb_get_line_number "breakpoint1"]
+gdb_test "trace test_function" \
+  "Tracepoint \[0-9\]+ at .*" \
+  "set tracepoint at test_function"
+gdb_trace_setactions "Set action for trace point 1" "" \
+  "collect var" "^$"
+gdb_test_no_output "tstart"
+gdb_test "continue" \
+  "Continuing.*Breakpoint $decimal.*" \
+  "run trace experiment 1"
+gdb_test "tstatus" \
+  ".*Trace stopped because the buffer was full.*" \
+  "buffer full check 1"
+
+# Use the default size -- the trace buffer should not end up
+# full this time
+clean_restart ${testfile}
+runto_main
+gdb_breakpoint ${srcfile}:[gdb_get_line_number "breakpoint1"]
+gdb_test "trace test_function" \
+  "Tracepoint \[0-9\]+ at .*" \
+  "set tracepoint at test_function"
+gdb_trace_setactions "Set action for trace point 2" "" \
+  "collect var" "^$"
+gdb_test_no_output "tstart"
+gdb_test "continue" \
+  "Continuing.*Breakpoint $decimal.*" \
+  "run trace experiment 2"
+gdb_test "tstatus" \
+  ".*Trace is running on the target.*" \
+  "buffer full check 2"
+gdb_test_no_output "tstop"
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 2a4b245..0413903 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -170,6 +170,11 @@ static int disconnected_tracing;
 
 static int circular_trace_buffer;
 
+/* This variable is the requested trace buffer size, or -1 to indicate
+   that we don't care and leave it up to the target to set a size.  */
+
+static int trace_buffer_size = -1;
+
 /* Textual notes applying to the current and/or future trace runs.  */
 
 char *trace_user = NULL;
@@ -1819,6 +1824,7 @@ start_tracing (char *notes)
   /* Set some mode flags.  */
   target_set_disconnected_tracing (disconnected_tracing);
   target_set_circular_trace_buffer (circular_trace_buffer);
+  target_set_trace_buffer_size (trace_buffer_size);
 
   if (!notes)
     notes = trace_notes;
@@ -3217,6 +3223,13 @@ set_circular_trace_buffer (char *args, int from_tty,
 }
 
 static void
+set_trace_buffer_size (char *args, int from_tty,
+			   struct cmd_list_element *c)
+{
+  target_set_trace_buffer_size (trace_buffer_size);
+}
+
+static void
 set_trace_user (char *args, int from_tty,
 		struct cmd_list_element *c)
 {
@@ -5409,6 +5422,16 @@ up and stopping the trace run."),
 			   &setlist,
 			   &showlist);
 
+  add_setshow_zuinteger_unlimited_cmd ("trace-buffer-size", no_class,
+			    &trace_buffer_size, _("\
+Set requested size of trace buffer."), _("\
+Show requested size of trace buffer."), _("\
+Use this to choose a size for the trace buffer.  Some targets\n\
+may have fixed or limited buffer sizes.  A value of -1 disables\n\
+any attempt to set the buffer size and lets the target choose."),
+			    set_trace_buffer_size, NULL,
+			    &setlist, &showlist);
+
   add_setshow_string_cmd ("trace-user", class_trace,
 			  &trace_user, _("\
 Set the user name to use for current and future trace runs"), _("\

Re: [patch] Change trace buffer size(v5)

[Eli Zaretskii]

> Date: Fri, 8 Mar 2013 11:29:43 +0000
> From: "Abid, Hafiz" <hafiz_abid@mentor.com>
> CC: <gdb-patches@sourceware.org>, <stan@codesourcery.com>,
> 	<yao@codesourcery.com>, <eliz@gnu.org>
> 
> 	gdb/
> 	* NEWS: Mention set and show trace-buffer-size commands.
> 	Mention new packet.
> 	* target.h (struct target_ops): New method
> 	to_set_trace_buffer_size.
> 	(target_set_trace_buffer_size): New macro.
> 	* target.c (update_current_target): Set up new method.
> 	* tracepoint.c (trace_buffer_size): New global.
> 	(start_tracing): Send it to the target.
> 	(set_trace_buffer_size): New function.
> 	(_initialize_tracepoint): Add new setshow for trace-buffer-size.
> 	* remote.c (remote_set_trace_buffer_size): New function.
> 	(_initialize_remote): Use it.
> 	(QTBuffer:size) New remote command.
> 	(PACKET_QTBuffer_size): New enum.
> 	(remote_protocol_features): Add an entry for
> 	PACKET_QTBuffer_size.
> 
> 	gdb/gdbserver/
> 	* tracepoint.c (trace_buffer_size): New global.
> 	(DEFAULT_TRACE_BUFFER_SIZE): New define.
> 	(init_trace_buffer): Change to one-argument function. Allocate
> 	trace buffer memory.
> 	(handle_tracepoint_general_set): Call cmd_bigqtbuffer_size to
> 	handle QTBuffer:size packet.
> 	(cmd_bigqtbuffer_size): New function.
> 	(initialize_tracepoint): Call init_trace_buffer with
> 	DEFAULT_TRACE_BUFFER_SIZE.
> 	* server.c (handle_query): Add QTBuffer:size in the
> 	supported packets.
> 
> 	gdb/doc/
> 	* gdb.texinfo (Starting and Stopping Trace Experiments):  
> Document
> 	trace-buffer-size set and show commands.
> 	(Tracepoint Packets): Document QTBuffer:size.
> 	(General Query Packets): Document QTBuffer:size.
> 
> 	gdb/testsuite/
> 	* gdb.trace/trace-buffer-size.exp: New file.
> 	* gdb.trace/trace-buffer-size.c: New file.

I already approved the documentation parts of this, didn't I?

Re: [patch] Change trace buffer size(v5)

[Abid, Hafiz]

On 08/03/13 14:08:27, Eli Zaretskii wrote:
> > Date: Fri, 8 Mar 2013 11:29:43 +0000
> > From: "Abid, Hafiz" <hafiz_abid@mentor.com>
> > CC: <gdb-patches@sourceware.org>, <stan@codesourcery.com>,
> > 	<yao@codesourcery.com>, <eliz@gnu.org>
> >
> > 	gdb/
> > 	* NEWS: Mention set and show trace-buffer-size commands.
> > 	Mention new packet.
> > 	* target.h (struct target_ops): New method
> > 	to_set_trace_buffer_size.
> > 	(target_set_trace_buffer_size): New macro.
> > 	* target.c (update_current_target): Set up new method.
> > 	* tracepoint.c (trace_buffer_size): New global.
> > 	(start_tracing): Send it to the target.
> > 	(set_trace_buffer_size): New function.
> > 	(_initialize_tracepoint): Add new setshow for trace-buffer-size.
> > 	* remote.c (remote_set_trace_buffer_size): New function.
> > 	(_initialize_remote): Use it.
> > 	(QTBuffer:size) New remote command.
> > 	(PACKET_QTBuffer_size): New enum.
> > 	(remote_protocol_features): Add an entry for
> > 	PACKET_QTBuffer_size.
> >
> > 	gdb/gdbserver/
> > 	* tracepoint.c (trace_buffer_size): New global.
> > 	(DEFAULT_TRACE_BUFFER_SIZE): New define.
> > 	(init_trace_buffer): Change to one-argument function. Allocate
> > 	trace buffer memory.
> > 	(handle_tracepoint_general_set): Call cmd_bigqtbuffer_size to
> > 	handle QTBuffer:size packet.
> > 	(cmd_bigqtbuffer_size): New function.
> > 	(initialize_tracepoint): Call init_trace_buffer with
> > 	DEFAULT_TRACE_BUFFER_SIZE.
> > 	* server.c (handle_query): Add QTBuffer:size in the
> > 	supported packets.
> >
> > 	gdb/doc/
> > 	* gdb.texinfo (Starting and Stopping Trace Experiments):
> > Document
> > 	trace-buffer-size set and show commands.
> > 	(Tracepoint Packets): Document QTBuffer:size.
> > 	(General Query Packets): Document QTBuffer:size.
> >
> > 	gdb/testsuite/
> > 	* gdb.trace/trace-buffer-size.exp: New file.
> > 	* gdb.trace/trace-buffer-size.c: New file.
> 
> I already approved the documentation parts of this, didn't I?
> 
Yes. Although some additions were done after that as suggested by Pedro.

Re: [patch] Change trace buffer size(v5)

[Pedro Alves]

On 03/08/2013 11:29 AM, Abid, Hafiz wrote:
> Hi Pedro,
>> It's always good for the submission to be explicit in these
>> things, and good to consider these issues upfront.  I'm interested
>> in hearing your thoughts on the subject upfront, and the plans
>> you had, if any.
>>
>> I gave it some thought, and looked at the code to refresh a bit,
>> and I think things will still work correctly if the IPA buffer is
>> either smaller or larger than gdbserver's.
>>
>> Running the whole set of gdb.trace tests (--directory=gdb.trace)
>> with a gdbserver and IPA hacked to default to different
>> buffer sizes once would provide some assurance, though
>> not that much.
> I have run the test cases as you mentioned and did not see any
> regression.

Thanks.

> +      sval = (LONGEST)val;

Space after cast.

> +
> +set BUFFER_SIZE 4
> +set default_size -1
> +
> +# Save default trace buffer size in 'default_size'.

Add:

set test "get default buffer size"

> +gdb_test_multiple "tstatus" "tstatus check 1" {

Make this:

gdb_test_multiple "tstatus" $test

> +    -re ".*Trace buffer has ($decimal) bytes of ($decimal) bytes free.*" {
> +        set default_size $expect_out(2,string)

Add:

      pass $test

> +    }
> +}
> +
> +# If we did not get the default size then there is no point in running the
> +# tests below.
> +if { $default_size < 0 } {
> +  fail "Get default buffer size."

and remove this "fail" (watch for lowercase).  gdb_test_multiple
will have already called FAIL with its second argument on failure.

> +  return -1;
> +}

OK with these fixes.

Thanks,
-- 
Pedro Alves

Re: [patch] Change trace buffer size(v5)

[Yao Qi]

Hi Abid,

On 03/08/2013 07:29 PM, Abid, Hafiz wrote:
> 	gdb/doc/
> 	* gdb.texinfo (Starting and Stopping Trace Experiments):
> Document
> 	trace-buffer-size set and show commands.

The indentation looks odd here.

> +
> +# Change buffer size to 'BUFFER_SIZE'.
> +gdb_test_no_output \
> +  "set trace-buffer-size $BUFFER_SIZE" \
> +  "set trace buffer size 1"

Minor suggestion, if they can fit in one line, better to write them in 
one line.  The first backslash is not necessary.  Here and somewhere 
else.  We can write them in two lines:

gdb_test_no_output "set trace-buffer-size $BUFFER_SIZE" \
   "set trace buffer size 1"

> +  add_setshow_zuinteger_unlimited_cmd ("trace-buffer-size", no_class,
> +			    &trace_buffer_size, _("\
> +Set requested size of trace buffer."), _("\
> +Show requested size of trace buffer."), _("\
> +Use this to choose a size for the trace buffer.  Some targets\n\
> +may have fixed or limited buffer sizes.  A value of -1 disables\n\
> +any attempt to set the buffer size and lets the target choose."),
> +			    set_trace_buffer_size, NULL,
> +			    &setlist, &showlist);
> +

Indentation looks odd to me.

-- 
Yao (齐尧)

Re: [patch] Change trace buffer size(v5)

[Abid, Hafiz]

Hi Yao,
Sorry, I had already applied the patch before I saw your email.

> Minor suggestion, if they can fit in one line, better to write them  
> in one line.  The first backslash is not necessary.  Here and  
> somewhere else.  We can write them in two lines:
> 
> gdb_test_no_output "set trace-buffer-size $BUFFER_SIZE" \
>   "set trace buffer size 1"
I will keep this in mind for future work.

>> Document
>> 	trace-buffer-size set and show commands.
> 
> The indentation looks odd here.
> 
>> +			    set_trace_buffer_size, NULL,
>> +			    &setlist, &showlist);
>> +
> 
> Indentation looks odd to me.

If you can let me know which portion is wrong then it will help me.

Thanks,
Abid

Re: [patch] Change trace buffer size(v5)

[Yao Qi]

On 03/08/2013 11:14 PM, Abid, Hafiz wrote:
> Sorry, I had already applied the patch before I saw your email.
> 

Never mind.

>>> >>Document
>>> >>	trace-buffer-size set and show commands.
>> >
>> >The indentation looks odd here.
>> >

The doc/ChangeLog is correct.  Looks it was caused by copying changelog
entry into the mail.

>>> >>+			    set_trace_buffer_size, NULL,
>>> >>+			    &setlist, &showlist);
>>> >>+
>> >
>> >Indentation looks odd to me.
> If you can let me know which portion is wrong then it will help me.

The patch below fixes the problem I mentioned.  Committed as it is obvious.
-- 
Yao (齐尧)

gdb/
2013-03-08  Yao Qi  <yao@codesourcery.com>

	* tracepoint.c (_initialize_tracepoint): Indent the code.

Index: tracepoint.c
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.c,v
retrieving revision 1.284
diff -u -r1.284 tracepoint.c
--- tracepoint.c	8 Mar 2013 15:06:35 -0000	1.284
+++ tracepoint.c	8 Mar 2013 15:18:43 -0000
@@ -5423,14 +5423,14 @@
 			   &showlist);
 
   add_setshow_zuinteger_unlimited_cmd ("trace-buffer-size", no_class,
-			    &trace_buffer_size, _("\
+				       &trace_buffer_size, _("\
 Set requested size of trace buffer."), _("\
 Show requested size of trace buffer."), _("\
 Use this to choose a size for the trace buffer.  Some targets\n\
 may have fixed or limited buffer sizes.  A value of -1 disables\n\
 any attempt to set the buffer size and lets the target choose."),
-			    set_trace_buffer_size, NULL,
-			    &setlist, &showlist);
+				       set_trace_buffer_size, NULL,
+				       &setlist, &showlist);
 
   add_setshow_string_cmd ("trace-user", class_trace,
 			  &trace_user, _("\

Re: [patch] Change trace buffer size(v5)

[Joel Brobecker]

> +@item QTBuffer:size
> +The remote stub supports the @samp{QTBuffer:size} (@pxref{QTBuffer:size})
> +packet that allows to change the size of the trace buffer.

This causes the documenation to fail, which also affect the nightly
packaging of the sources:

./gdb.texinfo:37555: Cross reference to nonexistent node `QTBuffer:size' (perhaps incorrect sectioning?).

I am about to fly back home, can you fix this error?

Thank you,
-- 
Joel

Re: [patch] Change trace buffer size(v5)

[Eli Zaretskii]

> Date: Fri, 8 Mar 2013 21:06:26 -0500
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> 
> > +@item QTBuffer:size
> > +The remote stub supports the @samp{QTBuffer:size} (@pxref{QTBuffer:size})
> > +packet that allows to change the size of the trace buffer.
> 
> This causes the documenation to fail, which also affect the nightly
> packaging of the sources:
> 
> ./gdb.texinfo:37555: Cross reference to nonexistent node `QTBuffer:size' (perhaps incorrect sectioning?).

I sincerely hope no one is committing changes to the manuals without
saying "make" in the gdb/doc directory first.

RE: [patch] Change trace buffer size(v5)

[Abid, Hafiz]

________________________________________
From: Eli Zaretskii [eliz@gnu.org]
Sent: Saturday, March 09, 2013 8:27 AM
To: Joel Brobecker
Cc: Abid, Hafiz; gdb-patches@sourceware.org
Subject: Re: [patch] Change trace buffer size(v5)

> Date: Fri, 8 Mar 2013 21:06:26 -0500
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
>
> > +@item QTBuffer:size
> > +The remote stub supports the @samp{QTBuffer:size} (@pxref{QTBuffer:size})
> > +packet that allows to change the size of the trace buffer.
>
> This causes the documenation to fail, which also affect the nightly
> packaging of the sources:
>
> ./gdb.texinfo:37555: Cross reference to nonexistent node `QTBuffer:size' (perhaps incorrect sectioning?).

>I sincerely hope no one is committing changes to the manuals without
>saying "make" in the gdb/doc directory first.

Joel, Eli,
I am committing the patch below to fix the build. I think it counts as obvious. Please let me know if there is any problem. Sorry for the inconvenience.

Thanks,
Abid

2012-03-09  Hafiz Abid Qadeer  <abidh@codesourcery.com>

	gdb/doc/
	* gdb.texinfo (QTBuffer:size): Add cindex and anchor.

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index f0caef1..ba03fca 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -38479,6 +38479,8 @@ This packet directs the target to use a circular trace buffer if
 @var{value} is 1, or a linear buffer if the value is 0.
 
 @item QTBuffer:size:@var{size}
+@anchor{QTBuffer:size}
+@cindex @samp{QTBuffer:size} packet
 This packet directs the target to make the trace buffer be of size
 @var{size} if possible.  A value of @code{-1} tells the target to
 use whatever size it prefers.
 

RE: [patch] Change trace buffer size(v5)

[Abid, Hafiz]

> > +@item QTBuffer:size
> > +The remote stub supports the @samp{QTBuffer:size} (@pxref{QTBuffer:size})
> > +packet that allows to change the size of the trace buffer.
>
> This causes the documenation to fail, which also affect the nightly
> packaging of the sources:
>
> ./gdb.texinfo:37555: Cross reference to nonexistent node `QTBuffer:size' (perhaps incorrect sectioning?).

>I sincerely hope no one is committing changes to the manuals without
>saying "make" in the gdb/doc directory first.

I noticed that year in the Changelog was wrong. Are we allowed to change that after the commit.

Thanks,
Abid

Re: [patch] Change trace buffer size(v5)

[Eli Zaretskii]

> From: "Abid, Hafiz" <Hafiz_Abid@mentor.com>
> CC: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
> Date: Sat, 9 Mar 2013 10:20:12 +0000
> 
> I am committing the patch below to fix the build. I think it counts as obvious. Please let me know if there is any problem. Sorry for the inconvenience.
> 
> Thanks,
> Abid
> 
> 2012-03-09  Hafiz Abid Qadeer  <abidh@codesourcery.com>
> 
> 	gdb/doc/
> 	* gdb.texinfo (QTBuffer:size): Add cindex and anchor.
> 
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index f0caef1..ba03fca 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -38479,6 +38479,8 @@ This packet directs the target to use a circular trace buffer if
>  @var{value} is 1, or a linear buffer if the value is 0.
>  
>  @item QTBuffer:size:@var{size}
> +@anchor{QTBuffer:size}
> +@cindex @samp{QTBuffer:size} packet

Sorry, but this isn't right.  You cannot have colons in @anchor or in
@cindex entries, because that will confuse Info readers when they try
to deduce the referenced spot.  (This is explained in the Texinfo
manual, in the nodes "Node Line Requirements", "@anchor", and
"Indexing Commands".)

Fixed thusly:

2013-03-09  Eli Zaretskii  <eliz@gnu.org>

	* gdb.texinfo (General Query Packets, Tracepoint Packets): Don't
	use colons in @anchor and @cindex entries.


Index: gdb/doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.1056
retrieving revision 1.1057
diff -u -r1.1056 -r1.1057
--- gdb/doc/gdb.texinfo	9 Mar 2013 10:21:42 -0000	1.1056
+++ gdb/doc/gdb.texinfo	9 Mar 2013 11:01:00 -0000	1.1057
@@ -37552,7 +37552,7 @@
 to be enabled and disabled while a trace experiment is running.
 
 @item QTBuffer:size
-The remote stub supports the @samp{QTBuffer:size} (@pxref{QTBuffer:size})
+The remote stub supports the @samp{QTBuffer:size} (@pxref{QTBuffer-size})
 packet that allows to change the size of the trace buffer.
 
 @item tracenz
@@ -38479,8 +38479,8 @@
 @var{value} is 1, or a linear buffer if the value is 0.
 
 @item QTBuffer:size:@var{size}
-@anchor{QTBuffer:size}
-@cindex @samp{QTBuffer:size} packet
+@anchor{QTBuffer-size}
+@cindex @samp{QTBuffer size} packet
 This packet directs the target to make the trace buffer be of size
 @var{size} if possible.  A value of @code{-1} tells the target to
 use whatever size it prefers.

Re: [patch] Change trace buffer size(v5)

[Eli Zaretskii]

> From: "Abid, Hafiz" <Hafiz_Abid@mentor.com>
> CC: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
> Date: Sat, 9 Mar 2013 10:39:29 +0000
> 
> I noticed that year in the Changelog was wrong. Are we allowed to change that after the commit.

Yes, as a separate commit.  Only the version history is immutable;
files are not.

Re: [patch] Change trace buffer size(v5)

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Eli> Sorry, but this isn't right.  You cannot have colons in @anchor or in
Eli> @cindex entries, because that will confuse Info readers when they try
Eli> to deduce the referenced spot.

There are some other instances in gdb.texinfo:

barimba. egrep '(anchor|cindex).*:' gdb.texinfo 
@cindex @code{::}, context for variables/functions
@cindex @samp{qSearch:memory} packet
@cindex <xi:include>

Tom

Re: [patch] Change trace buffer size(v5)

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Cc: "Abid\, Hafiz" <Hafiz_Abid@mentor.com>, brobecker@adacore.com,
>         gdb-patches@sourceware.org
> Date: Mon, 11 Mar 2013 12:32:54 -0600
> 
> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
> 
> Eli> Sorry, but this isn't right.  You cannot have colons in @anchor or in
> Eli> @cindex entries, because that will confuse Info readers when they try
> Eli> to deduce the referenced spot.
> 
> There are some other instances in gdb.texinfo:
> 
> barimba. egrep '(anchor|cindex).*:' gdb.texinfo 
> @cindex @code{::}, context for variables/functions
> @cindex @samp{qSearch:memory} packet
> @cindex <xi:include>

Thanks, fixed as below.  (The other 2 places already had the
@ifnotinfo guards around these index entries.)


2013-03-11  Eli Zaretskii  <eliz@gnu.org>

	* gdb.texinfo (General Query Packets): Don't use colon in index
	entries visible to Info format.


Index: gdb/doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.1059
diff -u -r1.1059 gdb.texinfo
--- gdb/doc/gdb.texinfo	11 Mar 2013 08:55:36 -0000	1.1059
+++ gdb/doc/gdb.texinfo	11 Mar 2013 19:43:00 -0000
@@ -37250,7 +37250,10 @@
 
 @item qSearch:memory:@var{address};@var{length};@var{search-pattern}
 @cindex searching memory, in remote debugging
+@ifnotinfo
 @cindex @samp{qSearch:memory} packet
+@end ifnotinfo
+@cindex @samp{qSearch memory} packet
 @anchor{qSearch memory}
 Search @var{length} bytes at @var{address} for @var{search-pattern}.
 @var{address} and @var{length} are encoded in hex.

Re: [patch] Change trace buffer size(v5)

[Pedro Alves]

Hi!

Unfortunately, I just noticed that this broke "tstart" if a
trace run is already running:

(gdb) tstart
A trace is running already.  Start a new run? (y or n) y
Sending packet: $QTinit#59...Packet received: OK
Sending packet: $QTDP:3:0000000000410296:E:0:0#49...Packet received: OK
Sending packet: $QTDPsrc:3:410296:at:0:4:6d61696e#8a...Packet received: OK
Sending packet: $QTDV:1:0000000000000000:1:74726163655f74696d657374616d70#4f...Packet received: OK
Sending packet: $:0000000000400200,000000000040021c:000000000040021c,000000000040023c:000000000040023c,0000000000400260:0000000000400260,00000000004002c0:00000000004002c0,0000000000400ff8:0000000000400ff8,000000000040149e:000000000040149e,00000000004015b8:00000000004015b8,0000000000401628:0000000000401628,0000000000401688:0000000000401688,0000000000402210:0000000000402210,000000000040221e:0000000000402220,00000000004029e0:00000000004029e0,000000000043a230:000000000043a230,000000000043a239:000000000043a240,000000000044cafd:000000000044cb00,000000000044e834:000000000044e838,00000000004560a4#ab...Packet received:
Sending packet: $QTDisconnected:0#e2...Packet received: OK
Sending packet: $QTBuffer:circular:0#f8...Packet received: OK
Sending packet: $QTBuffer:size:-1#8c...Packet received: E01
Target returns error code '01'.
(gdb)

Perhaps gdbserver's QTinit handling should be calling
stop_tracing?

-- 
Pedro Alves

Re: [patch] Change trace buffer size(v5)

[Abid, Hafiz]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="us-ascii"; delsp=Yes; format=Flowed, Size: 6974 bytes --]

On 02/05/13 18:31:36, Pedro Alves wrote:
> Hi!
> 
> Unfortunately, I just noticed that this broke "tstart" if a
> trace run is already running:
> 
> (gdb) tstart
> A trace is running already.  Start a new run? (y or n) y
> Sending packet: $QTinit#59...Packet received: OK
> Sending packet: $QTDP:3:0000000000410296:E:0:0#49...Packet received:  
> OK
> Sending packet: $QTDPsrc:3:410296:at:0:4:6d61696e#8a...Packet  
> received: OK
> Sending packet:  
> $QTDV:1:0000000000000000:1:74726163655f74696d657374616d70#4f...Packet  
> received: OK
> Sending packet:  
> $:0000000000400200,000000000040021c:000000000040021c,000000000040023c:000000000040023c,0000000000400260:0000000000400260,00000000004002c0:00000000004002c0,0000000000400ff8:0000000000400ff8,000000000040149e:000000000040149e,00000000004015b8:00000000004015b8,0000000000401628:0000000000401628,0000000000401688:0000000000401688,0000000000402210:0000000000402210,000000000040221e:0000000000402220,00000000004029e0:00000000004029e0,000000000043a230:000000000043a230,000000000043a239:000000000043a240,000000000044cafd:000000000044cb00,000000000044e834:000000000044e838,00000000004560a4#ab...Packet  
> received:
> Sending packet: $QTDisconnected:0#e2...Packet received: OK
> Sending packet: $QTBuffer:circular:0#f8...Packet received: OK
> Sending packet: $QTBuffer:size:-1#8c...Packet received: E01
> Target returns error code '01'.
> (gdb)
> 
> Perhaps gdbserver's QTinit handling should be calling
> stop_tracing?
> 
> --
> Pedro Alves
> 
> 
Hi Pedro,
Thanks for letting me know. There is a testcase in  
status-stop.exp(test_tstart_tstart) to check this case but it is not  
working as intended. I will work on a patch.

Regards,
Abid
From gdb-patches-return-101045-listarch-gdb-patches=sources.redhat.com@sourceware.org Fri May 03 12:57:41 2013
Return-Path: <gdb-patches-return-101045-listarch-gdb-patches=sources.redhat.com@sourceware.org>
Delivered-To: listarch-gdb-patches@sources.redhat.com
Received: (qmail 13035 invoked by alias); 3 May 2013 12:57:41 -0000
Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Id: <gdb-patches.sourceware.org>
List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org>
List-Archive: <http://sourceware.org/ml/gdb-patches/>
List-Post: <mailto:gdb-patches@sourceware.org>
List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs>
Sender: gdb-patches-owner@sourceware.org
Delivered-To: mailing list gdb-patches@sourceware.org
Received: (qmail 13000 invoked by uid 89); 3 May 2013 12:57:37 -0000
X-Spam-SWARE-Status: No, score=-1.1 required=5.0 testsºYES_00,FREEMAIL_FROM,SPF_NEUTRAL autolearn=no version=3.3.1
Received: from slaygeal.uusia.org (HELO ip.uusia.org) (62.109.10.102)    by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Fri, 03 May 2013 12:57:34 +0000
Received: from [2a02:2560:6d4:26ca::1] (helo=waterlily.siamics.net)	by ip.uusia.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32)	(Exim 4.72)	(envelope-from <oneingray@gmail.com>)	id 1UYFYI-0008Pd-5g; Fri, 03 May 2013 12:57:30 +0000
Received: from violet.siamics.net ([2001:470:1f13:1eb::1:1d])	by waterlily.siamics.net with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32)	(Exim 4.72)	(envelope-from <oneingray@gmail.com>)	id 1UYFYB-0007jE-Iu; Fri, 03 May 2013 19:57:23 +0700
Received: from localhost ([::1] helo=violet.siamics.net)	by violet.siamics.net with esmtp (Exim 4.72)	(envelope-from <oneingray@gmail.com>)	id 1UYFYA-000491-Mm; Fri, 03 May 2013 19:57:22 +0700
From: Ivan Shmakov <oneingray@gmail.com>
To: bug-hurd@gnu.org, gdb-patches@sourceware.org
Cc: Ivan Shmakov <oneingray@gmail.com>
Subject: Re: [patch] for mig check in GDB's configure
References: <CAB8fV=gfGtguD28FGa-A5DZT8jqvEA1AoaK4dO=cHMQcCVvB-w@mail.gmail.com>	<8738u4sc19.fsf@kepler.schwinge.homeip.net>	<CAB8fV=hrvCmRd1AkMwvFsYZq1tCzgExWEfHLAURu6iWo3pTQ@mail.gmail.com>
Date: Fri, 03 May 2013 12:57:00 -0000
In-Reply-To: <CAB8fV=hrvCmRd1AkMwvFsYZq1tCzgExWEfHLAURu6iWo3pTQ@mail.gmail.com>	(=?utf-8?B?IumZhuWysyIncw==?= message of "Fri, 3 May 2013 18:43:49 +0800")
Message-ID: <87txmkxlu6.fsf@violet.siamics.net>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.4 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
X-SW-Source: 2013-05/txt/msg00051.txt.bz2
Content-length: 2572

>>>>> 陆岳  <hacklu.newborn@gmail.com> writes:

[…]

	A few minor points.

 > From 13d3edd1f6dbbc20b2801cea1fc367bf9042f977 Mon Sep 17 00:00:00 2001
 > From: hacklu <hacklu.newborn@gmail.com>
 > Date: Fri, 3 May 2013 18:27:08 +0800
 > Subject: [PATCH] Patch check mig on GNU Hurd

 > 2013-05-3  hacklu <hacklu.newborn@gmail.com>

	There should be two spaces between the name and email, too.  And
	the date should be 2013-05-03, as per ISO 8601.  (One may want
	to check the respective Wikipedia article here.)  As in, e. g.:

$ date -uI 
2013-05-03
$ 

	Also, I'd urge you to use the “real” full name -- the same as
	you'd use when filing a GSoC application, or copyright
	assignment papers (as required by FSF), etc.

 >         * configure.ac : uncorrectly check for mig on GUN Hurd

	• There should be no space before “:”;

	• the sentence should begin with a capital letter, and end with
	  a period;

	• s/GUN/GNU/.

	Besides, I'm having trouble understanding the intended meaning
	of this sentence.  Shouldn't it be, say, as follows instead?

	* configure.ac: Ensure MIG is available when building for
	  GNU Hurd.

[…]

 > --- a/gdb/ChangeLog
 > +++ b/gdb/ChangeLog
 > @@ -1,3 +1,7 @@
 > +2013-05-3  hacklu <hacklu.newborn@gmail.com>
 > +
 > +       * configure.ac : uncorrectly check for mig on GUN Hurd
 > +       * configure: Regenerate.
 >  2013-04-30  Samuel Thibault  <samuel.thibault@gnu.org>

	The same applies here.  Also, there should be an empty line
	between the ChangeLog entries.

[…]

 > --- a/gdb/configure.ac
 > +++ b/gdb/configure.ac
 > @@ -488,6 +488,15 @@ AC_CHECK_TOOL(WINDRES, windres)

 >  # Needed for GNU/Hurd.
 >  AC_CHECK_TOOL(MIG, mig)
 > +case "${host}" in

	Note that ‘{}’ are superfluous here; case "$host" will do the
	same, but is a tiny bit shorter.

 > +       *-linux*|*-k*bsd-gnu*)
 > +               ;;
 > +       i[?]86-*-gnu*)
 > +           if test "$MIG" = "" ; then
 > +               AC_MSG_ERROR([MIG not found but required for $host])
 > +           fi
 > +           ;;

	I guess that both cases should use the same indentation level —
	either 4 spaces or 8 (or a TAB.)  Check the rest of the GDB .ac
	code for the currently preferred style.

	Also, it's possible to avoid an indentation in the first case
	altogether, like:

   +       *-linux*|*-k*bsd-gnu*) ;;

	Again, look at the rest of the GDB code and use a matching
	style.

 > +esac

[…]

-- 
FSF associate member #7257

Re: [patch] Change trace buffer size(v5)

[Pedro Alves]

On 05/03/2013 12:38 PM, Abid, Hafiz wrote:

> Hi Pedro,
> Thanks for letting me know. There is a testcase in status-stop.exp(test_tstart_tstart) to check this case but it is not working as intended. I will work on a patch.

Thanks.

-- 
Pedro Alves

Re: [patch] Change trace buffer size(v5)

[Abid, Hafiz]

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

On 03/05/13 15:04:52, Pedro Alves wrote:
> On 05/03/2013 12:38 PM, Abid, Hafiz wrote:
> 
> > Hi Pedro,
> > Thanks for letting me know. There is a testcase in  
> status-stop.exp(test_tstart_tstart) to check this case but it is not  
> working as intended. I will work on a patch.
> 
> Thanks.
> 
> --
> Pedro Alves
> 
Hi Pedro,
I have attached a patch. It calls 'stop_tracing' from the QTinit  
handler. I was wondering that setting 'tracing' to 0 may be enough. But  
stop_tracing seems to do a lot of other things which may be required.  
So I thought it is better to go with it. Another possibility is to let  
QTBuffer:size handler return ok if the new size is equal to current  
size irrespective of the value of 'tracing'.

As I mentioned earlier, a testcase was already present for this  
scenario but it was missing the error message that came after we press  
'y'. I have handled it but would like to know if there is a better way  
to do it. I saw no regression on native-gdbserver run.

Regards,
Abid

gdb/testsuite/ChangeLog:
2013-05-03  Hafiz Abid Qadeer  <abidh@codesourcery.com>

	* status-stop.exp (test_tstart_tstart): Check for error
	returned by the second 'tstart' command.


gdb/gdbserver/ChangeLog:
2013-05-03  Hafiz Abid Qadeer  <abidh@codesourcery.com>

	* tracepoint.c (cmd_qtinit): Call 'stop_tracing'.



[-- Attachment #2: test.patch --]
[-- Type: text/x-patch, Size: 1358 bytes --]

diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 419765b..1ff6114 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -2354,6 +2354,8 @@ cmd_qtinit (char *packet)
   /* Make sure we don't try to read from a trace frame.  */
   current_traceframe = -1;
 
+  stop_tracing ();
+
   trace_debug ("Initializing the trace");
 
   clear_installed_tracepoints ();
diff --git a/gdb/testsuite/gdb.trace/status-stop.exp b/gdb/testsuite/gdb.trace/status-stop.exp
index bca03d2..33c1eb9 100644
--- a/gdb/testsuite/gdb.trace/status-stop.exp
+++ b/gdb/testsuite/gdb.trace/status-stop.exp
@@ -66,6 +66,7 @@ proc test_tstart_tstop_tstart { } {
 
 proc test_tstart_tstart { } {
     with_test_prefix "tstart_tstart" {
+	global gdb_prompt
 	global executable
 	global hex
 
@@ -79,9 +80,13 @@ proc test_tstart_tstart { } {
 	gdb_test "trace func1" "Tracepoint \[0-9\] at $hex: file.*"
 	gdb_test_no_output "tstart"
 
-	gdb_test "tstart" "" "tstart again" \
-	    "A trace is running already.  Start a new run\\? \\(y or n\\) " \
-	    "y"
+	set test "tstart again"
+	gdb_test_multiple "tstart" $test {
+	    -re "A trace is running already.  Start a new run.*y or n.*" {
+		# Send 'y' and make sure that we don't get any error. 
+		gdb_test_no_output "y" $test
+	    }
+	}
     }
 }
 

Re: [patch] Change trace buffer size(v5)

[Pedro Alves]

On 05/03/2013 04:08 PM, Abid, Hafiz wrote:

> I have attached a patch. It calls 'stop_tracing' from the QTinit handler. I was wondering that setting 'tracing' to 0 may be enough. But stop_tracing seems to do a lot of other things which may be required. So I thought it is better to go with it.

Right.

> Another possibility is to let QTBuffer:size handler return ok if the new size is equal to current size irrespective of the value of 'tracing'.

I'd rather not add special cases.

> As I mentioned earlier, a testcase was already present for this scenario but 
> it was missing the error message that came after we press 'y'. I have handled
> it but would like to know if there is a better way to do it.

Looks like a good solution.

>  proc test_tstart_tstart { } {
>      with_test_prefix "tstart_tstart" {
> +	global gdb_prompt
>  	global executable
>  	global hex

This looks unnecessary.  Otherwise OK for mainline and 7.6.

Thanks!

-- 
Pedro Alves