Re: [libvirt] [PATCH v3 07/11] util: netdev: use VIR_AUTOFREE instead of VIR_FREE for scalar types
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
> ---
> 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(, "%lld", (long long) pidInNs) == -1)
> return -1;
>
> /* The 802.11 wireless devices only move together with their PHY. */
> if (virNetDevSysfsFile(_path, ifname, "phy80211/name") < 0)
> -goto cleanup;
> +return -1;
>
> if ((len = virFileReadAllQuiet(phy_path, 1024, )) <= 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, , 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(_link, "%s/subsystem", devpath) < 0)
> return false;
>
> if (!virFileExists(subsys_link))
> -goto cleanup;
> +return false;
>
> if (virFileResolveLink(subsys_link, _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(, 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(, ifname, "phys_port_id") < 0)
> -goto cleanup;
> +return -1;
>
> /* a failure to read just means the driver
[libvirt] [PATCH v3 07/11] util: netdev: 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
---
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(, "%lld", (long long) pidInNs) == -1)
return -1;
/* The 802.11 wireless devices only move together with their PHY. */
if (virNetDevSysfsFile(_path, ifname, "phy80211/name") < 0)
-goto cleanup;
+return -1;
if ((len = virFileReadAllQuiet(phy_path, 1024, )) <= 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, , 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(_link, "%s/subsystem", devpath) < 0)
return false;
if (!virFileExists(subsys_link))
-goto cleanup;
+return false;
if (virFileResolveLink(subsys_link, _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(, 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(, 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);
-
