Re: [libvirt] [PATCH v2 01/10] Add qemuMonitorJSONGetObjectListPaths() method for QMP qom-list command
On 07/11/2013 10:06 AM, Michal Privoznik wrote:
On 08.07.2013 21:20, John Ferlan wrote:
<...snip...>
> diff --git a/src/qemu/qemu_monitor_json.c
b/src/qemu/qemu_monitor_json.c
> index 3383c88..fc2b65f 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -4540,6 +4540,108 @@ cleanup:
> }
>
>
> +int qemuMonitorJSONGetObjectListPaths(qemuMonitorPtr mon,
> + const char *path,
> + qemuMonitorJSONListPathPtr **paths)
> +{
> + int ret;
> + virJSONValuePtr cmd;
> + virJSONValuePtr reply = NULL;
> + virJSONValuePtr data;
> + qemuMonitorJSONListPathPtr *pathlist = NULL;
> + int n = 0;
> + size_t i;
> +
> + *paths = NULL;
> +
> + if (!(cmd = qemuMonitorJSONMakeCommand("qom-list",
> + "s:path", path,
> + NULL)))
> + return -1;
> +
> + ret = qemuMonitorJSONCommand(mon, cmd, &reply);
> +
> + if (ret == 0)
> + ret = qemuMonitorJSONCheckError(cmd, reply);
> +
> + if (ret < 0)
> + goto cleanup;
> +
> + ret = -1;
> +
> + if (!(data = virJSONValueObjectGet(reply, "return"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("qom-list reply was missing return data"));
> + goto cleanup;
> + }
> +
> + if ((n = virJSONValueArraySize(data)) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("qom-list reply data was not an array"));
> + goto cleanup;
> + }
> +
> + /* null-terminated list */
Do we need NULL terminated list ... [1]
This same comment and sequence exists in many other places - this was a
lot of cut-n-paste
> + if (VIR_ALLOC_N(pathlist, n + 1) < 0) {
> + virReportOOMError();
This can be dropped now. But as you've said in one of previous e-mails
of your, you already did that.
Right - it was dropped as were the {}
> + goto cleanup;
> + }
> +
> + for (i = 0; i < n; i++) {
> + virJSONValuePtr child = virJSONValueArrayGet(data, i);
> + const char *tmp;
> + qemuMonitorJSONListPathPtr info;
> +
> + if (VIR_ALLOC(info) < 0) {
> + virReportOOMError();
> + goto cleanup;
> + }
> +
> + pathlist[i] = info;
> +
> + if (!(tmp = virJSONValueObjectGetString(child, "name"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("qom-list reply data was missing
'name'"));
> + goto cleanup;
> + }
> +
> + if (VIR_STRDUP(info->name, tmp) < 0)
> + goto cleanup;
> +
> + if (virJSONValueObjectHasKey(child, "type")) {
> + if (!(tmp = virJSONValueObjectGetString(child, "type"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("qom-list reply has malformed 'type'
data"));
> + goto cleanup;
> + }
> + if (VIR_STRDUP(info->type, tmp) < 0)
> + goto cleanup;
> + }
> + }
> +
> + ret = n;
1: ... if we are returning number of items in array? I'm not saying it's
something wrong, just curious.
Yes, we are returning the number of items in the array the callers then
use that to walk through the results.
> + *paths = pathlist;
> +
> +cleanup:
> + if (ret < 0 && pathlist) {
> + for (i = 0; i < n; i++)
> + qemuMonitorJSONListPathFree(pathlist[i]);
> + VIR_FREE(pathlist);
> + }
> + virJSONValueFree(cmd);
> + virJSONValueFree(reply);
> + return ret;
> +}
<...snip...>
John