This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]
Other format:	[Raw text]

Re: [patch] Change trace buffer size

From: Yao Qi <yao at codesourcery dot com>
To: "Abid, Hafiz" <hafiz_abid at mentor dot com>
Cc: <gdb-patches at sourceware dot org>, <stan at codesourcery dot com>
Date: Tue, 19 Feb 2013 10:51:53 +0800
Subject: Re: [patch] Change trace buffer size
References: <1361211216.2217.2@abidh-ubunto1104>

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 (éå)

Follow-Ups:
- Re: [patch] Change trace buffer size
  - From: Pedro Alves

References:
- [patch] Change trace buffer size
  - From: Abid, Hafiz

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]