On 09/04/2014 09:39 AM, Peter Krempa wrote:
> On 08/31/14 06:02, Eric Blake wrote:
>> Another layer of overly-multiplexed code that deserves to be
>> split into obviously separate paths for query vs. modify.
>> This continues the cleanup started in the previous patch.
>>
>> In the process, make some tweaks to simplify the logic when
>> parsing the JSON reply. There should be no user-visible
>> semantic changes.
>>
>> +int qemuMonitorBlockJobInfo(qemuMonitorPtr mon,
>> + const char *device,
>> + virDomainBlockJobInfoPtr info)
>> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
>
> Implementation of this function doesn't check for presence of the @info
> structure and dereferences it always. You should mark argument 3 nonnull
> too.
Good catch.
>> - for (i = 0; i < nr_results; i++) {
>> + for (i = 0, ret = 0; i < nr_results && ret == 0; i++) {
>> virJSONValuePtr entry = virJSONValueArrayGet(data, i);
>> if (!entry) {
>> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> _("missing array element"));
>> - return -1;
>> + ret = -1;
>> + goto cleanup;
>> }
>> - if (qemuMonitorJSONGetBlockJobInfoOne(entry, device, info) == 0)
>> - return 1;
>> + ret = qemuMonitorJSONGetBlockJobInfoOne(entry, device, info);
>
> This doesn't break the loop, thus if the device info isn't last in the
> structure you'd overwrite the return code. I presume the clients don't
> check that far but you've documented that semantics.
Actually, the loop DOES break, just on the next iteration. I added '&&
ret == 0' to the loop continuation condition. So the semantics are:
InfoOne fails and returns -1, the next iteration is aborted and overall
ret is -1
InfoOne returns 0 because the requested device didn't match, so the loop
continues to look at the next array member. If all array members are
exhausted, ret remains 0 and overall return is 0 for job not found.
InfoOne succeeds and returns 1, the next iteration is aborted and the
overall ret is 1 for info populated.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list