openkylin
/
libvirt
mirror of https://gitee.com/openkylin/libvirt.git
9
0
Fork 0
Code Issues Projects Releases Wiki Activity

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.

This also fixes a memory leak in udevInterfaceGetXMLDesc() which
called virInterfaceDefFree() only in successful path.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
This commit is contained in:
Michal Privoznik 2021-09-23 14:32:24 +02:00
parent 488711a6ed
commit b72419f387
7 changed files with 54 additions and 93 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
src/conf/interface_conf.c
View File

@ -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);
}

1
src/conf/interface_conf.h
View File

@ -153,6 +153,7 @@ struct _virInterfaceDef {
void
virInterfaceDefFree(virInterfaceDef *def);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virInterfaceDef, virInterfaceDefFree);
virInterfaceDef *
virInterfaceDefParseString(const char *xmlStr,

4
src/conf/virinterfaceobj.c
View File

@ -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)
@ -379,6 +379,7 @@ virInterfaceObjListCloneCb(void *payload,
if (!(obj = virInterfaceObjListAssignDef(data->dest, backup)))
goto error;
backup = NULL;
virInterfaceObjEndAPI(&obj);
virObjectUnlock(srcObj);
@ -387,7 +388,6 @@ virInterfaceObjListCloneCb(void *payload,
error:
data->error = true;
VIR_FREE(xml);
virInterfaceDefFree(backup);
virObjectUnlock(srcObj);
return 0;
}

47
src/interface/interface_backend_netcf.c
View File

@ -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;
}

29
src/interface/interface_backend_udev.c
View File

@ -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;
}

17
src/test/test_driver.c
View File

@ -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;

17
tests/interfacexml2xmltest.c
View File

@ -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