Re: [Qemu-devel] [PATCH v25 04/31] QemuOpts: change opt->name|str from (const char *) to (char *)

[Eric Blake]

On 04/10/2014 11:54 AM, Chunyan Liu wrote:
> qemu_opt_del() already assumes that all QemuOpt instances contain
> malloc'd name and value; but it had to cast away const because
> opts_start_struct() was doing its own thing and using static storage
> instead.  By using the correct type and malloced strings everywhere, the
> usage of this struct becomes clearer.
> 
> Reviewed-by: Leandro Dorileo <address@hidden>
> Signed-off-by: Chunyan Liu <address@hidden>
> ---
>  include/qemu/option_int.h |  4 ++--
>  qapi/opts-visitor.c       | 10 +++++++---
>  util/qemu-option.c        |  4 ++--
>  3 files changed, 11 insertions(+), 7 deletions(-)

> @@ -177,7 +177,11 @@ opts_end_struct(Visitor *v, Error **errp)
>      }
>      g_hash_table_destroy(ov->unprocessed_opts);
>      ov->unprocessed_opts = NULL;
> -    g_free(ov->fake_id_opt);
> +    if (ov->fake_id_opt) {
> +        g_free(ov->fake_id_opt->name);
> +        g_free(ov->fake_id_opt->str);
> +        g_free(ov->fake_id_opt);
> +    }

I wonder if this could have called qemu_opt_del() instead of open-coding
the three g_free() calls.  On the other hand...

>      ov->fake_id_opt = NULL;
>  }
>  
> diff --git a/util/qemu-option.c b/util/qemu-option.c
> index 84f0d5c..065ffb8 100644
> --- a/util/qemu-option.c
> +++ b/util/qemu-option.c
> @@ -664,8 +664,8 @@ static void qemu_opt_parse(QemuOpt *opt, Error **errp)
>  static void qemu_opt_del(QemuOpt *opt)
>  {
>      QTAILQ_REMOVE(&opt->opts->head, opt, next);

...I don't know if the additional QTAILQ_REMOVE would be safe on
fake_id_opt, and you'd have to make qemu_opt_del non-static to call it
from another file.

> -    g_free((/* !const */ char*)opt->name);
> -    g_free((/* !const */ char*)opt->str);
> +    g_free(opt->name);
> +    g_free(opt->str);
>      g_free(opt);
>  }

So I'm fine with leaving this patch as-is:

Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature