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 Hafiz Abid Qadeer 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.