2014-03-12 20:40 GMT+08:00 Eric Blake <ebl...@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 > >
