Re: [libvirt] [PATCH v3 04/18] blockjob: split out block info monitor handling

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.

> - 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.