This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 04/12] record btrace: add configuration struct
- From: Pedro Alves <palves at redhat dot com>
- To: Markus Metzger <markus dot t dot metzger at intel dot com>
- Cc: gdb-patches at sourceware dot org, Eli Zaretskii <eliz at gnu dot org>
- Date: Wed, 05 Nov 2014 20:45:32 +0000
- Subject: Re: [PATCH 04/12] record btrace: add configuration struct
- Authentication-results: sourceware.org; auth=none
- References: <1405346196-1804-1-git-send-email-markus dot t dot metzger at intel dot com> <1405346196-1804-5-git-send-email-markus dot t dot metzger at intel dot com>
On 07/14/2014 02:56 PM, Markus Metzger wrote:
> index 146d95f..c601e8d 100644
> --- a/gdb/gdbserver/server.c
> +++ b/gdb/gdbserver/server.c
> @@ -131,6 +131,9 @@ struct vstop_notif
> struct target_waitstatus status;
> };
>
> +/* The current btrace configuration. */
> +static struct btrace_config current_btrace_conf;
Why do we need this global? Please expand the comment.
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> +/* The target's btrace configuration. */
> +
> +static struct btrace_config btrace_target_config;
If this mirrors the state on the remote target, then
it should be a field of struct remote_state, not global.
> +/* Synchronize the configuration with the target. */
> +
> +static void btrace_sync_conf (const struct btrace_config *conf)
Line break after void.
> +{
> + /* Nothing to do for now. */
> +}
> +
> +/* Read the current thread's btrace configuration from the target and
> + store it into CONF. */
> +
> +static void btrace_read_config (struct btrace_config *conf)
> +{
Line break after void.
> +static void btrace_read_config (struct btrace_config *conf)
> +{
> + struct cleanup *cleanup;
Move this within the if block below.
> + char *xml;
> +
> + xml = target_read_stralloc (¤t_target,
> + TARGET_OBJECT_BTRACE_CONF, "");
> + if (xml != NULL)
> + {
> +
> + cleanup = make_cleanup (xfree, xml);
> + parse_xml_btrace_conf (conf, xml);
Spurious empty line.
Thanks,
Pedro Alves