On 24.01.2012 19:21, Jiri Denemark wrote:
On Tue, Jan 24, 2012 at 18:37:43 +0100, Michal Privoznik wrote:
> +
> + if (qemuAgentCommand(mon, cmd, &reply) < 0 ||
> + qemuAgentCheckError(cmd, reply) < 0)
> + goto cleanup;
> +
> + virJSONValueObjectGetNumberInt(reply, "return", &ret);
Doesn't the above function need to be checked for errors? That is, what if
there's no "return" key in the reply? In json monitor code, we usually
have
something like the following code:
if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0)
{
qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("guest-fsfreeze-freeze reply was missing return"
" data"));
}
In fact this check is done in qemuAgentCheckError(); which reports error
as well. But I agree I should check if returned value is integer and
throw an error. I have squashed this into my local branch:
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 3a5cc4b..9df5546 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1137,7 +1137,10 @@ int qemuAgentFSFreeze(qemuAgentPtr mon)
qemuAgentCheckError(cmd, reply) < 0)
goto cleanup;
- virJSONValueObjectGetNumberInt(reply, "return", &ret);
+ if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) {
+ qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("malformed return value"));
+ }
cleanup:
virJSONValueFree(cmd);
@@ -1171,7 +1174,10 @@ int qemuAgentFSThaw(qemuAgentPtr mon)
qemuAgentCheckError(cmd, reply) < 0)
goto cleanup;
- virJSONValueObjectGetNumberInt(reply, "return", &ret);
+ if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) {
+ qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("malformed return value"));
+ }
cleanup:
virJSONValueFree(cmd);