Re: [Xen-devel] [PATCH 4/6] libxl_qmp: Use qmp_parameters_* functions for param list of a QMP command.

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Anthony PERARD writes ("[PATCH 4/6] libxl_qmp: Use qmp_parameters_* functions 
for param list of a QMP command."):
> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>

Thanks:

> -    parameters = flexarray_make(6, 1);
> -    flexarray_append_pair(parameters, "driver", "xen-pci-passthrough");
> -    flexarray_append_pair(parameters, "id",
> +    args = qmp_parameters_add_string(gc, NULL, "driver", 
> "xen-pci-passthrough");
> +    if (!args)
> +        goto error_nomem;

This is a rather clumsy use pattern, threading args through all the
time.  If nothing else it might lead to mistakes because the first
call takes NULL whereas subsequent calls take args:

> +    args = qmp_parameters_add_string(gc, args, "id",
>                            libxl__sprintf(gc, PCI_PT_QDEV_ID,
>                                           pcidev->bus, pcidev->dev,
>                                           pcidev->func));

How about inventing functions that would allow
  libxl__json_object *args = NULL;
  libxl__qmp_param_add(gc, &args, "id", libxl__qmp_sprintf(gc, ....);
with perhaps a helper macro that allows
  QMP_PARAM_SPRINTF(args, "id", PCI_PT_QDEV_ID,
                    pcidev->bus, pcidev->dev, pcidev->func));
?

And again there's a lot of unnecessary error handling.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel