4:53 a.m.
The first two patches are intended to be merged. The last one is
incomplete. Intentionally. It should be viewed as RFC. If
upstream likes it, I can finish the work. But so far it's merely
just for you guys to express your opinion.
Michal Privoznik (3):
qemuMonitorJSONQueryRxFilter: Validate qemu reply prior parsing it
qemu_monitor_json: Drop redundant checks
qemu_monitor_json: Follow our coding style
src/qemu/qemu_monitor_json.c | 323 +++++++++++++++++++++----------------------
1 file changed, 156 insertions(+), 167 deletions(-)
--
2.8.1
4:53 a.m.
New subject: [libvirt] [PATCH 1/3] qemuMonitorJSONQueryRxFilter: Validate qemu reply prior parsing it
Usually, the flow in this area of the code is as follows:
qemuMonitorJSONMakeCommand()
qemuMonitorJSONCommand()
qemuMonitorJSONCheckError()
parseReply()
But in this function, for some reasons, the last two steps were
swapped. This makes no sense.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index a48a263..81970b9 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -3220,9 +3220,6 @@ qemuMonitorJSONQueryRxFilterParse(virJSONValuePtr msg,
size_t i;
virNetDevRxFilterPtr fil = virNetDevRxFilterNew();
- if (!fil)
- goto cleanup;
-
if (!(returnArray = virJSONValueObjectGetArray(msg, "return"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("query-rx-filter reply was missing return data"));
@@ -3401,14 +3398,14 @@ qemuMonitorJSONQueryRxFilter(qemuMonitorPtr mon, const char
*alias,
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
goto cleanup;
+ if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+ goto cleanup;
+
if (qemuMonitorJSONQueryRxFilterParse(reply, filter) < 0)
goto cleanup;
ret = 0;
cleanup:
- if (ret == 0)
- ret = qemuMonitorJSONCheckError(cmd, reply);
-
if (ret < 0) {
virNetDevRxFilterFree(*filter);
*filter = NULL;
--
2.8.1
6:09 a.m.
New subject: [libvirt] [PATCH 1/3] qemuMonitorJSONQueryRxFilter: Validate qemu reply prior parsing it
On Tue, May 03, 2016 at 11:53:19 +0200, Michal Privoznik wrote:
Usually, the flow in this area of the code is as follows:
qemuMonitorJSONMakeCommand()
qemuMonitorJSONCommand()
qemuMonitorJSONCheckError()
parseReply()
But in this function, for some reasons, the last two steps were
swapped. This makes no sense.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index a48a263..81970b9 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -3220,9 +3220,6 @@ qemuMonitorJSONQueryRxFilterParse(virJSONValuePtr msg,
size_t i;
virNetDevRxFilterPtr fil = virNetDevRxFilterNew();
- if (!fil)
- goto cleanup;
-
The code dereferences 'fil' a few lines below. Without this check it
might crash.
if (!(returnArray = virJSONValueObjectGetArray(msg,
"return"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("query-rx-filter reply was missing return data"));
@@ -3401,14 +3398,14 @@ qemuMonitorJSONQueryRxFilter(qemuMonitorPtr mon, const char
*alias,
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
goto cleanup;
+ if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+ goto cleanup;
+
if (qemuMonitorJSONQueryRxFilterParse(reply, filter) < 0)
goto cleanup;
ret = 0;
cleanup:
- if (ret == 0)
- ret = qemuMonitorJSONCheckError(cmd, reply);
-
if (ret < 0) {
virNetDevRxFilterFree(*filter);
*filter = NULL;
ACK to the last two hunks, the first one needs to be removed.
4:53 a.m.
New subject: [libvirt] [PATCH 2/3] qemu_monitor_json: Drop redundant checks
In these functions I'm fixing here, we do call
qemuMonitorJSONCheckError() followed by another check if qemu
reply contains 'return' object. If it wouldn't, the former
CheckError() function would error out and the flow would not even
get to the latter.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 19 +++----------------
1 file changed, 3 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 81970b9..4dbf33f 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1093,11 +1093,7 @@ qemuMonitorJSONHumanCommandWithFd(qemuMonitorPtr mon,
if (qemuMonitorJSONCheckError(cmd, reply))
goto cleanup;
- if (!(obj = virJSONValueObjectGet(reply, "return"))) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("human monitor command was missing return data"));
- goto cleanup;
- }
+ obj = virJSONValueObjectGet(reply, "return");
if (reply_str) {
const char *data;
@@ -2539,12 +2535,7 @@ qemuMonitorJSONGetMigrationCompression(qemuMonitorPtr mon,
if ((ret = qemuMonitorJSONCheckError(cmd, reply)) < 0)
goto cleanup;
- if (!(result = virJSONValueObjectGet(reply, "return"))) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("query-migrate-parameters reply was missing "
- "'return' data"));
- goto cleanup;
- }
+ result = virJSONValueObjectGet(reply, "return");
if (virJSONValueObjectGetNumberInt(result, "compress-level",
&compress->level) < 0) {
@@ -5392,11 +5383,7 @@ int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon,
ret = -1;
- if (!(data = virJSONValueObjectGet(reply, "return"))) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("qom-get reply was missing return data"));
- goto cleanup;
- }
+ data = virJSONValueObjectGet(reply, "return");
switch ((qemuMonitorJSONObjectPropertyType) prop->type) {
/* Simple cases of boolean, int, long, uint, ulong, double, and string
--
2.8.1
6:10 a.m.
New subject: [libvirt] [PATCH 2/3] qemu_monitor_json: Drop redundant checks
On Tue, May 03, 2016 at 11:53:20 +0200, Michal Privoznik wrote:
In these functions I'm fixing here, we do call
qemuMonitorJSONCheckError() followed by another check if qemu
reply contains 'return' object. If it wouldn't, the former
CheckError() function would error out and the flow would not even
get to the latter.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 19 +++----------------
1 file changed, 3 insertions(+), 16 deletions(-)
ACK
4:53 a.m.
New subject: [libvirt] [PATCH 3/3] qemu_monitor_json: Follow our coding style
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(a)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
@@ -1115,17 +1115,20 @@ qemuMonitorJSONHumanCommandWithFd(qemuMonitorPtr mon,
int
qemuMonitorJSONSetCapabilities(qemuMonitorPtr mon)
{
- int ret;
+ int ret = -1;
virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("qmp_capabilities",
NULL);
virJSONValuePtr reply = NULL;
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;
+ cleanup:
virJSONValueFree(cmd);
virJSONValueFree(reply);
return ret;
@@ -1173,17 +1176,20 @@ qemuMonitorJSONStartCPUs(qemuMonitorPtr mon,
int
qemuMonitorJSONStopCPUs(qemuMonitorPtr mon)
{
- int ret;
+ int ret = -1;
virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("stop", NULL);
virJSONValuePtr reply = NULL;
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;
+ cleanup:
virJSONValueFree(cmd);
virJSONValueFree(reply);
return ret;
@@ -1195,7 +1201,7 @@ qemuMonitorJSONGetStatus(qemuMonitorPtr mon,
bool *running,
virDomainPausedReason *reason)
{
- int ret;
+ int ret = -1;
const char *status;
virJSONValuePtr cmd;
virJSONValuePtr reply = NULL;
@@ -1207,15 +1213,11 @@ qemuMonitorJSONGetStatus(qemuMonitorPtr mon,
if (!(cmd = qemuMonitorJSONMakeCommand("query-status", NULL)))
return -1;
- ret = qemuMonitorJSONCommand(mon, cmd, &reply);
-
- if (ret == 0)
- ret = qemuMonitorJSONCheckError(cmd, reply);
-
- if (ret < 0)
+ if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
goto cleanup;
- ret = -1;
+ if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+ goto cleanup;
if (!(data = virJSONValueObjectGetObject(reply, "return"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -1247,17 +1249,20 @@ qemuMonitorJSONGetStatus(qemuMonitorPtr mon,
int qemuMonitorJSONSystemPowerdown(qemuMonitorPtr mon)
{
- int ret;
+ int ret = -1;
virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("system_powerdown",
NULL);
virJSONValuePtr reply = NULL;
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;
+ cleanup:
virJSONValueFree(cmd);
virJSONValueFree(reply);
return ret;
@@ -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:
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;
+ cleanup:
virJSONValueFree(cmd);
virJSONValueFree(reply);
return ret;
@@ -1367,7 +1379,7 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr reply,
int qemuMonitorJSONGetCPUInfo(qemuMonitorPtr mon,
int **pids)
{
- int ret;
+ int ret = -1;
virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-cpus",
NULL);
virJSONValuePtr reply = NULL;
@@ -1377,14 +1389,14 @@ int qemuMonitorJSONGetCPUInfo(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 (ret == 0)
- ret = qemuMonitorJSONExtractCPUInfo(reply, pids);
+ if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+ goto cleanup;
+ ret = qemuMonitorJSONExtractCPUInfo(reply, pids);
+ cleanup:
virJSONValueFree(cmd);
virJSONValueFree(reply);
return ret;
@@ -1394,42 +1406,41 @@ int qemuMonitorJSONGetCPUInfo(qemuMonitorPtr mon,
int qemuMonitorJSONGetVirtType(qemuMonitorPtr mon,
virDomainVirtType *virtType)
{
- int ret;
+ int ret = -1;
virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-kvm",
NULL);
virJSONValuePtr reply = NULL;
+ virJSONValuePtr data;
+ bool val = false;
*virtType = VIR_DOMAIN_VIRT_QEMU;
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;
- if (ret == 0) {
- virJSONValuePtr data;
- bool val = false;
- if (!(data = virJSONValueObjectGetObject(reply, "return"))) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("info kvm reply was missing return data"));
- ret = -1;
- goto cleanup;
- }
+ if (!(data = virJSONValueObjectGetObject(reply, "return"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("info kvm reply was missing return data"));
+ goto cleanup;
+ }
- if (virJSONValueObjectGetBoolean(data, "enabled", &val) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("info kvm reply missing 'enabled'
field"));
- ret = -1;
- goto cleanup;
- }
- if (val)
- *virtType = VIR_DOMAIN_VIRT_KVM;
+ if (virJSONValueObjectGetBoolean(data, "enabled", &val) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("info kvm reply missing 'enabled' field"));
+ goto cleanup;
}
+ if (val)
+ *virtType = VIR_DOMAIN_VIRT_KVM;
+
+ ret = 0;
cleanup:
virJSONValueFree(cmd);
virJSONValueFree(reply);
@@ -1564,7 +1575,9 @@ int
qemuMonitorJSONGetBalloonInfo(qemuMonitorPtr mon,
unsigned long long *currmem)
{
- int ret;
+ int ret = -1;
+ virJSONValuePtr data;
+ unsigned long long mem;
virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-balloon",
NULL);
virJSONValuePtr reply = NULL;
@@ -1574,41 +1587,35 @@ qemuMonitorJSONGetBalloonInfo(qemuMonitorPtr mon,
if (!cmd)
return -1;
- ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+ if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+ goto cleanup;
- if (ret == 0) {
- /* See if balloon soft-failed */
- if (qemuMonitorJSONHasError(reply, "DeviceNotActive") ||
- qemuMonitorJSONHasError(reply, "KVMMissingCap"))
- goto cleanup;
-
- /* See if any other fatal error occurred */
- ret = qemuMonitorJSONCheckError(cmd, reply);
-
- /* Success */
- if (ret == 0) {
- virJSONValuePtr data;
- unsigned long long mem;
+ /* See if balloon soft-failed */
+ if (qemuMonitorJSONHasError(reply, "DeviceNotActive") ||
+ qemuMonitorJSONHasError(reply, "KVMMissingCap")) {
+ ret = 0;
+ goto cleanup;
+ }
- if (!(data = virJSONValueObjectGetObject(reply, "return"))) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("info balloon reply was missing return
data"));
- ret = -1;
- goto cleanup;
- }
+ /* See if any other fatal error occurred */
+ if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+ goto cleanup;
- if (virJSONValueObjectGetNumberUlong(data, "actual", &mem) <
0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("info balloon reply was missing balloon
data"));
- ret = -1;
- goto cleanup;
- }
+ /* Success */
+ if (!(data = virJSONValueObjectGetObject(reply, "return"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("info balloon reply was missing return data"));
+ goto cleanup;
+ }
- *currmem = (mem/1024);
- ret = 1;
- }
+ if (virJSONValueObjectGetNumberUlong(data, "actual", &mem) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("info balloon reply was missing balloon data"));
+ goto cleanup;
}
+ *currmem = (mem/1024);
+ ret = 1;
cleanup:
virJSONValueFree(cmd);
virJSONValueFree(reply);
@@ -1658,7 +1665,7 @@ int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon,
virDomainMemoryStatPtr stats,
unsigned int nr_stats)
{
- int ret;
+ int ret = -1;
virJSONValuePtr cmd = NULL;
virJSONValuePtr reply = NULL;
virJSONValuePtr data;
@@ -1682,7 +1689,7 @@ int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon,
NULL)))
goto cleanup;
- if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0)
+ if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
goto cleanup;
if ((data = virJSONValueObjectGetObject(reply, "error"))) {
@@ -1697,7 +1704,7 @@ int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon,
}
}
- if ((ret = qemuMonitorJSONCheckError(cmd, reply)) < 0)
+ if (qemuMonitorJSONCheckError(cmd, reply) < 0)
goto cleanup;
if (!(data = virJSONValueObjectGetObject(reply, "return"))) {
@@ -1723,15 +1730,10 @@ int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon,
VIR_DOMAIN_MEMORY_STAT_UNUSED, 1024);
GET_BALLOON_STATS("stat-total-memory",
VIR_DOMAIN_MEMORY_STAT_AVAILABLE, 1024);
-
-
+ ret = got;
cleanup:
virJSONValueFree(cmd);
virJSONValueFree(reply);
-
- if (got > 0)
- ret = got;
-
return ret;
}
#undef GET_BALLOON_STATS
@@ -1764,7 +1766,7 @@ qemuMonitorJSONSetMemoryStatsPeriod(qemuMonitorPtr mon,
int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
virHashTablePtr table)
{
- int ret;
+ int ret = -1;
size_t i;
virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-block",
@@ -1775,13 +1777,11 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
if (!cmd)
return -1;
- ret = qemuMonitorJSONCommand(mon, cmd, &reply);
- if (ret == 0)
- ret = qemuMonitorJSONCheckError(cmd, reply);
- if (ret < 0)
+ if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
goto cleanup;
- ret = -1;
+ if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+ goto cleanup;
if (!(devices = virJSONValueObjectGetArray(reply, "return"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -1847,7 +1847,6 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
}
ret = 0;
-
cleanup:
virJSONValueFree(cmd);
virJSONValueFree(reply);
@@ -2114,7 +2113,7 @@ int qemuMonitorJSONBlockResize(qemuMonitorPtr mon,
const char *device,
unsigned long long size)
{
- int ret;
+ int ret = -1;
virJSONValuePtr cmd;
virJSONValuePtr reply = NULL;
@@ -2125,17 +2124,18 @@ int qemuMonitorJSONBlockResize(qemuMonitorPtr mon,
if (!cmd)
return -1;
- ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+ if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+ goto cleanup;
- if (ret == 0) {
- if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
- ret = -2;
- goto cleanup;
- }
-
- ret = qemuMonitorJSONCheckError(cmd, reply);
+ if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
+ ret = -2;
+ goto cleanup;
}
+ if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+ goto cleanup;
+
+ ret = 0;
cleanup:
virJSONValueFree(cmd);
virJSONValueFree(reply);
@@ -2145,7 +2145,7 @@ int qemuMonitorJSONBlockResize(qemuMonitorPtr mon,
int qemuMonitorJSONSetVNCPassword(qemuMonitorPtr mon,
const char *password)
{
- int ret;
+ int ret = -1;
virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("change",
"s:device",
"vnc",
"s:target",
"password",
@@ -2155,11 +2155,14 @@ int qemuMonitorJSONSetVNCPassword(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;
+ cleanup:
virJSONValueFree(cmd);
virJSONValueFree(reply);
return ret;
@@ -2171,7 +2174,7 @@ int qemuMonitorJSONSetPassword(qemuMonitorPtr mon,
const char *password,
const char *action_if_connected)
{
- int ret;
+ int ret = -1;
virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("set_password",
"s:protocol", protocol,
"s:password", password,
@@ -2181,17 +2184,18 @@ int qemuMonitorJSONSetPassword(qemuMonitorPtr mon,
if (!cmd)
return -1;
- ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+ if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+ goto cleanup;
- if (ret == 0) {
- if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
- ret = -2;
- goto cleanup;
- }
-
- ret = qemuMonitorJSONCheckError(cmd, reply);
+ if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
+ ret = -2;
+ goto cleanup;
}
+ if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+ goto cleanup;
+
+ ret = 0;
cleanup:
virJSONValueFree(cmd);
virJSONValueFree(reply);
@@ -2203,7 +2207,7 @@ int qemuMonitorJSONExpirePassword(qemuMonitorPtr mon,
const char *protocol,
const char *expire_time)
{
- int ret;
+ int ret = -1;
virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("expire_password",
"s:protocol", protocol,
"s:time", expire_time,
@@ -2212,17 +2216,18 @@ int qemuMonitorJSONExpirePassword(qemuMonitorPtr mon,
if (!cmd)
return -1;
- ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+ if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+ goto cleanup;
- if (ret == 0) {
- if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
- ret = -2;
- goto cleanup;
- }
-
- ret = qemuMonitorJSONCheckError(cmd, reply);
+ if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
+ ret = -2;
+ goto cleanup;
}
+ if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+ goto cleanup;
+
+ ret = 0;
cleanup:
virJSONValueFree(cmd);
virJSONValueFree(reply);
@@ -2234,7 +2239,7 @@ int
qemuMonitorJSONSetBalloon(qemuMonitorPtr mon,
unsigned long long newmem)
{
- int ret;
+ int ret = -1;
virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("balloon",
"U:value", newmem * 1024,
NULL);
@@ -2242,22 +2247,22 @@ qemuMonitorJSONSetBalloon(qemuMonitorPtr mon,
if (!cmd)
return -1;
- ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+ if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+ goto cleanup;
- if (ret == 0) {
- /* See if balloon soft-failed */
- if (qemuMonitorJSONHasError(reply, "DeviceNotActive") ||
- qemuMonitorJSONHasError(reply, "KVMMissingCap"))
- goto cleanup;
-
- /* See if any other fatal error occurred */
- ret = qemuMonitorJSONCheckError(cmd, reply);
-
- /* Real success */
- if (ret == 0)
- ret = 1;
+ /* See if balloon soft-failed */
+ if (qemuMonitorJSONHasError(reply, "DeviceNotActive") ||
+ qemuMonitorJSONHasError(reply, "KVMMissingCap")) {
+ ret = 0;
+ goto cleanup;
}
+ /* See if any other fatal error occurred */
+ if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+ goto cleanup;
+
+ /* Real success */
+ ret = 1;
cleanup:
virJSONValueFree(cmd);
virJSONValueFree(reply);
--
2.8.1
6:33 a.m.
New subject: [libvirt] [PATCH 3/3] qemu_monitor_json: Follow our coding style
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(a)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.
3161
days inactive
3161
days old
6 comments
2 participants
participants (2)
-
Michal Privoznik -
Peter Krempa