[libvirt PATCH 00/10] virHashNew refactorings - part III
"virHashNew" cannot return NULL, yet we check for NULL in various places. See https://listman.redhat.com/archives/libvir-list/2021-July/msg00074.html. Tim Wiederhake (10): testCompareXMLToArgvFiles: `virHashNew` cannot return NULL testQemuMonitorJSONqemuMonitorJSONGetAllBlockStatsInfo: Use automatic memory management testQemuMonitorJSONqemuMonitorJSONGetAllBlockStatsInfo: Remove superfluous `goto`s testQemuMonitorJSONqemuMonitorJSONGetAllBlockStatsInfo: `virHashNew` cannot return NULL testQemuMonitorJSONqemuMonitorJSONGetBlockInfo: Use automatic memory management testQemuMonitorJSONqemuMonitorJSONGetBlockInfo: Remove superfluous `goto`s testQemuMonitorJSONqemuMonitorJSONGetBlockInfo: `virHashNew` cannot return NULL testQemuMonitorJSONqemuMonitorJSONGetChardevInfo: Use automatic memory management testQemuMonitorJSONqemuMonitorJSONGetChardevInfo: Remove superfluous `goto`s testQemuMonitorJSONqemuMonitorJSONGetChardevInfo: `virHashNew` cannot return NULL tests/nwfilterxml2firewalltest.c | 3 -- tests/qemumonitorjsontest.c | 73 +++++++++++--------------------- 2 files changed, 24 insertions(+), 52 deletions(-) -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/nwfilterxml2firewalltest.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c index 6709cc15fd..1cde8e258e 100644 --- a/tests/nwfilterxml2firewalltest.c +++ b/tests/nwfilterxml2firewalltest.c @@ -376,9 +376,6 @@ static int testCompareXMLToArgvFiles(const char *xml, virCommandSetDryRun(dryRunToken, &buf, true, true, NULL, NULL); - if (!vars) - goto cleanup; - if (testSetDefaultParameters(vars) < 0) goto cleanup; -- 2.31.1
The function name in the subject is unfortunately ambiguous. Please prepend it with the test file name or something. On Mon, Jul 12, 2021 at 11:34:08 +0200, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/nwfilterxml2firewalltest.c | 3 --- 1 file changed, 3 deletions(-)
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/qemumonitorjsontest.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index e6746b806f..e993c30c97 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1536,7 +1536,7 @@ testQemuMonitorJSONqemuMonitorJSONGetAllBlockStatsInfo(const void *opaque) { const testGenericData *data = opaque; virDomainXMLOption *xmlopt = data->xmlopt; - GHashTable *blockstats = NULL; + g_autoptr(GHashTable) blockstats = NULL; qemuBlockStats *stats; int ret = -1; g_autoptr(qemuMonitorTest) test = NULL; @@ -1688,7 +1688,6 @@ testQemuMonitorJSONqemuMonitorJSONGetAllBlockStatsInfo(const void *opaque) #undef CHECK0FULL cleanup: - virHashFree(blockstats); return ret; } -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/qemumonitorjsontest.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index e993c30c97..b1332eb1df 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1538,7 +1538,6 @@ testQemuMonitorJSONqemuMonitorJSONGetAllBlockStatsInfo(const void *opaque) virDomainXMLOption *xmlopt = data->xmlopt; g_autoptr(GHashTable) blockstats = NULL; qemuBlockStats *stats; - int ret = -1; g_autoptr(qemuMonitorTest) test = NULL; const char *reply = @@ -1632,10 +1631,10 @@ testQemuMonitorJSONqemuMonitorJSONGetAllBlockStatsInfo(const void *opaque) return -1; if (!(blockstats = virHashNew(g_free))) - goto cleanup; + return -1; if (qemuMonitorTestAddItem(test, "query-blockstats", reply) < 0) - goto cleanup; + return -1; #define CHECK0FULL(var, value, varformat, valformat) \ if (stats->var != value) { \ @@ -1643,7 +1642,7 @@ testQemuMonitorJSONqemuMonitorJSONGetAllBlockStatsInfo(const void *opaque) "Invalid " #var " value: " varformat \ ", expected " valformat, \ stats->var, value); \ - goto cleanup; \ + return -1; \ } #define CHECK0(var, value) CHECK0FULL(var, value, "%lld", "%d") @@ -1654,7 +1653,7 @@ testQemuMonitorJSONqemuMonitorJSONGetAllBlockStatsInfo(const void *opaque) if (!(stats = virHashLookup(blockstats, NAME))) { \ virReportError(VIR_ERR_INTERNAL_ERROR, \ "block stats for device '%s' is missing", NAME); \ - goto cleanup; \ + return -1; \ } \ CHECK0(rd_req, RD_REQ) \ CHECK0(rd_bytes, RD_BYTES) \ @@ -1669,26 +1668,24 @@ testQemuMonitorJSONqemuMonitorJSONGetAllBlockStatsInfo(const void *opaque) if (qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorTestGetMonitor(test), blockstats, false) < 0) - goto cleanup; + return -1; if (!blockstats) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "qemuMonitorJSONGetAllBlockStatsInfo didn't return stats"); - goto cleanup; + return -1; } CHECK("virtio-disk0", 1279, 28505088, 640616474, 174, 2845696, 530699221, 0, 0, 5256018944ULL, true) CHECK("virtio-disk1", 85, 348160, 8232156, 0, 0, 0, 0, 0, 0ULL, true) CHECK("ide0-1-0", 16, 49250, 1004952, 0, 0, 0, 0, 0, 0ULL, false) - ret = 0; + return 0; #undef CHECK #undef CHECK0 #undef CHECK0FULL - cleanup: - return ret; } -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/qemumonitorjsontest.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index b1332eb1df..8895c7305f 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1536,7 +1536,7 @@ testQemuMonitorJSONqemuMonitorJSONGetAllBlockStatsInfo(const void *opaque) { const testGenericData *data = opaque; virDomainXMLOption *xmlopt = data->xmlopt; - g_autoptr(GHashTable) blockstats = NULL; + g_autoptr(GHashTable) blockstats = virHashNew(g_free); qemuBlockStats *stats; g_autoptr(qemuMonitorTest) test = NULL; @@ -1630,9 +1630,6 @@ testQemuMonitorJSONqemuMonitorJSONGetAllBlockStatsInfo(const void *opaque) if (!(test = qemuMonitorTestNewSchema(xmlopt, data->schema))) return -1; - if (!(blockstats = virHashNew(g_free))) - return -1; - if (qemuMonitorTestAddItem(test, "query-blockstats", reply) < 0) return -1; -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/qemumonitorjsontest.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 8895c7305f..e4a907e5b6 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1461,8 +1461,8 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockInfo(const void *opaque) const testGenericData *data = opaque; virDomainXMLOption *xmlopt = data->xmlopt; int ret = -1; - GHashTable *blockDevices = NULL; - GHashTable *expectedBlockDevices = NULL; + g_autoptr(GHashTable) blockDevices = NULL; + g_autoptr(GHashTable) expectedBlockDevices = NULL; struct qemuDomainDiskInfo *info; g_autoptr(qemuMonitorTest) test = NULL; @@ -1526,8 +1526,6 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockInfo(const void *opaque) ret = 0; cleanup: - virHashFree(blockDevices); - virHashFree(expectedBlockDevices); return ret; } -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/qemumonitorjsontest.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index e4a907e5b6..7fd58dc2d1 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1460,7 +1460,6 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockInfo(const void *opaque) { const testGenericData *data = opaque; virDomainXMLOption *xmlopt = data->xmlopt; - int ret = -1; g_autoptr(GHashTable) blockDevices = NULL; g_autoptr(GHashTable) expectedBlockDevices = NULL; struct qemuDomainDiskInfo *info; @@ -1471,14 +1470,14 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockInfo(const void *opaque) if (!(blockDevices = virHashNew(g_free)) || !(expectedBlockDevices = virHashNew(g_free))) - goto cleanup; + return -1; info = g_new0(struct qemuDomainDiskInfo, 1); if (virHashAddEntry(expectedBlockDevices, "virtio-disk0", info) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Unable to create expectedBlockDevices hash table"); - goto cleanup; + return -1; } info = g_new0(struct qemuDomainDiskInfo, 1); @@ -1486,7 +1485,7 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockInfo(const void *opaque) if (virHashAddEntry(expectedBlockDevices, "virtio-disk1", info) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Unable to create expectedBlockDevices hash table"); - goto cleanup; + return -1; } info = g_new0(struct qemuDomainDiskInfo, 1); @@ -1497,7 +1496,7 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockInfo(const void *opaque) if (virHashAddEntry(expectedBlockDevices, "ide0-1-0", info) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Unable to create expectedBlockDevices hash table"); - goto cleanup; + return -1; } info = g_new0(struct qemuDomainDiskInfo, 1); @@ -1509,24 +1508,22 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockInfo(const void *opaque) if (virHashAddEntry(expectedBlockDevices, "ide0-1-1", info) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Unable to create expectedBlockDevices hash table"); - goto cleanup; + return -1; } if (qemuMonitorTestAddItem(test, "query-block", queryBlockReply) < 0) - goto cleanup; + return -1; if (qemuMonitorJSONGetBlockInfo(qemuMonitorTestGetMonitor(test), blockDevices) < 0) - goto cleanup; + return -1; if (!virHashEqual(blockDevices, expectedBlockDevices, testHashEqualQemuDomainDiskInfo)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Hashtable is different to the expected one"); - goto cleanup; + return -1; } - ret = 0; - cleanup: - return ret; + return 0; } static int -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/qemumonitorjsontest.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 7fd58dc2d1..89ede3b59a 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1460,18 +1460,14 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockInfo(const void *opaque) { const testGenericData *data = opaque; virDomainXMLOption *xmlopt = data->xmlopt; - g_autoptr(GHashTable) blockDevices = NULL; - g_autoptr(GHashTable) expectedBlockDevices = NULL; + g_autoptr(GHashTable) blockDevices = virHashNew(g_free); + g_autoptr(GHashTable) expectedBlockDevices = virHashNew(g_free); struct qemuDomainDiskInfo *info; g_autoptr(qemuMonitorTest) test = NULL; if (!(test = qemuMonitorTestNewSchema(xmlopt, data->schema))) return -1; - if (!(blockDevices = virHashNew(g_free)) || - !(expectedBlockDevices = virHashNew(g_free))) - return -1; - info = g_new0(struct qemuDomainDiskInfo, 1); if (virHashAddEntry(expectedBlockDevices, "virtio-disk0", info) < 0) { -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/qemumonitorjsontest.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 89ede3b59a..89ace3bb33 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1812,8 +1812,8 @@ testQemuMonitorJSONqemuMonitorJSONGetChardevInfo(const void *opaque) const testGenericData *data = opaque; virDomainXMLOption *xmlopt = data->xmlopt; int ret = -1; - GHashTable *info = NULL; - GHashTable *expectedInfo = NULL; + g_autoptr(GHashTable) info = NULL; + g_autoptr(GHashTable) expectedInfo = NULL; qemuMonitorChardevInfo info0 = { NULL, VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT }; qemuMonitorChardevInfo info1 = { (char *) "/dev/pts/21", VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED }; qemuMonitorChardevInfo info2 = { (char *) "/dev/pts/20", VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT }; @@ -1874,8 +1874,6 @@ testQemuMonitorJSONqemuMonitorJSONGetChardevInfo(const void *opaque) ret = 0; cleanup: - virHashFree(info); - virHashFree(expectedInfo); return ret; } -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/qemumonitorjsontest.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 89ace3bb33..37d878b0ad 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1811,7 +1811,6 @@ testQemuMonitorJSONqemuMonitorJSONGetChardevInfo(const void *opaque) { const testGenericData *data = opaque; virDomainXMLOption *xmlopt = data->xmlopt; - int ret = -1; g_autoptr(GHashTable) info = NULL; g_autoptr(GHashTable) expectedInfo = NULL; qemuMonitorChardevInfo info0 = { NULL, VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT }; @@ -1825,7 +1824,7 @@ testQemuMonitorJSONqemuMonitorJSONGetChardevInfo(const void *opaque) if (!(info = virHashNew(qemuMonitorChardevInfoFree)) || !(expectedInfo = virHashNew(NULL))) - goto cleanup; + return -1; if (virHashAddEntry(expectedInfo, "charserial1", &info1) < 0 || virHashAddEntry(expectedInfo, "charserial0", &info2) < 0 || @@ -1833,7 +1832,7 @@ testQemuMonitorJSONqemuMonitorJSONGetChardevInfo(const void *opaque) virHashAddEntry(expectedInfo, "charserial2", &info3) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Unable to create expectedInfo hash table"); - goto cleanup; + return -1; } if (qemuMonitorTestAddItem(test, "query-chardev", @@ -1860,21 +1859,19 @@ testQemuMonitorJSONqemuMonitorJSONGetChardevInfo(const void *opaque) " ]," " \"id\": \"libvirt-15\"" "}") < 0) - goto cleanup; + return -1; if (qemuMonitorJSONGetChardevInfo(qemuMonitorTestGetMonitor(test), info) < 0) - goto cleanup; + return -1; if (!virHashEqual(info, expectedInfo, testHashEqualChardevInfo)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Hashtable is different to the expected one"); - goto cleanup; + return -1; } - ret = 0; - cleanup: - return ret; + return 0; } -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/qemumonitorjsontest.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 37d878b0ad..0b321e8ed8 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1811,8 +1811,8 @@ testQemuMonitorJSONqemuMonitorJSONGetChardevInfo(const void *opaque) { const testGenericData *data = opaque; virDomainXMLOption *xmlopt = data->xmlopt; - g_autoptr(GHashTable) info = NULL; - g_autoptr(GHashTable) expectedInfo = NULL; + g_autoptr(GHashTable) info = virHashNew(qemuMonitorChardevInfoFree); + g_autoptr(GHashTable) expectedInfo = virHashNew(NULL); qemuMonitorChardevInfo info0 = { NULL, VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT }; qemuMonitorChardevInfo info1 = { (char *) "/dev/pts/21", VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED }; qemuMonitorChardevInfo info2 = { (char *) "/dev/pts/20", VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT }; @@ -1822,10 +1822,6 @@ testQemuMonitorJSONqemuMonitorJSONGetChardevInfo(const void *opaque) if (!(test = qemuMonitorTestNewSchema(xmlopt, data->schema))) return -1; - if (!(info = virHashNew(qemuMonitorChardevInfoFree)) || - !(expectedInfo = virHashNew(NULL))) - return -1; - if (virHashAddEntry(expectedInfo, "charserial1", &info1) < 0 || virHashAddEntry(expectedInfo, "charserial0", &info2) < 0 || virHashAddEntry(expectedInfo, "charmonitor", &info0) < 0 || -- 2.31.1
On Mon, Jul 12, 2021 at 11:34:07 +0200, Tim Wiederhake wrote:
"virHashNew" cannot return NULL, yet we check for NULL in various places.
See https://listman.redhat.com/archives/libvir-list/2021-July/msg00074.html.
Series: Reviewed-by: Peter Krempa <pkrempa@redhat.com>
participants (2)
-
Peter Krempa -
Tim Wiederhake