RE: [PATCH] Add method/format information to =record-started

["Metzger, Markus T" <markus dot t dot metzger at intel dot com>]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Simon Marchi
> Sent: Friday, June 3, 2016 5:52 PM
> To: gdb-patches@sourceware.org
> Cc: Simon Marchi <simon.marchi@ericsson.com>
> Subject: [PATCH] Add method/format information to =record-started
> 
> Eclipse CDT now supports enabling execution recording using two methods
> (full and btrace) and both formats for btrace (bts and pt).  In the
> event that recording is enabled behind the back of the GUI (by the user
> on the command line, or a script), we need to know which method/format
> are being used, so it can be correctly reflected in the interface.  This
> patch adds this information to the =record-started async record.
> 
> Before:
> 
>   =record-started,thread-group="i1"
> 
> After:
> 
>   =record-started,thread-group="i1",method="btrace",format="bts"
>   =record-started,thread-group="i1",method="btrace",format="pt"
>   =record-started,thread-group="i1",method="full"
> 
> The "format" field is only present when the current method supports
> multiple formats (only the btrace method as of now).

Thanks for adding this.


> @@ -407,8 +409,30 @@ mi_record_changed (struct inferior *inferior, int started)
>    old_chain = make_cleanup_restore_target_terminal ();
>    target_terminal_ours_for_output ();
> 
> -  fprintf_unfiltered (mi->event_channel,  "record-%s,thread-group=\"i%d\"",
> -		      started ? "started" : "stopped", inferior->num);
> +  if (started)
> +    {
> +      if (format != NULL)
> +	{

Do we really need braces, here...

> +	  fprintf_unfiltered (
> +	    mi->event_channel,
> +	    "record-started,thread-
> group=\"i%d\",method=\"%s\",format=\"%s\"",
> +	    inferior->num, method, format);
> +	}
> +      else
> +	{
> +

...and here...

> +	  fprintf_unfiltered (
> +	    mi->event_channel,
> +	    "record-started,thread-group=\"i%d\",method=\"%s\"",
> +	    inferior->num, method);
> +	}
> +    }
> +  else
> +    {

...and here?

> +      fprintf_unfiltered (mi->event_channel,
> +			  "record-stopped,thread-group=\"i%d\"", inferior-
> >num);
> +    }
> +


> @@ -234,7 +235,8 @@ record_btrace_open (const char *args, int from_tty)
>  				  NULL);
>    record_btrace_generating_corefile = 0;
> 
> -  observer_notify_record_changed (current_inferior (),  1);
> +  format = record_btrace_conf.format == BTRACE_FORMAT_PT ? "pt" : "bts";

I'd use a switch here and not assume that format != pt means bts.  If we added
another format (LBR maybe?) we might miss this.


Regards,
Markus.

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928