2014-03-11 19:59 GMT+08:00 Eric Blake <ebl...@redhat.com>:
> On 03/10/2014 11:29 PM, Chunyan Liu wrote:
> >>
> >> Why are your later callers using a common helper function, but
> >> qemu_opt_get_del() repeating a lot of the body of qemu_opt_get()?
> >> Shouldn't qemu_opt_get() and qemu_opt_get_del() call into a common
> helper?
> >>
> >> qemu_opt_get_del must return (char *), but the existing qemu_opt_get
> > returns
> > (const char *). Could also change qemu_opt_get return value to (char *)
> > type,
> > then these two functions could use a common helper function. But there
> are
> > too
> > many places using qemu_opt_get already, if the return value type changes,
> > it will
> > affect a lot.
>
> Going from 'char *' to 'const char *' is automatic. Have your helper
> return 'char *', and qemu_opt_get can call it just fine and give its
> callers their desired const char *.
>
> >>
> >> Same problem as in 4/25 - I don't think you can rely on
> >> opt->value.boolean being valid if you don't track the union
> >> discriminator, and right now, the union discriminator is tracked only
> >> via opt->desc.
> >
> >
> > Couldn't find a reliable way if the option is passed through
> > opts_accept_any,
> > just no way to check the option type.
> >
>
> Like I said in 4/25, if the option is passed through opts_accept_any,
> treat it as string only; and make the opt_get_bool function either
> reject it outright, or to always do the string-to-bool conversion at the
> time of the lookup:
>
> if (opt->desc) {
> assert(opt->desc->type == QEMU_OPT_BOOL);
> return opt->value.boolean;
> } else {
> code to parse opt->str
> }
>
>
> >>
> > Could be if changing qemu_opt_get return value type, but just as said
> > before,
> > that will affect many codes.
>
> Also, changing an existing function that returns 'const char *' into now
> returning 'char *' will NOT break any callers if the semantics remain
> unchanged (what WILL break is if the semantics change where the callers
> must now free the result)
Yes, that's the only change, we need to free the result. There are more
than 200
places using qemu_opt_get in existing code, we need to checkout the related
files
and free the result one by one.
> But in general, it should still be just fine
> to have qemu_opt_get return 'const char *' even if the common helper
> returns 'char *'.
>
> --
> Eric Blake eblake redhat com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
>
