[PATCH] lib: Introduce and use g_autoptr() for virInterfaceDef

Tim Wiederhake twiederh at redhat.com
Tue Nov 2 09:04:46 UTC 2021


On Mon, 2021-11-01 at 16:25 +0100, Michal Privoznik wrote:
> There are a lot of places where we call virInterfaceDefFree()
> explicitly. We can define autoptr cleanup macro and annotate
> declarations with g_autoptr() and remove plenty of those explicit
> free calls.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/conf/interface_conf.c               | 32 ++++++++---------
>  src/conf/interface_conf.h               |  1 +
>  src/conf/virinterfaceobj.c              |  3 +-
>  src/interface/interface_backend_netcf.c | 47 ++++++++---------------
> --
>  src/interface/interface_backend_udev.c  | 29 +++++----------
>  src/test/test_driver.c                  | 17 ++++-----
>  tests/interfacexml2xmltest.c            | 17 ++++-----
>  7 files changed, 53 insertions(+), 93 deletions(-)
> 
> diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
> index b45dc37379..f2b3804bec 100644
> --- a/src/conf/interface_conf.c
> +++ b/src/conf/interface_conf.c
> @@ -679,7 +679,7 @@ static virInterfaceDef *
>  virInterfaceDefParseXML(xmlXPathContextPtr ctxt,
>                          int parentIfType)
>  {
> -    virInterfaceDef *def;
> +    g_autoptr(virInterfaceDef) def = NULL;
>      int type;
>      char *tmp;
>      VIR_XPATH_NODE_AUTORESTORE(ctxt)
> @@ -716,28 +716,28 @@ virInterfaceDefParseXML(xmlXPathContextPtr
> ctxt,
>          virReportError(VIR_ERR_XML_ERROR,
>                         _("interface has unsupported type '%s'"),
>                         virInterfaceTypeToString(type));
> -        goto error;
> +        return NULL;
>      }
>      def->type = type;
>  
>      if (virInterfaceDefParseName(def, ctxt) < 0)
> -       goto error;
> +       return NULL;
>  
>      if (parentIfType == VIR_INTERFACE_TYPE_LAST) {
>          /* only recognize these in toplevel bond interfaces */
>          if (virInterfaceDefParseStartMode(def, ctxt) < 0)
> -            goto error;
> +            return NULL;
>          if (virInterfaceDefParseMtu(def, ctxt) < 0)
> -            goto error;
> +            return NULL;
>          if (virInterfaceDefParseIfAdressing(def, ctxt) < 0)
> -            goto error;
> +            return NULL;
>      }
>  
>      if (type != VIR_INTERFACE_TYPE_BRIDGE) {
>          /* link status makes no sense for a bridge */
>          lnk = virXPathNode("./link", ctxt);
>          if (lnk && virInterfaceLinkParseXML(lnk, &def->lnk) < 0)
> -            goto error;
> +            return NULL;
>      }
>  
>      switch (type) {
> @@ -751,11 +751,11 @@ virInterfaceDefParseXML(xmlXPathContextPtr
> ctxt,
>              if (!(bridge = virXPathNode("./bridge[1]", ctxt))) {
>                  virReportError(VIR_ERR_XML_ERROR,
>                                 "%s", _("bridge interface misses the
> bridge element"));
> -                goto error;
> +                return NULL;
>              }
>              ctxt->node = bridge;
>              if (virInterfaceDefParseBridge(def, ctxt) < 0)
> -                goto error;
> +                return NULL;
>              break;
>          }
>          case VIR_INTERFACE_TYPE_BOND: {
> @@ -764,11 +764,11 @@ virInterfaceDefParseXML(xmlXPathContextPtr
> ctxt,
>              if (!(bond = virXPathNode("./bond[1]", ctxt))) {
>                  virReportError(VIR_ERR_XML_ERROR,
>                                 "%s", _("bond interface misses the
> bond element"));
> -                goto error;
> +                return NULL;
>              }
>              ctxt->node = bond;
>              if (virInterfaceDefParseBond(def, ctxt)  < 0)
> -                goto error;
> +                return NULL;
>              break;
>          }
>          case VIR_INTERFACE_TYPE_VLAN: {
> @@ -777,21 +777,17 @@ virInterfaceDefParseXML(xmlXPathContextPtr
> ctxt,
>              if (!(vlan = virXPathNode("./vlan[1]", ctxt))) {
>                  virReportError(VIR_ERR_XML_ERROR,
>                                 "%s", _("vlan interface misses the
> vlan element"));
> -                goto error;
> +                return NULL;
>              }
>              ctxt->node = vlan;
>              if (virInterfaceDefParseVlan(def, ctxt)  < 0)
> -                goto error;
> +                return NULL;
>              break;
>          }
>  
>      }
>  
> -    return def;
> -
> - error:
> -    virInterfaceDefFree(def);
> -    return NULL;
> +    return g_steal_pointer(&def);
>  }
>  
>  
> diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h
> index ea92e0fb31..510d83b2bf 100644
> --- a/src/conf/interface_conf.h
> +++ b/src/conf/interface_conf.h
> @@ -153,6 +153,7 @@ struct _virInterfaceDef {
>  
>  void
>  virInterfaceDefFree(virInterfaceDef *def);
> +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virInterfaceDef, virInterfaceDefFree);
>  
>  virInterfaceDef *
>  virInterfaceDefParseString(const char *xmlStr,
> diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
> index 9439bb3d0b..ceb3ae7595 100644
> --- a/src/conf/virinterfaceobj.c
> +++ b/src/conf/virinterfaceobj.c
> @@ -362,7 +362,7 @@ virInterfaceObjListCloneCb(void *payload,
>      virInterfaceObj *srcObj = payload;
>      struct _virInterfaceObjListCloneData *data = opaque;
>      char *xml = NULL;
> -    virInterfaceDef *backup = NULL;
> +    g_autoptr(virInterfaceDef) backup = NULL;
>      virInterfaceObj *obj;
>  
>      if (data->error)
> @@ -387,7 +387,6 @@ virInterfaceObjListCloneCb(void *payload,
>   error:
>      data->error = true;
>      VIR_FREE(xml);
> -    virInterfaceDefFree(backup);
>      virObjectUnlock(srcObj);
>      return 0;
>  }

I believe there is a `g_steal_pointer` or similar missing in the call
to `virInterfaceObjListAssignDef` (not shown in patch).

> diff --git a/src/interface/interface_backend_netcf.c
> b/src/interface/interface_backend_netcf.c
> index 78fd4f9bc7..146a703953 100644
> --- a/src/interface/interface_backend_netcf.c
> +++ b/src/interface/interface_backend_netcf.c
> @@ -387,7 +387,7 @@ static int
> netcfConnectNumOfInterfacesImpl(virConnectPtr conn,
>      }
>  
>      for (i = 0; i < count; i++) {
> -        virInterfaceDef *def;
> +        g_autoptr(virInterfaceDef) def = NULL;
>          struct netcf_if *iface;
>  
>          iface = ncf_lookup_by_name(driver->netcf, names[i]);
> @@ -416,11 +416,8 @@ static int
> netcfConnectNumOfInterfacesImpl(virConnectPtr conn,
>          }
>          ncf_if_free(iface);
>  
> -        if (!filter(conn, def)) {
> -            virInterfaceDefFree(def);
> +        if (!filter(conn, def))
>              continue;
> -        }
> -        virInterfaceDefFree(def);
>  
>          want++;
>      }
> @@ -481,7 +478,7 @@ static int
> netcfConnectListInterfacesImpl(virConnectPtr conn,
>      }
>  
>      for (i = 0; i < count && want < nnames; i++) {
> -        virInterfaceDef *def;
> +        g_autoptr(virInterfaceDef) def = NULL;
>          struct netcf_if *iface;
>  
>          iface = ncf_lookup_by_name(driver->netcf, allnames[i]);
> @@ -510,11 +507,8 @@ static int
> netcfConnectListInterfacesImpl(virConnectPtr conn,
>          }
>          ncf_if_free(iface);
>  
> -        if (!filter(conn, def)) {
> -            virInterfaceDefFree(def);
> +        if (!filter(conn, def))
>              continue;
> -        }
> -        virInterfaceDefFree(def);
>  
>          names[want++] = g_steal_pointer(&allnames[i]);
>      }
> @@ -665,7 +659,7 @@ netcfConnectListAllInterfaces(virConnectPtr conn,
>          tmp_iface_objs = g_new0(virInterfacePtr, count + 1);
>  
>      for (i = 0; i < count; i++) {
> -        virInterfaceDef *def;
> +        g_autoptr(virInterfaceDef) def = NULL;
>  
>          iface = ncf_lookup_by_name(driver->netcf, names[i]);
>          if (!iface) {
> @@ -693,20 +687,17 @@ netcfConnectListAllInterfaces(virConnectPtr
> conn,
>          if (!virConnectListAllInterfacesCheckACL(conn, def)) {
>              ncf_if_free(iface);
>              iface = NULL;
> -            virInterfaceDefFree(def);
>              continue;
>          }
>  
>          if (ifaces) {
> -            if (!(iface_obj = virGetInterface(conn, def->name, def-
> >mac))) {
> -                virInterfaceDefFree(def);
> +            if (!(iface_obj = virGetInterface(conn, def->name, def-
> >mac)))
>                  goto cleanup;
> -            }
> +
>              tmp_iface_objs[niface_objs] = iface_obj;
>          }
>          niface_objs++;
>  
> -        virInterfaceDefFree(def);
>          ncf_if_free(iface);
>          iface = NULL;
>      }
> @@ -743,7 +734,7 @@ static virInterfacePtr
> netcfInterfaceLookupByName(virConnectPtr conn,
>  {
>      struct netcf_if *iface;
>      virInterfacePtr ret = NULL;
> -    virInterfaceDef *def = NULL;
> +    g_autoptr(virInterfaceDef) def = NULL;
>  
>      virObjectLock(driver);
>      iface = ncf_lookup_by_name(driver->netcf, name);
> @@ -772,7 +763,6 @@ static virInterfacePtr
> netcfInterfaceLookupByName(virConnectPtr conn,
>  
>   cleanup:
>      ncf_if_free(iface);
> -    virInterfaceDefFree(def);
>      virObjectUnlock(driver);
>      return ret;
>  }
> @@ -783,7 +773,7 @@ static virInterfacePtr
> netcfInterfaceLookupByMACString(virConnectPtr conn,
>      struct netcf_if *iface;
>      int niface;
>      virInterfacePtr ret = NULL;
> -    virInterfaceDef *def = NULL;
> +    g_autoptr(virInterfaceDef) def = NULL;
>  
>      virObjectLock(driver);
>      niface = ncf_lookup_by_mac_string(driver->netcf, macstr, 1,
> &iface);
> @@ -820,7 +810,6 @@ static virInterfacePtr
> netcfInterfaceLookupByMACString(virConnectPtr conn,
>  
>   cleanup:
>      ncf_if_free(iface);
> -    virInterfaceDefFree(def);
>      virObjectUnlock(driver);
>      return ret;
>  }
> @@ -830,7 +819,7 @@ static char
> *netcfInterfaceGetXMLDesc(virInterfacePtr ifinfo,
>  {
>      struct netcf_if *iface = NULL;
>      char *xmlstr = NULL;
> -    virInterfaceDef *ifacedef = NULL;
> +    g_autoptr(virInterfaceDef) ifacedef = NULL;
>      char *ret = NULL;
>      bool active;
>  
> @@ -880,7 +869,6 @@ static char
> *netcfInterfaceGetXMLDesc(virInterfacePtr ifinfo,
>   cleanup:
>      ncf_if_free(iface);
>      VIR_FREE(xmlstr);
> -    virInterfaceDefFree(ifacedef);
>      virObjectUnlock(driver);
>      return ret;
>  }
> @@ -891,7 +879,7 @@ static virInterfacePtr
> netcfInterfaceDefineXML(virConnectPtr conn,
>  {
>      struct netcf_if *iface = NULL;
>      char *xmlstr = NULL;
> -    virInterfaceDef *ifacedef = NULL;
> +    g_autoptr(virInterfaceDef) ifacedef = NULL;
>      virInterfacePtr ret = NULL;
>  
>      virCheckFlags(VIR_INTERFACE_DEFINE_VALIDATE, NULL);
> @@ -929,7 +917,6 @@ static virInterfacePtr
> netcfInterfaceDefineXML(virConnectPtr conn,
>   cleanup:
>      ncf_if_free(iface);
>      VIR_FREE(xmlstr);
> -    virInterfaceDefFree(ifacedef);
>      virObjectUnlock(driver);
>      return ret;
>  }
> @@ -937,7 +924,7 @@ static virInterfacePtr
> netcfInterfaceDefineXML(virConnectPtr conn,
>  static int netcfInterfaceUndefine(virInterfacePtr ifinfo)
>  {
>      struct netcf_if *iface = NULL;
> -    virInterfaceDef *def = NULL;
> +    g_autoptr(virInterfaceDef) def = NULL;
>      int ret = -1;
>  
>      virObjectLock(driver);
> @@ -968,7 +955,6 @@ static int netcfInterfaceUndefine(virInterfacePtr
> ifinfo)
>  
>   cleanup:
>      ncf_if_free(iface);
> -    virInterfaceDefFree(def);
>      virObjectUnlock(driver);
>      return ret;
>  }
> @@ -977,7 +963,7 @@ static int netcfInterfaceCreate(virInterfacePtr
> ifinfo,
>                                  unsigned int flags)
>  {
>      struct netcf_if *iface = NULL;
> -    virInterfaceDef *def = NULL;
> +    g_autoptr(virInterfaceDef) def = NULL;
>      int ret = -1;
>      bool active;
>  
> @@ -1020,7 +1006,6 @@ static int netcfInterfaceCreate(virInterfacePtr
> ifinfo,
>  
>   cleanup:
>      ncf_if_free(iface);
> -    virInterfaceDefFree(def);
>      virObjectUnlock(driver);
>      return ret;
>  }
> @@ -1029,7 +1014,7 @@ static int
> netcfInterfaceDestroy(virInterfacePtr ifinfo,
>                                   unsigned int flags)
>  {
>      struct netcf_if *iface = NULL;
> -    virInterfaceDef *def = NULL;
> +    g_autoptr(virInterfaceDef) def = NULL;
>      int ret = -1;
>      bool active;
>  
> @@ -1072,7 +1057,6 @@ static int
> netcfInterfaceDestroy(virInterfacePtr ifinfo,
>  
>   cleanup:
>      ncf_if_free(iface);
> -    virInterfaceDefFree(def);
>      virObjectUnlock(driver);
>      return ret;
>  }
> @@ -1080,7 +1064,7 @@ static int
> netcfInterfaceDestroy(virInterfacePtr ifinfo,
>  static int netcfInterfaceIsActive(virInterfacePtr ifinfo)
>  {
>      struct netcf_if *iface = NULL;
> -    virInterfaceDef *def = NULL;
> +    g_autoptr(virInterfaceDef) def = NULL;
>      int ret = -1;
>      bool active;
>  
> @@ -1105,7 +1089,6 @@ static int
> netcfInterfaceIsActive(virInterfacePtr ifinfo)
>  
>   cleanup:
>      ncf_if_free(iface);
> -    virInterfaceDefFree(def);
>      virObjectUnlock(driver);
>      return ret;
>  }
> diff --git a/src/interface/interface_backend_udev.c
> b/src/interface/interface_backend_udev.c
> index 0217f16607..8c417714e5 100644
> --- a/src/interface/interface_backend_udev.c
> +++ b/src/interface/interface_backend_udev.c
> @@ -165,7 +165,7 @@ udevNumOfInterfacesByStatus(virConnectPtr conn,
> virUdevStatus status,
>      udev_list_entry_foreach(dev_entry, devices) {
>          struct udev_device *dev;
>          const char *path;
> -        virInterfaceDef *def;
> +        g_autoptr(virInterfaceDef) def = NULL;
>  
>          path = udev_list_entry_get_name(dev_entry);
>          dev = udev_device_new_from_syspath(udev, path);
> @@ -174,7 +174,6 @@ udevNumOfInterfacesByStatus(virConnectPtr conn,
> virUdevStatus status,
>          if (filter(conn, def))
>              count++;
>          udev_device_unref(dev);
> -        virInterfaceDefFree(def);
>      }
>  
>   cleanup:
> @@ -218,7 +217,7 @@ udevListInterfacesByStatus(virConnectPtr conn,
>      udev_list_entry_foreach(dev_entry, devices) {
>          struct udev_device *dev;
>          const char *path;
> -        virInterfaceDef *def;
> +        g_autoptr(virInterfaceDef) def = NULL;
>  
>          /* Ensure we won't exceed the size of our array */
>          if (count > names_len)
> @@ -233,7 +232,6 @@ udevListInterfacesByStatus(virConnectPtr conn,
>              count++;
>          }
>          udev_device_unref(dev);
> -        virInterfaceDefFree(def);
>      }
>  
>      udev_enumerate_unref(enumerate);
> @@ -355,7 +353,7 @@ udevConnectListAllInterfaces(virConnectPtr conn,
>          const char *path;
>          const char *name;
>          const char *macaddr;
> -        virInterfaceDef *def;
> +        g_autoptr(virInterfaceDef) def = NULL;
>  
>          path = udev_list_entry_get_name(dev_entry);
>          dev = udev_device_new_from_syspath(udev, path);
> @@ -366,10 +364,8 @@ udevConnectListAllInterfaces(virConnectPtr conn,
>          def = udevGetMinimalDefForDevice(dev);
>          if (!virConnectListAllInterfacesCheckACL(conn, def)) {
>              udev_device_unref(dev);
> -            virInterfaceDefFree(def);
>              continue;
>          }
> -        virInterfaceDefFree(def);
>  
>          /* Filter the results */
>          if (MATCH(VIR_CONNECT_LIST_INTERFACES_FILTERS_ACTIVE) &&
> @@ -413,7 +409,7 @@ udevInterfaceLookupByName(virConnectPtr conn,
> const char *name)
>      struct udev *udev = udev_ref(driver->udev);
>      struct udev_device *dev;
>      virInterfacePtr ret = NULL;
> -    virInterfaceDef *def = NULL;
> +    g_autoptr(virInterfaceDef) def = NULL;
>  
>      /* get a device reference based on the device name */
>      dev = udev_device_new_from_subsystem_sysname(udev, "net", name);
> @@ -435,7 +431,6 @@ udevInterfaceLookupByName(virConnectPtr conn,
> const char *name)
>  
>   cleanup:
>      udev_unref(udev);
> -    virInterfaceDefFree(def);
>  
>      return ret;
>  }
> @@ -447,7 +442,7 @@ udevInterfaceLookupByMACString(virConnectPtr
> conn, const char *macstr)
>      struct udev_enumerate *enumerate = NULL;
>      struct udev_list_entry *dev_entry;
>      struct udev_device *dev;
> -    virInterfaceDef *def = NULL;
> +    g_autoptr(virInterfaceDef) def = NULL;
>      virInterfacePtr ret = NULL;
>  
>      enumerate = udevGetDevices(udev, VIR_UDEV_IFACE_ALL);
> @@ -499,7 +494,6 @@ udevInterfaceLookupByMACString(virConnectPtr
> conn, const char *macstr)
>      if (enumerate)
>          udev_enumerate_unref(enumerate);
>      udev_unref(udev);
> -    virInterfaceDefFree(def);
>  
>      return ret;
>  }
> @@ -945,7 +939,7 @@ static virInterfaceDef * ATTRIBUTE_NONNULL(1)
>  udevGetIfaceDef(struct udev *udev, const char *name)
>  {
>      struct udev_device *dev = NULL;
> -    virInterfaceDef *ifacedef;
> +    g_autoptr(virInterfaceDef) ifacedef = NULL;
>      unsigned int mtu;
>      const char *mtu_str;
>      char *vlan_parent_dev = NULL;
> @@ -1038,13 +1032,11 @@ udevGetIfaceDef(struct udev *udev, const char
> *name)
>  
>      udev_device_unref(dev);
>  
> -    return ifacedef;
> +    return g_steal_pointer(&ifacedef);
>  
>   error:
>      udev_device_unref(dev);
>  
> -    virInterfaceDefFree(ifacedef);
> -
>      return NULL;
>  }
>  
> @@ -1053,7 +1045,7 @@ udevInterfaceGetXMLDesc(virInterfacePtr ifinfo,
>                          unsigned int flags)
>  {
>      struct udev *udev = udev_ref(driver->udev);
> -    virInterfaceDef *ifacedef;
> +    g_autoptr(virInterfaceDef) ifacedef = NULL;
>      char *xmlstr = NULL;
>  
>      virCheckFlags(VIR_INTERFACE_XML_INACTIVE, NULL);
> @@ -1071,8 +1063,6 @@ udevInterfaceGetXMLDesc(virInterfacePtr ifinfo,
>  
>      xmlstr = virInterfaceDefFormat(ifacedef);
>  
> -    virInterfaceDefFree(ifacedef);
> -

This used to be a memory leak if the call to
`virInterfaceGetXMLDescEnsureACL` (not shown in the patch) failed,
isn't it? If so, we should mention that in the commit message.

Otherwise:
Reviewed-by: Tim Wiederhake <twiederh at redhat.com>

>   cleanup:
>      /* decrement our udev ptr */
>      udev_unref(udev);
> @@ -1085,7 +1075,7 @@ udevInterfaceIsActive(virInterfacePtr ifinfo)
>  {
>      struct udev *udev = udev_ref(driver->udev);
>      struct udev_device *dev;
> -    virInterfaceDef *def = NULL;
> +    g_autoptr(virInterfaceDef) def = NULL;
>      int status = -1;
>  
>      dev = udev_device_new_from_subsystem_sysname(udev, "net",
> @@ -1110,7 +1100,6 @@ udevInterfaceIsActive(virInterfacePtr ifinfo)
>  
>   cleanup:
>      udev_unref(udev);
> -    virInterfaceDefFree(def);
>  
>      return status;
>  }
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 13d07e570e..ea474d55ac 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -1096,7 +1096,7 @@ testParseNetworks(testDriver *privconn,
>          return -1;
>  
>      for (i = 0; i < num; i++) {
> -        virNetworkDef *def;
> +        g_autoptr(virNetworkDef) def = NULL;
>          xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file,
> "network");
>          if (!node)
>              return -1;
> @@ -1105,10 +1105,9 @@ testParseNetworks(testDriver *privconn,
>          if (!def)
>              return -1;
>  
> -        if (!(obj = virNetworkObjAssignDef(privconn->networks, def,
> 0))) {
> -            virNetworkDefFree(def);
> +        if (!(obj = virNetworkObjAssignDef(privconn->networks, def,
> 0)))
>              return -1;
> -        }
> +        def = NULL;
>  
>          virNetworkObjSetActive(obj, true);
>          virNetworkObjEndAPI(&obj);
> @@ -1133,7 +1132,7 @@ testParseInterfaces(testDriver *privconn,
>          return -1;
>  
>      for (i = 0; i < num; i++) {
> -        virInterfaceDef *def;
> +        g_autoptr(virInterfaceDef) def = NULL;
>          xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file,
>                                                     "interface");
>          if (!node)
> @@ -1143,10 +1142,9 @@ testParseInterfaces(testDriver *privconn,
>          if (!def)
>              return -1;
>  
> -        if (!(obj = virInterfaceObjListAssignDef(privconn->ifaces,
> def))) {
> -            virInterfaceDefFree(def);
> +        if (!(obj = virInterfaceObjListAssignDef(privconn->ifaces,
> def)))
>              return -1;
> -        }
> +        def = NULL;
>  
>          virInterfaceObjSetActive(obj, true);
>          virInterfaceObjEndAPI(&obj);
> @@ -6195,7 +6193,7 @@ testInterfaceDefineXML(virConnectPtr conn,
>                         unsigned int flags)
>  {
>      testDriver *privconn = conn->privateData;
> -    virInterfaceDef *def;
> +    g_autoptr(virInterfaceDef) def = NULL;
>      virInterfaceObj *obj = NULL;
>      virInterfaceDef *objdef;
>      virInterfacePtr ret = NULL;
> @@ -6214,7 +6212,6 @@ testInterfaceDefineXML(virConnectPtr conn,
>      ret = virGetInterface(conn, objdef->name, objdef->mac);
>  
>   cleanup:
> -    virInterfaceDefFree(def);
>      virInterfaceObjEndAPI(&obj);
>      virObjectUnlock(privconn);
>      return ret;
> diff --git a/tests/interfacexml2xmltest.c
> b/tests/interfacexml2xmltest.c
> index 8235e701a9..15fab88107 100644
> --- a/tests/interfacexml2xmltest.c
> +++ b/tests/interfacexml2xmltest.c
> @@ -18,28 +18,23 @@ testCompareXMLToXMLFiles(const char *xml)
>  {
>      g_autofree char *xmlData = NULL;
>      g_autofree char *actual = NULL;
> -    int ret = -1;
> -    virInterfaceDef *dev = NULL;
> +    g_autoptr(virInterfaceDef) dev = NULL;
>  
>      if (virTestLoadFile(xml, &xmlData) < 0)
> -        goto fail;
> +        return -1;
>  
>      if (!(dev = virInterfaceDefParseString(xmlData, 0)))
> -        goto fail;
> +        return -1;
>  
>      if (!(actual = virInterfaceDefFormat(dev)))
> -        goto fail;
> +        return -1;
>  
>      if (STRNEQ(xmlData, actual)) {
>          virTestDifferenceFull(stderr, xmlData, xml, actual, NULL);
> -        goto fail;
> +        return -1;
>      }
>  
> -    ret = 0;
> -
> - fail:
> -    virInterfaceDefFree(dev);
> -    return ret;
> +    return 0;
>  }
>  
>  static int





More information about the libvir-list mailing list