Re: [Qemu-devel] [PATCH v23 06/32] add qemu_opt_get_*_del functions for replace work

[Eric Blake]

On 03/21/2014 04:12 AM, Chunyan Liu wrote:
> Add qemu_opt_get_del, qemu_opt_get_bool_del and qemu_opt_get_number_gel

s/gel/del/

Any reason you don't mention qemu_opt_get_size_del here?

> to replace the same handling of QEMUOptionParamter (get and delete).

s/Paramter/Parameter/

> 
> get_*_del purpose:
> 
> In specific driver (like "qcow2"), it only handles expected options, then
> delete them from option list. Leave the left options to be passed down
> to 2nd driver (like "raw") to do 2nd handling.

Awkward wording.  I suggest replacing these two paragraphs with:

Several drivers are coded to parse a known subset of options, then
remove them for the list before handing all remaining options to a
second driver for further option processing.  get_*_del makes it easier
to retrieve a known option (or its default) and remove it from the list
all in one action.

> +++ b/util/qemu-option.c
> @@ -588,6 +588,29 @@ const char *qemu_opt_get(QemuOpts *opts, const char 
> *name)
>      return opt ? opt->str : NULL;
>  }
>  
> +char *qemu_opt_get_del(QemuOpts *opts, const char *name)

It would help to add documentation to this function; particularly that
the caller must use g_free() on the result (in contrast to qemu_opt_get
where they must NOT use g_free).

> +    str = g_strdup(opt->str);

We could play a game with transfer semantics for fewer mallocs, by
writing this as:

str = opt->str;
opt->str = NULL;

but not the end of the world if you keep it as-is.

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

signature.asc
Description: OpenPGP digital signature