[libvirt] [PATCH] nodedev: use autofree for more local variables

Jonathon Jongsma posted 1 patch 4 years, 2 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200110215107.8628-1-jjongsma@redhat.com
src/conf/node_device_conf.h          |  2 ++
src/node_device/node_device_driver.c | 47 +++++++++++-----------------
2 files changed, 20 insertions(+), 29 deletions(-)
[libvirt] [PATCH] nodedev: use autofree for more local variables
Posted by Jonathon Jongsma 4 years, 2 months ago
Simplify function logic by using g_autofree to free local variables so
that we can remove some goto statements that are used for cleanup.

Introduce a g_autoptr cleanup function for virNodeDeviceDef.

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
---
 src/conf/node_device_conf.h          |  2 ++
 src/node_device/node_device_driver.c | 47 +++++++++++-----------------
 2 files changed, 20 insertions(+), 29 deletions(-)

diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index bf7939fbb3..4ff616ee90 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -344,6 +344,8 @@ virNodeDeviceGetWWNs(virNodeDeviceDefPtr def,
 void
 virNodeDeviceDefFree(virNodeDeviceDefPtr def);
 
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNodeDeviceDef, virNodeDeviceDefFree);
+
 void
 virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps);
 
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index b630be4399..fda77ede24 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -106,10 +106,9 @@ int nodeConnectIsAlive(virConnectPtr conn G_GNUC_UNUSED)
 static int
 nodeDeviceUpdateDriverName(virNodeDeviceDefPtr def)
 {
-    char *driver_link = NULL;
-    char *devpath = NULL;
+    g_autofree char *driver_link = NULL;
+    g_autofree char *devpath = NULL;
     char *p;
-    int ret = -1;
 
     VIR_FREE(def->driver);
 
@@ -117,26 +116,20 @@ nodeDeviceUpdateDriverName(virNodeDeviceDefPtr def)
 
     /* Some devices don't have an explicit driver, so just return
        without a name */
-    if (access(driver_link, R_OK) < 0) {
-        ret = 0;
-        goto cleanup;
-    }
+    if (access(driver_link, R_OK) < 0)
+        return 0;
 
     if (virFileResolveLink(driver_link, &devpath) < 0) {
         virReportSystemError(errno,
                              _("cannot resolve driver link %s"), driver_link);
-        goto cleanup;
+        return -1;
     }
 
     p = strrchr(devpath, '/');
     if (p)
         def->driver = g_strdup(p + 1);
-    ret = 0;
 
- cleanup:
-    VIR_FREE(driver_link);
-    VIR_FREE(devpath);
-    return ret;
+    return 0;
 }
 #else
 /* XXX: Implement me for non-linux */
@@ -468,8 +461,9 @@ nodeDeviceCreateXML(virConnectPtr conn,
                     const char *xmlDesc,
                     unsigned int flags)
 {
-    virNodeDeviceDefPtr def = NULL;
-    char *wwnn = NULL, *wwpn = NULL;
+    g_autoptr(virNodeDeviceDef) def = NULL;
+    g_autofree char *wwnn = NULL;
+    g_autofree char *wwpn = NULL;
     int parent_host = -1;
     virNodeDevicePtr device = NULL;
     const char *virt_type = NULL;
@@ -478,19 +472,19 @@ nodeDeviceCreateXML(virConnectPtr conn,
     virt_type  = virConnectGetType(conn);
 
     if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, virt_type)))
-        goto cleanup;
+        return NULL;
 
     if (virNodeDeviceCreateXMLEnsureACL(conn, def) < 0)
-        goto cleanup;
+        return NULL;
 
     if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) == -1)
-        goto cleanup;
+        return NULL;
 
     if ((parent_host = virNodeDeviceObjListGetParentHost(driver->devs, def)) < 0)
-        goto cleanup;
+        return NULL;
 
     if (virVHBAManageVport(parent_host, wwpn, wwnn, VPORT_CREATE) < 0)
-        goto cleanup;
+        return NULL;
 
     device = nodeDeviceFindNewDevice(conn, wwnn, wwpn);
     /* We don't check the return value, because one way or another,
@@ -501,10 +495,7 @@ nodeDeviceCreateXML(virConnectPtr conn,
                        _("no node device for '%s' with matching "
                          "wwnn '%s' and wwpn '%s'"),
                        def->name, wwnn, wwpn);
- cleanup:
-    virNodeDeviceDefFree(def);
-    VIR_FREE(wwnn);
-    VIR_FREE(wwpn);
+
     return device;
 }
 
@@ -515,8 +506,9 @@ nodeDeviceDestroy(virNodeDevicePtr device)
     int ret = -1;
     virNodeDeviceObjPtr obj = NULL;
     virNodeDeviceDefPtr def;
-    char *parent = NULL;
-    char *wwnn = NULL, *wwpn = NULL;
+    g_autofree char *parent = NULL;
+    g_autofree char *wwnn = NULL;
+    g_autofree char *wwpn = NULL;
     unsigned int parent_host;
 
     if (!(obj = nodeDeviceObjFindByName(device->name)))
@@ -554,9 +546,6 @@ nodeDeviceDestroy(virNodeDevicePtr device)
 
  cleanup:
     virNodeDeviceObjEndAPI(&obj);
-    VIR_FREE(parent);
-    VIR_FREE(wwnn);
-    VIR_FREE(wwpn);
     return ret;
 }
 
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] nodedev: use autofree for more local variables
Posted by Erik Skultety 4 years, 2 months ago
On Fri, Jan 10, 2020 at 03:51:07PM -0600, Jonathon Jongsma wrote:
> Simplify function logic by using g_autofree to free local variables so
> that we can remove some goto statements that are used for cleanup.
>
> Introduce a g_autoptr cleanup function for virNodeDeviceDef.
>
> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>