[libvirt] [PATCH v1 21/40] util: pci: use VIR_AUTOPTR for aggregate types

Sukrit Bhatnagar posted 40 patches 7 years, 6 months ago
There is a newer version of this series
[libvirt] [PATCH v1 21/40] util: pci: use VIR_AUTOPTR for aggregate types
Posted by Sukrit Bhatnagar 7 years, 6 months ago
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOPTR macro for declaring aggregate pointer variables,
majority of the calls to *Free functions can be dropped, which
in turn leads to getting rid of most of our cleanup sections.

Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
---
 src/util/virpci.c | 51 ++++++++++++++++++---------------------------------
 1 file changed, 18 insertions(+), 33 deletions(-)

diff --git a/src/util/virpci.c b/src/util/virpci.c
index 1f6ac0b..7b0964c 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -490,8 +490,6 @@ virPCIDeviceIterDevices(virPCIDeviceIterPredicate predicate,
             ret = 1;
             break;
         }
-
-        virPCIDeviceFree(check);
     }
     VIR_DIR_CLOSE(dir);
     return ret;
@@ -781,7 +779,8 @@ virPCIDeviceTrySecondaryBusReset(virPCIDevicePtr dev,
                                  int cfgfd,
                                  virPCIDeviceList *inactiveDevs)
 {
-    virPCIDevicePtr parent, conflict;
+    VIR_AUTOPTR(virPCIDevice) parent = NULL;
+    VIR_AUTOPTR(virPCIDevice) conflict = NULL;
     uint8_t config_space[PCI_CONF_LEN];
     uint16_t ctl;
     int ret = -1;
@@ -795,7 +794,6 @@ virPCIDeviceTrySecondaryBusReset(virPCIDevicePtr dev,
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Active %s devices on bus with %s, not doing bus reset"),
                        conflict->name, dev->name);
-        virPCIDeviceFree(conflict);
         return -1;
     }
 
@@ -848,7 +846,6 @@ virPCIDeviceTrySecondaryBusReset(virPCIDevicePtr dev,
 
  out:
     virPCIDeviceConfigClose(parent, parentfd);
-    virPCIDeviceFree(parent);
     return ret;
 }
 
@@ -1270,8 +1267,8 @@ virPCIDeviceBindToStubWithNewid(virPCIDevicePtr dev)
     VIR_AUTOFREE(char *) stubDriverPath = NULL;
     VIR_AUTOFREE(char *) driverLink = NULL;
     VIR_AUTOFREE(char *) path = NULL; /* reused for different purposes */
+    VIR_AUTOPTR(virError) err = NULL;
     const char *stubDriverName = NULL;
-    virErrorPtr err = NULL;
 
     /* Check the device is configured to use one of the known stub drivers */
     if (dev->stubDriver == VIR_PCI_STUB_DRIVER_NONE) {
@@ -1406,7 +1403,6 @@ virPCIDeviceBindToStubWithNewid(virPCIDevicePtr dev)
 
     if (err)
         virSetError(err);
-    virFreeError(err);
 
     return result;
 }
@@ -1679,19 +1675,16 @@ virPCIGetAddrString(unsigned int domain,
                     unsigned int function,
                     char **pciConfigAddr)
 {
-    virPCIDevicePtr dev = NULL;
-    int ret = -1;
+    VIR_AUTOPTR(virPCIDevice) dev = NULL;
 
     dev = virPCIDeviceNew(domain, bus, slot, function);
     if (dev != NULL) {
         if (VIR_STRDUP(*pciConfigAddr, dev->name) < 0)
-            goto cleanup;
-        ret = 0;
+            return -1;
+        return 0;
     }
 
- cleanup:
-    virPCIDeviceFree(dev);
-    return ret;
+    return -1;
 }
 
 virPCIDevicePtr
@@ -1962,10 +1955,10 @@ virPCIDeviceListAddCopy(virPCIDeviceListPtr list, virPCIDevicePtr dev)
 
     if (!copy)
         return -1;
-    if (virPCIDeviceListAdd(list, copy) < 0) {
-        virPCIDeviceFree(copy);
+    if (virPCIDeviceListAdd(list, copy) < 0)
         return -1;
-    }
+
+    copy = NULL;
     return 0;
 }
 
@@ -2013,8 +2006,7 @@ void
 virPCIDeviceListDel(virPCIDeviceListPtr list,
                     virPCIDevicePtr dev)
 {
-    virPCIDevicePtr ret = virPCIDeviceListSteal(list, dev);
-    virPCIDeviceFree(ret);
+    VIR_AUTOPTR(virPCIDevice) ret = virPCIDeviceListSteal(list, dev);
 }
 
 int
