Let's pass along / fill @niothreads rather than trying to make dual
use as a return value and thread count.
This resolves a Coverity issue detected in qemuDomainGetIOThreadsMon
where if qemuDomainObjExitMonitor failed, then a -1 was returned and
overwrite @niothreads causing a memory leak.
Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
Since v1, updated the logic to pass @niothreads around rather than
rely on the dual meaning. Took the full plunge.
src/qemu/qemu_driver.c | 23 +++++++++++------------
src/qemu/qemu_monitor.c | 8 +++++---
src/qemu/qemu_monitor.h | 3 ++-
src/qemu/qemu_monitor_json.c | 6 ++++--
src/qemu/qemu_monitor_json.h | 3 ++-
src/qemu/qemu_process.c | 4 ++--
tests/qemumonitorjsontest.c | 4 ++--
7 files changed, 28 insertions(+), 23 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index bca1c84630..65725b2ef2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4972,17 +4972,18 @@ qemuDomainGetMaxVcpus(virDomainPtr dom)
static int
qemuDomainGetIOThreadsMon(virQEMUDriverPtr driver,
virDomainObjPtr vm,
- qemuMonitorIOThreadInfoPtr **iothreads)
+ qemuMonitorIOThreadInfoPtr **iothreads,
+ int *niothreads)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
- int niothreads = 0;
+ int ret = -1;
qemuDomainObjEnterMonitor(driver, vm);
- niothreads = qemuMonitorGetIOThreads(priv->mon, iothreads);
- if (qemuDomainObjExitMonitor(driver, vm) < 0 || niothreads < 0)
+ ret = qemuMonitorGetIOThreads(priv->mon, iothreads, niothreads);
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
return -1;
- return niothreads;
+ return ret;
}
@@ -5014,7 +5015,7 @@ qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver,
goto endjob;
}
- if ((niothreads = qemuDomainGetIOThreadsMon(driver, vm, &iothreads)) < 0)
+ if ((ret = qemuDomainGetIOThreadsMon(driver, vm, &iothreads, &niothreads)) < 0)
goto endjob;
/* Nothing to do */
@@ -5314,8 +5315,7 @@ qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver,
* IOThreads thread_id's, adjust the cgroups, thread affinity,
* and add the thread_id to the vm->def->iothreadids list.
*/
- if ((new_niothreads = qemuMonitorGetIOThreads(priv->mon,
- &new_iothreads)) < 0)
+ if (qemuMonitorGetIOThreads(priv->mon, &new_iothreads, &new_niothreads) < 0)
goto exit_monitor;
if (qemuDomainObjExitMonitor(driver, vm) < 0)
@@ -5425,8 +5425,7 @@ qemuDomainHotplugDelIOThread(virQEMUDriverPtr driver,
if (rc < 0)
goto exit_monitor;
- if ((new_niothreads = qemuMonitorGetIOThreads(priv->mon,
- &new_iothreads)) < 0)
+ if (qemuMonitorGetIOThreads(priv->mon, &new_iothreads, &new_niothreads) < 0)
goto exit_monitor;
if (qemuDomainObjExitMonitor(driver, vm) < 0)
@@ -18507,7 +18506,7 @@ qemuDomainGetStatsIOThread(virQEMUDriverPtr driver,
qemuDomainObjPrivatePtr priv = dom->privateData;
size_t i;
qemuMonitorIOThreadInfoPtr *iothreads = NULL;
- int niothreads;
+ int niothreads = 0;
int ret = -1;
if (!HAVE_JOB(privflags) || !virDomainObjIsActive(dom))
@@ -18516,7 +18515,7 @@ qemuDomainGetStatsIOThread(virQEMUDriverPtr driver,
if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD))
return 0;
- if ((niothreads = qemuDomainGetIOThreadsMon(driver, dom, &iothreads)) < 0)
+ if (qemuDomainGetIOThreadsMon(driver, dom, &iothreads, &niothreads) < 0)
return -1;
/* qemuDomainGetIOThreadsMon returns a NULL-terminated list, so we must free
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index ce1a06c4c8..551b65e778 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4211,22 +4211,24 @@ qemuMonitorRTCResetReinjection(qemuMonitorPtr mon)
* qemuMonitorGetIOThreads:
* @mon: Pointer to the monitor
* @iothreads: Location to return array of IOThreadInfo data
+ * @niothreads: Count of the number of IOThreads in the array
*
* Issue query-iothreads command.
* Retrieve the list of iothreads defined/running for the machine
*
- * Returns count of IOThreadInfo structures on success
+ * Returns 0 on success
* -1 on error.
*/
int
qemuMonitorGetIOThreads(qemuMonitorPtr mon,
- qemuMonitorIOThreadInfoPtr **iothreads)
+ qemuMonitorIOThreadInfoPtr **iothreads,
+ int *niothreads)
{
VIR_DEBUG("iothreads=%p", iothreads);
QEMU_CHECK_MONITOR(mon);
- return qemuMonitorJSONGetIOThreads(mon, iothreads);
+ return qemuMonitorJSONGetIOThreads(mon, iothreads, niothreads);
}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 8bc092870b..49be2d5412 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1365,7 +1365,8 @@ struct _qemuMonitorIOThreadInfo {
bool set_poll_shrink;
};
int qemuMonitorGetIOThreads(qemuMonitorPtr mon,
- qemuMonitorIOThreadInfoPtr **iothreads);
+ qemuMonitorIOThreadInfoPtr **iothreads,
+ int *niothreads);
int qemuMonitorSetIOThread(qemuMonitorPtr mon,
qemuMonitorIOThreadInfoPtr iothreadInfo);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 5acc1a10aa..f70490d9b0 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -8072,7 +8072,8 @@ qemuMonitorJSONRTCResetReinjection(qemuMonitorPtr mon)
*/
int
qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon,
- qemuMonitorIOThreadInfoPtr **iothreads)
+ qemuMonitorIOThreadInfoPtr **iothreads,
+ int *niothreads)
{
int ret = -1;
virJSONValuePtr cmd;
@@ -8149,9 +8150,10 @@ qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon,
info->poll_valid = true;
}
- ret = n;
+ *niothreads = n;
*iothreads = infolist;
infolist = NULL;
+ ret = 0;
cleanup:
if (infolist) {
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index d2928b0ffc..4eb0f667a2 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -550,7 +550,8 @@ int qemuMonitorJSONGetGuestCPU(qemuMonitorPtr mon,
int qemuMonitorJSONRTCResetReinjection(qemuMonitorPtr mon);
int qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon,
- qemuMonitorIOThreadInfoPtr **iothreads)
+ qemuMonitorIOThreadInfoPtr **iothreads,
+ int *niothreads)
ATTRIBUTE_NONNULL(2);
int qemuMonitorJSONSetIOThread(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 20e90026e1..01afe66ec9 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2498,10 +2498,10 @@ qemuProcessDetectIOThreadPIDs(virQEMUDriverPtr driver,
/* Get the list of IOThreads from qemu */
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
goto cleanup;
- niothreads = qemuMonitorGetIOThreads(priv->mon, &iothreads);
+ ret = qemuMonitorGetIOThreads(priv->mon, &iothreads, &niothreads);
if (qemuDomainObjExitMonitor(driver, vm) < 0)
goto cleanup;
- if (niothreads < 0)
+ if (ret < 0)
goto cleanup;
if (niothreads != vm->def->niothreadids) {
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 79ef2a545e..d0c37967d5 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -2377,8 +2377,8 @@ testQemuMonitorJSONGetIOThreads(const void *opaque)
"}") < 0)
goto cleanup;
- if ((ninfo = qemuMonitorGetIOThreads(qemuMonitorTestGetMonitor(test),
- &info)) < 0)
+ if (qemuMonitorGetIOThreads(qemuMonitorTestGetMonitor(test),
+ &info, &ninfo) < 0)
goto cleanup;
if (ninfo != 2) {
--
2.28.0
