[libvirt PATCH 0/6] Use virURIFree less (glib chronicles)
Ján Tomko (6): Use g_auto for virURI almost everywhere tests: remove pointless label in testURIParse vz: refactor testURIParse vmx: use g_autofree in virVMXParseSerial qemu: remove cleanup label from qemuMigrationSrcGraphicsRelocate libxl: remove cleanup label from libxlDomainMigrationSrcPerform src/esx/esx_driver.c | 3 +-- src/libxl/libxl_migration.c | 17 ++++++----------- src/qemu/qemu_migration.c | 30 ++++++++++++------------------ src/vmx/vmx.c | 13 ++++--------- src/vz/vz_driver.c | 3 +-- src/vz/vz_sdk.c | 31 +++++++++++-------------------- tests/viruritest.c | 14 +++++--------- 7 files changed, 40 insertions(+), 71 deletions(-) -- 2.34.1
Convert all the cases where we can unconditionally free the virURI at the end of scope. In libxlDomainMigrationDstPrepare, uri is only filled if uri_in was present, so moving the virURIFree out of the condition is safe. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/esx/esx_driver.c | 3 +-- src/libxl/libxl_migration.c | 7 ++----- src/qemu/qemu_migration.c | 3 +-- src/vmx/vmx.c | 3 +-- src/vz/vz_driver.c | 3 +-- src/vz/vz_sdk.c | 3 +-- tests/viruritest.c | 3 +-- 7 files changed, 8 insertions(+), 17 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 79b991f36f..9dc5489411 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -3658,7 +3658,7 @@ esxDomainMigratePerform(virDomainPtr domain, { int result = -1; esxPrivate *priv = domain->conn->privateData; - virURI *parsedUri = NULL; + g_autoptr(virURI) parsedUri = NULL; char *saveptr; char *path_resourcePool; char *path_hostSystem; @@ -3779,7 +3779,6 @@ esxDomainMigratePerform(virDomainPtr domain, result = 0; cleanup: - virURIFree(parsedUri); esxVI_ObjectContent_Free(&virtualMachine); esxVI_Event_Free(&eventList); esxVI_ManagedObjectReference_Free(&task); diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index 0fd2b3206e..c5ec80139f 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -637,7 +637,7 @@ libxlDomainMigrationDstPrepare(virConnectPtr dconn, char *xmlout = NULL; unsigned short port; char portstr[100]; - virURI *uri = NULL; + g_autoptr(virURI) uri = NULL; virNetSocket **socks = NULL; size_t nsocks = 0; int nsocks_listen = 0; @@ -795,8 +795,6 @@ libxlDomainMigrationDstPrepare(virConnectPtr dconn, libxlMigrationCookieFree(mig); if (!uri_in) VIR_FREE(hostname); - else - virURIFree(uri); virObjectUnref(args); virDomainObjEndAPI(&vm); virObjectUnref(cfg); @@ -1183,7 +1181,7 @@ libxlDomainMigrationSrcPerform(libxlDriverPrivate *driver, char *hostname = NULL; unsigned short port = 0; char portstr[100]; - virURI *uri = NULL; + g_autoptr(virURI) uri = NULL; virNetSocket *sock; int sockfd = -1; int ret = -1; @@ -1233,7 +1231,6 @@ libxlDomainMigrationSrcPerform(libxlDriverPrivate *driver, cleanup: VIR_FORCE_CLOSE(sockfd); - virURIFree(uri); return ret; } diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 6e5b3839fb..3d7d5efda3 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2107,7 +2107,7 @@ qemuMigrationSrcGraphicsRelocate(virQEMUDriver *driver, int ret = -1; const char *listenAddress = NULL; virSocketAddr addr; - virURI *uri = NULL; + g_autoptr(virURI) uri = NULL; int type = -1; int port = -1; int tlsPort = -1; @@ -2192,7 +2192,6 @@ qemuMigrationSrcGraphicsRelocate(virQEMUDriver *driver, } cleanup: - virURIFree(uri); return ret; } diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index e24e9211aa..282e9562e0 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2903,7 +2903,7 @@ virVMXParseSerial(virVMXContext *ctx, virConf *conf, int port, char network_endPoint_name[48] = ""; char *network_endPoint = NULL; - virURI *parsedUri = NULL; + g_autoptr(virURI) parsedUri = NULL; if (def == NULL || *def != NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); @@ -3051,7 +3051,6 @@ virVMXParseSerial(virVMXContext *ctx, virConf *conf, int port, VIR_FREE(fileType); VIR_FREE(fileName); VIR_FREE(network_endPoint); - virURIFree(parsedUri); return result; } diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 82323cb1b2..017c084ede 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -3074,7 +3074,7 @@ vzDomainMigratePerformStep(virDomainObj *dom, { int ret = -1; struct vzDomObj *privdom = dom->privateData; - virURI *vzuri = NULL; + g_autoptr(virURI) vzuri = NULL; const char *miguri = NULL; const char *dname = NULL; vzMigrationCookie *mig = NULL; @@ -3117,7 +3117,6 @@ vzDomainMigratePerformStep(virDomainObj *dom, cleanup: if (job) vzDomainObjEndJob(dom); - virURIFree(vzuri); vzMigrationCookieFree(mig); return ret; diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 43d528820e..5ea3f29a8f 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1133,7 +1133,7 @@ prlsdkGetSerialInfo(PRL_HANDLE serialPort, virDomainChrDef *chr) char *friendlyName = NULL; PRL_SERIAL_PORT_SOCKET_OPERATION_MODE socket_mode; char *uristr = NULL; - virURI *uri = NULL; + g_autoptr(virURI) uri = NULL; int ret = -1; chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; @@ -1196,7 +1196,6 @@ prlsdkGetSerialInfo(PRL_HANDLE serialPort, virDomainChrDef *chr) cleanup: VIR_FREE(friendlyName); VIR_FREE(uristr); - virURIFree(uri); return ret; } diff --git a/tests/viruritest.c b/tests/viruritest.c index 94c8aa806d..7eb7c9cb67 100644 --- a/tests/viruritest.c +++ b/tests/viruritest.c @@ -46,7 +46,7 @@ struct URIParseData { static int testURIParse(const void *args) { int ret = -1; - virURI *uri = NULL; + g_autoptr(virURI) uri = NULL; const struct URIParseData *data = args; g_autofree char *uristr = NULL; size_t i; @@ -133,7 +133,6 @@ static int testURIParse(const void *args) ret = 0; cleanup: - virURIFree(uri); return ret; } -- 2.34.1
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/viruritest.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/viruritest.c b/tests/viruritest.c index 7eb7c9cb67..cd6ce57371 100644 --- a/tests/viruritest.c +++ b/tests/viruritest.c @@ -45,7 +45,6 @@ struct URIParseData { static int testURIParse(const void *args) { - int ret = -1; g_autoptr(virURI) uri = NULL; const struct URIParseData *data = args; g_autofree char *uristr = NULL; @@ -53,7 +52,7 @@ static int testURIParse(const void *args) bool fail = false; if (!(uri = virURIParse(data->uri))) - goto cleanup; + return -1; if (STRNEQ(uri->scheme, data->scheme)) { VIR_TEST_DEBUG("Expected scheme '%s', actual '%s'", @@ -120,7 +119,7 @@ static int testURIParse(const void *args) uri->query = virURIFormatParams(uri); if (!(uristr = virURIFormat(uri))) - goto cleanup; + return -1; if (STRNEQ(uristr, data->uri_out)) { VIR_TEST_DEBUG("URI did not roundtrip, expect '%s', actual '%s'", @@ -129,11 +128,9 @@ static int testURIParse(const void *args) } if (fail) - goto cleanup; + return -1; - ret = 0; - cleanup: - return ret; + return 0; } -- 2.34.1
Use g_autofree for the two strings still using manual cleanup and remove the pointless cleanup label. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vz/vz_sdk.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 5ea3f29a8f..ecf610d7db 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1130,26 +1130,25 @@ prlsdkGetSerialInfo(PRL_HANDLE serialPort, virDomainChrDef *chr) PRL_RESULT pret; PRL_UINT32 serialPortIndex; PRL_UINT32 emulatedType; - char *friendlyName = NULL; + g_autofree char *friendlyName = NULL; PRL_SERIAL_PORT_SOCKET_OPERATION_MODE socket_mode; - char *uristr = NULL; + g_autofree char *uristr = NULL; g_autoptr(virURI) uri = NULL; - int ret = -1; chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; pret = PrlVmDev_GetIndex(serialPort, &serialPortIndex); - prlsdkCheckRetGoto(pret, cleanup); + prlsdkCheckRetExit(pret, -1); chr->target.port = serialPortIndex; pret = PrlVmDev_GetEmulatedType(serialPort, &emulatedType); - prlsdkCheckRetGoto(pret, cleanup); + prlsdkCheckRetExit(pret, -1); if (!(friendlyName = prlsdkGetStringParamVar(PrlVmDev_GetFriendlyName, serialPort))) - goto cleanup; + return -1; pret = PrlVmDevSerial_GetSocketMode(serialPort, &socket_mode); - prlsdkCheckRetGoto(pret, cleanup); + prlsdkCheckRetExit(pret, -1); switch (emulatedType) { case PDT_USE_OUTPUT_FILE: @@ -1169,7 +1168,7 @@ prlsdkGetSerialInfo(PRL_HANDLE serialPort, virDomainChrDef *chr) chr->source->type = VIR_DOMAIN_CHR_TYPE_TCP; uristr = g_strdup_printf("tcp://%s", friendlyName); if (!(uri = virURIParse(uristr))) - goto cleanup; + return -1; chr->source->data.tcp.host = g_strdup(uri->server); chr->source->data.tcp.service = g_strdup_printf("%d", uri->port); chr->source->data.tcp.listen = socket_mode == PSP_SERIAL_SOCKET_SERVER; @@ -1178,7 +1177,7 @@ prlsdkGetSerialInfo(PRL_HANDLE serialPort, virDomainChrDef *chr) chr->source->type = VIR_DOMAIN_CHR_TYPE_UDP; uristr = g_strdup_printf("udp://%s", friendlyName); if (!(uri = virURIParse(uristr))) - goto cleanup; + return -1; chr->source->data.udp.bindHost = g_strdup(uri->server); chr->source->data.udp.bindService = g_strdup_printf("%d", uri->port); chr->source->data.udp.connectHost = g_strdup(uri->server); @@ -1187,17 +1186,10 @@ prlsdkGetSerialInfo(PRL_HANDLE serialPort, virDomainChrDef *chr) default: virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown serial type: %X"), emulatedType); - goto cleanup; - break; + return -1; } - ret = 0; - - cleanup: - VIR_FREE(friendlyName); - VIR_FREE(uristr); - - return ret; + return 0; } -- 2.34.1
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vmx/vmx.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 282e9562e0..43ddee5bb6 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2895,13 +2895,13 @@ virVMXParseSerial(virVMXContext *ctx, virConf *conf, int port, bool startConnected = false; char fileType_name[48] = ""; - char *fileType = NULL; + g_autofree char *fileType = NULL; char fileName_name[48] = ""; - char *fileName = NULL; + g_autofree char *fileName = NULL; char network_endPoint_name[48] = ""; - char *network_endPoint = NULL; + g_autofree char *network_endPoint = NULL; g_autoptr(virURI) parsedUri = NULL; @@ -3048,10 +3048,6 @@ virVMXParseSerial(virVMXContext *ctx, virConf *conf, int port, g_clear_pointer(def, virDomainChrDefFree); } - VIR_FREE(fileType); - VIR_FREE(fileName); - VIR_FREE(network_endPoint); - return result; } -- 2.34.1
Remove the label and use 'rc' instead of 'ret'. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_migration.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 3d7d5efda3..0ed0066e27 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2104,7 +2104,6 @@ qemuMigrationSrcGraphicsRelocate(virQEMUDriver *driver, const char *graphicsuri) { qemuDomainObjPrivate *priv = vm->privateData; - int ret = -1; const char *listenAddress = NULL; virSocketAddr addr; g_autoptr(virURI) uri = NULL; @@ -2112,12 +2111,13 @@ qemuMigrationSrcGraphicsRelocate(virQEMUDriver *driver, int port = -1; int tlsPort = -1; const char *tlsSubject = NULL; + int rc = -1; if (!cookie || (!cookie->graphics && !graphicsuri)) return 0; if (graphicsuri && !(uri = virURIParse(graphicsuri))) - goto cleanup; + return -1; if (cookie->graphics) { type = cookie->graphics->type; @@ -2140,7 +2140,7 @@ qemuMigrationSrcGraphicsRelocate(virQEMUDriver *driver, if ((type = virDomainGraphicsTypeFromString(uri->scheme)) < 0) { virReportError(VIR_ERR_INVALID_ARG, _("unknown graphics type %s"), uri->scheme); - goto cleanup; + return -1; } if (uri->server) @@ -2156,7 +2156,7 @@ qemuMigrationSrcGraphicsRelocate(virQEMUDriver *driver, virReportError(VIR_ERR_INVALID_ARG, _("invalid tlsPort number: %s"), param->value); - goto cleanup; + return -1; } } else if (STRCASEEQ(param->name, "tlsSubject")) { tlsSubject = param->value; @@ -2167,32 +2167,27 @@ qemuMigrationSrcGraphicsRelocate(virQEMUDriver *driver, /* QEMU doesn't support VNC relocation yet, so * skip it to avoid generating an error */ - if (type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - ret = 0; - goto cleanup; - } + if (type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) + return 0; /* Older libvirt sends port == 0 for listen type='none' graphics. It's * safe to ignore such requests since relocation to unknown port does * not make sense in general. */ - if (port <= 0 && tlsPort <= 0) { - ret = 0; - goto cleanup; - } + if (port <= 0 && tlsPort <= 0) + return 0; if (qemuDomainObjEnterMonitorAsync(driver, vm, VIR_ASYNC_JOB_MIGRATION_OUT) == 0) { qemuDomainJobPrivate *jobPriv = priv->job.privateData; - ret = qemuMonitorGraphicsRelocate(priv->mon, type, listenAddress, + rc = qemuMonitorGraphicsRelocate(priv->mon, type, listenAddress, port, tlsPort, tlsSubject); - jobPriv->spiceMigration = !ret; + jobPriv->spiceMigration = !rc; qemuDomainObjExitMonitor(vm); } - cleanup: - return ret; + return rc; } -- 2.34.1
On Wed, Jun 22, 2022 at 07:35:33AM +0200, Ján Tomko wrote:
Remove the label and use 'rc' instead of 'ret'.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_migration.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 3d7d5efda3..0ed0066e27 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c
[...]
@@ -2167,32 +2167,27 @@ qemuMigrationSrcGraphicsRelocate(virQEMUDriver *driver, /* QEMU doesn't support VNC relocation yet, so * skip it to avoid generating an error */ - if (type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - ret = 0; - goto cleanup; - } + if (type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) + return 0;
/* Older libvirt sends port == 0 for listen type='none' graphics. It's * safe to ignore such requests since relocation to unknown port does * not make sense in general. */ - if (port <= 0 && tlsPort <= 0) { - ret = 0; - goto cleanup; - } + if (port <= 0 && tlsPort <= 0) + return 0;
if (qemuDomainObjEnterMonitorAsync(driver, vm, VIR_ASYNC_JOB_MIGRATION_OUT) == 0) { qemuDomainJobPrivate *jobPriv = priv->job.privateData;
- ret = qemuMonitorGraphicsRelocate(priv->mon, type, listenAddress, + rc = qemuMonitorGraphicsRelocate(priv->mon, type, listenAddress, port, tlsPort, tlsSubject);
Wrong indentation.
- jobPriv->spiceMigration = !ret; + jobPriv->spiceMigration = !rc; qemuDomainObjExitMonitor(vm); }
- cleanup: - return ret; + return rc; }
-- 2.34.1
Use VIR_AUTOCLOSE for the remaining file descriptor that uses manual cleanup and remove the label. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/libxl/libxl_migration.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index c5ec80139f..800a6b0365 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -1183,13 +1183,13 @@ libxlDomainMigrationSrcPerform(libxlDriverPrivate *driver, char portstr[100]; g_autoptr(virURI) uri = NULL; virNetSocket *sock; - int sockfd = -1; + VIR_AUTOCLOSE sockfd = -1; int ret = -1; /* parse dst host:port from uri */ uri = virURIParse(uri_str); if (uri == NULL || uri->server == NULL || uri->port == 0) - goto cleanup; + return -1; hostname = uri->server; port = uri->port; @@ -1199,11 +1199,11 @@ libxlDomainMigrationSrcPerform(libxlDriverPrivate *driver, if (virNetSocketNewConnectTCP(hostname, portstr, AF_UNSPEC, &sock) < 0) - goto cleanup; + return -1; if (virNetSocketSetBlocking(sock, true) < 0) { virObjectUnref(sock); - goto cleanup; + return -1; } sockfd = virNetSocketDupFD(sock, true); @@ -1229,8 +1229,6 @@ libxlDomainMigrationSrcPerform(libxlDriverPrivate *driver, libxlDomainObjEndJob(driver, vm); } - cleanup: - VIR_FORCE_CLOSE(sockfd); return ret; } -- 2.34.1
On Wed, Jun 22, 2022 at 07:35:28AM +0200, Ján Tomko wrote:
Ján Tomko (6): Use g_auto for virURI almost everywhere tests: remove pointless label in testURIParse vz: refactor testURIParse vmx: use g_autofree in virVMXParseSerial qemu: remove cleanup label from qemuMigrationSrcGraphicsRelocate libxl: remove cleanup label from libxlDomainMigrationSrcPerform
With the indentation fixed Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
participants (2)
-
Ján Tomko -
Pavel Hrdina