[PATCH] libxl: Use g_autofree for char* where easily possible

Jim Fehlig posted 1 patch 3 years, 2 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20210217014022.30958-1-jfehlig@suse.com
src/libxl/libxl_capabilities.c |  3 +--
src/libxl/libxl_domain.c       | 17 +++++------------
src/libxl/libxl_driver.c       | 32 +++++++++++---------------------
src/libxl/libxl_logger.c       | 21 ++++++---------------
src/libxl/libxl_migration.c    | 15 +++++----------
src/libxl/xen_common.c         | 17 ++++++-----------
src/libxl/xen_xl.c             |  6 ++----
7 files changed, 36 insertions(+), 75 deletions(-)
[PATCH] libxl: Use g_autofree for char* where easily possible
Posted by Jim Fehlig 3 years, 2 months ago
All of these strings are allocated once, freed once, and are never
returned out of the function where they are declared.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 src/libxl/libxl_capabilities.c |  3 +--
 src/libxl/libxl_domain.c       | 17 +++++------------
 src/libxl/libxl_driver.c       | 32 +++++++++++---------------------
 src/libxl/libxl_logger.c       | 21 ++++++---------------
 src/libxl/libxl_migration.c    | 15 +++++----------
 src/libxl/xen_common.c         | 17 ++++++-----------
 src/libxl/xen_xl.c             |  6 ++----
 7 files changed, 36 insertions(+), 75 deletions(-)

diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
index 2f3ed5bc83..10e5d46cdd 100644
--- a/src/libxl/libxl_capabilities.c
+++ b/src/libxl/libxl_capabilities.c
@@ -769,7 +769,7 @@ libxlDomainGetEmulatorType(const virDomainDef *def)
 {
     int ret = LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
     virCommandPtr cmd = NULL;
-    char *output = NULL;
+    g_autofree char *output = NULL;
 
     if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
         if (def->emulator) {
@@ -790,7 +790,6 @@ libxlDomainGetEmulatorType(const virDomainDef *def)
     }
 
  cleanup:
-    VIR_FREE(output);
     virCommandFree(cmd);
     return ret;
 }
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 3bcd369d12..365ac36f2b 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -748,7 +748,7 @@ libxlDomainSaveImageOpen(libxlDriverPrivatePtr driver,
     int fd;
     virDomainDefPtr def = NULL;
     libxlSavefileHeader hdr;
-    char *xml = NULL;
+    g_autofree char *xml = NULL;
 
     if ((fd = virFileOpenAs(from, O_RDONLY, 0, -1, -1, 0)) < 0) {
         virReportSystemError(-fd,
@@ -792,15 +792,12 @@ libxlDomainSaveImageOpen(libxlDriverPrivatePtr driver,
                                         VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)))
         goto error;
 
-    VIR_FREE(xml);
-
     *ret_def = def;
     *ret_hdr = hdr;
 
     return fd;
 
  error:
-    VIR_FREE(xml);
     virDomainDefFree(def);
     VIR_FORCE_CLOSE(fd);
     return -1;
@@ -951,7 +948,7 @@ libxlDomainAutoCoreDump(libxlDriverPrivatePtr driver,
     g_autoptr(libxlDriverConfig) cfg = libxlDriverConfigGet(driver);
     g_autoptr(GDateTime) now = g_date_time_new_now_local();
     g_autofree char *nowstr = NULL;
-    char *dumpfile = NULL;
+    g_autofree char *dumpfile = NULL;
 
     nowstr = g_date_time_format(now, "%Y-%m-%d-%H:%M:%S");
 
@@ -963,7 +960,6 @@ libxlDomainAutoCoreDump(libxlDriverPrivatePtr driver,
     libxl_domain_core_dump(cfg->ctx, vm->def->id, dumpfile, NULL);
     virObjectLock(vm);
 
-    VIR_FREE(dumpfile);
     return 0;
 }
 
@@ -1262,8 +1258,8 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
     libxlSavefileHeader hdr;
     int ret = -1;
     uint32_t domid = 0;
-    char *dom_xml = NULL;
-    char *managed_save_path = NULL;
+    g_autofree char *dom_xml = NULL;
+    g_autofree char *managed_save_path = NULL;
     int managed_save_fd = -1;
     libxlDomainObjPrivatePtr priv = vm->privateData;
     g_autoptr(libxlDriverConfig) cfg = libxlDriverConfigGet(driver);
@@ -1271,7 +1267,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
     libxl_asyncprogress_how aop_console_how;
     libxl_domain_restore_params params;
     unsigned int hostdev_flags = VIR_HOSTDEV_SP_PCI;
-    char *config_json = NULL;
+    g_autofree char *config_json = NULL;
 
 #ifdef LIBXL_HAVE_PVUSB
     hostdev_flags |= VIR_HOSTDEV_SP_USB;
@@ -1513,9 +1509,6 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
 
  cleanup:
     libxl_domain_config_dispose(&d_config);
-    VIR_FREE(config_json);
-    VIR_FREE(dom_xml);
-    VIR_FREE(managed_save_path);
     virDomainDefFree(def);
     VIR_FORCE_CLOSE(managed_save_fd);
     return ret;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 2e002b2930..fcb203ab98 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -533,7 +533,7 @@ libxlDriverShouldLoad(bool privileged)
 
     if (virFileExists(HYPERVISOR_CAPABILITIES)) {
         int status;
-        char *output = NULL;
+        g_autofree char *output = NULL;
         /*
          * Don't load if not running on a Xen control domain (dom0). It is not
          * sufficient to check for the file to exist as any guest can mount
@@ -542,7 +542,6 @@ libxlDriverShouldLoad(bool privileged)
         status = virFileReadAll(HYPERVISOR_CAPABILITIES, 10, &output);
         if (status >= 0)
             status = strncmp(output, "control_d", 9);
-        VIR_FREE(output);
         if (status) {
             VIR_INFO("No Xen capabilities detected, probably not running "
                      "in a Xen Dom0.  Disabling libxenlight driver");
@@ -1819,7 +1818,7 @@ libxlDoDomainSave(libxlDriverPrivatePtr driver,
     libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
     libxlSavefileHeader hdr;
     virObjectEventPtr event = NULL;
-    char *xml = NULL;
+    g_autofree char *xml = NULL;
     uint32_t xml_len;
     int fd = -1;
     int ret = -1;
@@ -1889,7 +1888,6 @@ libxlDoDomainSave(libxlDriverPrivatePtr driver,
     ret = 0;
 
  cleanup:
-    VIR_FREE(xml);
     if (VIR_CLOSE(fd) < 0)
         virReportSystemError(errno, "%s", _("cannot close file"));
     virObjectEventStateQueue(driver->domainEventState, event);
@@ -2117,7 +2115,7 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int flags)
 {
     libxlDriverPrivatePtr driver = dom->conn->privateData;
     virDomainObjPtr vm = NULL;
-    char *name = NULL;
+    g_autofree char *name = NULL;
     int ret = -1;
 
     virCheckFlags(0, -1);
@@ -2160,7 +2158,6 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int flags)
 
  cleanup:
     virDomainObjEndAPI(&vm);
-    VIR_FREE(name);
     return ret;
 }
 
@@ -2213,7 +2210,7 @@ libxlDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
     libxlDriverPrivatePtr driver = dom->conn->privateData;
     virDomainObjPtr vm = NULL;
     int ret = -1;
-    char *name = NULL;
+    g_autofree char *name = NULL;
 
     virCheckFlags(0, -1);
 
@@ -2231,7 +2228,6 @@ libxlDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
     vm->hasManagedSave = false;
 
  cleanup:
-    VIR_FREE(name);
     virDomainObjEndAPI(&vm);
     return ret;
 }
@@ -2917,7 +2913,7 @@ libxlDomainUndefineFlags(virDomainPtr dom,
     libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
     virDomainObjPtr vm;
     virObjectEventPtr event = NULL;
-    char *name = NULL;
+    g_autofree char *name = NULL;
     int ret = -1;
 
     virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE, -1);
@@ -2967,7 +2963,6 @@ libxlDomainUndefineFlags(virDomainPtr dom,
     ret = 0;
 
  cleanup:
-    VIR_FREE(name);
     virDomainObjEndAPI(&vm);
     virObjectEventStateQueue(driver->domainEventState, event);
     virObjectUnref(cfg);
@@ -4518,7 +4513,8 @@ libxlDomainSetAutostart(virDomainPtr dom, int autostart)
     libxlDriverPrivatePtr driver = dom->conn->privateData;
     libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
     virDomainObjPtr vm;
-    char *configFile = NULL, *autostartLink = NULL;
+    g_autofree char *configFile = NULL;
+    g_autofree char *autostartLink = NULL;
     int ret = -1;
 
     if (!(vm = libxlDomObjFromDomain(dom)))
@@ -4577,8 +4573,6 @@ libxlDomainSetAutostart(virDomainPtr dom, int autostart)
     libxlDomainObjEndJob(driver, vm);
 
  cleanup:
-    VIR_FREE(configFile);
-    VIR_FREE(autostartLink);
     virDomainObjEndAPI(&vm);
     virObjectUnref(cfg);
     return ret;
@@ -4882,7 +4876,7 @@ libxlDomainGetNumaParameters(virDomainPtr dom,
     virDomainObjPtr vm;
     libxl_bitmap nodemap;
     virBitmapPtr nodes = NULL;
-    char *nodeset = NULL;
+    g_autofree char *nodeset = NULL;
     int rc, ret = -1;
     size_t i, j;
 
@@ -4981,7 +4975,6 @@ libxlDomainGetNumaParameters(virDomainPtr dom,
     ret = 0;
 
  cleanup:
-    VIR_FREE(nodeset);
     virBitmapFree(nodes);
     libxl_bitmap_dispose(&nodemap);
     virDomainObjEndAPI(&vm);
@@ -5783,7 +5776,7 @@ libxlNodeDeviceDetachFlags(virNodeDevicePtr dev,
     virPCIDeviceAddress devAddr;
     int ret = -1;
     virNodeDeviceDefPtr def = NULL;
-    char *xml = NULL;
+    g_autofree char *xml = NULL;
     libxlDriverPrivatePtr driver = dev->conn->privateData;
     virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
     virConnectPtr nodeconn = NULL;
@@ -5839,7 +5832,6 @@ libxlNodeDeviceDetachFlags(virNodeDevicePtr dev,
     virNodeDeviceDefFree(def);
     virObjectUnref(nodedev);
     virObjectUnref(nodeconn);
-    VIR_FREE(xml);
     return ret;
 }
 
@@ -5856,7 +5848,7 @@ libxlNodeDeviceReAttach(virNodeDevicePtr dev)
     virPCIDeviceAddress devAddr;
     int ret = -1;
     virNodeDeviceDefPtr def = NULL;
-    char *xml = NULL;
+    g_autofree char *xml = NULL;
     libxlDriverPrivatePtr driver = dev->conn->privateData;
     virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
     virConnectPtr nodeconn = NULL;
@@ -5903,7 +5895,6 @@ libxlNodeDeviceReAttach(virNodeDevicePtr dev)
     virNodeDeviceDefFree(def);
     virObjectUnref(nodedev);
     virObjectUnref(nodeconn);
-    VIR_FREE(xml);
     return ret;
 }
 
@@ -5914,7 +5905,7 @@ libxlNodeDeviceReset(virNodeDevicePtr dev)
     virPCIDeviceAddress devAddr;
     int ret = -1;
     virNodeDeviceDefPtr def = NULL;
-    char *xml = NULL;
+    g_autofree char *xml = NULL;
     libxlDriverPrivatePtr driver = dev->conn->privateData;
     virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
     virConnectPtr nodeconn = NULL;
@@ -5961,7 +5952,6 @@ libxlNodeDeviceReset(virNodeDevicePtr dev)
     virNodeDeviceDefFree(def);
     virObjectUnref(nodedev);
     virObjectUnref(nodeconn);
-    VIR_FREE(xml);
     return ret;
 }
 
diff --git a/src/libxl/libxl_logger.c b/src/libxl/libxl_logger.c
index 93a9c76b25..39e59d9231 100644
--- a/src/libxl/libxl_logger.c
+++ b/src/libxl/libxl_logger.c
@@ -65,7 +65,7 @@ libvirt_vmessage(xentoollog_logger *logger_in,
     xentoollog_logger_libvirt *lg = (xentoollog_logger_libvirt *)logger_in;
     FILE *logFile = lg->defaultLogFile;
     char timestamp[VIR_TIME_STRING_BUFLEN];
-    char *message = NULL;
+    g_autofree char *message = NULL;
     char *start, *end;
 
     VIR_DEBUG("libvirt_vmessage: context='%s' format='%s'", context, format);
@@ -107,8 +107,6 @@ libvirt_vmessage(xentoollog_logger *logger_in,
 
     fputc('\n', logFile);
     fflush(logFile);
-
-    VIR_FREE(message);
 }
 
 static void
@@ -135,7 +133,7 @@ libxlLoggerNew(const char *logDir, virLogPriority minLevel)
 {
     xentoollog_logger_libvirt logger;
     libxlLoggerPtr logger_out = NULL;
-    char *path = NULL;
+    g_autofree char *path = NULL;
 
     switch (minLevel) {
     case VIR_LOG_DEBUG:
@@ -164,7 +162,6 @@ libxlLoggerNew(const char *logDir, virLogPriority minLevel)
     logger_out = XTL_NEW_LOGGER(libvirt, logger);
 
  cleanup:
-    VIR_FREE(path);
     return logger_out;
 
  error:
@@ -188,9 +185,9 @@ libxlLoggerOpenFile(libxlLoggerPtr logger,
                     const char *name,
                     const char *domain_config)
 {
-    char *path = NULL;
+    g_autofree char *path = NULL;
     FILE *logFile = NULL;
-    char *domidstr = NULL;
+    g_autofree char *domidstr = NULL;
 
     path = g_strdup_printf("%s/%s.log", logger->logDir, name);
     domidstr = g_strdup_printf("%d", id);
@@ -198,7 +195,7 @@ libxlLoggerOpenFile(libxlLoggerPtr logger,
     if (!(logFile = fopen(path, "a"))) {
         VIR_WARN("Failed to open log file %s: %s",
                  path, g_strerror(errno));
-        goto cleanup;
+        return;
     }
     ignore_value(virHashAddEntry(logger->files, domidstr, logFile));
 
@@ -207,19 +204,13 @@ libxlLoggerOpenFile(libxlLoggerPtr logger,
         fprintf(logFile, "Domain start: %s\n", domain_config);
         fflush(logFile);
     }
-
- cleanup:
-    VIR_FREE(path);
-    VIR_FREE(domidstr);
 }
 
 void
 libxlLoggerCloseFile(libxlLoggerPtr logger, int id)
 {
-    char *domidstr = NULL;
+    g_autofree char *domidstr = NULL;
     domidstr = g_strdup_printf("%d", id);
 
     ignore_value(virHashRemoveEntry(logger->files, domidstr));
-
-    VIR_FREE(domidstr);
 }
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index c56fdd98ab..631f67930d 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -150,7 +150,7 @@ libxlMigrationEatCookie(const char *cookiein,
     libxlMigrationCookiePtr mig = NULL;
     xmlDocPtr doc = NULL;
     xmlXPathContextPtr ctxt = NULL;
-    char *uuidstr = NULL;
+    g_autofree char *uuidstr = NULL;
     int ret = -1;
 
     /*
@@ -216,7 +216,6 @@ libxlMigrationEatCookie(const char *cookiein,
     libxlMigrationCookieFree(mig);
 
  cleanup:
-    VIR_FREE(uuidstr);
     xmlXPathFreeContext(ctxt);
     xmlFreeDoc(doc);
     return ret;
@@ -840,7 +839,7 @@ struct _libxlTunnelMigrationThread {
 static void libxlTunnel3MigrationSrcFunc(void *arg)
 {
     libxlTunnelMigrationThread *data = (libxlTunnelMigrationThread *)arg;
-    char *buffer = NULL;
+    g_autofree char *buffer = NULL;
     struct pollfd fds[1];
     int timeout = -1;
 
@@ -858,7 +857,7 @@ static void libxlTunnel3MigrationSrcFunc(void *arg)
                 continue;
             virReportError(errno, "%s",
                            _("poll failed in libxlTunnel3MigrationSrcFunc"));
-            goto cleanup;
+            return;
         }
 
         if (ret == 0) {
@@ -874,13 +873,13 @@ static void libxlTunnel3MigrationSrcFunc(void *arg)
                 /* Write to dest stream */
                 if (virStreamSend(data->st, buffer, nbytes) < 0) {
                     virStreamAbort(data->st);
-                    goto cleanup;
+                    return;
                 }
             } else if (nbytes < 0) {
                 virReportError(errno, "%s",
                                _("tunnelled migration failed to read from xen side"));
                 virStreamAbort(data->st);
-                goto cleanup;
+                return;
             } else {
                 /* EOF; transferred all data */
                 break;
@@ -889,10 +888,6 @@ static void libxlTunnel3MigrationSrcFunc(void *arg)
     }
 
     ignore_value(virStreamFinish(data->st));
-
- cleanup:
-    VIR_FREE(buffer);
-
     return;
 }
 
diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c
index f852032d8a..781483f496 100644
--- a/src/libxl/xen_common.c
+++ b/src/libxl/xen_common.c
@@ -2102,29 +2102,24 @@ xenFormatCharDev(virConfPtr conf, virDomainDefPtr def,
 static int
 xenFormatCPUAllocation(virConfPtr conf, virDomainDefPtr def)
 {
-    int ret = -1;
-    char *cpus = NULL;
+    g_autofree char *cpus = NULL;
 
     if (virDomainDefGetVcpus(def) < virDomainDefGetVcpusMax(def) &&
         xenConfigSetInt(conf, "maxvcpus", virDomainDefGetVcpusMax(def)) < 0)
-        goto cleanup;
+        return -1;
     if (xenConfigSetInt(conf, "vcpus", virDomainDefGetVcpus(def)) < 0)
-        goto cleanup;
+        return -1;
 
     if ((def->cpumask != NULL) &&
         ((cpus = virBitmapFormat(def->cpumask)) == NULL)) {
-        goto cleanup;
+        return -1;
     }
 
     if (cpus &&
         xenConfigSetString(conf, "cpus", cpus) < 0)
-        goto cleanup;
+        return -1;
 
-    ret = 0;
-
- cleanup:
-    VIR_FREE(cpus);
-    return ret;
+    return 0;
 }
 
 
diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c
index 494b5f3dd2..4113be8cd1 100644
--- a/src/libxl/xen_xl.c
+++ b/src/libxl/xen_xl.c
@@ -1373,7 +1373,7 @@ static int
 xenFormatXLCPUID(virConfPtr conf, virDomainDefPtr def)
 {
     char **cpuid_pairs = NULL;
-    char *cpuid_string = NULL;
+    g_autofree char *cpuid_string = NULL;
     size_t i, j;
     int ret = -1;
 
@@ -1431,7 +1431,6 @@ xenFormatXLCPUID(virConfPtr conf, virDomainDefPtr def)
 
  cleanup:
     g_strfreev(cpuid_pairs);
-    VIR_FREE(cpuid_string);
     return ret;
 }
 
@@ -1686,7 +1685,7 @@ xenFormatXLDisk(virConfValuePtr list, virDomainDiskDefPtr disk)
     virConfValuePtr val, tmp;
     int format = virDomainDiskGetFormat(disk);
     const char *driver = virDomainDiskGetDriver(disk);
-    char *target = NULL;
+    g_autofree char *target = NULL;
     int ret = -1;
 
     /* format */
@@ -1772,7 +1771,6 @@ xenFormatXLDisk(virConfValuePtr list, virDomainDiskDefPtr disk)
     ret = 0;
 
  cleanup:
-    VIR_FREE(target);
     return ret;
 }
 
-- 
2.29.2


Re: [PATCH] libxl: Use g_autofree for char* where easily possible
Posted by Jim Fehlig 3 years, 2 months ago
On 2/16/21 6:40 PM, Jim Fehlig wrote:
> All of these strings are allocated once, freed once, and are never
> returned out of the function where they are declared.
> 
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
>   src/libxl/libxl_capabilities.c |  3 +--
>   src/libxl/libxl_domain.c       | 17 +++++------------
>   src/libxl/libxl_driver.c       | 32 +++++++++++---------------------
>   src/libxl/libxl_logger.c       | 21 ++++++---------------
>   src/libxl/libxl_migration.c    | 15 +++++----------
>   src/libxl/xen_common.c         | 17 ++++++-----------
>   src/libxl/xen_xl.c             |  6 ++----
>   7 files changed, 36 insertions(+), 75 deletions(-)

[snip]

> @@ -135,7 +133,7 @@ libxlLoggerNew(const char *logDir, virLogPriority minLevel)
>   {
>       xentoollog_logger_libvirt logger;
>       libxlLoggerPtr logger_out = NULL;
> -    char *path = NULL;
> +    g_autofree char *path = NULL;
>   
>       switch (minLevel) {
>       case VIR_LOG_DEBUG:
> @@ -164,7 +162,6 @@ libxlLoggerNew(const char *logDir, virLogPriority minLevel)
>       logger_out = XTL_NEW_LOGGER(libvirt, logger);
>   
>    cleanup:
> -    VIR_FREE(path);
>       return logger_out;
>   
>    error:

With nothing more to cleanup, this function could be reduced a bit more. I've 
squashed the following into my local branch

diff --git a/src/libxl/libxl_logger.c b/src/libxl/libxl_logger.c
index 39e59d9231..1da3357c6d 100644
--- a/src/libxl/libxl_logger.c
+++ b/src/libxl/libxl_logger.c
@@ -132,7 +132,6 @@ libxlLoggerPtr
  libxlLoggerNew(const char *logDir, virLogPriority minLevel)
  {
      xentoollog_logger_libvirt logger;
-    libxlLoggerPtr logger_out = NULL;
      g_autofree char *path = NULL;

      switch (minLevel) {
@@ -156,17 +155,12 @@ libxlLoggerNew(const char *logDir, virLogPriority minLevel)

      path = g_strdup_printf("%s/libxl-driver.log", logDir);

-    if ((logger.defaultLogFile = fopen(path, "a")) == NULL)
-        goto error;
+    if ((logger.defaultLogFile = fopen(path, "a")) == NULL) {
+        virHashFree(logger.files);
+        return NULL;
+    }

-    logger_out = XTL_NEW_LOGGER(libvirt, logger);
-
- cleanup:
-    return logger_out;
-
- error:
-    virHashFree(logger.files);
-    goto cleanup;
+    return XTL_NEW_LOGGER(libvirt, logger);
  }

  void

Re: [PATCH] libxl: Use g_autofree for char* where easily possible
Posted by Ján Tomko 3 years, 2 months ago
On a Tuesday in 2021, Jim Fehlig wrote:
>All of these strings are allocated once, freed once, and are never
>returned out of the function where they are declared.
>
>Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>---
> src/libxl/libxl_capabilities.c |  3 +--
> src/libxl/libxl_domain.c       | 17 +++++------------
> src/libxl/libxl_driver.c       | 32 +++++++++++---------------------
> src/libxl/libxl_logger.c       | 21 ++++++---------------
> src/libxl/libxl_migration.c    | 15 +++++----------
> src/libxl/xen_common.c         | 17 ++++++-----------
> src/libxl/xen_xl.c             |  6 ++----
> 7 files changed, 36 insertions(+), 75 deletions(-)
>
>@@ -1262,8 +1258,8 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
>     libxlSavefileHeader hdr;
>     int ret = -1;
>     uint32_t domid = 0;
>-    char *dom_xml = NULL;
>-    char *managed_save_path = NULL;
>+    g_autofree char *dom_xml = NULL;
>+    g_autofree char *managed_save_path = NULL;
>     int managed_save_fd = -1;
>     libxlDomainObjPrivatePtr priv = vm->privateData;
>     g_autoptr(libxlDriverConfig) cfg = libxlDriverConfigGet(driver);
>@@ -1271,7 +1267,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
>     libxl_asyncprogress_how aop_console_how;
>     libxl_domain_restore_params params;
>     unsigned int hostdev_flags = VIR_HOSTDEV_SP_PCI;
>-    char *config_json = NULL;
>+    g_autofree char *config_json = NULL;
>
> #ifdef LIBXL_HAVE_PVUSB
>     hostdev_flags |= VIR_HOSTDEV_SP_USB;

There is one more free that can be removed in the if (restore_fd < 0) block:

              vm->hasManagedSave = false;
          }
          VIR_FREE(managed_save_path);
      }

      if (virDomainObjSetDefTransient(driver->xmlopt, vm, NULL) < 0)
          goto cleanup;


>@@ -1513,9 +1509,6 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
>
>  cleanup:
>     libxl_domain_config_dispose(&d_config);
>-    VIR_FREE(config_json);
>-    VIR_FREE(dom_xml);
>-    VIR_FREE(managed_save_path);
>     virDomainDefFree(def);
>     VIR_FORCE_CLOSE(managed_save_fd);
>     return ret;

[...]

>@@ -4882,7 +4876,7 @@ libxlDomainGetNumaParameters(virDomainPtr dom,
>     virDomainObjPtr vm;
>     libxl_bitmap nodemap;
>     virBitmapPtr nodes = NULL;
>-    char *nodeset = NULL;
>+    g_autofree char *nodeset = NULL;

This variable is used inside the loop, please move its declaration
there (even though it's only used once).

>     int rc, ret = -1;
>     size_t i, j;
>
>@@ -4981,7 +4975,6 @@ libxlDomainGetNumaParameters(virDomainPtr dom,
>     ret = 0;
>
>  cleanup:
>-    VIR_FREE(nodeset);
>     virBitmapFree(nodes);
>     libxl_bitmap_dispose(&nodemap);
>     virDomainObjEndAPI(&vm);

With the logger changes from your reply:

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano