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

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(a)redhat.com&gt; --- 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; } 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); - 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 -- 2.32.0