[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v22 05/25] add some QemuOpts functions for repla
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v22 05/25] add some QemuOpts functions for replace work |
Date: |
Tue, 11 Mar 2014 05:59:04 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 |
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). 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
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v22 03/25] improve some functions in qemu-option.c, Leandro Dorileo, 2014/03/17
[Qemu-devel] [PATCH v22 05/25] add some QemuOpts functions for replace work, Chunyan Liu, 2014/03/10
- Re: [Qemu-devel] [PATCH v22 05/25] add some QemuOpts functions for replace work, Eric Blake, 2014/03/10
- Re: [Qemu-devel] [PATCH v22 05/25] add some QemuOpts functions for replace work, Chunyan Liu, 2014/03/11
- Re: [Qemu-devel] [PATCH v22 05/25] add some QemuOpts functions for replace work,
Eric Blake <=
- Re: [Qemu-devel] [PATCH v22 05/25] add some QemuOpts functions for replace work, Chunyan Liu, 2014/03/11
- Re: [Qemu-devel] [PATCH v22 05/25] add some QemuOpts functions for replace work, Eric Blake, 2014/03/12
- Re: [Qemu-devel] [PATCH v22 05/25] add some QemuOpts functions for replace work, Chunyan Liu, 2014/03/13
- Re: [Qemu-devel] [PATCH v22 05/25] add some QemuOpts functions for replace work, Chunyan Liu, 2014/03/18
Re: [Qemu-devel] [PATCH v22 05/25] add some QemuOpts functions for replace work, Leandro Dorileo, 2014/03/17
[Qemu-devel] [PATCH v22 04/25] improve assertion in qemu_opt_get functions, Chunyan Liu, 2014/03/10
[Qemu-devel] [PATCH v22 06/25] add convert functions between QEMUOptionParameter to QemuOpts, Chunyan Liu, 2014/03/10