On Thu, Aug 09, 2018 at 09:42:15AM +0530, Sukrit Bhatnagar wrote:
> 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@xxxxxxxxx>
> ---
> src/util/virnetdev.c | 343 +++++++++++++++++++--------------------------------
> 1 file changed, 125 insertions(+), 218 deletions(-)
>
> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
> index 8eac419..edb7393 100644
> --- a/src/util/virnetdev.c
> +++ b/src/util/virnetdev.c
> @@ -535,18 +535,17 @@ int virNetDevSetMTUFromDevice(const char *ifname,
> */
> int virNetDevSetNamespace(const char *ifname, pid_t pidInNs)
> {
> - int ret = -1;
> - char *pid = NULL;
> - char *phy = NULL;
> - char *phy_path = NULL;
> int len;
> + VIR_AUTOFREE(char *) pid = NULL;
> + VIR_AUTOFREE(char *) phy = NULL;
> + VIR_AUTOFREE(char *) phy_path = NULL;
>
> if (virAsprintf(&pid, "%lld", (long long) pidInNs) == -1)
> return -1;
>
> /* The 802.11 wireless devices only move together with their PHY. */
> if (virNetDevSysfsFile(&phy_path, ifname, "phy80211/name") < 0)
> - goto cleanup;
> + return -1;
>
> if ((len = virFileReadAllQuiet(phy_path, 1024, &phy)) <= 0) {
> /* Not a wireless device. */
> @@ -556,7 +555,7 @@ int virNetDevSetNamespace(const char *ifname, pid_t pidInNs)
>
> argv[5] = pid;
> if (virRun(argv, NULL) < 0)
> - goto cleanup;
> + return -1;
>
> } else {
> const char *argv[] = {
> @@ -569,15 +568,10 @@ int virNetDevSetNamespace(const char *ifname, pid_t pidInNs)
> argv[2] = phy;
> argv[5] = pid;
> if (virRun(argv, NULL) < 0)
> - goto cleanup;
> + return -1;
> }
>
> - ret = 0;
> - cleanup:
> - VIR_FREE(phy_path);
> - VIR_FREE(phy);
> - VIR_FREE(pid);
> - return ret;
> + return 0;
> }
>
> #if defined(SIOCSIFNAME) && defined(HAVE_STRUCT_IFREQ)
> @@ -969,25 +963,21 @@ int virNetDevGetIndex(const char *ifname ATTRIBUTE_UNUSED,
> int
> virNetDevGetMaster(const char *ifname, char **master)
> {
> - int ret = -1;
> - void *nlData = NULL;
> struct nlattr *tb[IFLA_MAX + 1] = {NULL, };
> + VIR_AUTOFREE(void *) nlData = NULL;
>
> *master = NULL;
>
> if (virNetlinkDumpLink(ifname, -1, &nlData, tb, 0, 0) < 0)
> - goto cleanup;
> + return -1;
>
> if (tb[IFLA_MASTER]) {
> if (!(*master = virNetDevGetName(*(int *)RTA_DATA(tb[IFLA_MASTER]))))
> - goto cleanup;
> + return -1;
> }
>
> VIR_DEBUG("IFLA_MASTER for %s is %s", ifname, *master ? *master : "(none)");
> - ret = 0;
> - cleanup:
> - VIR_FREE(nlData);
> - return ret;
> + return 0;
> }
>
>
> @@ -1168,39 +1158,33 @@ virNetDevSysfsDeviceFile(char **pf_sysfs_device_link, const char *ifname,
> static bool
> virNetDevIsPCIDevice(const char *devpath)
> {
> - char *subsys_link = NULL;
> - char *abs_path = NULL;
> char *subsys = NULL;
> - bool ret = false;
> + VIR_AUTOFREE(char *) subsys_link = NULL;
> + VIR_AUTOFREE(char *) abs_path = NULL;
>
> if (virAsprintf(&subsys_link, "%s/subsystem", devpath) < 0)
> return false;
>
> if (!virFileExists(subsys_link))
> - goto cleanup;
> + return false;
>
> if (virFileResolveLink(subsys_link, &abs_path) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Unable to resolve device subsystem symlink %s"),
> subsys_link);
> - goto cleanup;
> + return false;
> }
>
> subsys = last_component(abs_path);
> - ret = STRPREFIX(subsys, "pci");
> -
> - cleanup:
> - VIR_FREE(subsys_link);
> - VIR_FREE(abs_path);
> - return ret;
> + return STRPREFIX(subsys, "pci");
> }
>
> static virPCIDevicePtr
> virNetDevGetPCIDevice(const char *devName)
> {
> - char *vfSysfsDevicePath = NULL;
> virPCIDeviceAddressPtr vfPCIAddr = NULL;
> virPCIDevicePtr vfPCIDevice = NULL;
> + VIR_AUTOFREE(char *) vfSysfsDevicePath = NULL;
>
> if (virNetDevSysfsFile(&vfSysfsDevicePath, devName, "device") < 0)
> goto cleanup;
> @@ -1216,7 +1200,6 @@ virNetDevGetPCIDevice(const char *devName)
> vfPCIAddr->slot, vfPCIAddr->function);
>
> cleanup:
> - VIR_FREE(vfSysfsDevicePath);
> VIR_FREE(vfPCIAddr);
>
> return vfPCIDevice;
> @@ -1241,25 +1224,20 @@ int
> virNetDevGetPhysPortID(const char *ifname,
> char **physPortID)
> {
> - int ret = -1;
> - char *physPortIDFile = NULL;
> + VIR_AUTOFREE(char *) physPortIDFile = NULL;
>
> *physPortID = NULL;
>
> if (virNetDevSysfsFile(&physPortIDFile, ifname, "phys_port_id") < 0)
> - goto cleanup;
> + return -1;
>
> /* a failure to read just means the driver doesn't support
> - * phys_port_id, so set success now and ignore the return from
> - * virFileReadAllQuiet().
> + * phys_port_id, so ignore the return from virFileReadAllQuiet()
> + * and return success.
> */
> - ret = 0;
> -
> ignore_value(virFileReadAllQuiet(physPortIDFile, 1024, physPortID));
>
> - cleanup:
> - VIR_FREE(physPortIDFile);
> - return ret;
> + return 0;
> }
>
>
> @@ -1280,67 +1258,61 @@ virNetDevGetVirtualFunctions(const char *pfname,
> size_t *n_vfname,
> unsigned int *max_vfs)
> {
> - int ret = -1;
> size_t i;
> - char *pf_sysfs_device_link = NULL;
> - char *pci_sysfs_device_link = NULL;
> - char *pciConfigAddr = NULL;
> - char *pfPhysPortID = NULL;
> + VIR_AUTOFREE(char *) pf_sysfs_device_link = NULL;
> + VIR_AUTOFREE(char *) pci_sysfs_device_link = NULL;
> + VIR_AUTOFREE(char *) pciConfigAddr = NULL;
> + VIR_AUTOFREE(char *) pfPhysPortID = NULL;
> + VIR_AUTOFREE(char **) tmpVfname = NULL;
^this should be virString and come with the next patch.
> + VIR_AUTOFREE(virPCIDeviceAddressPtr *) tmpVirtFns = NULL;
We're not controlling external types where we'd have to typedef significantly,
but this our internal type and per Andrea's comments in the last version, this
should get its own wrapper and thus be used with VIR_AUTOPTR.
I haven't found any other issues.
Erik
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
