Re: [PATCH] Support for enabling/disabling tracepoints while a trace experiment is running

[Pedro Alves <pedro@codesourcery.com>]

On Thursday 05 May 2011 18:27:11, Kwok Cheung Yeung wrote:

>  	* gdbserver/tracepoint.c (clear_installed_tracepoints): Uninstall
>  	disabled tracepoints.

gdbserver has its own gdbserver/ChangeLog file:

  	* tracepoint.c (clear_installed_tracepoints): Uninstall
  	disabled tracepoints.


>  	* NEWS: Add news item for the new behaviour of the enable and disable
>  	GDB commands when applied to tracepoints.
>  	Add news items for the new remote packets QTEnable and QTDisable.

> 
> 	gdb/doc/
> 	* gdb.texinfo: Document change in the behaviour of the enable and
> 	disable GDB commands when applied to tracepoints.
> 	Document QTEnable and QTDisable in the list of tracepoint packets.

Eli is our documentation maintainer and will review these bits.

The code looks okay to me.

>  static void
> +cmd_qtenable_disable (char *own_buf, int enable)
> +{
> +  char *packet = own_buf;
> +  ULONGEST num, addr;
> +  struct tracepoint *tp;
> +
> +  packet += strlen (enable ? "QTEnable:" : "QTDisable:");
> +  packet = unpack_varlen_hex (packet, &num);
> +  ++packet; /* skip a colon */
> +  packet = unpack_varlen_hex (packet, &addr);
> +
> +  tp = find_tracepoint (num, addr);
> +
> +  if (tp)
> +    {
> +      if ((enable && tp->enabled) || (!enable && !tp->enabled))

but isn't this just:

 if (enable != tp->enabled)

?

> +	{
> +	  trace_debug ("Tracepoint %d at 0x%s is already %s",
> +		       (int) num, paddress (addr),
> +		       enable ? "enabled" : "disabled");
> +	  write_ok (own_buf);
> +	  return;
> +	}
> +

-- 
Pedro Alves