From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22940 invoked by alias); 19 Feb 2013 02:53:13 -0000 Received: (qmail 22930 invoked by uid 22791); 19 Feb 2013 02:53:12 -0000 X-SWARE-Spam-Status: No, hits=-4.5 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,TW_XS X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 19 Feb 2013 02:53:04 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1U7dKJ-0001Rh-JQ from Yao_Qi@mentor.com for gdb-patches@sourceware.org; Mon, 18 Feb 2013 18:53:03 -0800 Received: from SVR-ORW-FEM-03.mgc.mentorg.com ([147.34.97.39]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Mon, 18 Feb 2013 18:53:03 -0800 Received: from qiyao.dyndns.org (147.34.91.1) by svr-orw-fem-03.mgc.mentorg.com (147.34.97.39) with Microsoft SMTP Server id 14.1.289.1; Mon, 18 Feb 2013 18:53:01 -0800 Message-ID: <5122E8C9.2070205@codesourcery.com> Date: Tue, 19 Feb 2013 02:53:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/17.0 Thunderbird/17.0 MIME-Version: 1.0 To: "Abid, Hafiz" CC: , Subject: Re: [patch] Change trace buffer size References: <1361211216.2217.2@abidh-ubunto1104> In-Reply-To: <1361211216.2217.2@abidh-ubunto1104> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2013-02/txt/msg00485.txt.bz2 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. */ > + > +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. > + > +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 (齐尧)