[PATCH 0/5] trivial qemuProcessQMPPtr related cleanups
Hi, These are some simple cleanups related to the qemuProcessQMPPtr pointer I ended up making while trying to understand how and when qemuProcessQMPFree() is called. Daniel Henrique Barboza (5): qemu_process.h: register AUTOPTR_CLEANUP_FUNC for qemuProcessQMPPtr qemu_process.c: modernize qemuProcessQMPNew() qemu_driver.c: modernize qemuConnectCPUModelBaseline() qemu_driver.c: modernize qemuConnectCPUModelComparison() qemu_capabilities.c: use g_autoptr() in virQEMUCapsInitQMPSingle() src/qemu/qemu_capabilities.c | 3 +-- src/qemu/qemu_driver.c | 41 ++++++++++++++---------------------- src/qemu/qemu_process.c | 13 ++++-------- src/qemu/qemu_process.h | 1 + 4 files changed, 22 insertions(+), 36 deletions(-) -- 2.26.2
Next patches will use g_autoptr() in qemuProcessQMPPtr pointers for some cleanups in QMP code. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_process.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 15e67b9762..125508f9fe 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -234,5 +234,6 @@ qemuProcessQMPPtr qemuProcessQMPNew(const char *binary, bool forceTCG); void qemuProcessQMPFree(qemuProcessQMPPtr proc); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuProcessQMP, qemuProcessQMPFree); int qemuProcessQMPStart(qemuProcessQMPPtr proc); -- 2.26.2
Use g_autoptr() and remove the 'cleanup' label. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_process.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 70fc24b993..a11eedda16 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8395,8 +8395,7 @@ qemuProcessQMPNew(const char *binary, gid_t runGid, bool forceTCG) { - qemuProcessQMPPtr ret = NULL; - qemuProcessQMPPtr proc = NULL; + g_autoptr(qemuProcessQMP) proc = NULL; const char *threadSuffix; g_autofree char *threadName = NULL; @@ -8404,7 +8403,7 @@ qemuProcessQMPNew(const char *binary, binary, libDir, runUid, runGid, forceTCG); if (VIR_ALLOC(proc) < 0) - goto cleanup; + return NULL; proc->binary = g_strdup(binary); proc->libDir = g_strdup(libDir); @@ -8421,13 +8420,9 @@ qemuProcessQMPNew(const char *binary, threadName = g_strdup_printf("qmp-%s", threadSuffix); if (!(proc->eventThread = virEventThreadNew(threadName))) - goto cleanup; - - ret = g_steal_pointer(&proc); + return NULL; - cleanup: - qemuProcessQMPFree(proc); - return ret; + return g_steal_pointer(&proc); } -- 2.26.2
Use g_autoptr() on pointers and remove the unneeded 'cleanup' label. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d185666ed8..d73d093465 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13342,50 +13342,44 @@ qemuConnectCPUModelBaseline(virQEMUCapsPtr qemuCaps, virCPUDefPtr *cpus, int ncpus) { - qemuProcessQMPPtr proc; - virCPUDefPtr ret = NULL; - virCPUDefPtr baseline = NULL; + g_autoptr(qemuProcessQMP) proc = NULL; + g_autoptr(virCPUDef) baseline = NULL; qemuMonitorCPUModelInfoPtr result = NULL; size_t i; if (!(proc = qemuProcessQMPNew(virQEMUCapsGetBinary(qemuCaps), libDir, runUid, runGid, false))) - goto cleanup; + return NULL; if (qemuProcessQMPStart(proc) < 0) - goto cleanup; + return NULL; if (VIR_ALLOC(baseline) < 0) - goto cleanup; + return NULL; if (virCPUDefCopyModel(baseline, cpus[0], false)) - goto cleanup; + return NULL; for (i = 1; i < ncpus; i++) { if (qemuMonitorGetCPUModelBaseline(proc->mon, baseline, cpus[i], &result) < 0) - goto cleanup; + return NULL; if (qemuConnectStealCPUModelFromInfo(baseline, &result) < 0) - goto cleanup; + return NULL; } if (expand_features) { if (qemuMonitorGetCPUModelExpansion(proc->mon, QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL, baseline, true, false, &result) < 0) - goto cleanup; + return NULL; if (qemuConnectStealCPUModelFromInfo(baseline, &result) < 0) - goto cleanup; + return NULL; } - ret = g_steal_pointer(&baseline); - - cleanup: - qemuProcessQMPFree(proc); - virCPUDefFree(baseline); - return ret; + return g_steal_pointer(&baseline); } -- 2.26.2
Use g_auto* on pointers to avoid using the 'cleanup' label. In theory the 'ret' variable can also be discarded if the flow of the logic is reworked. Perhaps another time. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d73d093465..393a7e27cb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13147,19 +13147,19 @@ qemuConnectCPUModelComparison(virQEMUCapsPtr qemuCaps, virCPUDefPtr cpu_b, bool failIncompatible) { - qemuProcessQMPPtr proc = NULL; - char *result = NULL; + g_autoptr(qemuProcessQMP) proc = NULL; + g_autofree char *result = NULL; int ret = VIR_CPU_COMPARE_ERROR; if (!(proc = qemuProcessQMPNew(virQEMUCapsGetBinary(qemuCaps), libDir, runUid, runGid, false))) - goto cleanup; + return VIR_CPU_COMPARE_ERROR; if (qemuProcessQMPStart(proc) < 0) - goto cleanup; + return VIR_CPU_COMPARE_ERROR; if (qemuMonitorGetCPUModelComparison(proc->mon, cpu_a, cpu_b, &result) < 0) - goto cleanup; + return VIR_CPU_COMPARE_ERROR; if (STREQ(result, "identical")) ret = VIR_CPU_COMPARE_IDENTICAL; @@ -13170,9 +13170,6 @@ qemuConnectCPUModelComparison(virQEMUCapsPtr qemuCaps, else ret = VIR_CPU_COMPARE_INCOMPATIBLE; - cleanup: - VIR_FREE(result); - qemuProcessQMPFree(proc); return ret; } -- 2.26.2
On Fri, Jul 17, 2020 at 18:15:55 -0300, Daniel Henrique Barboza wrote:
Use g_auto* on pointers to avoid using the 'cleanup' label.
In theory the 'ret' variable can also be discarded if the flow of the logic is reworked. Perhaps another time.
Doing so would actually be pretty easy, mostly ret = ...; would need to be replaced with direct return. I'll send a trivial patch doing this. Jirka
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_capabilities.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 98ddd2dcdc..1a2cb874c0 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5382,7 +5382,7 @@ virQEMUCapsInitQMPSingle(virQEMUCapsPtr qemuCaps, gid_t runGid, bool onlyTCG) { - qemuProcessQMPPtr proc = NULL; + g_autoptr(qemuProcessQMP) proc = NULL; int ret = -1; if (!(proc = qemuProcessQMPNew(qemuCaps->binary, libDir, @@ -5401,7 +5401,6 @@ virQEMUCapsInitQMPSingle(virQEMUCapsPtr qemuCaps, if (ret < 0) virQEMUCapsLogProbeFailure(qemuCaps->binary); - qemuProcessQMPFree(proc); return ret; } -- 2.26.2
On Fri, Jul 17, 2020 at 18:15:51 -0300, Daniel Henrique Barboza wrote:
Hi,
These are some simple cleanups related to the qemuProcessQMPPtr pointer I ended up making while trying to understand how and when qemuProcessQMPFree() is called.
Daniel Henrique Barboza (5): qemu_process.h: register AUTOPTR_CLEANUP_FUNC for qemuProcessQMPPtr qemu_process.c: modernize qemuProcessQMPNew() qemu_driver.c: modernize qemuConnectCPUModelBaseline() qemu_driver.c: modernize qemuConnectCPUModelComparison() qemu_capabilities.c: use g_autoptr() in virQEMUCapsInitQMPSingle()
src/qemu/qemu_capabilities.c | 3 +-- src/qemu/qemu_driver.c | 41 ++++++++++++++---------------------- src/qemu/qemu_process.c | 13 ++++-------- src/qemu/qemu_process.h | 1 + 4 files changed, 22 insertions(+), 36 deletions(-)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com> And pushed.
participants (2)
-
Daniel Henrique Barboza -
Jiri Denemark