2014-03-12 20:40 GMT+08:00 Eric Blake <eblake@redhat.com>:

> On 03/11/2014 09:10 PM, Chunyan Liu wrote:
>
> >>>>
> >>> 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.
>
> You can still write your helper function so that if 'del' is true, you
> strdup, if 'del' is false, you return the in-place memory.  You'll have
> to cast away const in the helper, but qemu_opt_get can then restore the
> const and you don't have to adjust any existing callers, while still
> sharing the underlying implementation rather than duplicating code.


After trying, still has blocking here. qemu_opt_get returns either opt->str
(this
is char *) or desc->def_value_str (this is const char *). If not g_strdup
the result,
return (const char *) is OK, but return (char *) will has build error when
casting
(const char *) to (char *).

And since assert(opt->desc && opt->desc->type == xx) doesn't need change
for this patch series now, qemu_opt_get_bool could still return
opt->value.boolean, doesn't need to call qemu_opt_get to get opt->str and
parse that to bool. Same to qemu_opt_get_size/number. So, it's not so urgent
that qemu_opt_get/qemu_opt_get_del share common helper function. I'm
inclined
to keep duplicating code.


> But
> I guess I'll wait for v23 to see what you actually do in response to my
> suggestions.  If nothing else, please document design decisions in your
> commit message (for example, if you choose to duplicate code rather than
> share code in a common helper, explain that the duplication is because
> one function returns malloc'd memory while the other returns in-place
> memory).
>

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