[libvirt PATCH 0/5] virnetdevopenvswitch: fix overindented label (glib chronicles)

By removing it, of course. Patch 1/5 also has the potential to fix the build on FreeBSD, but it failed with an internal server error for me. Ján Tomko (5): build: only build virnetdevopenvswitchtest on Linux tests: virnetdevopenvswitch: use g_auto tests: virnetdevbandwidthtest: use g_auto tests: introduce testVirNetDevBandwidthParse tests: virnetdev*: remove unnecessary labels tests/meson.build | 6 ++- tests/virnetdevbandwidthtest.c | 56 ++++++++++++------------- tests/virnetdevopenvswitchtest.c | 70 ++++++++++++++------------------ 3 files changed, 61 insertions(+), 71 deletions(-) -- 2.31.1

Now that it uses virnetdevbandwidthmock which we only build on Linux. Fixes: eb55e8a897a8c9d68664daa213b1a76b0cb3c05d Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/meson.build | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/meson.build b/tests/meson.build index 3c73cbe3b5..dfbc2c01e2 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -350,6 +350,11 @@ if host_machine.system() == 'linux' { 'name': 'virscsitest' }, { 'name': 'virusbtest' }, ] + if conf.has('WITH_YAJL') + tests += [ + { 'name': 'virnetdevopenvswitchtest' }, + ] + endif endif if conf.has('WITH_BHYVE') @@ -543,7 +548,6 @@ if conf.has('WITH_YAJL') tests += [ { 'name': 'virjsontest' }, { 'name': 'virmacmaptest' }, - { 'name': 'virnetdevopenvswitchtest' }, ] endif -- 2.31.1

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/virnetdevopenvswitchtest.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/virnetdevopenvswitchtest.c b/tests/virnetdevopenvswitchtest.c index 8c5034a5c6..89ffc1e4b5 100644 --- a/tests/virnetdevopenvswitchtest.c +++ b/tests/virnetdevopenvswitchtest.c @@ -48,8 +48,8 @@ struct testClearQosStruct { #define PARSE(xml, var) \ do { \ int rc; \ - xmlDocPtr doc; \ - xmlXPathContextPtr ctxt = NULL; \ + g_autoptr(xmlDoc) doc = NULL; \ + g_autoptr(xmlXPathContext) ctxt = NULL; \ \ if (!xml) \ break; \ @@ -63,8 +63,6 @@ struct testClearQosStruct { NULL, \ ctxt->node, \ true); \ - xmlFreeDoc(doc); \ - xmlXPathFreeContext(ctxt); \ if (rc < 0) \ goto cleanup; \ } while (0) @@ -160,7 +158,7 @@ testVirNetDevOpenvswitchInterfaceSetQos(const void *data) const char *iface = info->iface; g_autoptr(virNetDevBandwidth) band = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - char *actual_cmd = NULL; + g_autofree char *actual_cmd = NULL; g_autoptr(virCommandDryRunToken) dryRunToken = virCommandDryRunTokenNew(); PARSE(info->band, band); @@ -187,7 +185,6 @@ testVirNetDevOpenvswitchInterfaceSetQos(const void *data) ret = 0; cleanup: - VIR_FREE(actual_cmd); return ret; } @@ -198,7 +195,7 @@ testVirNetDevOpenvswitchInterfaceClearQos(const void *data) int ret = -1; const struct testClearQosStruct *info = data; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - char *actual_cmd = NULL; + g_autofree char *actual_cmd = NULL; const char *iface = info->iface; const unsigned char *vmid = info->vmid; g_autoptr(virCommandDryRunToken) dryRunToken = virCommandDryRunTokenNew(); @@ -222,7 +219,6 @@ testVirNetDevOpenvswitchInterfaceClearQos(const void *data) ret = 0; cleanup: - VIR_FREE(actual_cmd); return ret; } -- 2.31.1

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/virnetdevbandwidthtest.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c index 83e7b2089f..197d936479 100644 --- a/tests/virnetdevbandwidthtest.c +++ b/tests/virnetdevbandwidthtest.c @@ -69,7 +69,7 @@ testVirNetDevBandwidthSet(const void *data) const char *iface = info->iface; g_autoptr(virNetDevBandwidth) band = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - char *actual_cmd = NULL; + g_autofree char *actual_cmd = NULL; g_autoptr(virCommandDryRunToken) dryRunToken = virCommandDryRunTokenNew(); PARSE(info->band, band); @@ -96,7 +96,6 @@ testVirNetDevBandwidthSet(const void *data) ret = 0; cleanup: - VIR_FREE(actual_cmd); return ret; } -- 2.31.1

