10:36 a.m.
"virHashNew" cannot return NULL, yet we check for NULL in various places.
This series is the first of several that remove these checks. Where
applicable, the functions are refactored to use automatic memory management
by means of g_autoptr etc. as well.
Tim Wiederhake (10):
qemuMonitorGetAllBlockStatsInfo: Clean up line break
qemuMonitorGetAllBlockStatsInfo: Remove superfluous variable
initialization
qemuMonitorGetAllBlockStatsInfo: Assign hash table only on success
qemuMonitorGetAllBlockStatsInfo: Use automatic memory management
qemuMonitorGetBlockInfo: Remove superfluous variable "ret"
qemuMonitorGetBlockInfo: Use automatic memory management
qemuMonitorGetBlockInfo: `virHashNew` cannot return NULL
qemuMonitorGetChardevInfo: Remove superfluous variable "ret"
qemuMonitorGetChardevInfo: Use automatic memory management
qemuMonitorGetChardevInfo: `virHashNew` cannot return NULL
src/qemu/qemu_monitor.c | 54 ++++++++++++-----------------------------
1 file changed, 16 insertions(+), 38 deletions(-)
--
2.31.1
10:36 a.m.
New subject: [libvirt PATCH 01/10] qemuMonitorGetAllBlockStatsInfo: Clean up line break
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/qemu/qemu_monitor.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 8f35b4240f..df1e41f68f 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2162,8 +2162,7 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitor *mon,
if (!(*ret_stats = virHashNew(g_free)))
goto error;
- ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, *ret_stats,
- backingChain);
+ ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, *ret_stats, backingChain);
if (ret < 0)
goto error;
--
2.31.1
10:36 a.m.
New subject: [libvirt PATCH 02/10] qemuMonitorGetAllBlockStatsInfo: Remove superfluous variable initialization
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/qemu/qemu_monitor.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index df1e41f68f..19fcc5658b 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2154,7 +2154,7 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitor *mon,
GHashTable **ret_stats,
bool backingChain)
{
- int ret = -1;
+ int ret;
VIR_DEBUG("ret_stats=%p, backing=%d", ret_stats, backingChain);
QEMU_CHECK_MONITOR(mon);
--
2.31.1
10:36 a.m.
New subject: [libvirt PATCH 03/10] qemuMonitorGetAllBlockStatsInfo: Assign hash table only on success
`virHashNew` cannot return NULL, the check is not needed.
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/qemu/qemu_monitor.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 19fcc5658b..86aabc98c3 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2155,23 +2155,23 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitor *mon,
bool backingChain)
{
int ret;
+ GHashTable *stats = virHashNew(g_free);
+
VIR_DEBUG("ret_stats=%p, backing=%d", ret_stats, backingChain);
QEMU_CHECK_MONITOR(mon);
- if (!(*ret_stats = virHashNew(g_free)))
- goto error;
-
- ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, *ret_stats, backingChain);
+ *ret_stats = NULL;
+ ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, stats, backingChain);
if (ret < 0)
goto error;
+ *ret_stats = stats;
return ret;
error:
- virHashFree(*ret_stats);
- *ret_stats = NULL;
+ virHashFree(stats);
return -1;
}
--
2.31.1
10:47 a.m.
New subject: [libvirt PATCH 03/10] qemuMonitorGetAllBlockStatsInfo: Assign hash table only on success
On Mon, Jul 05, 2021 at 05:36:42PM +0200, Tim Wiederhake wrote:
`virHashNew` cannot return NULL, the check is not needed.
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/qemu/qemu_monitor.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 19fcc5658b..86aabc98c3 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2155,23 +2155,23 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitor *mon,
bool backingChain)
{
int ret;
+ GHashTable *stats = virHashNew(g_free);
+
VIR_DEBUG("ret_stats=%p, backing=%d", ret_stats, backingChain);
QEMU_CHECK_MONITOR(mon);
This macro expands with 'return -1', so this change causes a memory
leak of 'stats'. This obscurity is why i don't like macros containing
hidden control flow.
- if (!(*ret_stats = virHashNew(g_free)))
- goto error;
-
- ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, *ret_stats, backingChain);
+ *ret_stats = NULL;
+ ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, stats, backingChain);
if (ret < 0)
goto error;
+ *ret_stats = stats;
return ret;
error:
- virHashFree(*ret_stats);
- *ret_stats = NULL;
+ virHashFree(stats);
return -1;
}
--
2.31.1
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
10:36 a.m.
New subject: [libvirt PATCH 04/10] qemuMonitorGetAllBlockStatsInfo: Use automatic memory management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/qemu/qemu_monitor.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 86aabc98c3..f08b43bbfb 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2155,7 +2155,7 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitor *mon,
bool backingChain)
{
int ret;
- GHashTable *stats = virHashNew(g_free);
+ g_autoptr(GHashTable) stats = virHashNew(g_free);
VIR_DEBUG("ret_stats=%p, backing=%d", ret_stats, backingChain);
@@ -2165,14 +2165,10 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitor *mon,
ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, stats, backingChain);
if (ret < 0)
- goto error;
+ return -1;
- *ret_stats = stats;
+ *ret_stats = g_steal_pointer(&stats);
return ret;
-
- error:
- virHashFree(stats);
- return -1;
}
--
2.31.1
10:47 a.m.
New subject: [libvirt PATCH 04/10] qemuMonitorGetAllBlockStatsInfo: Use automatic memory management
On Mon, Jul 05, 2021 at 05:36:43PM +0200, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/qemu/qemu_monitor.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 86aabc98c3..f08b43bbfb 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2155,7 +2155,7 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitor *mon,
bool backingChain)
{
int ret;
- GHashTable *stats = virHashNew(g_free);
+ g_autoptr(GHashTable) stats = virHashNew(g_free);
VIR_DEBUG("ret_stats=%p, backing=%d", ret_stats, backingChain);
@@ -2165,14 +2165,10 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitor *mon,
ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, stats, backingChain);
if (ret < 0)
- goto error;
+ return -1;
- *ret_stats = stats;
+ *ret_stats = g_steal_pointer(&stats);
return ret;
-
- error:
- virHashFree(stats);
- return -1;
}
This fixes the bug introduced by the previous patch. The changes need
to be reversed to ensure bisect works correctly.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
10:36 a.m.
New subject: [libvirt PATCH 05/10] qemuMonitorGetBlockInfo: Remove superfluous variable "ret"
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/qemu/qemu_monitor.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index f08b43bbfb..de0f6ccf7d 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2103,7 +2103,6 @@ qemuDomainDiskInfoFree(void *value)
GHashTable *
qemuMonitorGetBlockInfo(qemuMonitor *mon)
{
- int ret;
GHashTable *table;
QEMU_CHECK_MONITOR_NULL(mon);
@@ -2111,9 +2110,7 @@ qemuMonitorGetBlockInfo(qemuMonitor *mon)
if (!(table = virHashNew(qemuDomainDiskInfoFree)))
return NULL;
- ret = qemuMonitorJSONGetBlockInfo(mon, table);
-
- if (ret < 0) {
+ if (qemuMonitorJSONGetBlockInfo(mon, table) < 0) {
virHashFree(table);
return NULL;
}
--
2.31.1
10:36 a.m.
New subject: [libvirt PATCH 06/10] qemuMonitorGetBlockInfo: Use automatic memory management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/qemu/qemu_monitor.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index de0f6ccf7d..9f0b20db09 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2103,7 +2103,7 @@ qemuDomainDiskInfoFree(void *value)
GHashTable *
qemuMonitorGetBlockInfo(qemuMonitor *mon)
{
- GHashTable *table;
+ g_autoptr(GHashTable) table = NULL;
QEMU_CHECK_MONITOR_NULL(mon);
@@ -2111,11 +2111,10 @@ qemuMonitorGetBlockInfo(qemuMonitor *mon)
return NULL;
if (qemuMonitorJSONGetBlockInfo(mon, table) < 0) {
- virHashFree(table);
return NULL;
}
- return table;
+ return g_steal_pointer(&table);
}
--
2.31.1
10:36 a.m.
New subject: [libvirt PATCH 07/10] qemuMonitorGetBlockInfo: `virHashNew` cannot return NULL
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/qemu/qemu_monitor.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 9f0b20db09..dba2fb1982 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2103,13 +2103,10 @@ qemuDomainDiskInfoFree(void *value)
GHashTable *
qemuMonitorGetBlockInfo(qemuMonitor *mon)
{
- g_autoptr(GHashTable) table = NULL;
+ g_autoptr(GHashTable) table = virHashNew(qemuDomainDiskInfoFree);
QEMU_CHECK_MONITOR_NULL(mon);
- if (!(table = virHashNew(qemuDomainDiskInfoFree)))
- return NULL;
-
if (qemuMonitorJSONGetBlockInfo(mon, table) < 0) {
return NULL;
}
--
2.31.1
10:36 a.m.
New subject: [libvirt PATCH 08/10] qemuMonitorGetChardevInfo: Remove superfluous variable "ret"
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/qemu/qemu_monitor.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index dba2fb1982..f03d6106f9 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2885,7 +2885,6 @@ int
qemuMonitorGetChardevInfo(qemuMonitor *mon,
GHashTable **retinfo)
{
- int ret;
GHashTable *info = NULL;
VIR_DEBUG("retinfo=%p", retinfo);
@@ -2895,9 +2894,7 @@ qemuMonitorGetChardevInfo(qemuMonitor *mon,
if (!(info = virHashNew(qemuMonitorChardevInfoFree)))
goto error;
- ret = qemuMonitorJSONGetChardevInfo(mon, info);
-
- if (ret < 0)
+ if (qemuMonitorJSONGetChardevInfo(mon, info) < 0)
goto error;
*retinfo = info;
--
2.31.1
10:36 a.m.
New subject: [libvirt PATCH 09/10] qemuMonitorGetChardevInfo: Use automatic memory management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/qemu/qemu_monitor.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index f03d6106f9..2c43cc0788 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2885,25 +2885,21 @@ int
qemuMonitorGetChardevInfo(qemuMonitor *mon,
GHashTable **retinfo)
{
- GHashTable *info = NULL;
+ g_autoptr(GHashTable) info = NULL;
VIR_DEBUG("retinfo=%p", retinfo);
- QEMU_CHECK_MONITOR_GOTO(mon, error);
+ QEMU_CHECK_MONITOR(mon);
+ *retinfo = NULL;
if (!(info = virHashNew(qemuMonitorChardevInfoFree)))
- goto error;
+ return -1;
if (qemuMonitorJSONGetChardevInfo(mon, info) < 0)
- goto error;
+ return -1;
- *retinfo = info;
+ *retinfo = g_steal_pointer(&info);
return 0;
-
- error:
- virHashFree(info);
- *retinfo = NULL;
- return -1;
}
--
2.31.1
10:36 a.m.
New subject: [libvirt PATCH 10/10] qemuMonitorGetChardevInfo: `virHashNew` cannot return NULL
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/qemu/qemu_monitor.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 2c43cc0788..a63056f0a0 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2885,16 +2885,13 @@ int
qemuMonitorGetChardevInfo(qemuMonitor *mon,
GHashTable **retinfo)
{
- g_autoptr(GHashTable) info = NULL;
+ g_autoptr(GHashTable) info = virHashNew(qemuMonitorChardevInfoFree);
VIR_DEBUG("retinfo=%p", retinfo);
QEMU_CHECK_MONITOR(mon);
*retinfo = NULL;
- if (!(info = virHashNew(qemuMonitorChardevInfoFree)))
- return -1;
-
if (qemuMonitorJSONGetChardevInfo(mon, info) < 0)
return -1;
--
2.31.1
1237
days inactive
1237
days old
12 comments
2 participants
participants (2)
-
Daniel P. Berrangé -
Tim Wiederhake