On Tue, May 03, 2016 at 11:53:21 +0200, Michal Privoznik wrote:
In majority of our functions we have this variable @ret that is overwritten a lot. In other areas of the code we use 'goto cleanup;' just so that this wouldn't happen. But here.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_monitor_json.c | 295 ++++++++++++++++++++++--------------------- 1 file changed, 150 insertions(+), 145 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 4dbf33f..47bb812 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c
[...]
@@ -1278,12 +1283,16 @@ int qemuMonitorJSONSetLink(qemuMonitorPtr mon, if (!cmd) return -1;
- if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup;
+ if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + ret = 0; + cleanup:
Your compiler should have complained that 'ret' might be uninitialized.
virJSONValueFree(cmd); virJSONValueFree(reply); - return ret; }
@@ -1295,11 +1304,14 @@ int qemuMonitorJSONSystemReset(qemuMonitorPtr mon) if (!cmd) return -1;
- ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup;
- if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup;
+ ret = 0;
Here too.
+ cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret;
[...]
@@ -1847,7 +1847,6 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, }
ret = 0; -
Spurious whitespace modification.
cleanup: virJSONValueFree(cmd); virJSONValueFree(reply);
I usually require a v2 for stuff that does not compile cleanly but I don't really want to see this code again. ACK if you fix the mistakes pointed out.