On 03/21/2014 04:12 AM, Chunyan Liu wrote: > Add two temp convert functions between QEMUOptionParameter to QemuOpts,
s/convert/conversion/ here and in subject
> so that next patch can use it. It will simplify later patch for easier
> review. And will be finally removed after all backend drivers switch to
> QemuOpts.
>
> Signed-off-by: Chunyan Liu <cy...@suse.com>
> ---
> +++ b/include/qemu/option.h
> @@ -103,6 +103,11 @@ typedef struct QemuOptDesc {
> } QemuOptDesc;
>
> struct QemuOptsList {
> + /* FIXME: Temp used for QEMUOptionParamter->QemuOpts conversion to
> + * indicate free memory. Will remove after all drivers switch to
> QemuOpts.
> + */
> + bool mallocd;
Spelling looks odd; I might have used 'allocated'. But as it gets
deleted later, I don't care.
> + num_opts = count_option_parameters(list);
> + opts = g_malloc0(sizeof(QemuOptsList) +
> + (num_opts + 1) * sizeof(QemuOptDesc));
[1]...
> + while (list && list->name) {
> + opts->desc[i].name = g_strdup(list->name);
> + opts->desc[i].help = g_strdup(list->help);
> + switch (list->type) {
> + case OPT_FLAG:
> + opts->desc[i].type = QEMU_OPT_BOOL;
> + opts->desc[i].def_value_str =
> + g_strdup(list->value.n ? "on" : "off");
This always sets def_value_str...
> + break;
> +
> + case OPT_NUMBER:
> + opts->desc[i].type = QEMU_OPT_NUMBER;
> + if (list->value.n) {
> + opts->desc[i].def_value_str =
> + g_strdup_printf("%" PRIu64, list->value.n);
> + }
...whereas this only sets def_value_str for non-zero values. But can't
0 be a valid setting? Is this mismatch in what gets converted going to
bite us? Should you be paying attention to list->assigned instead or in
addition to just checking for non-zero values?
> + break;
> +
> + case OPT_SIZE:
> + opts->desc[i].type = QEMU_OPT_SIZE;
> + if (list->value.n) {
> + opts->desc[i].def_value_str =
> + g_strdup_printf("%" PRIu64, list->value.n);
> + }
Same question for 0 values.
> + break;
> +
> + case OPT_STRING:
> + opts->desc[i].type = QEMU_OPT_STRING;
> + opts->desc[i].def_value_str = g_strdup(list->value.s);
> + break;
> + }
> +
> + i++;
> + list++;
> + opts->desc[i].name = NULL;
...[1] This assignment is dead code, because you used malloc0 which
guarantees that desc[i].name is already NULL.
> +/* convert QemuOpts to QEMUOptionParamter
s/Paramter/Parameter/
> + * Note: result QEMUOptionParameter has shorter lifetime than
> + * input QemuOpts.
> + * FIXME: this function will be removed after all drivers
> + * switch to QemuOpts
> + */
> +QEMUOptionParameter *opts_to_params(QemuOpts *opts)
> +{
> + num_opts = count_opts_list(opts->list);
> + dest = g_malloc0((num_opts + 1) * sizeof(QEMUOptionParameter));
> +
> + i++;
> + desc++;
> + dest[i].name = NULL;
Another dead assignment.
> + }
> +
> + return dest;
> +}
> +
> +void qemu_opts_free(QemuOptsList *list)
> +{
> + /* List members point to new malloced space and need to free.
> + * FIXME:
> + * Introduced for QEMUOptionParamter->QemuOpts conversion.
> + * Will remove after all drivers switch to QemuOpts.
> + */
> + if (list && list->mallocd) {
> + QemuOptDesc *desc = list->desc;
> + while (desc && desc->name) {
> + g_free((char *)desc->name);
> + g_free((char *)desc->help);
Are these casts still necessary, given patch 4?
> + g_free((char *)desc->def_value_str);
However, it looks like you are correct that this one is casting away const.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
