[libvirt] [PATCH v1 20/40] util: pci: use VIR_AUTOFREE instead of VIR_FREE for scalar types

By making use of GNU C's cleanup attribute handled by the VIR_AUTOFREE macro for declaring scalar variables, majority of the VIR_FREE calls can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virpci.c | 271 ++++++++++++++++++------------------------------------ 1 file changed, 87 insertions(+), 184 deletions(-) diff --git a/src/util/virpci.c b/src/util/virpci.c index a606462..1f6ac0b 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -253,7 +253,7 @@ int virPCIDeviceGetDriverPathAndName(virPCIDevicePtr dev, char **path, char **name) { int ret = -1; - char *drvlink = NULL; + VIR_AUTOFREE(char *) drvlink = NULL; *path = *name = NULL; /* drvlink = "/sys/bus/pci/dddd:bb:ss.ff/driver" */ @@ -285,7 +285,6 @@ virPCIDeviceGetDriverPathAndName(virPCIDevicePtr dev, char **path, char **name) ret = 0; cleanup: - VIR_FREE(drvlink); if (ret < 0) { VIR_FREE(*path); VIR_FREE(*name); @@ -375,32 +374,27 @@ virPCIDeviceRead32(virPCIDevicePtr dev, int cfgfd, unsigned int pos) static int virPCIDeviceReadClass(virPCIDevicePtr dev, uint16_t *device_class) { - char *path = NULL; - char *id_str = NULL; - int ret = -1; + VIR_AUTOFREE(char *) path = NULL; + VIR_AUTOFREE(char *) id_str = NULL; unsigned int value; if (!(path = virPCIFile(dev->name, "class"))) - return ret; + return -1; /* class string is '0xNNNNNN\n' ... i.e. 9 bytes */ if (virFileReadAll(path, 9, &id_str) < 0) - goto cleanup; + return -1; id_str[8] = '\0'; if (virStrToLong_ui(id_str, NULL, 16, &value) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unusual value in %s/devices/%s/class: %s"), PCI_SYSFS, dev->name, id_str); - goto cleanup; + return -1; } *device_class = (value >> 8) & 0xFFFF; - ret = 0; - cleanup: - VIR_FREE(id_str); - VIR_FREE(path); - return ret; + return 0; } static int @@ -574,7 +568,7 @@ virPCIDeviceDetectFunctionLevelReset(virPCIDevicePtr dev, int cfgfd) { uint32_t caps; uint8_t pos; - char *path; + VIR_AUTOFREE(char *) path = NULL; int found; /* The PCIe Function Level Reset capability allows @@ -614,7 +608,6 @@ virPCIDeviceDetectFunctionLevelReset(virPCIDevicePtr dev, int cfgfd) return -1; found = virFileExists(path); - VIR_FREE(path); if (found) { VIR_DEBUG("%s %s: buggy device didn't advertise FLR, but is a VF; forcing flr on", dev->id, dev->name); @@ -926,8 +919,8 @@ virPCIDeviceReset(virPCIDevicePtr dev, virPCIDeviceList *activeDevs, virPCIDeviceList *inactiveDevs) { - char *drvPath = NULL; - char *drvName = NULL; + VIR_AUTOFREE(char *) drvPath = NULL; + VIR_AUTOFREE(char *) drvName = NULL; int ret = -1; int fd = -1; int hdrType = -1; @@ -1000,8 +993,6 @@ virPCIDeviceReset(virPCIDevicePtr dev, } cleanup: - VIR_FREE(drvPath); - VIR_FREE(drvName); virPCIDeviceConfigClose(dev, fd); return ret; } @@ -1011,7 +1002,7 @@ static int virPCIProbeStubDriver(virPCIStubDriver driver) { const char *drvname = NULL; - char *drvpath = NULL; + VIR_AUTOFREE(char *) drvpath = NULL; bool probed = false; if (driver == VIR_PCI_STUB_DRIVER_NONE || @@ -1023,20 +1014,15 @@ virPCIProbeStubDriver(virPCIStubDriver driver) } recheck: - if ((drvpath = virPCIDriverDir(drvname)) && virFileExists(drvpath)) { + if ((drvpath = virPCIDriverDir(drvname)) && virFileExists(drvpath)) /* driver already loaded, return */ - VIR_FREE(drvpath); return 0; - } - - VIR_FREE(drvpath); if (!probed) { - char *errbuf = NULL; + VIR_AUTOFREE(char *) errbuf = NULL; probed = true; if ((errbuf = virKModLoad(drvname, true))) { VIR_WARN("failed to load driver %s: %s", drvname, errbuf); - VIR_FREE(errbuf); goto cleanup; } @@ -1064,38 +1050,30 @@ virPCIProbeStubDriver(virPCIStubDriver driver) int virPCIDeviceUnbind(virPCIDevicePtr dev) { - char *path = NULL; - char *drvpath = NULL; - char *driver = NULL; - int ret = -1; + VIR_AUTOFREE(char *) path = NULL; + VIR_AUTOFREE(char *) drvpath = NULL; + VIR_AUTOFREE(char *) driver = NULL; if (virPCIDeviceGetDriverPathAndName(dev, &drvpath, &driver) < 0) - goto cleanup; + return -1; - if (!driver) { + if (!driver) /* The device is not bound to any driver */ - ret = 0; - goto cleanup; - } + return 0; if (!(path = virPCIFile(dev->name, "driver/unbind"))) - goto cleanup; + return -1; if (virFileExists(path)) { if (virFileWriteStr(path, dev->name, 0) < 0) { virReportSystemError(errno, _("Failed to unbind PCI device '%s' from %s"), dev->name, driver); - goto cleanup; + return -1; } } - ret = 0; - cleanup: - VIR_FREE(path); - VIR_FREE(drvpath); - VIR_FREE(driver); - return ret; + return 0; } @@ -1138,8 +1116,7 @@ static int virPCIDeviceBindWithDriverOverride(virPCIDevicePtr dev, const char *driverName) { - int ret = -1; - char *path; + VIR_AUTOFREE(char *) path = NULL; if (!(path = virPCIFile(dev->name, "driver_override"))) return -1; @@ -1149,26 +1126,22 @@ virPCIDeviceBindWithDriverOverride(virPCIDevicePtr dev, _("Failed to add driver '%s' to driver_override " " interface of PCI device '%s'"), driverName, dev->name); - goto cleanup; + return -1; } if (virPCIDeviceRebind(dev) < 0) - goto cleanup; + return -1; - ret = 0; - - cleanup: - VIR_FREE(path); - return ret; + return 0; } static int virPCIDeviceUnbindFromStubWithNewid(virPCIDevicePtr dev) { int result = -1; - char *drvdir = NULL; - char *path = NULL; - char *driver = NULL; + VIR_AUTOFREE(char *) drvdir = NULL; + VIR_AUTOFREE(char *) path = NULL; + VIR_AUTOFREE(char *) driver = NULL; /* If the device is currently bound to one of the "well known" * stub drivers, then unbind it, otherwise ignore it. @@ -1257,10 +1230,6 @@ virPCIDeviceUnbindFromStubWithNewid(virPCIDevicePtr dev) dev->remove_slot = false; dev->reprobe = false; - VIR_FREE(drvdir); - VIR_FREE(path); - VIR_FREE(driver); - return result; } @@ -1278,8 +1247,7 @@ virPCIDeviceUnbindFromStubWithOverride(virPCIDevicePtr dev) static int virPCIDeviceUnbindFromStub(virPCIDevicePtr dev) { - int ret; - char *path; + VIR_AUTOFREE(char *) path = NULL; /* * Prefer using the device's driver_override interface, falling back @@ -1289,12 +1257,9 @@ virPCIDeviceUnbindFromStub(virPCIDevicePtr dev) return -1; if (virFileExists(path)) - ret = virPCIDeviceUnbindFromStubWithOverride(dev); + return virPCIDeviceUnbindFromStubWithOverride(dev); else - ret = virPCIDeviceUnbindFromStubWithNewid(dev); - - VIR_FREE(path); - return ret; + return virPCIDeviceUnbindFromStubWithNewid(dev); } static int @@ -1302,9 +1267,9 @@ virPCIDeviceBindToStubWithNewid(virPCIDevicePtr dev) { int result = -1; bool reprobe = false; - char *stubDriverPath = NULL; - char *driverLink = NULL; - char *path = NULL; /* reused for different purposes */ + VIR_AUTOFREE(char *) stubDriverPath = NULL; + VIR_AUTOFREE(char *) driverLink = NULL; + VIR_AUTOFREE(char *) path = NULL; /* reused for different purposes */ const char *stubDriverName = NULL; virErrorPtr err = NULL; @@ -1436,10 +1401,6 @@ virPCIDeviceBindToStubWithNewid(virPCIDevicePtr dev) } cleanup: - VIR_FREE(stubDriverPath); - VIR_FREE(driverLink); - VIR_FREE(path); - if (result < 0) virPCIDeviceUnbindFromStub(dev); @@ -1453,10 +1414,9 @@ virPCIDeviceBindToStubWithNewid(virPCIDevicePtr dev) static int virPCIDeviceBindToStubWithOverride(virPCIDevicePtr dev) { - int ret = -1; const char *stubDriverName; - char *stubDriverPath = NULL; - char *driverLink = NULL; + VIR_AUTOFREE(char *) stubDriverPath = NULL; + VIR_AUTOFREE(char *) driverLink = NULL; /* Check the device is configured to use one of the known stub drivers */ if (dev->stubDriver == VIR_PCI_STUB_DRIVER_NONE) { @@ -1473,35 +1433,28 @@ virPCIDeviceBindToStubWithOverride(virPCIDevicePtr dev) if (!(stubDriverPath = virPCIDriverDir(stubDriverName)) || !(driverLink = virPCIFile(dev->name, "driver"))) - goto cleanup; + return -1; if (virFileExists(driverLink)) { if (virFileLinkPointsTo(driverLink, stubDriverPath)) { /* The device is already bound to the correct driver */ VIR_DEBUG("Device %s is already bound to %s", dev->name, stubDriverName); - ret = 0; - goto cleanup; + return 0; } } if (virPCIDeviceBindWithDriverOverride(dev, stubDriverName) < 0) - goto cleanup; + return -1; dev->unbind_from_stub = true; - ret = 0; - - cleanup: - VIR_FREE(stubDriverPath); - VIR_FREE(driverLink); - return ret; + return 0; } static int virPCIDeviceBindToStub(virPCIDevicePtr dev) { - int ret; - char *path; + VIR_AUTOFREE(char *) path = NULL; /* * Prefer using the device's driver_override interface, falling back @@ -1511,12 +1464,9 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev) return -1; if (virFileExists(path)) - ret = virPCIDeviceBindToStubWithOverride(dev); + return virPCIDeviceBindToStubWithOverride(dev); else - ret = virPCIDeviceBindToStubWithNewid(dev); - - VIR_FREE(path); - return ret; + return virPCIDeviceBindToStubWithNewid(dev); } /* virPCIDeviceDetach: @@ -1700,19 +1650,15 @@ virPCIDeviceWaitForCleanup(virPCIDevicePtr dev, const char *matcher) static char * virPCIDeviceReadID(virPCIDevicePtr dev, const char *id_name) { - char *path = NULL; + VIR_AUTOFREE(char *) path = NULL; char *id_str; if (!(path = virPCIFile(dev->name, id_name))) return NULL; /* ID string is '0xNNNN\n' ... i.e. 7 bytes */ - if (virFileReadAll(path, 7, &id_str) < 0) { - VIR_FREE(path); + if (virFileReadAll(path, 7, &id_str) < 0) return NULL; - } - - VIR_FREE(path); /* Check for 0x suffix */ if (id_str[0] != '0' || id_str[1] != 'x') { @@ -1755,8 +1701,8 @@ virPCIDeviceNew(unsigned int domain, unsigned int function) { virPCIDevicePtr dev; - char *vendor = NULL; - char *product = NULL; + VIR_AUTOFREE(char *) vendor = NULL; + VIR_AUTOFREE(char *) product = NULL; if (VIR_ALLOC(dev) < 0) return NULL; @@ -1805,15 +1751,11 @@ virPCIDeviceNew(unsigned int domain, VIR_DEBUG("%s %s: initialized", dev->id, dev->name); - cleanup: - VIR_FREE(product); - VIR_FREE(vendor); return dev; error: virPCIDeviceFree(dev); - dev = NULL; - goto cleanup; + return NULL; } @@ -2129,8 +2071,7 @@ int virPCIDeviceFileIterate(virPCIDevicePtr dev, virPCIDeviceFileActor actor, void *opaque) { - char *pcidir = NULL; - char *file = NULL; + VIR_AUTOFREE(char *) pcidir = NULL; DIR *dir = NULL; int ret = -1; struct dirent *ent; @@ -2145,6 +2086,7 @@ int virPCIDeviceFileIterate(virPCIDevicePtr dev, goto cleanup; while ((direrr = virDirRead(dir, &ent, pcidir)) > 0) { + VIR_AUTOFREE(char *) file = NULL; /* Device assignment requires: * $PCIDIR/config, $PCIDIR/resource, $PCIDIR/resourceNNN, * $PCIDIR/rom, $PCIDIR/reset, $PCIDIR/vendor, $PCIDIR/device @@ -2159,8 +2101,6 @@ int virPCIDeviceFileIterate(virPCIDevicePtr dev, goto cleanup; if ((actor)(dev, file, opaque) < 0) goto cleanup; - - VIR_FREE(file); } } if (direrr < 0) @@ -2170,8 +2110,6 @@ int virPCIDeviceFileIterate(virPCIDevicePtr dev, cleanup: VIR_DIR_CLOSE(dir); - VIR_FREE(file); - VIR_FREE(pcidir); return ret; } @@ -2186,7 +2124,7 @@ virPCIDeviceAddressIOMMUGroupIterate(virPCIDeviceAddressPtr orig, virPCIDeviceAddressActor actor, void *opaque) { - char *groupPath = NULL; + VIR_AUTOFREE(char *) groupPath = NULL; DIR *groupDir = NULL; int ret = -1; struct dirent *ent; @@ -2222,7 +2160,6 @@ virPCIDeviceAddressIOMMUGroupIterate(virPCIDeviceAddressPtr orig, ret = 0; cleanup: - VIR_FREE(groupPath); VIR_DIR_CLOSE(groupDir); return ret; } @@ -2341,28 +2278,25 @@ virPCIDeviceAddressGetIOMMUGroupAddresses(virPCIDeviceAddressPtr devAddr, int virPCIDeviceAddressGetIOMMUGroupNum(virPCIDeviceAddressPtr addr) { - char *devName = NULL; - char *devPath = NULL; - char *groupPath = NULL; + VIR_AUTOFREE(char *) devName = NULL; + VIR_AUTOFREE(char *) devPath = NULL; + VIR_AUTOFREE(char *) groupPath = NULL; const char *groupNumStr; unsigned int groupNum; - int ret = -1; if (virAsprintf(&devName, "%.4x:%.2x:%.2x.%.1x", addr->domain, addr->bus, addr->slot, addr->function) < 0) - goto cleanup; + return -1; if (!(devPath = virPCIFile(devName, "iommu_group"))) - goto cleanup; - if (virFileIsLink(devPath) != 1) { - ret = -2; - goto cleanup; - } + return -1; + if (virFileIsLink(devPath) != 1) + return -2; if (virFileResolveLink(devPath, &groupPath) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to resolve device %s iommu_group symlink %s"), devName, devPath); - goto cleanup; + return -1; } groupNumStr = last_component(groupPath); @@ -2371,16 +2305,10 @@ virPCIDeviceAddressGetIOMMUGroupNum(virPCIDeviceAddressPtr addr) _("device %s iommu_group symlink %s has " "invalid group number %s"), devName, groupPath, groupNumStr); - ret = -1; - goto cleanup; + return -1; } - ret = groupNum; - cleanup: - VIR_FREE(devName); - VIR_FREE(devPath); - VIR_FREE(groupPath); - return ret; + return groupNum; } @@ -2390,30 +2318,28 @@ virPCIDeviceAddressGetIOMMUGroupNum(virPCIDeviceAddressPtr addr) char * virPCIDeviceGetIOMMUGroupDev(virPCIDevicePtr dev) { - char *devPath = NULL; - char *groupPath = NULL; + VIR_AUTOFREE(char *) devPath = NULL; + VIR_AUTOFREE(char *) groupPath = NULL; char *groupDev = NULL; if (!(devPath = virPCIFile(dev->name, "iommu_group"))) - goto cleanup; + return NULL; if (virFileIsLink(devPath) != 1) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid device %s iommu_group file %s is not a symlink"), dev->name, devPath); - goto cleanup; + return NULL; } if (virFileResolveLink(devPath, &groupPath) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to resolve device %s iommu_group symlink %s"), dev->name, devPath); - goto cleanup; + return NULL; } if (virAsprintf(&groupDev, "/dev/vfio/%s", last_component(groupPath)) < 0) - goto cleanup; - cleanup: - VIR_FREE(devPath); - VIR_FREE(groupPath); + return NULL; + return groupDev; } @@ -2614,7 +2540,7 @@ virPCIGetDeviceAddressFromSysfsLink(const char *device_link) { virPCIDeviceAddressPtr bdf = NULL; char *config_address = NULL; - char *device_path = NULL; + VIR_AUTOFREE(char *) device_path = NULL; if (!virFileExists(device_link)) { VIR_DEBUG("'%s' does not exist", device_link); @@ -2631,19 +2557,16 @@ virPCIGetDeviceAddressFromSysfsLink(const char *device_link) config_address = last_component(device_path); if (VIR_ALLOC(bdf) < 0) - goto out; + return NULL; if (virPCIDeviceAddressParse(config_address, bdf) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to parse PCI config address '%s'"), config_address); VIR_FREE(bdf); - goto out; + return NULL; } - out: - VIR_FREE(device_path); - return bdf; } @@ -2665,7 +2588,7 @@ int virPCIGetPhysicalFunction(const char *vf_sysfs_path, virPCIDeviceAddressPtr *pf) { - char *device_link = NULL; + VIR_AUTOFREE(char *) device_link = NULL; *pf = NULL; @@ -2678,7 +2601,6 @@ virPCIGetPhysicalFunction(const char *vf_sysfs_path, VIR_DEBUG("PF for VF device '%s': %.4x:%.2x:%.2x.%.1x", vf_sysfs_path, (*pf)->domain, (*pf)->bus, (*pf)->slot, (*pf)->function); } - VIR_FREE(device_link); return 0; } @@ -2695,9 +2617,9 @@ virPCIGetVirtualFunctions(const char *sysfs_path, { int ret = -1; size_t i; - char *device_link = NULL; + VIR_AUTOFREE(char *) totalvfs_file = NULL; + VIR_AUTOFREE(char *) totalvfs_str = NULL; virPCIDeviceAddressPtr config_addr = NULL; - char *totalvfs_file = NULL, *totalvfs_str = NULL; *virtual_functions = NULL; *num_virtual_functions = 0; @@ -2719,6 +2641,7 @@ virPCIGetVirtualFunctions(const char *sysfs_path, } do { + VIR_AUTOFREE(char *) device_link = NULL; /* look for virtfn%d links until one isn't found */ if (virAsprintf(&device_link, "%s/virtfn%zu", sysfs_path, *num_virtual_functions) < 0) goto error; @@ -2736,18 +2659,13 @@ virPCIGetVirtualFunctions(const char *sysfs_path, if (VIR_APPEND_ELEMENT(*virtual_functions, *num_virtual_functions, config_addr) < 0) goto error; - VIR_FREE(device_link); - } while (1); VIR_DEBUG("Found %zu virtual functions for %s", *num_virtual_functions, sysfs_path); ret = 0; cleanup: - VIR_FREE(device_link); VIR_FREE(config_addr); - VIR_FREE(totalvfs_file); - VIR_FREE(totalvfs_str); return ret; error: @@ -2765,18 +2683,13 @@ virPCIGetVirtualFunctions(const char *sysfs_path, int virPCIIsVirtualFunction(const char *vf_sysfs_device_link) { - char *vf_sysfs_physfn_link = NULL; - int ret = -1; + VIR_AUTOFREE(char *) vf_sysfs_physfn_link = NULL; if (virAsprintf(&vf_sysfs_physfn_link, "%s/physfn", vf_sysfs_device_link) < 0) - return ret; + return -1; - ret = virFileExists(vf_sysfs_physfn_link); - - VIR_FREE(vf_sysfs_physfn_link); - - return ret; + return virFileExists(vf_sysfs_physfn_link); } /* @@ -2868,12 +2781,12 @@ virPCIGetNetName(const char *device_link_sysfs_path, char *physPortID, char **netname) { - char *pcidev_sysfs_net_path = NULL; + VIR_AUTOFREE(char *) pcidev_sysfs_net_path = NULL; + VIR_AUTOFREE(char *) firstEntryName = NULL; + VIR_AUTOFREE(char *) thisPhysPortID = NULL; int ret = -1; DIR *dir = NULL; struct dirent *entry = NULL; - char *firstEntryName = NULL; - char *thisPhysPortID = NULL; size_t i = 0; if (virBuildPath(&pcidev_sysfs_net_path, device_link_sysfs_path, @@ -2946,9 +2859,6 @@ virPCIGetNetName(const char *device_link_sysfs_path, } cleanup: VIR_DIR_CLOSE(dir); - VIR_FREE(pcidev_sysfs_net_path); - VIR_FREE(thisPhysPortID); - VIR_FREE(firstEntryName); return ret; } @@ -2959,9 +2869,9 @@ virPCIGetVirtualFunctionInfo(const char *vf_sysfs_device_path, int *vf_index) { virPCIDeviceAddressPtr pf_config_address = NULL; - char *pf_sysfs_device_path = NULL; - char *vfname = NULL; - char *vfPhysPortID = NULL; + VIR_AUTOFREE(char *) pf_sysfs_device_path = NULL; + VIR_AUTOFREE(char *) vfname = NULL; + VIR_AUTOFREE(char *) vfPhysPortID = NULL; int ret = -1; if (virPCIGetPhysicalFunction(vf_sysfs_device_path, &pf_config_address) < 0) @@ -3016,9 +2926,6 @@ virPCIGetVirtualFunctionInfo(const char *vf_sysfs_device_path, ret = 0; cleanup: VIR_FREE(pf_config_address); - VIR_FREE(pf_sysfs_device_path); - VIR_FREE(vfname); - VIR_FREE(vfPhysPortID); return ret; } @@ -3032,8 +2939,7 @@ virPCIGetMdevTypes(const char *sysfspath, int dirret = -1; DIR *dir = NULL; struct dirent *entry; - char *types_path = NULL; - char *tmppath = NULL; + VIR_AUTOFREE(char *) types_path = NULL; virMediatedDeviceTypePtr mdev_type = NULL; virMediatedDeviceTypePtr *mdev_types = NULL; size_t ntypes = 0; @@ -3051,6 +2957,7 @@ virPCIGetMdevTypes(const char *sysfspath, } while ((dirret = virDirRead(dir, &entry, types_path)) > 0) { + VIR_AUTOFREE(char *) tmppath = NULL; /* append the type id to the path and read the attributes from there */ if (virAsprintf(&tmppath, "%s/%s", types_path, entry->d_name) < 0) goto cleanup; @@ -3060,8 +2967,6 @@ virPCIGetMdevTypes(const char *sysfspath, if (VIR_APPEND_ELEMENT(mdev_types, ntypes, mdev_type) < 0) goto cleanup; - - VIR_FREE(tmppath); } if (dirret < 0) @@ -3075,8 +2980,6 @@ virPCIGetMdevTypes(const char *sysfspath, for (i = 0; i < ntypes; i++) virMediatedDeviceTypeFree(mdev_types[i]); VIR_FREE(mdev_types); - VIR_FREE(types_path); - VIR_FREE(tmppath); VIR_DIR_CLOSE(dir); return ret; } -- 1.8.3.1