Re: PATCH: Circular trace buffers

[Pedro Alves <pedro at codesourcery dot com>]

On Tuesday 16 March 2010 21:42:02, Stan Shebs wrote:
> This patch adds a flag that requests the target agent to make the trace 
> buffer circular, so that instead of filling it up and then stopping, the 
> agent discards the oldest trace frames as necessary to accommodate new 
> ones.  Any hairy memory management code is going to be on the target 
> side; GDB just has to transmit the setting (and now always via target 
> vector), and report back status, which may now include a total number of 
> frames that were created.  This also adds complete documentation of the 
> qTStatus reply, per request.  Any comments before I commit?

Playing devil's advogate here, I'm still not 100% convinced that
"set circular-trace-buffer" is 100% well defined and that
is isn't confusing in some cases; it applies on the fly
in some cases, does somewhat not-completly clear
things in other cases, and errors out in others.  I wonder
if we defined "set circular-trace-buffer" as another flag
that is respected at "tstart" time only, and made the
presently running trace run's circular-trace-buffer-ness reported
through "tstatus", and define "show circular-trace-buffer" as the
"circular-trace-buffer-ness" intent at next trace run start,
things would be more consistent and clear.  If not, vis.:

E.g.,

 - applies on the fly in non-stop mode.

 - all-stop/async + trace running + "set circular-trace-buffer"
   errors out because you can't talk to the target if it
   is running in all-stop.

 - E.g., what does "show circular-trace-buffer" mean when
   debugging a tfile?  "set circular-trace-buffer" changes
   the local GDB flag, and "show circular-trace-buffer"
   shows the according change, but, then we have no
   way of knowing when debugging a tfile had been
   in circular-trace-buffer mode or not when the tfile
   was created.


That said, I'm not opposed to the patch.  Just
dumping my thoughts.  :-)

-- 
Pedro Alves