RE: [PATCH v6 07/10] btrace, gdbserver: Add ptwrite to btrace_config_pt.

["Metzger, Markus T" <markus.t.metzger@intel.com>]

Hello Felix,

>This enables gdb and gdbserver to communicate about ptwrite support.  If
>ptwrite support would be enabled unconditionally, GDBs with older libipt
>versions would break.
>---
> gdb/btrace.c                 |  8 +++++++-
> gdb/doc/gdb.texinfo          | 21 +++++++++++++++++++++
> gdb/features/btrace-conf.dtd |  1 +
> gdb/remote.c                 | 30 ++++++++++++++++++++++++++++++
> gdbserver/linux-low.cc       |  2 ++
> gdbserver/server.cc          | 18 ++++++++++++++++++
> gdbsupport/btrace-common.h   |  6 ++++++
> 7 files changed, 85 insertions(+), 1 deletion(-)

Looks good to me with one nit:

>--- a/gdb/features/btrace-conf.dtd
>+++ b/gdb/features/btrace-conf.dtd
>@@ -12,3 +12,4 @@
>
> <!ELEMENT pt	EMPTY>
> <!ATTLIST pt	size	CDATA	#IMPLIED>
>+<!ATTLIST pt	ptwrite	(yes | no) #IMPLIED>

Should there be a space between 'ptwrite' and '(yes|no)'?

>diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc
>index 2f71360d3bd..1333ec13dbd 100644
>--- a/gdbserver/linux-low.cc
>+++ b/gdbserver/linux-low.cc
>@@ -6806,6 +6806,8 @@ linux_process_target::read_btrace_conf (const
>btrace_target_info *tinfo,
> 	case BTRACE_FORMAT_PT:
> 	  buffer_xml_printf (buffer, "<pt");
> 	  buffer_xml_printf (buffer, " size=\"0x%x\"", conf->pt.size);
>+	  const char *ptw = conf->pt.ptwrite ? "yes" : "no";
>+	  buffer_xml_printf (buffer, " ptwrite=\"%s\"", ptw);

We don't really need the PTW variable.

thanks,
markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928