On Mon, Oct 03, 2016 at 10:46:07 -0400, John Ferlan wrote:
> [...]
>
> >> Since you didn't explicitly state which checks, I'm assuming you mean
> >> either:
> >>
> >> if (!(devices = virJSONValueObjectGetArray(reply, "return"))) {
> >> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> >> _("block info reply was missing device list"));
> >> goto cleanup;
> >> }
> >>
> >> I did consider moving this into the helper and returning devices but ran
> >> into the problem that the helper would virJSONValueFree(reply) while
> >> returning devices which is then used by the for loop to get each device
> >> (dev = virJSONValueArrayGet(devices, i)).
> >
> > What's the problem wit that? Creating a function that steals the value
> > from a JSON object should be pretty simple. We have one that steals
> > array members: virJSONValueArraySteal.
> >
> >> Since devices points into reply, I didn't think that would be such a
> >> good idea to free reply and then return devices. Returning reply anda
> >
> > Why not?
> >
>
>
> Let's assume the helper is:
>
> +static virJSONValuePtr
> +qemuMonitorJSONQueryBlock(qemuMonitorPtr mon)
> +{
> + virJSONValuePtr cmd;
> + virJSONValuePtr reply = NULL;
> + virJSONValuePtr devices = NULL;
> +
> + if (!(cmd = qemuMonitorJSONMakeCommand("query-block", NULL)))
> + return NULL;
> +
> + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0 ||
> + qemuMonitorJSONCheckError(cmd, reply) < 0)
> + goto cleanup;
> +
> + if (!(devices = virJSONValueObjectGetArray(reply, "return"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("query-block reply was missing device list"));
> + goto cleanup;
> + }
> +
> + cleanup:
> + virJSONValueFree(cmd);
> + virJSONValueFree(reply);
> + return devices;
> +}
>
> Wouldn't "reply" being free'd cause access problems for devices? Is
> there other code somewhere else that gets the array, free's the object,
> then parses the array? I didn't find any (for good reason)...
It would as virJSONValueFree is recursive and thus everything would be
freed. Hence my comment of adding the "steal" function.
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list