Re: [Qemu-devel] [PATCH 3/7] trace: move qemu_trace_opts to trace/control.c

From: Stefan Hajnoczi <stefanha@redhat.com>
To: "Denis V. Lunev" <den@openvz.org>
Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org,
	Paolo Bonzini <pbonzini@redhat.com>,
	Kevin Wolf <kwolf@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 3/7] trace: move qemu_trace_opts to trace/control.c
Date: Fri, 17 Jun 2016 14:48:22 +0100	[thread overview]
Message-ID: <20160617134822.GE28550@stefanha-x1.localdomain> (raw)
In-Reply-To: <1466061312-7373-4-git-send-email-den@openvz.org>

[-- Attachment #1: Type: text/plain, Size: 1590 bytes --]

On Thu, Jun 16, 2016 at 10:15:08AM +0300, Denis V. Lunev wrote:
> +/**
> + * Definition of QEMU options describing trace subsystem configuration
> + */
> +extern QemuOptsList qemu_trace_opts;
> +
> +/**
> + * trace_opt_parse:
> + * @optarg: A string argument of --trace command line argument
> + * @trace_file: current filename to save traces to
> + *
> + * Initialize tracing subsystem.

+Returns the filename to save trace to.  It must be freed with g_free().

> @@ -3867,23 +3847,8 @@ int main(int argc, char **argv, char **envp)
>                  xen_mode = XEN_ATTACH;
>                  break;
>              case QEMU_OPTION_trace:
> -            {
> -                opts = qemu_opts_parse_noisily(qemu_find_opts("trace"),
> -                                               optarg, true);
> -                if (!opts) {
> -                    exit(1);
> -                }
> -                if (qemu_opt_get(opts, "enable")) {
> -                    trace_enable_events(qemu_opt_get(opts, "enable"));
> -                }
> -                trace_init_events(qemu_opt_get(opts, "events"));
> -                if (trace_file) {
> -                    g_free(trace_file);
> -                }
> -                trace_file = g_strdup(qemu_opt_get(opts, "file"));
> -                qemu_opts_del(opts);
> +                trace_file = trace_opt_parse(optarg, trace_file);

It's clearer if the caller frees trace_file:

  g_free(trace_file);
  trace_file = trace_opt_parse(optarg);

Now the memory management is explicit and there is no need to pass
trace_file to trace_opt_parse().

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]