|
From: | Chunyan Liu |
Subject: | Re: [Qemu-devel] [PATCH v22 05/25] add some QemuOpts functions for replace work |
Date: | Wed, 12 Mar 2014 11:10:03 +0800 |
On 03/10/2014 11:29 PM, Chunyan Liu wrote:Going from 'char *' to 'const char *' is automatic. Have your helper
>>
>> 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.
return 'char *', and qemu_opt_get can call it just fine and give its
callers their desired const char *.
Like I said in 4/25, if the option is passed through opts_accept_any,
>>
>> 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.
>
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:
return opt->value.boolean;
if (opt->desc) {
assert(opt->desc->type == QEMU_OPT_BOOL);
} else {
code to parse opt->str
}Also, changing an existing function that returns 'const char *' into now
>>
> Could be if changing qemu_opt_get return value type, but just as said
> before,
> that will affect many codes.
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 *'.
[Prev in Thread] | Current Thread | [Next in Thread] |