[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]