The 'PARSE' macro does not use '#' or '##' directives, or anything from outside of the macro other than the cleanup label. Turn it into a function. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/virnetdevbandwidthtest.c | 44 ++++++++++++++++---------------- tests/virnetdevopenvswitchtest.c | 44 ++++++++++++++++---------------- 2 files changed, 44 insertions(+), 44 deletions(-) diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c index 197d936479..a17737ec40 100644 --- a/tests/virnetdevbandwidthtest.c +++ b/tests/virnetdevbandwidthtest.c @@ -39,27 +39,26 @@ struct testSetStruct { const bool hierarchical_class; }; -#define PARSE(xml, var) \ - do { \ - int rc; \ - g_autoptr(xmlDoc) doc = NULL; \ - g_autoptr(xmlXPathContext) ctxt = NULL; \ - \ - if (!xml) \ - break; \ - \ - if (!(doc = virXMLParseStringCtxt((xml), \ - "bandwidth definition", \ - &ctxt))) \ - goto cleanup; \ - \ - rc = virNetDevBandwidthParse(&(var), \ - NULL, \ - ctxt->node, \ - true); \ - if (rc < 0) \ - goto cleanup; \ - } while (0) +static int +testVirNetDevBandwidthParse(virNetDevBandwidth **var, + const char *xml) +{ + g_autoptr(xmlDoc) doc = NULL; + g_autoptr(xmlXPathContext) ctxt = NULL; + + if (!xml) + return 0; + + if (!(doc = virXMLParseStringCtxt((xml), + "bandwidth definition", + &ctxt))) + return -1; + + return virNetDevBandwidthParse(var, + NULL, + ctxt->node, + true); +} static int testVirNetDevBandwidthSet(const void *data) @@ -72,7 +71,8 @@ testVirNetDevBandwidthSet(const void *data) g_autofree char *actual_cmd = NULL; g_autoptr(virCommandDryRunToken) dryRunToken = virCommandDryRunTokenNew(); - PARSE(info->band, band); + if (testVirNetDevBandwidthParse(&band, info->band) < 0) + return -1; if (!iface) iface = "eth0"; diff --git a/tests/virnetdevopenvswitchtest.c b/tests/virnetdevopenvswitchtest.c index 89ffc1e4b5..1026c890cd 100644 --- a/tests/virnetdevopenvswitchtest.c +++ b/tests/virnetdevopenvswitchtest.c @@ -45,27 +45,26 @@ struct testClearQosStruct { const unsigned char *vmid; }; -#define PARSE(xml, var) \ - do { \ - int rc; \ - g_autoptr(xmlDoc) doc = NULL; \ - g_autoptr(xmlXPathContext) ctxt = NULL; \ - \ - if (!xml) \ - break; \ - \ - if (!(doc = virXMLParseStringCtxt((xml), \ - "bandwidth definition", \ - &ctxt))) \ - goto cleanup; \ - \ - rc = virNetDevBandwidthParse(&(var), \ - NULL, \ - ctxt->node, \ - true); \ - if (rc < 0) \ - goto cleanup; \ - } while (0) +static int +testVirNetDevBandwidthParse(virNetDevBandwidth **var, + const char *xml) +{ + g_autoptr(xmlDoc) doc = NULL; + g_autoptr(xmlXPathContext) ctxt = NULL; + + if (!xml) + return 0; + + if (!(doc = virXMLParseStringCtxt((xml), + "bandwidth definition", + &ctxt))) + return -1; + + return virNetDevBandwidthParse(var, + NULL, + ctxt->node, + true); +} static const unsigned char vm_id[VIR_UUID_BUFLEN] = "fakeuuid"; @@ -161,7 +160,8 @@ testVirNetDevOpenvswitchInterfaceSetQos(const void *data) g_autofree char *actual_cmd = NULL; g_autoptr(virCommandDryRunToken) dryRunToken = virCommandDryRunTokenNew(); - PARSE(info->band, band); + if (testVirNetDevBandwidthParse(&band, info->band) < 0) + return -1; if (!iface) iface = "tap-fake"; -- 2.31.1

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/virnetdevbandwidthtest.c | 9 +++------ tests/virnetdevopenvswitchtest.c | 18 ++++++------------ 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c index a17737ec40..eb4f47dfce 100644 --- a/tests/virnetdevbandwidthtest.c +++ b/tests/virnetdevbandwidthtest.c @@ -63,7 +63,6 @@ testVirNetDevBandwidthParse(virNetDevBandwidth **var, static int testVirNetDevBandwidthSet(const void *data) { - int ret = -1; const struct testSetStruct *info = data; const char *iface = info->iface; g_autoptr(virNetDevBandwidth) band = NULL; @@ -80,7 +79,7 @@ testVirNetDevBandwidthSet(const void *data) virCommandSetDryRun(dryRunToken, &buf, false, false, NULL, NULL); if (virNetDevBandwidthSet(iface, band, info->hierarchical_class, true) < 0) - goto cleanup; + return -1; if (!(actual_cmd = virBufferContentAndReset(&buf))) { /* This is interesting, no command has been executed. @@ -91,12 +90,10 @@ testVirNetDevBandwidthSet(const void *data) virTestDifference(stderr, NULLSTR(info->exp_cmd), NULLSTR(actual_cmd)); - goto cleanup; + return -1; } - ret = 0; - cleanup: - return ret; + return 0; } static int diff --git a/tests/virnetdevopenvswitchtest.c b/tests/virnetdevopenvswitchtest.c index 1026c890cd..2a20ba82d0 100644 --- a/tests/virnetdevopenvswitchtest.c +++ b/tests/virnetdevopenvswitchtest.c @@ -152,7 +152,6 @@ testNameEscape(const void *opaque) static int testVirNetDevOpenvswitchInterfaceSetQos(const void *data) { - int ret = -1; const struct testSetQosStruct *info = data; const char *iface = info->iface; g_autoptr(virNetDevBandwidth) band = NULL; @@ -169,7 +168,7 @@ testVirNetDevOpenvswitchInterfaceSetQos(const void *data) virCommandSetDryRun(dryRunToken, &buf, false, false, NULL, NULL); if (virNetDevOpenvswitchInterfaceSetQos(iface, band, vm_id, true) < 0) - goto cleanup; + return -1; if (!(actual_cmd = virBufferContentAndReset(&buf))) { /* This is interesting, no command has been executed. @@ -180,19 +179,16 @@ testVirNetDevOpenvswitchInterfaceSetQos(const void *data) virTestDifference(stderr, NULLSTR(info->exp_cmd), NULLSTR(actual_cmd)); - goto cleanup; + return -1; } - ret = 0; - cleanup: - return ret; + return 0; } static int testVirNetDevOpenvswitchInterfaceClearQos(const void *data) { - int ret = -1; const struct testClearQosStruct *info = data; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; g_autofree char *actual_cmd = NULL; @@ -203,7 +199,7 @@ testVirNetDevOpenvswitchInterfaceClearQos(const void *data) virCommandSetDryRun(dryRunToken, &buf, false, false, NULL, NULL); if (virNetDevOpenvswitchInterfaceClearQos(iface, vmid) < 0) - goto cleanup; + return -1; if (!(actual_cmd = virBufferContentAndReset(&buf))) { /* This is interesting, no command has been executed. @@ -214,12 +210,10 @@ testVirNetDevOpenvswitchInterfaceClearQos(const void *data) virTestDifference(stderr, NULLSTR(info->exp_cmd), NULLSTR(actual_cmd)); - goto cleanup; + return -1; } - ret = 0; - cleanup: - return ret; + return 0; } static int -- 2.31.1

On 8/20/21 3:15 PM, Ján Tomko wrote:
By removing it, of course.
Patch 1/5 also has the potential to fix the build on FreeBSD, but it failed with an internal server error for me.
Ján Tomko (5): build: only build virnetdevopenvswitchtest on Linux tests: virnetdevopenvswitch: use g_auto tests: virnetdevbandwidthtest: use g_auto tests: introduce testVirNetDevBandwidthParse tests: virnetdev*: remove unnecessary labels
tests/meson.build | 6 ++- tests/virnetdevbandwidthtest.c | 56 ++++++++++++------------- tests/virnetdevopenvswitchtest.c | 70 ++++++++++++++------------------ 3 files changed, 61 insertions(+), 71 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Ján Tomko
-
Michal Prívozník