Re: [patch v9 23/23] btrace, remote: drop qbtrace packet

Jan Kratochvil
2013-03-05T20:15:06Z

On Mon, 04 Mar 2013 18:06:10 +0100, Markus Metzger wrote:
> Use the qXfer:btrace:read packet's annex to encode a trace read method:
>
> all ... read all trace
> new ... read all trace if the trace has changed

BTW fixing previous patches by adding new patches is not right. It clutters
GDB history. There will be now target_btrace_has_changed in the history
despite it has never been approved for FSF GDB check-in.

I understand it is easier this way.

But GDB existing in many source variants and people (yes, incl. me) dig into
its history for various backports, regressions finding etc. so a minimal
logical changes are preferred.

Please merge it with the appropriate parts.

>
> This obsoletes the need for the qbtrace packet.
>
> CC: Pedro Alves <palves@redhat.com>
> CC: Jan Kratochvil <jan.kratochvil@redhat.com>
> CC: Eli Zaretskii <eliz@gnu.org>
>
> 2013-03-04 Markus Metzger <markus.t.metzger@intel.com>
>
> * target.h (target_ops) <to_read_btrace>: Add type parameter.
> (target_ops) <to_btrace_has_changed>: Remove.
> (target_btrace_has_changed): Remove.
> (target_read_btrace): Add type parameter.
> * target.c (target_read_btrace): Add type parameter.
> (target_btrace_has_changed): Remove.
> * remote.c (PACKET_qbtrace): Remove.
> (remote_protocol_features): Remove qbtrace.
> (remote_supports_btrace): Remove check for qbtrace.
> (remote_btrace_has_changed): Remove.
> (remote_read_btrace): Consider read type.
> (init_remote_ops): Remove to_disable_btrace.
> (_initialize_remote): Remove qbtrace.
> * i386-linux-nat.c (_initialize_i386_linux_nat): Remove
> initialization of to_btrace_has_changed.
> * common/linux-btrace.h (linux_read_btrace): Add type parameter.
> (linux_btrace_has_changed): Removed.
> * common/linux-btrace.c (linux_btrace_has_changed): Moved.
> Changed to static.
> (linux_read_btrace): Consider read type.
> * common/btrace-common.h (btrace_read_type): New enum.
> * btrace.c (btrace_fetch): Only update branch trace if it
> changed.
> * amd64-linux-nat.c (_initialize_amd64_linux_nat): Remove
> initialization of to_btrace_has_changed.
>
> gdbserver/
>
> * target.h (target_ops): Remove btrace_has_changed. Add type
> parameter to read_btrace. Update target_ macros.
> * server.c: Include btrace-common.h.
> (handle_qxfer_btrace): Consider read type in annex.
> (handle_btrace_query): Removed.
> (handle_query): Remove qbtrace from qSupported. Remove call to
> handle_btrace_query.
> * linux-low.c (linux_low_read_btrace): Consider read type.
> (linux_target_ops): Remove linux_btrace_has_changed.
>
> doc/
>
> * gdb.texinfo (Remote Configuration): Remove qbtrace.
> Describe annex of Qbtrace.
[...]
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
[...]
> /* Read the branch trace. */
>
> static VEC (btrace_block_s) *
> -remote_read_btrace (struct btrace_target_info *tinfo)
> +remote_read_btrace (struct btrace_target_info *tinfo,
> + enum btrace_read_type type)
> {
> struct packet_config *packet = &remote_protocol_packets[PACKET_qXfer_btrace];
> struct remote_state *rs = get_remote_state ();
> VEC (btrace_block_s) *btrace = NULL;
> + const char *annex;
> char *xml;
>
> if (packet->support != PACKET_ENABLE)
> @@ -11268,8 +11223,21 @@ remote_read_btrace (struct btrace_target_info *tinfo)
> error (_("Cannot process branch tracing result. XML parsing not supported."));
> #endif
>
> + switch (type)
> + {
> + case btrace_read_all:
> + annex = "all";
> + break;
> + case btrace_read_new:
> + annex = "new";
> + break;
> + default:

Here should be internal_error call. It just cannot happen in valid GDB
binary.

> + annex = NULL;
> + break;
> + }
> +
> xml = target_read_stralloc (&current_target,
> - TARGET_OBJECT_BTRACE, NULL);
> + TARGET_OBJECT_BTRACE, annex);
> if (xml != NULL)
> {
> struct cleanup *cleanup = make_cleanup (xfree, xml);