@@ -2168,22 +2160,18 @@ virPCIDeviceAddressIOMMUGroupIterate(virPCIDeviceAddressPtr orig,
 static int
 virPCIDeviceGetIOMMUGroupAddOne(virPCIDeviceAddressPtr newDevAddr, void *opaque)
 {
-    int ret = -1;
     virPCIDeviceListPtr groupList = opaque;
-    virPCIDevicePtr newDev;
+    VIR_AUTOPTR(virPCIDevice) newDev = NULL;
 
     if (!(newDev = virPCIDeviceNew(newDevAddr->domain, newDevAddr->bus,
                                    newDevAddr->slot, newDevAddr->function)))
-        goto cleanup;
+        return -1;
 
     if (virPCIDeviceListAdd(groupList, newDev) < 0)
-        goto cleanup;
+        return -1;
 
     newDev = NULL; /* it's now on the list */
-    ret = 0;
- cleanup:
-    virPCIDeviceFree(newDev);
-    return ret;
+    return 0;
 }
 
 
@@ -2395,7 +2383,7 @@ virPCIDeviceDownstreamLacksACS(virPCIDevicePtr dev)
 static int
 virPCIDeviceIsBehindSwitchLackingACS(virPCIDevicePtr dev)
 {
-    virPCIDevicePtr parent;
+    VIR_AUTOPTR(virPCIDevice) parent = NULL;
 
     if (virPCIDeviceGetParent(dev, &parent) < 0)
         return -1;
@@ -2419,14 +2407,13 @@ virPCIDeviceIsBehindSwitchLackingACS(virPCIDevicePtr dev)
      * parent can be found
      */
     do {
-        virPCIDevicePtr tmp;
+        VIR_AUTOPTR(virPCIDevice) tmp = NULL;
         int acs;
         int ret;
 
         acs = virPCIDeviceDownstreamLacksACS(parent);
 
         if (acs) {
-            virPCIDeviceFree(parent);
             if (acs < 0)
                 return -1;
             else
@@ -2435,7 +2422,6 @@ virPCIDeviceIsBehindSwitchLackingACS(virPCIDevicePtr dev)
 
         tmp = parent;
         ret = virPCIDeviceGetParent(parent, &parent);
-        virPCIDeviceFree(tmp);
         if (ret < 0)
             return -1;
     } while (parent);
@@ -2940,7 +2926,7 @@ virPCIGetMdevTypes(const char *sysfspath,
     DIR *dir = NULL;
     struct dirent *entry;
     VIR_AUTOFREE(char *) types_path = NULL;
-    virMediatedDeviceTypePtr mdev_type = NULL;
+    VIR_AUTOPTR(virMediatedDeviceType) mdev_type = NULL;
     virMediatedDeviceTypePtr *mdev_types = NULL;
     size_t ntypes = 0;
     size_t i;
@@ -2976,7 +2962,6 @@ virPCIGetMdevTypes(const char *sysfspath,
     ret = ntypes;
     ntypes = 0;
  cleanup:
-    virMediatedDeviceTypeFree(mdev_type);
     for (i = 0; i < ntypes; i++)
         virMediatedDeviceTypeFree(mdev_types[i]);
     VIR_FREE(mdev_types);
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 21/40] util: pci: use VIR_AUTOPTR for aggregate types
Posted by Erik Skultety 7 years, 6 months ago
On Sat, Jul 21, 2018 at 05:36:53PM +0530, Sukrit Bhatnagar wrote:
> By making use of GNU C's cleanup attribute handled by the
> VIR_AUTOPTR macro for declaring aggregate pointer variables,
> majority of the calls to *Free functions can be dropped, which
> in turn leads to getting rid of most of our cleanup sections.
>
> Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
> ---
>  src/util/virpci.c | 51 ++++++++++++++++++---------------------------------
>  1 file changed, 18 insertions(+), 33 deletions(-)
>
> diff --git a/src/util/virpci.c b/src/util/virpci.c
> index 1f6ac0b..7b0964c 100644
> --- a/src/util/virpci.c
> +++ b/src/util/virpci.c
> @@ -490,8 +490,6 @@ virPCIDeviceIterDevices(virPCIDeviceIterPredicate predicate,
>              ret = 1;
>              break;
>          }
> -
> -        virPCIDeviceFree(check);
>      }
>      VIR_DIR_CLOSE(dir);
>      return ret;

...

> @@ -1679,19 +1675,16 @@ virPCIGetAddrString(unsigned int domain,
>                      unsigned int function,
>                      char **pciConfigAddr)
>  {
> -    virPCIDevicePtr dev = NULL;
> -    int ret = -1;
> +    VIR_AUTOPTR(virPCIDevice) dev = NULL;
>
>      dev = virPCIDeviceNew(domain, bus, slot, function);
>      if (dev != NULL) {
>          if (VIR_STRDUP(*pciConfigAddr, dev->name) < 0)
> -            goto cleanup;
> -        ret = 0;
> +            return -1;
> +        return 0;


if (!dev || VIR_STRDUP(*pciConfigAddr, dev->name) < 0))
    return -1;

>      }
>
> - cleanup:
> -    virPCIDeviceFree(dev);
> -    return ret;
> +    return -1;

^return 0;

>  }
>
>  virPCIDevicePtr
> @@ -1962,10 +1955,10 @@ virPCIDeviceListAddCopy(virPCIDeviceListPtr list, virPCIDevicePtr dev)
>
>      if (!copy)
>          return -1;
> -    if (virPCIDeviceListAdd(list, copy) < 0) {
> -        virPCIDeviceFree(copy);
> +    if (virPCIDeviceListAdd(list, copy) < 0)
>          return -1;
> -    }
> +
> +    copy = NULL;

You'll first need to define copy as VIR_AUTOPTR, if you want to use ^this
assignment, otherwise it's a NOP.

There may be some leftovers from the previous patch that will need to go into
this one, otherwise looks fine.

Erik

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