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@gmail.com> --- src/util/viriscsi.c | 31 ++++++++++--------------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c index 653b4fd..48f4106 100644 --- a/src/util/viriscsi.c +++ b/src/util/viriscsi.c @@ -88,7 +88,7 @@ virISCSIGetSession(const char *devpath, .session = NULL, .devpath = devpath, }; - char *error = NULL; + VIR_AUTOFREE(char *) error = NULL; int exitstatus = 0; virCommandPtr cmd = virCommandNewArgList(ISCSIADM, "--mode", @@ -109,7 +109,6 @@ virISCSIGetSession(const char *devpath, NULLSTR(error)); cleanup: - VIR_FREE(error); virCommandFree(cmd); return cbdata.session; } @@ -125,10 +124,10 @@ virStorageBackendIQNFound(const char *initiatoriqn, char **ifacename) { int ret = IQN_ERROR; - char *outbuf = NULL; char *line = NULL; - char *iface = NULL; - char *iqn = NULL; + VIR_AUTOFREE(char *) outbuf = NULL; + VIR_AUTOFREE(char *) iface = NULL; + VIR_AUTOFREE(char *) iqn = NULL; virCommandPtr cmd = virCommandNewArgList(ISCSIADM, "--mode", "iface", NULL); @@ -197,9 +196,6 @@ virStorageBackendIQNFound(const char *initiatoriqn, if (ret == IQN_MISSING) VIR_DEBUG("Could not find interface with IQN '%s'", iqn); - VIR_FREE(iqn); - VIR_FREE(iface); - VIR_FREE(outbuf); virCommandFree(cmd); return ret; @@ -216,7 +212,7 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn, char **ifacename) { int ret = -1, exitstatus = -1; - char *temp_ifacename; + VIR_AUTOFREE(char *) temp_ifacename = NULL; virCommandPtr cmd = NULL; if (virAsprintf(&temp_ifacename, @@ -277,7 +273,6 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn, cleanup: virCommandFree(cmd); - VIR_FREE(temp_ifacename); if (ret != 0) VIR_FREE(*ifacename); return ret; @@ -299,7 +294,7 @@ virISCSIConnection(const char *portal, NULL }; virCommandPtr cmd; - char *ifacename = NULL; + VIR_AUTOFREE(char *) ifacename = NULL; cmd = virCommandNewArgs(baseargv); virCommandAddArgSet(cmd, extraargv); @@ -339,7 +334,6 @@ virISCSIConnection(const char *portal, cleanup: virCommandFree(cmd); - VIR_FREE(ifacename); return ret; } @@ -390,15 +384,13 @@ virISCSIGetTargets(char **const groups, void *data) { struct virISCSITargetList *list = data; - char *target; + VIR_AUTOFREE(char *) target = NULL; if (VIR_STRDUP(target, groups[1]) < 0) return -1; - if (VIR_APPEND_ELEMENT(list->targets, list->ntargets, target) < 0) { - VIR_FREE(target); + if (VIR_APPEND_ELEMENT(list->targets, list->ntargets, target) < 0) return -1; - } return 0; } @@ -498,8 +490,7 @@ virISCSIScanTargets(const char *portal, size_t *ntargets, char ***targets) { - char *ifacename = NULL; - int ret = -1; + VIR_AUTOFREE(char *) ifacename = NULL; if (ntargets) *ntargets = 0; @@ -522,10 +513,8 @@ virISCSIScanTargets(const char *portal, } } - ret = virISCSIScanTargetsInternal(portal, ifacename, + return virISCSIScanTargetsInternal(portal, ifacename, persist, ntargets, targets); - VIR_FREE(ifacename); - return ret; } -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- src/util/viriscsi.c | 68 +++++++++++++++++------------------------------------ 1 file changed, 22 insertions(+), 46 deletions(-) diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c index 48f4106..81404f8 100644 --- a/src/util/viriscsi.c +++ b/src/util/viriscsi.c @@ -91,7 +91,7 @@ virISCSIGetSession(const char *devpath, VIR_AUTOFREE(char *) error = NULL; int exitstatus = 0; - virCommandPtr cmd = virCommandNewArgList(ISCSIADM, "--mode", + VIR_AUTOPTR(virCommand) cmd = virCommandNewArgList(ISCSIADM, "--mode", "session", NULL); virCommandSetErrorBuffer(cmd, &error); @@ -101,15 +101,13 @@ virISCSIGetSession(const char *devpath, vars, virISCSIExtractSession, &cbdata, NULL, &exitstatus) < 0) - goto cleanup; + return NULL; if (cbdata.session == NULL && !probe) virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find iscsiadm session: %s"), NULLSTR(error)); - cleanup: - virCommandFree(cmd); return cbdata.session; } @@ -128,7 +126,7 @@ virStorageBackendIQNFound(const char *initiatoriqn, VIR_AUTOFREE(char *) outbuf = NULL; VIR_AUTOFREE(char *) iface = NULL; VIR_AUTOFREE(char *) iqn = NULL; - virCommandPtr cmd = virCommandNewArgList(ISCSIADM, + VIR_AUTOPTR(virCommand) cmd = virCommandNewArgList(ISCSIADM, "--mode", "iface", NULL); *ifacename = NULL; @@ -196,7 +194,6 @@ virStorageBackendIQNFound(const char *initiatoriqn, if (ret == IQN_MISSING) VIR_DEBUG("Could not find interface with IQN '%s'", iqn); - virCommandFree(cmd); return ret; error: @@ -213,7 +210,7 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn, { int ret = -1, exitstatus = -1; VIR_AUTOFREE(char *) temp_ifacename = NULL; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; if (virAsprintf(&temp_ifacename, "libvirt-iface-%08llx", @@ -272,7 +269,6 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn, ret = 0; cleanup: - virCommandFree(cmd); if (ret != 0) VIR_FREE(*ifacename); return ret; @@ -285,7 +281,6 @@ virISCSIConnection(const char *portal, const char *target, const char **extraargv) { - int ret = -1; const char *const baseargv[] = { ISCSIADM, "--mode", "node", @@ -293,7 +288,7 @@ virISCSIConnection(const char *portal, "--targetname", target, NULL }; - virCommandPtr cmd; + VIR_AUTOPTR(virCommand) cmd = NULL; VIR_AUTOFREE(char *) ifacename = NULL; cmd = virCommandNewArgs(baseargv); @@ -306,7 +301,7 @@ virISCSIConnection(const char *portal, break; case IQN_MISSING: if (virStorageBackendCreateIfaceIQN(initiatoriqn, &ifacename) != 0) - goto cleanup; + return -1; /* * iscsiadm doesn't let you send commands to the Interface IQN, * unless you've first issued a 'sendtargets' command to the @@ -317,25 +312,20 @@ virISCSIConnection(const char *portal, */ if (virISCSIScanTargetsInternal(portal, ifacename, true, NULL, NULL) < 0) - goto cleanup; + return -1; break; case IQN_ERROR: default: - goto cleanup; + return -1; } virCommandAddArgList(cmd, "--interface", ifacename, NULL); } if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return -1; - ret = 0; - - cleanup: - virCommandFree(cmd); - - return ret; + return 0; } @@ -362,14 +352,12 @@ virISCSIConnectionLogout(const char *portal, int virISCSIRescanLUNs(const char *session) { - virCommandPtr cmd = virCommandNewArgList(ISCSIADM, + VIR_AUTOPTR(virCommand) cmd = virCommandNewArgList(ISCSIADM, "--mode", "session", "-r", session, "-R", NULL); - int ret = virCommandRun(cmd, NULL); - virCommandFree(cmd); - return ret; + return virCommandRun(cmd, NULL); } @@ -419,8 +407,7 @@ virISCSIScanTargetsInternal(const char *portal, int vars[] = { 2 }; struct virISCSITargetList list; size_t i; - int ret = -1; - virCommandPtr cmd = virCommandNewArgList(ISCSIADM, + VIR_AUTOPTR(virCommand) cmd = virCommandNewArgList(ISCSIADM, "--mode", "discovery", "--type", "sendtargets", "--portal", portal, @@ -446,7 +433,7 @@ virISCSIScanTargetsInternal(const char *portal, vars, virISCSIGetTargets, &list, NULL, NULL) < 0) - goto cleanup; + return -1; if (ntargetsret && targetsret) { *ntargetsret = list.ntargets; @@ -457,10 +444,7 @@ virISCSIScanTargetsInternal(const char *portal, VIR_FREE(list.targets); } - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; + return 0; } @@ -538,9 +522,8 @@ int virISCSINodeNew(const char *portal, const char *target) { - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; int status; - int ret = -1; cmd = virCommandNewArgList(ISCSIADM, "--mode", "node", @@ -553,20 +536,17 @@ virISCSINodeNew(const char *portal, virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed new node mode for target '%s'"), target); - goto cleanup; + return -1; } if (status != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("%s failed new mode for target '%s' with status '%d'"), ISCSIADM, target, status); - goto cleanup; + return -1; } - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; + return 0; } @@ -576,9 +556,8 @@ virISCSINodeUpdate(const char *portal, const char *name, const char *value) { - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; int status; - int ret = -1; cmd = virCommandNewArgList(ISCSIADM, "--mode", "node", @@ -594,11 +573,8 @@ virISCSINodeUpdate(const char *portal, virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to update '%s' of node mode for target '%s'"), name, target); - goto cleanup; + return -1; } - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; + return 0; } -- 1.8.3.1

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@gmail.com> --- src/util/virnetdevbridge.c | 45 +++++++++++++++------------------------------ 1 file changed, 15 insertions(+), 30 deletions(-) diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index e46ac35..bf30d7c 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -126,8 +126,7 @@ static int virNetDevBridgeSet(const char *brname, int fd, /* control socket */ struct ifreq *ifr) /* pre-filled bridge name */ { - char *path = NULL; - int ret = -1; + VIR_AUTOFREE(char *) path = NULL; if (virAsprintf(&path, SYSFS_NET_DIR "%s/bridge/%s", brname, paramname) < 0) return -1; @@ -138,7 +137,7 @@ static int virNetDevBridgeSet(const char *brname, if (virFileWriteStr(path, valuestr, 0) < 0) { virReportSystemError(errno, _("Unable to set bridge %s %s"), brname, paramname); - goto cleanup; + return -1; } } else { unsigned long paramid; @@ -149,21 +148,18 @@ static int virNetDevBridgeSet(const char *brname, } else { virReportSystemError(EINVAL, _("Unable to set bridge %s %s"), brname, paramname); - goto cleanup; + return -1; } unsigned long args[] = { paramid, value, 0, 0 }; ifr->ifr_data = (char*)&args; if (ioctl(fd, SIOCDEVPRIVATE, ifr) < 0) { virReportSystemError(errno, _("Unable to set bridge %s %s"), brname, paramname); - goto cleanup; + return -1; } } - ret = 0; - cleanup: - VIR_FREE(path); - return ret; + return 0; } @@ -171,7 +167,7 @@ static int virNetDevBridgeGet(const char *brname, const char *paramname, /* sysfs param name */ unsigned long *value) /* current value */ { - char *path = NULL; + VIR_AUTOFREE(char *) path = NULL; int ret = -1; int fd = -1; struct ifreq ifr; @@ -180,7 +176,7 @@ static int virNetDevBridgeGet(const char *brname, return -1; if (virFileExists(path)) { - char *valuestr; + VIR_AUTOFREE(char *) valuestr = NULL; if (virFileReadAll(path, INT_BUFSIZE_BOUND(unsigned long), &valuestr) < 0) goto cleanup; @@ -189,10 +185,8 @@ static int virNetDevBridgeGet(const char *brname, virReportSystemError(EINVAL, _("Unable to get bridge %s %s"), brname, paramname); - VIR_FREE(valuestr); goto cleanup; } - VIR_FREE(valuestr); } else { struct __bridge_info info; unsigned long args[] = { BRCTL_GET_BRIDGE_INFO, (unsigned long)&info, 0, 0 }; @@ -221,7 +215,6 @@ static int virNetDevBridgeGet(const char *brname, ret = 0; cleanup: VIR_FORCE_CLOSE(fd); - VIR_FREE(path); return ret; } #endif /* __linux__ */ @@ -233,7 +226,7 @@ virNetDevBridgePortSet(const char *brname, const char *paramname, unsigned long value) { - char *path = NULL; + VIR_AUTOFREE(char *) path = NULL; char valuestr[INT_BUFSIZE_BOUND(value)]; int ret = -1; @@ -254,7 +247,6 @@ virNetDevBridgePortSet(const char *brname, brname, ifname, paramname, valuestr); } - VIR_FREE(path); return ret; } @@ -265,29 +257,24 @@ virNetDevBridgePortGet(const char *brname, const char *paramname, unsigned long *value) { - char *path = NULL; - char *valuestr = NULL; - int ret = -1; + VIR_AUTOFREE(char *) path = NULL; + VIR_AUTOFREE(char *) valuestr = NULL; if (virAsprintf(&path, SYSFS_NET_DIR "%s/brif/%s/%s", brname, ifname, paramname) < 0) return -1; if (virFileReadAll(path, INT_BUFSIZE_BOUND(unsigned long), &valuestr) < 0) - goto cleanup; + return -1; if (virStrToLong_ul(valuestr, NULL, 10, value) < 0) { virReportSystemError(EINVAL, _("Unable to get bridge %s port %s %s"), brname, ifname, paramname); - goto cleanup; + return -1; } - ret = 0; - cleanup: - VIR_FREE(path); - VIR_FREE(valuestr); - return ret; + return 0; } @@ -430,7 +417,7 @@ virNetDevBridgeCreate(const char *brname) /* use a netlink RTM_NEWLINK message to create the bridge */ const char *type = "bridge"; int rc = -1; - struct nlmsghdr *resp = NULL; + VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; struct nlmsgerr *err; struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC }; unsigned int recvbuflen; @@ -495,7 +482,6 @@ virNetDevBridgeCreate(const char *brname) rc = 0; cleanup: nlmsg_free(nl_msg); - VIR_FREE(resp); return rc; malformed_resp: @@ -1069,7 +1055,7 @@ virNetDevBridgeFDBAddDel(const virMacAddr *mac, const char *ifname, unsigned int flags, bool isAdd) { int ret = -1; - struct nlmsghdr *resp = NULL; + VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; struct nlmsgerr *err; unsigned int recvbuflen; struct nl_msg *nl_msg; @@ -1142,7 +1128,6 @@ virNetDevBridgeFDBAddDel(const virMacAddr *mac, const char *ifname, ret = 0; cleanup: nlmsg_free(nl_msg); - VIR_FREE(resp); return ret; malformed_resp: -- 1.8.3.1

Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in src/util/viralloc.h, define a new wrapper around an existing cleanup function which will be called when a variable declared with VIR_AUTOPTR macro goes out of scope. Also, drop the redundant viralloc.h include, since that has moved from the source module into the header. When a variable of type virMacAddrPtr is declared using VIR_AUTOPTR, the function virMacAddrFree will be run automatically on it when it goes out of scope. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- src/util/virmacaddr.c | 6 ++++++ src/util/virmacaddr.h | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/src/util/virmacaddr.c b/src/util/virmacaddr.c index 7afe032..e739775 100644 --- a/src/util/virmacaddr.c +++ b/src/util/virmacaddr.c @@ -252,3 +252,9 @@ virMacAddrIsBroadcastRaw(const unsigned char s[VIR_MAC_BUFLEN]) { return memcmp(virMacAddrBroadcastAddrRaw, s, sizeof(*s)) == 0; } + +void +virMacAddrFree(virMacAddrPtr addr) +{ + VIR_FREE(addr); +} diff --git a/src/util/virmacaddr.h b/src/util/virmacaddr.h index d0dd4a4..39dd51b 100644 --- a/src/util/virmacaddr.h +++ b/src/util/virmacaddr.h @@ -25,6 +25,7 @@ # define __VIR_MACADDR_H__ # include "internal.h" +# include "viralloc.h" # define VIR_MAC_BUFLEN 6 # define VIR_MAC_HEXLEN (VIR_MAC_BUFLEN * 2) @@ -64,5 +65,8 @@ int virMacAddrParseHex(const char* str, bool virMacAddrIsUnicast(const virMacAddr *addr); bool virMacAddrIsMulticast(const virMacAddr *addr); bool virMacAddrIsBroadcastRaw(const unsigned char s[VIR_MAC_BUFLEN]); +void virMacAddrFree(virMacAddrPtr addr); + +VIR_DEFINE_AUTOPTR_FUNC(virMacAddr, virMacAddrFree) #endif /* __VIR_MACADDR_H__ */ -- 1.8.3.1

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@gmail.com> --- src/util/virnetdev.c | 276 ++++++++++++++++++--------------------------------- 1 file changed, 96 insertions(+), 180 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 9eca786..7653f8b 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -535,10 +535,9 @@ 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; + VIR_AUTOFREE(char *) pid = NULL; + VIR_AUTOFREE(char *) phy = NULL; + VIR_AUTOFREE(char *) phy_path = NULL; int len; if (virAsprintf(&pid, "%lld", (long long) pidInNs) == -1) @@ -546,7 +545,7 @@ int virNetDevSetNamespace(const char *ifname, pid_t pidInNs) /* 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; + VIR_AUTOFREE(void *) nlData = NULL; struct nlattr *tb[IFLA_MAX + 1] = {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,37 +1158,31 @@ virNetDevSysfsDeviceFile(char **pf_sysfs_device_link, const char *ifname, static bool virNetDevIsPCIDevice(const char *devpath) { - char *subsys_link = NULL; - char *abs_path = NULL; + VIR_AUTOFREE(char *) subsys_link = NULL; + VIR_AUTOFREE(char *) abs_path = NULL; char *subsys = NULL; - bool ret = false; 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; + VIR_AUTOFREE(char *) vfSysfsDevicePath = NULL; virPCIDeviceAddressPtr vfPCIAddr = NULL; virPCIDevicePtr vfPCIDevice = NULL; @@ -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; } @@ -1282,10 +1260,11 @@ virNetDevGetVirtualFunctions(const char *pfname, { 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 **) tempVfname = NULL; *virt_fns = NULL; *n_vfname = 0; @@ -1333,13 +1312,9 @@ virNetDevGetVirtualFunctions(const char *pfname, cleanup: if (ret < 0) { - VIR_FREE(*vfname); + VIR_STEAL_PTR(tempVfname, *vfname); VIR_FREE(*virt_fns); } - VIR_FREE(pfPhysPortID); - VIR_FREE(pf_sysfs_device_link); - VIR_FREE(pci_sysfs_device_link); - VIR_FREE(pciConfigAddr); return ret; } @@ -1355,17 +1330,12 @@ virNetDevGetVirtualFunctions(const char *pfname, int virNetDevIsVirtualFunction(const char *ifname) { - char *if_sysfs_device_link = NULL; - int ret = -1; + VIR_AUTOFREE(char *) if_sysfs_device_link = NULL; if (virNetDevSysfsFile(&if_sysfs_device_link, ifname, "device") < 0) - return ret; + return -1; - ret = virPCIIsVirtualFunction(if_sysfs_device_link); - - VIR_FREE(if_sysfs_device_link); - - return ret; + return virPCIIsVirtualFunction(if_sysfs_device_link); } /** @@ -1383,25 +1353,19 @@ int virNetDevGetVirtualFunctionIndex(const char *pfname, const char *vfname, int *vf_index) { - char *pf_sysfs_device_link = NULL, *vf_sysfs_device_link = NULL; - int ret = -1; + VIR_AUTOFREE(char *) pf_sysfs_device_link = NULL; + VIR_AUTOFREE(char *) vf_sysfs_device_link = NULL; if (virNetDevSysfsFile(&pf_sysfs_device_link, pfname, "device") < 0) - return ret; + return -1; - if (virNetDevSysfsFile(&vf_sysfs_device_link, vfname, "device") < 0) { - VIR_FREE(pf_sysfs_device_link); - return ret; - } + if (virNetDevSysfsFile(&vf_sysfs_device_link, vfname, "device") < 0) + return -1; - ret = virPCIGetVirtualFunctionIndex(pf_sysfs_device_link, + return virPCIGetVirtualFunctionIndex(pf_sysfs_device_link, vf_sysfs_device_link, vf_index); - VIR_FREE(pf_sysfs_device_link); - VIR_FREE(vf_sysfs_device_link); - - return ret; } /** @@ -1417,19 +1381,18 @@ virNetDevGetVirtualFunctionIndex(const char *pfname, const char *vfname, int virNetDevGetPhysicalFunction(const char *ifname, char **pfname) { - char *physfn_sysfs_path = NULL; - char *vfPhysPortID = NULL; - int ret = -1; + VIR_AUTOFREE(char *) physfn_sysfs_path = NULL; + VIR_AUTOFREE(char *) vfPhysPortID = NULL; if (virNetDevGetPhysPortID(ifname, &vfPhysPortID) < 0) - goto cleanup; + return -1; if (virNetDevSysfsDeviceFile(&physfn_sysfs_path, ifname, "physfn") < 0) - goto cleanup; + return -1; if (virPCIGetNetName(physfn_sysfs_path, 0, vfPhysPortID, pfname) < 0) { - goto cleanup; + return -1; } if (!*pfname) { @@ -1439,14 +1402,10 @@ virNetDevGetPhysicalFunction(const char *ifname, char **pfname) virReportError(VIR_ERR_INTERNAL_ERROR, _("The PF device for VF %s has no network device name"), ifname); - goto cleanup; + return -1; } - ret = 0; - cleanup: - VIR_FREE(vfPhysPortID); - VIR_FREE(physfn_sysfs_path); - return ret; + return 0; } @@ -1470,26 +1429,25 @@ virNetDevGetPhysicalFunction(const char *ifname, char **pfname) int virNetDevPFGetVF(const char *pfname, int vf, char **vfname) { - char *virtfnName = NULL; - char *virtfnSysfsPath = NULL; - char *pfPhysPortID = NULL; - int ret = -1; + VIR_AUTOFREE(char *) virtfnName = NULL; + VIR_AUTOFREE(char *) virtfnSysfsPath = NULL; + VIR_AUTOFREE(char *) pfPhysPortID = NULL; /* a VF may have multiple "ports", each one having its own netdev, * and each netdev having a different phys_port_id. Be sure we get * the VF netdev with a phys_port_id matchine that of pfname */ if (virNetDevGetPhysPortID(pfname, &pfPhysPortID) < 0) - goto cleanup; + return -1; if (virAsprintf(&virtfnName, "virtfn%d", vf) < 0) - goto cleanup; + return -1; /* this provides the path to the VF's directory in sysfs, * e.g. "/sys/class/net/enp2s0f0/virtfn3" */ if (virNetDevSysfsDeviceFile(&virtfnSysfsPath, pfname, virtfnName) < 0) - goto cleanup; + return -1; /* and this gets the netdev name associated with it, which is a * directory entry in [virtfnSysfsPath]/net, @@ -1498,14 +1456,7 @@ virNetDevPFGetVF(const char *pfname, int vf, char **vfname) * isn't bound to a netdev driver, it won't have a netdev name, * and vfname will be NULL). */ - ret = virPCIGetNetName(virtfnSysfsPath, 0, pfPhysPortID, vfname); - - cleanup: - VIR_FREE(virtfnName); - VIR_FREE(virtfnSysfsPath); - VIR_FREE(pfPhysPortID); - - return ret; + return virPCIGetNetName(virtfnSysfsPath, 0, pfPhysPortID, vfname); } @@ -1522,13 +1473,14 @@ int virNetDevGetVirtualFunctionInfo(const char *vfname, char **pfname, int *vf) { - char *pf_sysfs_path = NULL, *vf_sysfs_path = NULL; - int ret = -1; + VIR_AUTOFREE(char *) pf_sysfs_path = NULL; + VIR_AUTOFREE(char *) vf_sysfs_path = NULL; + VIR_AUTOFREE(char *) tempPfname = NULL; *pfname = NULL; if (virNetDevGetPhysicalFunction(vfname, pfname) < 0) - return ret; + return -1; if (virNetDevSysfsFile(&pf_sysfs_path, *pfname, "device") < 0) goto cleanup; @@ -1536,16 +1488,12 @@ virNetDevGetVirtualFunctionInfo(const char *vfname, char **pfname, if (virNetDevSysfsFile(&vf_sysfs_path, vfname, "device") < 0) goto cleanup; - ret = virPCIGetVirtualFunctionIndex(pf_sysfs_path, vf_sysfs_path, vf); + return virPCIGetVirtualFunctionIndex(pf_sysfs_path, vf_sysfs_path, vf); cleanup: - if (ret < 0) - VIR_FREE(*pfname); + VIR_STEAL_PTR(tempPfname, *pfname); - VIR_FREE(vf_sysfs_path); - VIR_FREE(pf_sysfs_path); - - return ret; + return -1; } #else /* !__linux__ */ @@ -1657,7 +1605,7 @@ virNetDevSetVfConfig(const char *ifname, int vf, { int rc = -1; char macstr[VIR_MAC_STRING_BUFLEN]; - struct nlmsghdr *resp = NULL; + VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; struct nlmsgerr *err; unsigned int recvbuflen = 0; struct nl_msg *nl_msg; @@ -1769,7 +1717,6 @@ virNetDevSetVfConfig(const char *ifname, int vf, vlanid, rc < 0 ? "Fail" : "Success"); nlmsg_free(nl_msg); - VIR_FREE(resp); return rc; malformed_resp: @@ -1843,19 +1790,15 @@ virNetDevGetVfConfig(const char *ifname, int vf, virMacAddrPtr mac, int *vlanid) { int rc = -1; - void *nlData = NULL; + VIR_AUTOFREE(void *) nlData = NULL; struct nlattr *tb[IFLA_MAX + 1] = {NULL, }; int ifindex = -1; rc = virNetlinkDumpLink(ifname, ifindex, &nlData, tb, 0, 0); if (rc < 0) - goto cleanup; + return rc; - rc = virNetDevParseVfConfig(tb, vf, mac, vlanid); - - cleanup: - VIR_FREE(nlData); - return rc; + return virNetDevParseVfConfig(tb, vf, mac, vlanid); } @@ -1914,13 +1857,13 @@ virNetDevSaveNetConfig(const char *linkdev, int vf, { int ret = -1; const char *pfDevName = NULL; - char *pfDevOrig = NULL; - char *vfDevOrig = NULL; + VIR_AUTOFREE(char *) pfDevOrig = NULL; + VIR_AUTOFREE(char *) vfDevOrig = NULL; + VIR_AUTOFREE(char *) filePath = NULL; + VIR_AUTOFREE(char *) fileStr = NULL; virMacAddr oldMAC; char MACStr[VIR_MAC_STRING_BUFLEN]; int oldVlanTag = -1; - char *filePath = NULL; - char *fileStr = NULL; virJSONValuePtr configJSON = NULL; if (vf >= 0) { @@ -2030,10 +1973,6 @@ virNetDevSaveNetConfig(const char *linkdev, int vf, ret = 0; cleanup: - VIR_FREE(pfDevOrig); - VIR_FREE(vfDevOrig); - VIR_FREE(filePath); - VIR_FREE(fileStr); virJSONValueFree(configJSON); return ret; } @@ -2069,10 +2008,10 @@ virNetDevReadNetConfig(const char *linkdev, int vf, { int ret = -1; const char *pfDevName = NULL; - char *pfDevOrig = NULL; - char *vfDevOrig = NULL; - char *filePath = NULL; - char *fileStr = NULL; + VIR_AUTOFREE(char *) pfDevOrig = NULL; + VIR_AUTOFREE(char *) vfDevOrig = NULL; + VIR_AUTOFREE(char *) filePath = NULL; + VIR_AUTOFREE(char *) fileStr = NULL; virJSONValuePtr configJSON = NULL; const char *MACStr = NULL; const char *adminMACStr = NULL; @@ -2245,10 +2184,6 @@ virNetDevReadNetConfig(const char *linkdev, int vf, VIR_FREE(*vlan); } - VIR_FREE(pfDevOrig); - VIR_FREE(vfDevOrig); - VIR_FREE(filePath); - VIR_FREE(fileStr); virJSONValueFree(configJSON); return ret; } @@ -2282,8 +2217,8 @@ virNetDevSetNetConfig(const char *linkdev, int vf, int ret = -1; char MACStr[VIR_MAC_STRING_BUFLEN]; const char *pfDevName = NULL; - char *pfDevOrig = NULL; - char *vfDevOrig = NULL; + VIR_AUTOFREE(char *) pfDevOrig = NULL; + VIR_AUTOFREE(char *) vfDevOrig = NULL; int vlanTag = -1; virPCIDevicePtr vfPCIDevice = NULL; @@ -2462,8 +2397,6 @@ virNetDevSetNetConfig(const char *linkdev, int vf, ret = 0; cleanup: - VIR_FREE(pfDevOrig); - VIR_FREE(vfDevOrig); virPCIDeviceFree(vfPCIDevice); return ret; } @@ -2543,28 +2476,27 @@ int virNetDevGetLinkInfo(const char *ifname, virNetDevIfLinkPtr lnk) { - int ret = -1; - char *path = NULL; - char *buf = NULL; + VIR_AUTOFREE(char *) path = NULL; + VIR_AUTOFREE(char *) buf = NULL; char *tmp; int tmp_state; unsigned int tmp_speed; if (virNetDevSysfsFile(&path, ifname, "operstate") < 0) - goto cleanup; + return -1; if (virFileReadAll(path, 1024, &buf) < 0) { virReportSystemError(errno, _("unable to read: %s"), path); - goto cleanup; + return -1; } if (!(tmp = strchr(buf, '\n'))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to parse: %s"), buf); - goto cleanup; + return -1; } *tmp = '\0'; @@ -2575,7 +2507,7 @@ virNetDevGetLinkInfo(const char *ifname, virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to parse: %s"), buf); - goto cleanup; + return -1; } lnk->state = tmp_state; @@ -2586,26 +2518,24 @@ virNetDevGetLinkInfo(const char *ifname, * speed if that's the case. */ if (lnk->state != VIR_NETDEV_IF_STATE_UP) { lnk->speed = 0; - ret = 0; - goto cleanup; + return 0; } VIR_FREE(path); VIR_FREE(buf); if (virNetDevSysfsFile(&path, ifname, "speed") < 0) - goto cleanup; + return -1; if (virFileReadAllQuiet(path, 1024, &buf) < 0) { /* Some devices doesn't report speed, in which case we get EINVAL */ - if (errno == EINVAL) { - ret = 0; - goto cleanup; - } + if (errno == EINVAL) + return 0; + virReportSystemError(errno, _("unable to read: %s"), path); - goto cleanup; + return -1; } if (virStrToLong_ui(buf, &tmp, 10, &tmp_speed) < 0 || @@ -2613,16 +2543,12 @@ virNetDevGetLinkInfo(const char *ifname, virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to parse: %s"), buf); - goto cleanup; + return -1; } lnk->speed = tmp_speed; - ret = 0; - cleanup: - VIR_FREE(buf); - VIR_FREE(path); - return ret; + return 0; } #else @@ -2830,7 +2756,7 @@ static int virNetDevGetMcastList(const char *ifname, virNetDevMcastListPtr mcast) { char *cur = NULL; - char *buf = NULL; + VIR_AUTOFREE(char *) buf = NULL; char *next = NULL; int ret = -1, len; virNetDevMcastEntryPtr entry = NULL; @@ -2866,7 +2792,6 @@ static int virNetDevGetMcastList(const char *ifname, ret = 0; cleanup: - VIR_FREE(buf); VIR_FREE(entry); return ret; @@ -3006,10 +2931,8 @@ static int virNetDevRDMAFeature(const char *ifname, virBitmapPtr *out) { - char *eth_devpath = NULL; - char *ib_devpath = NULL; - char *eth_res_buf = NULL; - char *ib_res_buf = NULL; + VIR_AUTOFREE(char *) eth_devpath = NULL; + VIR_AUTOFREE(char *) eth_res_buf = NULL; DIR *dirp = NULL; struct dirent *dp; int ret = -1; @@ -3028,6 +2951,9 @@ virNetDevRDMAFeature(const char *ifname, goto cleanup; while (virDirRead(dirp, &dp, SYSFS_INFINIBAND_DIR) > 0) { + VIR_AUTOFREE(char *) ib_devpath = NULL; + VIR_AUTOFREE(char *) ib_res_buf = NULL; + if (virAsprintf(&ib_devpath, SYSFS_INFINIBAND_DIR "%s/device/resource", dp->d_name) < 0) continue; @@ -3036,17 +2962,11 @@ virNetDevRDMAFeature(const char *ifname, ignore_value(virBitmapSetBit(*out, VIR_NET_DEV_FEAT_RDMA)); break; } - VIR_FREE(ib_devpath); - VIR_FREE(ib_res_buf); } ret = 0; cleanup: VIR_DIR_CLOSE(dirp); - VIR_FREE(eth_devpath); - VIR_FREE(ib_devpath); - VIR_FREE(eth_res_buf); - VIR_FREE(ib_res_buf); return ret; } @@ -3205,7 +3125,7 @@ static uint32_t virNetDevGetFamilyId(const char *family_name) { struct nl_msg *nl_msg = NULL; - struct nlmsghdr *resp = NULL; + VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; struct genlmsghdr* gmsgh = NULL; struct nlattr *tb[CTRL_ATTR_MAX + 1] = {NULL, }; unsigned int recvbuflen; @@ -3245,7 +3165,6 @@ virNetDevGetFamilyId(const char *family_name) cleanup: nlmsg_free(nl_msg); - VIR_FREE(resp); return family_id; } @@ -3265,13 +3184,13 @@ virNetDevSwitchdevFeature(const char *ifname, virBitmapPtr *out) { struct nl_msg *nl_msg = NULL; - struct nlmsghdr *resp = NULL; + VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; unsigned int recvbuflen; struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {NULL, }; virPCIDevicePtr pci_device_ptr = NULL; struct genlmsghdr* gmsgh = NULL; const char *pci_name; - char *pfname = NULL; + VIR_AUTOFREE(char *) pfname = NULL; int is_vf = -1; int ret = -1; uint32_t family_id; @@ -3333,8 +3252,6 @@ virNetDevSwitchdevFeature(const char *ifname, cleanup: nlmsg_free(nl_msg); virPCIDeviceFree(pci_device_ptr); - VIR_FREE(resp); - VIR_FREE(pfname); return ret; } # else @@ -3375,7 +3292,7 @@ virNetDevGetEthtoolGFeatures(virBitmapPtr bitmap, int fd, struct ifreq *ifr) { - struct ethtool_gfeatures *g_cmd; + VIR_AUTOFREE(struct ethtool_gfeatures *) g_cmd = NULL; if (VIR_ALLOC_VAR(g_cmd, struct ethtool_get_features_block, GFEATURES_SIZE) < 0) @@ -3385,7 +3302,6 @@ virNetDevGetEthtoolGFeatures(virBitmapPtr bitmap, g_cmd->size = GFEATURES_SIZE; if (virNetDevGFeatureAvailable(fd, ifr, g_cmd)) ignore_value(virBitmapSetBit(bitmap, VIR_NET_DEV_FEAT_TXUDPTNL)); - VIR_FREE(g_cmd); return 0; } # else -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- src/util/virnetdev.c | 129 ++++++++++++++++++++++----------------------------- 1 file changed, 55 insertions(+), 74 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 7653f8b..c5871b4 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1855,16 +1855,15 @@ virNetDevSaveNetConfig(const char *linkdev, int vf, const char *stateDir, bool saveVlan) { - int ret = -1; const char *pfDevName = NULL; VIR_AUTOFREE(char *) pfDevOrig = NULL; VIR_AUTOFREE(char *) vfDevOrig = NULL; VIR_AUTOFREE(char *) filePath = NULL; VIR_AUTOFREE(char *) fileStr = NULL; + VIR_AUTOPTR(virJSONValue) configJSON = NULL; virMacAddr oldMAC; char MACStr[VIR_MAC_STRING_BUFLEN]; int oldVlanTag = -1; - virJSONValuePtr configJSON = NULL; if (vf >= 0) { /* linkdev is the PF */ @@ -1872,7 +1871,7 @@ virNetDevSaveNetConfig(const char *linkdev, int vf, /* linkdev should get the VF's netdev name (or NULL if none) */ if (virNetDevPFGetVF(pfDevName, vf, &vfDevOrig) < 0) - goto cleanup; + return -1; linkdev = vfDevOrig; saveVlan = true; @@ -1884,12 +1883,12 @@ virNetDevSaveNetConfig(const char *linkdev, int vf, */ if (virNetDevGetPhysicalFunction(linkdev, &pfDevOrig) < 0) - goto cleanup; + return -1; pfDevName = pfDevOrig; if (virNetDevGetVirtualFunctionIndex(pfDevName, linkdev, &vf) < 0) - goto cleanup; + return -1; } if (pfDevName) { @@ -1907,7 +1906,7 @@ virNetDevSaveNetConfig(const char *linkdev, int vf, * explicitly enable the PF in the host system network config. */ if (virNetDevGetOnline(pfDevName, &pfIsOnline) < 0) - goto cleanup; + return -1; if (!pfIsOnline) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1916,12 +1915,12 @@ virNetDevSaveNetConfig(const char *linkdev, int vf, "change host network config to put the " "PF online."), vf, pfDevName); - goto cleanup; + return -1; } } if (!(configJSON = virJSONValueNewObject())) - goto cleanup; + return -1; /* if there is a PF, it's now in pfDevName, and linkdev is either * the VF's name, or NULL (if the VF isn't bound to a net driver @@ -1930,11 +1929,11 @@ virNetDevSaveNetConfig(const char *linkdev, int vf, if (pfDevName && saveVlan) { if (virAsprintf(&filePath, "%s/%s_vf%d", stateDir, pfDevName, vf) < 0) - goto cleanup; + return -1; /* get admin MAC and vlan tag */ if (virNetDevGetVfConfig(pfDevName, vf, &oldMAC, &oldVlanTag) < 0) - goto cleanup; + return -1; if (virJSONValueObjectAppendString(configJSON, VIR_NETDEV_KEYNAME_ADMIN_MAC, @@ -1942,39 +1941,36 @@ virNetDevSaveNetConfig(const char *linkdev, int vf, virJSONValueObjectAppendNumberInt(configJSON, VIR_NETDEV_KEYNAME_VLAN_TAG, oldVlanTag) < 0) { - goto cleanup; + return -1; } } else { if (virAsprintf(&filePath, "%s/%s", stateDir, linkdev) < 0) - goto cleanup; + return -1; } if (linkdev) { if (virNetDevGetMAC(linkdev, &oldMAC) < 0) - goto cleanup; + return -1; /* for interfaces with no pfDevName (i.e. not a VF, this will * be the only value in the file. */ if (virJSONValueObjectAppendString(configJSON, VIR_NETDEV_KEYNAME_MAC, virMacAddrFormat(&oldMAC, MACStr)) < 0) - goto cleanup; + return -1; } if (!(fileStr = virJSONValueToString(configJSON, true))) - goto cleanup; + return -1; if (virFileWriteStr(filePath, fileStr, O_CREAT|O_TRUNC|O_WRONLY) < 0) { virReportSystemError(errno, _("Unable to preserve mac/vlan tag " "for device = %s, vf = %d"), linkdev, vf); - goto cleanup; + return -1; } - ret = 0; - cleanup: - virJSONValueFree(configJSON); - return ret; + return 0; } @@ -2012,7 +2008,10 @@ virNetDevReadNetConfig(const char *linkdev, int vf, VIR_AUTOFREE(char *) vfDevOrig = NULL; VIR_AUTOFREE(char *) filePath = NULL; VIR_AUTOFREE(char *) fileStr = NULL; - virJSONValuePtr configJSON = NULL; + VIR_AUTOPTR(virJSONValue) configJSON = NULL; + VIR_AUTOPTR(virMacAddr) tempAdminMAC = NULL; + VIR_AUTOPTR(virMacAddr) tempMAC = NULL; + VIR_AUTOPTR(virNetDevVlan) tempVlan = NULL; const char *MACStr = NULL; const char *adminMACStr = NULL; int vlanTag = -1; @@ -2176,15 +2175,12 @@ virNetDevReadNetConfig(const char *linkdev, int vf, /* we won't need the file again */ ignore_value(unlink(filePath)); - ret = 0; + return 0; cleanup: - if (ret < 0) { - VIR_FREE(*adminMAC); - VIR_FREE(*MAC); - VIR_FREE(*vlan); - } + VIR_STEAL_PTR(tempAdminMAC, *adminMAC); + VIR_STEAL_PTR(tempMAC, *MAC); + VIR_STEAL_PTR(tempVlan, *vlan); - virJSONValueFree(configJSON); return ret; } @@ -2214,13 +2210,12 @@ virNetDevSetNetConfig(const char *linkdev, int vf, const virMacAddr *MAC, bool setVlan) { - int ret = -1; char MACStr[VIR_MAC_STRING_BUFLEN]; const char *pfDevName = NULL; VIR_AUTOFREE(char *) pfDevOrig = NULL; VIR_AUTOFREE(char *) vfDevOrig = NULL; + VIR_AUTOPTR(virPCIDevice) vfPCIDevice = NULL; int vlanTag = -1; - virPCIDevicePtr vfPCIDevice = NULL; if (vf >= 0) { /* linkdev is the PF */ @@ -2228,7 +2223,7 @@ virNetDevSetNetConfig(const char *linkdev, int vf, /* linkdev should get the VF's netdev name (or NULL if none) */ if (virNetDevPFGetVF(pfDevName, vf, &vfDevOrig) < 0) - goto cleanup; + return -1; linkdev = vfDevOrig; @@ -2239,12 +2234,12 @@ virNetDevSetNetConfig(const char *linkdev, int vf, */ if (virNetDevGetPhysicalFunction(linkdev, &pfDevOrig) < 0) - goto cleanup; + return -1; pfDevName = pfDevOrig; if (virNetDevGetVirtualFunctionIndex(pfDevName, linkdev, &vf) < 0) - goto cleanup; + return -1; } @@ -2256,14 +2251,14 @@ virNetDevSetNetConfig(const char *linkdev, int vf, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("admin MAC can only be set for SR-IOV VFs, but " "%s is not a VF"), linkdev); - goto cleanup; + return -1; } if (vlan) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("vlan can only be set for SR-IOV VFs, but " "%s is not a VF"), linkdev); - goto cleanup; + return -1; } } else { @@ -2272,14 +2267,14 @@ virNetDevSetNetConfig(const char *linkdev, int vf, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("vlan trunking is not supported " "by SR-IOV network devices")); - goto cleanup; + return -1; } if (!setVlan) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("vlan tag set for interface %s but " "caller requested it not be set")); - goto cleanup; + return -1; } vlanTag = vlan->tag[0]; @@ -2297,7 +2292,7 @@ virNetDevSetNetConfig(const char *linkdev, int vf, _("VF %d of PF '%s' is not bound to a net driver, " "so its MAC address cannot be set to %s"), vf, pfDevName, virMacAddrFormat(MAC, MACStr)); - goto cleanup; + return -1; } setMACrc = virNetDevSetMACInternal(linkdev, MAC, !!pfDevOrig); @@ -2308,7 +2303,7 @@ virNetDevSetNetConfig(const char *linkdev, int vf, /* if pfDevOrig == NULL, this isn't a VF, so we've failed */ if (!pfDevOrig || (errno != EADDRNOTAVAIL && errno != EPERM)) - goto cleanup; + return -1; /* Otherwise this is a VF, and virNetDevSetMAC failed with * EADDRNOTAVAIL/EPERM, which could be due to the @@ -2322,18 +2317,18 @@ virNetDevSetNetConfig(const char *linkdev, int vf, if (virNetDevSetVfConfig(pfDevName, vf, MAC, vlanTag, &allowRetry) < 0) { - goto cleanup; + return -1; } /* admin MAC is set, now we need to construct a virPCIDevice * object so we can call virPCIDeviceRebind() */ if (!(vfPCIDevice = virNetDevGetPCIDevice(linkdev))) - goto cleanup; + return -1; /* Rebind the device. This should set the proper MAC address */ if (virPCIDeviceRebind(vfPCIDevice) < 0) - goto cleanup; + return -1; /* Wait until virNetDevGetIndex for the VF netdev returns success. * This indicates that the device is ready to be used. If we don't @@ -2385,20 +2380,17 @@ virNetDevSetNetConfig(const char *linkdev, int vf, * with the "locally administered" bit set. */ if (!allowRetry) - goto cleanup; + return -1; allowRetry = false; if (virNetDevSetVfConfig(pfDevName, vf, &altZeroMAC, vlanTag, &allowRetry) < 0) { - goto cleanup; + return -1; } } } - ret = 0; - cleanup: - virPCIDeviceFree(vfPCIDevice); - return ret; + return 0; } @@ -2873,30 +2865,31 @@ virNetDevRxFilterFree(virNetDevRxFilterPtr filter) int virNetDevGetRxFilter(const char *ifname, virNetDevRxFilterPtr *filter) { - int ret = -1; bool receive = false; - virNetDevRxFilterPtr fil = virNetDevRxFilterNew(); + VIR_AUTOPTR(virNetDevRxFilter) fil = virNetDevRxFilterNew(); + + *filter = NULL; if (!fil) - goto cleanup; + return -1; if (virNetDevGetMAC(ifname, &fil->mac)) - goto cleanup; + return -1; if (virNetDevGetMulticastTable(ifname, fil)) - goto cleanup; + return -1; if (virNetDevGetPromiscuous(ifname, &fil->promiscuous)) - goto cleanup; + return -1; if (virNetDevGetRcvAllMulti(ifname, &receive)) - goto cleanup; + return -1; if (receive) { fil->multicast.mode = VIR_NETDEV_RX_FILTER_MODE_ALL; } else { if (virNetDevGetRcvMulti(ifname, &receive)) - goto cleanup; + return -1; if (receive) fil->multicast.mode = VIR_NETDEV_RX_FILTER_MODE_NORMAL; @@ -2904,15 +2897,8 @@ int virNetDevGetRxFilter(const char *ifname, fil->multicast.mode = VIR_NETDEV_RX_FILTER_MODE_NONE; } - ret = 0; - cleanup: - if (ret < 0) { - virNetDevRxFilterFree(fil); - fil = NULL; - } - - *filter = fil; - return ret; + VIR_STEAL_PTR(*filter, fil); + return 0; } #if defined(SIOCETHTOOL) && defined(HAVE_STRUCT_IFREQ) @@ -3185,12 +3171,12 @@ virNetDevSwitchdevFeature(const char *ifname, { struct nl_msg *nl_msg = NULL; VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; + VIR_AUTOFREE(char *) pfname = NULL; + VIR_AUTOPTR(virPCIDevice) pci_device_ptr = NULL; unsigned int recvbuflen; struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {NULL, }; - virPCIDevicePtr pci_device_ptr = NULL; struct genlmsghdr* gmsgh = NULL; const char *pci_name; - VIR_AUTOFREE(char *) pfname = NULL; int is_vf = -1; int ret = -1; uint32_t family_id; @@ -3251,7 +3237,6 @@ virNetDevSwitchdevFeature(const char *ifname, cleanup: nlmsg_free(nl_msg); - virPCIDeviceFree(pci_device_ptr); return ret; } # else @@ -3507,8 +3492,7 @@ int virNetDevSetCoalesce(const char *ifname, int virNetDevRunEthernetScript(const char *ifname, const char *script) { - virCommandPtr cmd; - int ret; + VIR_AUTOPTR(virCommand) cmd = NULL; /* Not a bug! Previously we did accept script="" as a NO-OP. */ if (STREQ(script, "")) @@ -3522,8 +3506,5 @@ virNetDevRunEthernetScript(const char *ifname, const char *script) #endif virCommandAddEnvPassCommon(cmd); - ret = virCommandRun(cmd, NULL); - - virCommandFree(cmd); - return ret; + return virCommandRun(cmd, NULL); } -- 1.8.3.1

Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in src/util/viralloc.h, define a new wrapper around an existing cleanup function which will be called when a variable declared with VIR_AUTOPTR macro goes out of scope. Also, drop the redundant viralloc.h include, since that has moved from the source module into the header. When a variable of type virSocketAddrPtr is declared using VIR_AUTOPTR, the function virSocketAddrFree will be run automatically on it when it goes out of scope. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- src/util/virsocketaddr.c | 7 ++++++- src/util/virsocketaddr.h | 9 +++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 5c3bfad..6b52a3d 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -26,7 +26,6 @@ #include "virsocketaddr.h" #include "virerror.h" #include "virstring.h" -#include "viralloc.h" #include "virbuffer.h" #include <netdb.h> @@ -1253,3 +1252,9 @@ virSocketAddrPTRDomain(const virSocketAddr *addr, ret = -2; goto cleanup; } + +void +virSocketAddrFree(virSocketAddrPtr addr) +{ + VIR_FREE(addr); +} diff --git a/src/util/virsocketaddr.h b/src/util/virsocketaddr.h index 3029338..66f5998 100644 --- a/src/util/virsocketaddr.h +++ b/src/util/virsocketaddr.h @@ -24,14 +24,15 @@ #ifndef __VIR_SOCKETADDR_H__ # define __VIR_SOCKETADDR_H__ -# include "internal.h" - # include <netinet/in.h> # include <sys/socket.h> # ifdef HAVE_SYS_UN_H # include <sys/un.h> # endif +# include "internal.h" +# include "viralloc.h" + /* On architectures which lack these limits, define them (ie. Cygwin). * Note that the libvirt code should be robust enough to handle the * case where actual value is longer than these limits (eg. by setting @@ -162,4 +163,8 @@ int virSocketAddrPTRDomain(const virSocketAddr *addr, char **ptr) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); +void virSocketAddrFree(virSocketAddrPtr addr); + +VIR_DEFINE_AUTOPTR_FUNC(virSocketAddr, virSocketAddrFree) + #endif /* __VIR_SOCKETADDR_H__ */ -- 1.8.3.1

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@gmail.com> --- src/util/virsocketaddr.c | 70 ++++++++++++++++++++---------------------------- 1 file changed, 29 insertions(+), 41 deletions(-) diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 6b52a3d..eee725d 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -432,10 +432,10 @@ virSocketAddrFormatFull(const virSocketAddr *addr, if (withService) { if (virAsprintf(&addrstr, VIR_LOOPBACK_IPV4_ADDR"%s0", separator ? separator : ":") < 0) - goto error; + return NULL; } else { if (VIR_STRDUP(addrstr, VIR_LOOPBACK_IPV4_ADDR) < 0) - goto error; + return NULL; } return addrstr; } @@ -452,33 +452,27 @@ virSocketAddrFormatFull(const virSocketAddr *addr, } if (withService) { - char *ipv6_host = NULL; + VIR_AUTOFREE(char *) ipv6_host = NULL; /* sasl_new_client demands the socket address to be in an odd format: * a.b.c.d;port or e:f:g:h:i:j:k:l;port, so use square brackets for * IPv6 only if no separator is passed to the function */ if (!separator && VIR_SOCKET_ADDR_FAMILY(addr) == AF_INET6) { if (virAsprintf(&ipv6_host, "[%s]", host) < 0) - goto error; + return NULL; } if (virAsprintf(&addrstr, "%s%s%s", ipv6_host ? ipv6_host : host, separator ? separator : ":", port) == -1) { - VIR_FREE(ipv6_host); - goto error; + return NULL; } - - VIR_FREE(ipv6_host); } else { if (VIR_STRDUP(addrstr, host) < 0) - goto error; + return NULL; } return addrstr; - - error: - return NULL; } @@ -759,24 +753,26 @@ virSocketAddrGetRange(virSocketAddrPtr start, virSocketAddrPtr end, int ret = 0; size_t i; virSocketAddr netmask; - char *startStr = NULL, *endStr = NULL, *netStr = NULL; + VIR_AUTOFREE(char *) startStr = NULL; + VIR_AUTOFREE(char *) endStr = NULL; + VIR_AUTOFREE(char *) netStr = NULL; if (start == NULL || end == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("NULL argument - %p %p"), start, end); - goto error; + return -1; } startStr = virSocketAddrFormat(start); endStr = virSocketAddrFormat(end); if (!startStr || !endStr) - goto error; /*error already reported */ + return -1; /*error already reported */ if (VIR_SOCKET_ADDR_FAMILY(start) != VIR_SOCKET_ADDR_FAMILY(end)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("mismatch of address family in range %s - %s"), startStr, endStr); - goto error; + return -1; } if (network) { @@ -784,14 +780,14 @@ virSocketAddrGetRange(virSocketAddrPtr start, virSocketAddrPtr end, * network the range should be within */ if (!(netStr = virSocketAddrFormat(network))) - goto error; + return -1; if (VIR_SOCKET_ADDR_FAMILY(start) != VIR_SOCKET_ADDR_FAMILY(network)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("mismatch of address family in " "range %s - %s for network %s"), startStr, endStr, netStr); - goto error; + return -1; } if (prefix < 0 || @@ -801,7 +797,7 @@ virSocketAddrGetRange(virSocketAddrPtr start, virSocketAddrPtr end, _("bad prefix %d for network %s when " " checking range %s - %s"), prefix, netStr, startStr, endStr); - goto error; + return -1; } /* both start and end of range need to be within network */ @@ -811,7 +807,7 @@ virSocketAddrGetRange(virSocketAddrPtr start, virSocketAddrPtr end, _("range %s - %s is not entirely within " "network %s/%d"), startStr, endStr, netStr, prefix); - goto error; + return -1; } if (VIR_SOCKET_ADDR_IS_FAMILY(start, AF_INET)) { @@ -823,7 +819,7 @@ virSocketAddrGetRange(virSocketAddrPtr start, virSocketAddrPtr end, _("failed to construct broadcast or network " "address for network %s/%d"), netStr, prefix); - goto error; + return -1; } /* Don't allow the start of the range to be the network @@ -838,7 +834,7 @@ virSocketAddrGetRange(virSocketAddrPtr start, virSocketAddrPtr end, _("start of range %s - %s in network %s/%d " "is the network address"), startStr, endStr, netStr, prefix); - goto error; + return -1; } if (virSocketAddrEqual(end, &broadcast)) { @@ -846,7 +842,7 @@ virSocketAddrGetRange(virSocketAddrPtr start, virSocketAddrPtr end, _("end of range %s - %s in network %s/%d " "is the broadcast address"), startStr, endStr, netStr, prefix); - goto error; + return -1; } } } @@ -860,7 +856,7 @@ virSocketAddrGetRange(virSocketAddrPtr start, virSocketAddrPtr end, _("failed to get IPv4 address " "for start or end of range %s - %s"), startStr, endStr); - goto error; + return -1; } /* legacy check that everything except the last two bytes @@ -868,10 +864,10 @@ virSocketAddrGetRange(virSocketAddrPtr start, virSocketAddrPtr end, */ for (i = 0; i < 2; i++) { if (t1[i] != t2[i]) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("range %s - %s is too large (> 65535)"), - startStr, endStr); - goto error; + virReportError(VIR_ERR_INTERNAL_ERROR, + _("range %s - %s is too large (> 65535)"), + startStr, endStr); + return -1; } } ret = (t2[2] - t1[2]) * 256 + (t2[3] - t1[3]); @@ -879,7 +875,7 @@ virSocketAddrGetRange(virSocketAddrPtr start, virSocketAddrPtr end, virReportError(VIR_ERR_INTERNAL_ERROR, _("range %s - %s is reversed "), startStr, endStr); - goto error; + return -1; } ret++; } else if (VIR_SOCKET_ADDR_IS_FAMILY(start, AF_INET6)) { @@ -891,7 +887,7 @@ virSocketAddrGetRange(virSocketAddrPtr start, virSocketAddrPtr end, _("failed to get IPv6 address " "for start or end of range %s - %s"), startStr, endStr); - goto error; + return -1; } /* legacy check that everything except the last two bytes are @@ -902,7 +898,7 @@ virSocketAddrGetRange(virSocketAddrPtr start, virSocketAddrPtr end, virReportError(VIR_ERR_INTERNAL_ERROR, _("range %s - %s is too large (> 65535)"), startStr, endStr); - goto error; + return -1; } } ret = t2[7] - t1[7]; @@ -910,7 +906,7 @@ virSocketAddrGetRange(virSocketAddrPtr start, virSocketAddrPtr end, virReportError(VIR_ERR_INTERNAL_ERROR, _("range %s - %s start larger than end"), startStr, endStr); - goto error; + return -1; } ret++; } else { @@ -918,18 +914,10 @@ virSocketAddrGetRange(virSocketAddrPtr start, virSocketAddrPtr end, _("unsupported address family " "for range %s - %s, must be ipv4 or ipv6"), startStr, endStr); - goto error; + return -1; } - cleanup: - VIR_FREE(startStr); - VIR_FREE(endStr); - VIR_FREE(netStr); return ret; - - error: - ret = -1; - goto cleanup; } -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- src/util/virsocketaddr.c | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index eee725d..1b195cd 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -1193,52 +1193,46 @@ virSocketAddrPTRDomain(const virSocketAddr *addr, unsigned int prefix, char **ptr) { - virBuffer buf = VIR_BUFFER_INITIALIZER; + VIR_AUTOPTR(virBuffer) buf = NULL; size_t i; - int ret = -1; + + if (VIR_ALLOC(buf) < 0) + return -1; if (VIR_SOCKET_ADDR_IS_FAMILY(addr, AF_INET)) { virSocketAddrIPv4 ip; if (prefix == 0 || prefix >= 32 || prefix % 8 != 0) - goto unsupported; + return -2; if (virSocketAddrGetIPv4Addr(addr, &ip) < 0) - goto cleanup; + return -1; for (i = prefix / 8; i > 0; i--) - virBufferAsprintf(&buf, "%u.", ip[i - 1]); + virBufferAsprintf(buf, "%u.", ip[i - 1]); - virBufferAddLit(&buf, VIR_SOCKET_ADDR_IPV4_ARPA); + virBufferAddLit(buf, VIR_SOCKET_ADDR_IPV4_ARPA); } else if (VIR_SOCKET_ADDR_IS_FAMILY(addr, AF_INET6)) { virSocketAddrIPv6Nibbles ip; if (prefix == 0 || prefix >= 128 || prefix % 4 != 0) - goto unsupported; + return -2; if (virSocketAddrGetIPv6Nibbles(addr, &ip) < 0) - goto cleanup; + return -1; for (i = prefix / 4; i > 0; i--) - virBufferAsprintf(&buf, "%x.", ip[i - 1]); + virBufferAsprintf(buf, "%x.", ip[i - 1]); - virBufferAddLit(&buf, VIR_SOCKET_ADDR_IPV6_ARPA); + virBufferAddLit(buf, VIR_SOCKET_ADDR_IPV6_ARPA); } else { - goto unsupported; + return -2; } - if (!(*ptr = virBufferContentAndReset(&buf))) - goto cleanup; + if (!(*ptr = virBufferContentAndReset(buf))) + return -1; - ret = 0; - - cleanup: - virBufferFreeAndReset(&buf); - return ret; - - unsupported: - ret = -2; - goto cleanup; + return 0; } void -- 1.8.3.1

Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in src/util/viralloc.h, define a new wrapper around an existing cleanup function which will be called when a variable declared with VIR_AUTOPTR macro goes out of scope. Also, drop the redundant viralloc.h include, since that has moved from the source module into the header. When a variable of type virNetlinkCallbackDataPtr is declared using VIR_AUTOPTR, the function virNetlinkCallbackDataFree will be run automatically on it when it goes out of scope. This commit also adds an intermediate typedef for virNetlinkCallbackData type for use with the cleanup macros. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- src/util/virnetdevmacvlan.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index fb41bf9..91c6244 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -576,7 +576,7 @@ static const uint32_t modeMap[VIR_NETDEV_MACVLAN_MODE_LAST] = { }; /* Struct to hold the state and configuration of a 802.1qbg port */ -struct virNetlinkCallbackData { +struct _virNetlinkCallbackData { char *cr_ifname; virNetDevVPortProfilePtr virtPortProfile; virMacAddr macaddress; @@ -587,7 +587,8 @@ struct virNetlinkCallbackData { unsigned int linkState; }; -typedef struct virNetlinkCallbackData *virNetlinkCallbackDataPtr; +typedef struct _virNetlinkCallbackData virNetlinkCallbackData; +typedef virNetlinkCallbackData *virNetlinkCallbackDataPtr; # define INSTANCE_STRLEN 36 @@ -870,6 +871,8 @@ virNetlinkCallbackDataFree(virNetlinkCallbackDataPtr calld) VIR_FREE(calld); } +VIR_DEFINE_AUTOPTR_FUNC(virNetlinkCallbackData, virNetlinkCallbackDataFree) + /** * virNetDevMacVLanVPortProfileDestroyCallback: * -- 1.8.3.1

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@gmail.com> --- src/util/virnetdevmacvlan.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index 91c6244..a2ed65c 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -308,7 +308,7 @@ virNetDevMacVLanCreate(const char *ifname, int *retry) { int rc = -1; - struct nlmsghdr *resp = NULL; + VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; struct nlmsgerr *err; struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC }; int ifindex; @@ -403,7 +403,6 @@ virNetDevMacVLanCreate(const char *ifname, rc = 0; cleanup: nlmsg_free(nl_msg); - VIR_FREE(resp); return rc; malformed_resp: @@ -452,7 +451,7 @@ virNetDevMacVLanTapOpen(const char *ifname, { int ret = -1; int ifindex; - char *tapname = NULL; + VIR_AUTOFREE(char *) tapname = NULL; size_t i = 0; if (virNetDevGetIndex(ifname, &ifindex) < 0) @@ -487,7 +486,6 @@ virNetDevMacVLanTapOpen(const char *ifname, while (i--) VIR_FORCE_CLOSE(tapfd[i]); } - VIR_FREE(tapname); return ret; } -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- src/util/virnetdevmacvlan.c | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index a2ed65c..d01e5ef 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -898,19 +898,20 @@ virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname, virNetDevVPortProfilePtr virtPortProfile, virNetDevVPortProfileOp vmOp) { - virNetlinkCallbackDataPtr calld = NULL; + VIR_AUTOPTR(virNetlinkCallbackData) calld = NULL; + virNetlinkCallbackDataPtr temp ATTRIBUTE_UNUSED = NULL; if (virtPortProfile && virNetlinkEventServiceIsRunning(NETLINK_ROUTE)) { if (VIR_ALLOC(calld) < 0) - goto error; + return -1; if (VIR_STRDUP(calld->cr_ifname, ifname) < 0) - goto error; + return -1; if (VIR_ALLOC(calld->virtPortProfile) < 0) - goto error; + return -1; memcpy(calld->virtPortProfile, virtPortProfile, sizeof(*virtPortProfile)); virMacAddrSet(&calld->macaddress, macaddress); if (VIR_STRDUP(calld->linkdev, linkdev) < 0) - goto error; + return -1; memcpy(calld->vmuuid, vmuuid, sizeof(calld->vmuuid)); calld->vmOp = vmOp; @@ -918,14 +919,12 @@ virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname, if (virNetlinkEventAddClient(virNetDevMacVLanVPortProfileCallback, virNetDevMacVLanVPortProfileDestroyCallback, calld, macaddress, NETLINK_ROUTE) < 0) - goto error; + return -1; } + VIR_STEAL_PTR(temp, calld); + return 0; - - error: - virNetlinkCallbackDataFree(calld); - return -1; } @@ -1184,9 +1183,9 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname, } if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) { - virMacAddrPtr MAC = NULL; - virMacAddrPtr adminMAC = NULL; - virNetDevVlanPtr vlan = NULL; + VIR_AUTOPTR(virMacAddr) MAC = NULL; + VIR_AUTOPTR(virMacAddr) adminMAC = NULL; + VIR_AUTOPTR(virNetDevVlan) vlan = NULL; if ((virNetDevReadNetConfig(linkdev, -1, stateDir, &adminMAC, &vlan, &MAC) == 0) && @@ -1194,9 +1193,6 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname, ignore_value(virNetDevSetNetConfig(linkdev, -1, adminMAC, vlan, MAC, !!vlan)); - VIR_FREE(MAC); - VIR_FREE(adminMAC); - virNetDevVlanFree(vlan); } } -- 1.8.3.1

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@gmail.com> --- src/util/virnetdevopenvswitch.c | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index d1c5cf4..a9c5e2a 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -149,10 +149,10 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, char macaddrstr[VIR_MAC_STRING_BUFLEN]; char ifuuidstr[VIR_UUID_STRING_BUFLEN]; char vmuuidstr[VIR_UUID_STRING_BUFLEN]; - char *attachedmac_ex_id = NULL; - char *ifaceid_ex_id = NULL; - char *profile_ex_id = NULL; - char *vmid_ex_id = NULL; + VIR_AUTOFREE(char *) attachedmac_ex_id = NULL; + VIR_AUTOFREE(char *) ifaceid_ex_id = NULL; + VIR_AUTOFREE(char *) profile_ex_id = NULL; + VIR_AUTOFREE(char *) vmid_ex_id = NULL; virMacAddrFormat(macaddr, macaddrstr); virUUIDFormat(ovsport->interfaceID, ifuuidstr); @@ -209,10 +209,6 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, ret = 0; cleanup: - VIR_FREE(attachedmac_ex_id); - VIR_FREE(ifaceid_ex_id); - VIR_FREE(vmid_ex_id); - VIR_FREE(profile_ex_id); virCommandFree(cmd); return ret; } @@ -339,7 +335,7 @@ virNetDevOpenvswitchInterfaceStats(const char *ifname, virDomainInterfaceStatsPtr stats) { virCommandPtr cmd = NULL; - char *output; + VIR_AUTOFREE(char *) output = NULL; char *tmp; bool gotStats = false; int ret = -1; @@ -399,7 +395,6 @@ virNetDevOpenvswitchInterfaceStats(const char *ifname, ret = 0; cleanup: - VIR_FREE(output); virCommandFree(cmd); return ret; } @@ -424,7 +419,6 @@ int virNetDevOpenvswitchInterfaceGetMaster(const char *ifname, char **master) { virCommandPtr cmd = NULL; - int ret = -1; int exitstatus; *master = NULL; @@ -438,7 +432,7 @@ virNetDevOpenvswitchInterfaceGetMaster(const char *ifname, char **master) virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to run command to get OVS master for " "interface %s"), ifname); - goto cleanup; + return -1; } /* non-0 exit code just means that the interface has no master in OVS */ @@ -454,9 +448,7 @@ virNetDevOpenvswitchInterfaceGetMaster(const char *ifname, char **master) VIR_DEBUG("OVS master for %s is %s", ifname, *master ? *master : "(none)"); - ret = 0; - cleanup: - return ret; + return 0; } @@ -476,12 +468,12 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path, char **ifname) { virCommandPtr cmd = NULL; + VIR_AUTOFREE(char *) ovs_timeout = NULL; char *tmpIfname = NULL; char **tokens = NULL; size_t ntokens = 0; int status; int ret = -1; - char *ovs_timeout = NULL; /* Openvswitch vhostuser path are hardcoded to * /<runstatedir>/openvswitch/<ifname> @@ -513,7 +505,6 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path, cleanup: virStringListFreeCount(tokens, ntokens); virCommandFree(cmd); - VIR_FREE(ovs_timeout); return ret; } -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- src/util/virnetdevopenvswitch.c | 106 +++++++++++++++------------------------- 1 file changed, 40 insertions(+), 66 deletions(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index a9c5e2a..eae5861 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -76,9 +76,7 @@ virNetDevOpenvswitchAddTimeout(virCommandPtr cmd) static int virNetDevOpenvswitchConstructVlans(virCommandPtr cmd, virNetDevVlanPtr virtVlan) { - int ret = -1; size_t i = 0; - virBuffer buf = VIR_BUFFER_INITIALIZER; if (!virtVlan || !virtVlan->nTags) return 0; @@ -98,7 +96,12 @@ virNetDevOpenvswitchConstructVlans(virCommandPtr cmd, virNetDevVlanPtr virtVlan) } if (virtVlan->trunk) { - virBufferAddLit(&buf, "trunk="); + VIR_AUTOPTR(virBuffer) buf = NULL; + + if (VIR_ALLOC(buf) < 0) + return -1; + + virBufferAddLit(buf, "trunk="); /* * Trunk ports have at least one VLAN. Do the first one @@ -106,24 +109,21 @@ virNetDevOpenvswitchConstructVlans(virCommandPtr cmd, virNetDevVlanPtr virtVlan) * start of the for loop if there are more than one VLANs * on this trunk port. */ - virBufferAsprintf(&buf, "%d", virtVlan->tag[i]); + virBufferAsprintf(buf, "%d", virtVlan->tag[i]); for (i = 1; i < virtVlan->nTags; i++) { - virBufferAddLit(&buf, ","); - virBufferAsprintf(&buf, "%d", virtVlan->tag[i]); + virBufferAddLit(buf, ","); + virBufferAsprintf(buf, "%d", virtVlan->tag[i]); } - if (virBufferCheckError(&buf) < 0) - goto cleanup; - virCommandAddArg(cmd, virBufferCurrentContent(&buf)); + if (virBufferCheckError(buf) < 0) + return -1; + virCommandAddArg(cmd, virBufferCurrentContent(buf)); } else if (virtVlan->nTags) { virCommandAddArgFormat(cmd, "tag=%d", virtVlan->tag[0]); } - ret = 0; - cleanup: - virBufferFreeAndReset(&buf); - return ret; + return 0; } /** @@ -144,11 +144,10 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, virNetDevVPortProfilePtr ovsport, virNetDevVlanPtr virtVlan) { - int ret = -1; - virCommandPtr cmd = NULL; char macaddrstr[VIR_MAC_STRING_BUFLEN]; char ifuuidstr[VIR_UUID_STRING_BUFLEN]; char vmuuidstr[VIR_UUID_STRING_BUFLEN]; + VIR_AUTOPTR(virCommand) cmd = NULL; VIR_AUTOFREE(char *) attachedmac_ex_id = NULL; VIR_AUTOFREE(char *) ifaceid_ex_id = NULL; VIR_AUTOFREE(char *) profile_ex_id = NULL; @@ -160,17 +159,17 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, if (virAsprintf(&attachedmac_ex_id, "external-ids:attached-mac=\"%s\"", macaddrstr) < 0) - goto cleanup; + return -1; if (virAsprintf(&ifaceid_ex_id, "external-ids:iface-id=\"%s\"", ifuuidstr) < 0) - goto cleanup; + return -1; if (virAsprintf(&vmid_ex_id, "external-ids:vm-id=\"%s\"", vmuuidstr) < 0) - goto cleanup; + return -1; if (ovsport->profileID[0] != '\0') { if (virAsprintf(&profile_ex_id, "external-ids:port-profile=\"%s\"", ovsport->profileID) < 0) - goto cleanup; + return -1; } cmd = virCommandNew(OVSVSCTL); @@ -179,7 +178,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, ifname, "--", "add-port", brname, ifname, NULL); if (virNetDevOpenvswitchConstructVlans(cmd, virtVlan) < 0) - goto cleanup; + return -1; if (ovsport->profileID[0] == '\0') { virCommandAddArgList(cmd, @@ -204,13 +203,10 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to add port %s to OVS bridge %s"), ifname, brname); - goto cleanup; + return -1; } - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; + return 0; } /** @@ -223,8 +219,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, */ int virNetDevOpenvswitchRemovePort(const char *brname ATTRIBUTE_UNUSED, const char *ifname) { - int ret = -1; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; cmd = virCommandNew(OVSVSCTL); virNetDevOpenvswitchAddTimeout(cmd); @@ -233,13 +228,10 @@ int virNetDevOpenvswitchRemovePort(const char *brname ATTRIBUTE_UNUSED, const ch if (virCommandRun(cmd, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to delete port %s from OVS"), ifname); - goto cleanup; + return -1; } - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; + return 0; } /** @@ -253,9 +245,8 @@ int virNetDevOpenvswitchRemovePort(const char *brname ATTRIBUTE_UNUSED, const ch */ int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname) { - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; size_t len; - int ret = -1; cmd = virCommandNew(OVSVSCTL); virNetDevOpenvswitchAddTimeout(cmd); @@ -269,7 +260,7 @@ int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname) virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to run command to get OVS port data for " "interface %s"), ifname); - goto cleanup; + return -1; } /* Wipeout the newline, if it exists */ @@ -277,10 +268,7 @@ int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname) if (len > 0) (*migrate)[len - 1] = '\0'; - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; + return 0; } /** @@ -294,8 +282,7 @@ int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname) */ int virNetDevOpenvswitchSetMigrateData(char *migrate, const char *ifname) { - virCommandPtr cmd = NULL; - int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; if (!migrate) { VIR_DEBUG("No OVS port data for interface %s", ifname); @@ -312,13 +299,10 @@ int virNetDevOpenvswitchSetMigrateData(char *migrate, const char *ifname) virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to run command to set OVS port data for " "interface %s"), ifname); - goto cleanup; + return -1; } - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; + return 0; } /** @@ -334,11 +318,10 @@ int virNetDevOpenvswitchInterfaceStats(const char *ifname, virDomainInterfaceStatsPtr stats) { - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; VIR_AUTOFREE(char *) output = NULL; char *tmp; bool gotStats = false; - int ret = -1; /* Just ensure the interface exists in ovs */ cmd = virCommandNew(OVSVSCTL); @@ -350,7 +333,7 @@ virNetDevOpenvswitchInterfaceStats(const char *ifname, /* no ovs-vsctl or interface 'ifname' doesn't exists in ovs */ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Interface not found")); - goto cleanup; + return -1; } #define GET_STAT(name, member) \ @@ -369,7 +352,7 @@ virNetDevOpenvswitchInterfaceStats(const char *ifname, *tmp != '\n') { \ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", \ _("Fail to parse ovs-vsctl output")); \ - goto cleanup; \ + return -1; \ } \ gotStats = true; \ } \ @@ -389,14 +372,10 @@ virNetDevOpenvswitchInterfaceStats(const char *ifname, if (!gotStats) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Interface doesn't have any statistics")); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - virCommandFree(cmd); - return ret; + return 0; } @@ -467,7 +446,7 @@ int virNetDevOpenvswitchGetVhostuserIfname(const char *path, char **ifname) { - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; VIR_AUTOFREE(char *) ovs_timeout = NULL; char *tmpIfname = NULL; char **tokens = NULL; @@ -504,7 +483,6 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path, cleanup: virStringListFreeCount(tokens, ntokens); - virCommandFree(cmd); return ret; } @@ -520,8 +498,7 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path, int virNetDevOpenvswitchUpdateVlan(const char *ifname, virNetDevVlanPtr virtVlan) { - int ret = -1; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; cmd = virCommandNew(OVSVSCTL); virNetDevOpenvswitchAddTimeout(cmd); @@ -532,16 +509,13 @@ int virNetDevOpenvswitchUpdateVlan(const char *ifname, "--", "--if-exists", "set", "Port", ifname, NULL); if (virNetDevOpenvswitchConstructVlans(cmd, virtVlan) < 0) - goto cleanup; + return -1; if (virCommandRun(cmd, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to set vlan configuration on port %s"), ifname); - goto cleanup; + return -1; } - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; + return 0; } -- 1.8.3.1

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@gmail.com> --- src/util/virnetdevtap.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index d432577..c2c2e73 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -400,16 +400,14 @@ int virNetDevTapCreate(char **ifname, if (strstr(*ifname, "%d") != NULL) { size_t i; for (i = 0; i <= IF_MAXUNIT; i++) { - char *newname; + VIR_AUTOFREE(char *) newname = NULL; if (virAsprintf(&newname, *ifname, i) < 0) goto cleanup; if (virNetDevExists(newname) == 0) { - newifname = newname; + VIR_STEAL_PTR(newifname, newname); break; } - - VIR_FREE(newname); } if (newifname) { VIR_FREE(*ifname); @@ -423,7 +421,7 @@ int virNetDevTapCreate(char **ifname, } if (tapfd) { - char *dev_path = NULL; + VIR_AUTOFREE(char *) dev_path = NULL; if (virAsprintf(&dev_path, "/dev/%s", ifr.ifr_name) < 0) goto cleanup; @@ -431,11 +429,8 @@ int virNetDevTapCreate(char **ifname, virReportSystemError(errno, _("Unable to open %s"), dev_path); - VIR_FREE(dev_path); goto cleanup; } - - VIR_FREE(dev_path); } if (virNetDevSetName(ifr.ifr_name, *ifname) == -1) -- 1.8.3.1

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@gmail.com> --- src/util/virnetdevveth.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/util/virnetdevveth.c b/src/util/virnetdevveth.c index 6905168..8c1a7f3 100644 --- a/src/util/virnetdevveth.c +++ b/src/util/virnetdevveth.c @@ -46,12 +46,11 @@ virMutex virNetDevVethCreateMutex = VIR_MUTEX_INITIALIZER; static int virNetDevVethExists(int devNum) { int ret; - char *path = NULL; + VIR_AUTOFREE(char *) path = NULL; if (virAsprintf(&path, SYSFS_NET_DIR "vnet%d/", devNum) < 0) return -1; ret = virFileExists(path) ? 1 : 0; VIR_DEBUG("Checked dev vnet%d usage: %d", devNum, ret); - VIR_FREE(path); return ret; } @@ -111,8 +110,6 @@ static int virNetDevVethGetFreeNum(int startDev) int virNetDevVethCreate(char** veth1, char** veth2) { int ret = -1; - char *veth1auto = NULL; - char *veth2auto = NULL; int vethNum = 0; virCommandPtr cmd = NULL; size_t i; @@ -125,6 +122,9 @@ int virNetDevVethCreate(char** veth1, char** veth2) #define MAX_VETH_RETRIES 10 for (i = 0; i < MAX_VETH_RETRIES; i++) { + VIR_AUTOFREE(char *) veth1auto = NULL; + VIR_AUTOFREE(char *) veth2auto = NULL; + int status; if (!*veth1) { int veth1num; @@ -173,8 +173,6 @@ int virNetDevVethCreate(char** veth1, char** veth2) *veth1 ? *veth1 : veth1auto, *veth2 ? *veth2 : veth2auto, status); - VIR_FREE(veth1auto); - VIR_FREE(veth2auto); virCommandFree(cmd); cmd = NULL; } @@ -186,8 +184,6 @@ int virNetDevVethCreate(char** veth1, char** veth2) cleanup: virMutexUnlock(&virNetDevVethCreateMutex); virCommandFree(cmd); - VIR_FREE(veth1auto); - VIR_FREE(veth2auto); return ret; } -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- src/util/virnetdevveth.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/src/util/virnetdevveth.c b/src/util/virnetdevveth.c index 8c1a7f3..0b94f73 100644 --- a/src/util/virnetdevveth.c +++ b/src/util/virnetdevveth.c @@ -111,7 +111,6 @@ int virNetDevVethCreate(char** veth1, char** veth2) { int ret = -1; int vethNum = 0; - virCommandPtr cmd = NULL; size_t i; /* @@ -124,6 +123,7 @@ int virNetDevVethCreate(char** veth1, char** veth2) for (i = 0; i < MAX_VETH_RETRIES; i++) { VIR_AUTOFREE(char *) veth1auto = NULL; VIR_AUTOFREE(char *) veth2auto = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; int status; if (!*veth1) { @@ -173,8 +173,6 @@ int virNetDevVethCreate(char** veth1, char** veth2) *veth1 ? *veth1 : veth1auto, *veth2 ? *veth2 : veth2auto, status); - virCommandFree(cmd); - cmd = NULL; } virReportError(VIR_ERR_INTERNAL_ERROR, @@ -183,7 +181,6 @@ int virNetDevVethCreate(char** veth1, char** veth2) cleanup: virMutexUnlock(&virNetDevVethCreateMutex); - virCommandFree(cmd); return ret; } @@ -200,26 +197,22 @@ int virNetDevVethCreate(char** veth1, char** veth2) */ int virNetDevVethDelete(const char *veth) { - virCommandPtr cmd = virCommandNewArgList("ip", "link", "del", veth, NULL); + VIR_AUTOPTR(virCommand) cmd = NULL; + cmd = virCommandNewArgList("ip", "link", "del", veth, NULL); int status; - int ret = -1; if (virCommandRun(cmd, &status) < 0) - goto cleanup; + return -1; if (status != 0) { if (!virNetDevExists(veth)) { VIR_DEBUG("Device %s already deleted (by kernel namespace cleanup)", veth); - ret = 0; - goto cleanup; + return 0; } virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to delete veth device %s"), veth); - goto cleanup; + return -1; } - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; + return 0; } -- 1.8.3.1

Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in src/util/viralloc.h, define a new wrapper around an existing cleanup function which will be called when a variable declared with VIR_AUTOPTR macro goes out of scope. Also, drop the redundant viralloc.h include, since that has moved from the source module into the header. This commit also adds a typedef for virNetlinkHandlePtr for use with the cleanup macros. When a variable of type virNetlinkHandlePtr is declared using VIR_AUTOPTR, the function virNetlinkFree will be run automatically on it when it goes out of scope. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- src/util/virnetlink.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index fa1ba3e..8d28387 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -72,6 +72,10 @@ typedef struct nl_handle virNetlinkHandle; typedef struct nl_sock virNetlinkHandle; # endif +typedef virNetlinkHandle *virNetlinkHandlePtr; + +VIR_DEFINE_AUTOPTR_FUNC(virNetlinkHandle, virNetlinkFree) + typedef struct _virNetlinkEventSrvPrivate virNetlinkEventSrvPrivate; typedef virNetlinkEventSrvPrivate *virNetlinkEventSrvPrivatePtr; struct _virNetlinkEventSrvPrivate { @@ -79,7 +83,7 @@ struct _virNetlinkEventSrvPrivate { virMutex lock; int eventwatch; int netlinkfd; - virNetlinkHandle *netlinknh; + virNetlinkHandlePtr netlinknh; /*Events*/ int handled; size_t handlesCount; @@ -102,7 +106,7 @@ static int nextWatch = 1; /* Linux kernel supports up to MAX_LINKS (32 at the time) individual * netlink protocols. */ static virNetlinkEventSrvPrivatePtr server[MAX_LINKS] = {NULL}; -static virNetlinkHandle *placeholder_nlhandle; +static virNetlinkHandlePtr placeholder_nlhandle; /* Function definitions */ @@ -172,10 +176,10 @@ virNetlinkShutdown(void) * Returns a handle to the new netlink socket, or 0 if there was a failure. * */ -static virNetlinkHandle * +static virNetlinkHandlePtr virNetlinkCreateSocket(int protocol) { - virNetlinkHandle *nlhandle = NULL; + virNetlinkHandlePtr nlhandle = NULL; if (!(nlhandle = virNetlinkAlloc())) { virReportSystemError(errno, "%s", @@ -209,7 +213,7 @@ virNetlinkCreateSocket(int protocol) goto cleanup; } -static virNetlinkHandle * +static virNetlinkHandlePtr virNetlinkSendRequest(struct nl_msg *nl_msg, uint32_t src_pid, struct sockaddr_nl nladdr, unsigned int protocol, unsigned int groups) @@ -217,7 +221,7 @@ virNetlinkSendRequest(struct nl_msg *nl_msg, uint32_t src_pid, ssize_t nbytes; int fd; int n; - virNetlinkHandle *nlhandle = NULL; + virNetlinkHandlePtr nlhandle = NULL; struct pollfd fds[1]; struct nlmsghdr *nlmsg = nlmsg_hdr(nl_msg); @@ -303,7 +307,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg, .nl_groups = 0, }; struct pollfd fds[1]; - virNetlinkHandle *nlhandle = NULL; + virNetlinkHandlePtr nlhandle = NULL; int len = 0; memset(fds, 0, sizeof(fds)); @@ -353,7 +357,7 @@ virNetlinkDumpCommand(struct nl_msg *nl_msg, .nl_pid = dst_pid, .nl_groups = 0, }; - virNetlinkHandle *nlhandle = NULL; + virNetlinkHandlePtr nlhandle = NULL; if (!(nlhandle = virNetlinkSendRequest(nl_msg, src_pid, nladdr, protocol, groups))) -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- src/util/virnetlink.c | 48 +++++++++++++++++++----------------------------- 1 file changed, 19 insertions(+), 29 deletions(-) diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index 8d28387..6b00559 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -221,30 +221,31 @@ virNetlinkSendRequest(struct nl_msg *nl_msg, uint32_t src_pid, ssize_t nbytes; int fd; int n; - virNetlinkHandlePtr nlhandle = NULL; + VIR_AUTOPTR(virNetlinkHandle) nlhandle = NULL; + virNetlinkHandlePtr temp = NULL; struct pollfd fds[1]; struct nlmsghdr *nlmsg = nlmsg_hdr(nl_msg); if (protocol >= MAX_LINKS) { virReportSystemError(EINVAL, _("invalid protocol argument: %d"), protocol); - goto error; + return NULL; } if (!(nlhandle = virNetlinkCreateSocket(protocol))) - goto error; + return NULL; fd = nl_socket_get_fd(nlhandle); if (fd < 0) { virReportSystemError(errno, "%s", _("cannot get netlink socket fd")); - goto error; + return NULL; } if (groups && nl_socket_add_membership(nlhandle, groups) < 0) { virReportSystemError(errno, "%s", _("cannot add netlink membership")); - goto error; + return NULL; } nlmsg_set_dst(nl_msg, &nladdr); @@ -255,7 +256,7 @@ virNetlinkSendRequest(struct nl_msg *nl_msg, uint32_t src_pid, if (nbytes < 0) { virReportSystemError(errno, "%s", _("cannot send to netlink socket")); - goto error; + return NULL; } memset(fds, 0, sizeof(fds)); @@ -273,11 +274,9 @@ virNetlinkSendRequest(struct nl_msg *nl_msg, uint32_t src_pid, _("no valid netlink response was received")); } - return nlhandle; + VIR_STEAL_PTR(temp, nlhandle); - error: - virNetlinkFree(nlhandle); - return NULL; + return temp; } /** @@ -307,7 +306,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg, .nl_groups = 0, }; struct pollfd fds[1]; - virNetlinkHandlePtr nlhandle = NULL; + VIR_AUTOPTR(virNetlinkHandle) nlhandle = NULL; int len = 0; memset(fds, 0, sizeof(fds)); @@ -335,7 +334,6 @@ int virNetlinkCommand(struct nl_msg *nl_msg, *respbuflen = 0; } - virNetlinkFree(nlhandle); return ret; } @@ -346,10 +344,8 @@ virNetlinkDumpCommand(struct nl_msg *nl_msg, unsigned int protocol, unsigned int groups, void *opaque) { - int ret = -1; bool end = false; int len = 0; - struct nlmsghdr *resp = NULL; struct nlmsghdr *msg = NULL; struct sockaddr_nl nladdr = { @@ -357,13 +353,14 @@ virNetlinkDumpCommand(struct nl_msg *nl_msg, .nl_pid = dst_pid, .nl_groups = 0, }; - virNetlinkHandlePtr nlhandle = NULL; + VIR_AUTOPTR(virNetlinkHandle) nlhandle = NULL; if (!(nlhandle = virNetlinkSendRequest(nl_msg, src_pid, nladdr, protocol, groups))) - goto cleanup; + return -1; while (!end) { + VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; len = nl_recv(nlhandle, &nladdr, (unsigned char **)&resp, NULL); VIR_WARNINGS_NO_CAST_ALIGN for (msg = resp; NLMSG_OK(msg, len); msg = NLMSG_NEXT(msg, len)) { @@ -372,20 +369,14 @@ virNetlinkDumpCommand(struct nl_msg *nl_msg, end = true; if (virNetlinkGetErrorCode(msg, len) < 0) - goto cleanup; + return -1; if (callback(msg, opaque) < 0) - goto cleanup; + return -1; } - VIR_FREE(resp); } - ret = 0; - - cleanup: - VIR_FREE(resp); - virNetlinkFree(nlhandle); - return ret; + return 0; } /** @@ -527,7 +518,7 @@ int virNetlinkDelLink(const char *ifname, virNetlinkDelLinkFallback fallback) { int rc = -1; - struct nlmsghdr *resp = NULL; + VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; struct nlmsgerr *err; struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC }; unsigned int recvbuflen; @@ -582,7 +573,6 @@ virNetlinkDelLink(const char *ifname, virNetlinkDelLinkFallback fallback) rc = 0; cleanup: nlmsg_free(nl_msg); - VIR_FREE(resp); return rc; malformed_resp: @@ -771,7 +761,7 @@ virNetlinkEventCallback(int watch, void *opaque) { virNetlinkEventSrvPrivatePtr srv = opaque; - struct nlmsghdr *msg; + VIR_AUTOFREE(struct nlmsghdr *) msg = NULL; struct sockaddr_nl peer; struct ucred *creds = NULL; size_t i; @@ -806,7 +796,7 @@ virNetlinkEventCallback(int watch, if (!handled) VIR_DEBUG("event not handled."); - VIR_FREE(msg); + virNetlinkEventServerUnlock(srv); } -- 1.8.3.1

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@gmail.com> --- src/util/virnuma.c | 79 +++++++++++++++++++++--------------------------------- 1 file changed, 31 insertions(+), 48 deletions(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 784db0a..841c7cb 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -252,8 +252,8 @@ int virNumaGetNodeCPUs(int node, virBitmapPtr *cpus) { - unsigned long *mask = NULL; - unsigned long *allonesmask = NULL; + VIR_AUTOFREE(unsigned long *) mask = NULL; + VIR_AUTOFREE(unsigned long *) allonesmask = NULL; virBitmapPtr cpumap = NULL; int ncpus = 0; int max_n_cpus = virNumaGetMaxCPUs(); @@ -300,8 +300,6 @@ virNumaGetNodeCPUs(int node, ret = ncpus; cleanup: - VIR_FREE(mask); - VIR_FREE(allonesmask); virBitmapFree(cpumap); return ret; @@ -566,52 +564,47 @@ virNumaGetHugePageInfo(int node, unsigned long long *page_avail, unsigned long long *page_free) { - int ret = -1; - char *path = NULL; - char *buf = NULL; char *end; if (page_avail) { + VIR_AUTOFREE(char *) path = NULL; + VIR_AUTOFREE(char *) buf = NULL; if (virNumaGetHugePageInfoPath(&path, node, page_size, "nr_hugepages") < 0) - goto cleanup; + return -1; if (virFileReadAll(path, 1024, &buf) < 0) - goto cleanup; + return -1; if (virStrToLong_ull(buf, &end, 10, page_avail) < 0 || *end != '\n') { virReportError(VIR_ERR_INTERNAL_ERROR, _("unable to parse: %s"), buf); - goto cleanup; + return -1; } - VIR_FREE(buf); - VIR_FREE(path); } if (page_free) { + VIR_AUTOFREE(char *) path = NULL; + VIR_AUTOFREE(char *) buf = NULL; if (virNumaGetHugePageInfoPath(&path, node, page_size, "free_hugepages") < 0) - goto cleanup; + return -1; if (virFileReadAll(path, 1024, &buf) < 0) - goto cleanup; + return -1; if (virStrToLong_ull(buf, &end, 10, page_free) < 0 || *end != '\n') { virReportError(VIR_ERR_INTERNAL_ERROR, _("unable to parse: %s"), buf); - goto cleanup; + return -1; } } - ret = 0; - cleanup: - VIR_FREE(buf); - VIR_FREE(path); - return ret; + return 0; } /** @@ -714,13 +707,13 @@ virNumaGetPages(int node, size_t *npages) { int ret = -1; - char *path = NULL; + VIR_AUTOFREE(char *) path = NULL; + VIR_AUTOFREE(unsigned int *) tmp_size = NULL; + VIR_AUTOFREE(unsigned long long *) tmp_avail = NULL; + VIR_AUTOFREE(unsigned long long *) tmp_free = NULL; DIR *dir = NULL; int direrr = 0; struct dirent *entry; - unsigned int *tmp_size = NULL; - unsigned long long *tmp_avail = NULL; - unsigned long long *tmp_free = NULL; unsigned int ntmp = 0; size_t i; bool exchange; @@ -828,11 +821,7 @@ virNumaGetPages(int node, *npages = ntmp; ret = 0; cleanup: - VIR_FREE(tmp_free); - VIR_FREE(tmp_avail); - VIR_FREE(tmp_size); VIR_DIR_CLOSE(dir); - VIR_FREE(path); return ret; } @@ -843,8 +832,8 @@ virNumaSetPagePoolSize(int node, unsigned long long page_count, bool add) { - int ret = -1; - char *nr_path = NULL, *nr_buf = NULL; + VIR_AUTOFREE(char *) nr_path = NULL; + VIR_AUTOFREE(char *) nr_buf = NULL; char *end; unsigned long long nr_count; @@ -853,37 +842,35 @@ virNumaSetPagePoolSize(int node, * differently to huge pages. */ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("system pages pool can't be modified")); - goto cleanup; + return -1; } if (virNumaGetHugePageInfoPath(&nr_path, node, page_size, "nr_hugepages") < 0) - goto cleanup; + return -1; /* Firstly check, if there's anything for us to do */ if (virFileReadAll(nr_path, 1024, &nr_buf) < 0) - goto cleanup; + return -1; if (virStrToLong_ull(nr_buf, &end, 10, &nr_count) < 0 || *end != '\n') { virReportError(VIR_ERR_OPERATION_FAILED, _("invalid number '%s' in '%s'"), nr_buf, nr_path); - goto cleanup; + return -1; } if (add) { if (!page_count) { VIR_DEBUG("Nothing left to do: add = true page_count = 0"); - ret = 0; - goto cleanup; + return 0; } page_count += nr_count; } else { if (nr_count == page_count) { VIR_DEBUG("Nothing left to do: nr_count = page_count = %llu", page_count); - ret = 0; - goto cleanup; + return 0; } } @@ -896,40 +883,36 @@ virNumaSetPagePoolSize(int node, */ VIR_FREE(nr_buf); if (virAsprintf(&nr_buf, "%llu", page_count) < 0) - goto cleanup; + return -1; if (virFileWriteStr(nr_path, nr_buf, 0) < 0) { virReportSystemError(errno, _("Unable to write to: %s"), nr_path); - goto cleanup; + return -1; } /* And now do the check. */ VIR_FREE(nr_buf); if (virFileReadAll(nr_path, 1024, &nr_buf) < 0) - goto cleanup; + return -1; if (virStrToLong_ull(nr_buf, &end, 10, &nr_count) < 0 || *end != '\n') { virReportError(VIR_ERR_OPERATION_FAILED, _("invalid number '%s' in '%s'"), nr_buf, nr_path); - goto cleanup; + return -1; } if (nr_count != page_count) { virReportError(VIR_ERR_OPERATION_FAILED, _("Unable to allocate %llu pages. Allocated only %llu"), page_count, nr_count); - goto cleanup; + return -1; } - ret = 0; - cleanup: - VIR_FREE(nr_buf); - VIR_FREE(nr_path); - return ret; + return 0; } -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- src/util/virnuma.c | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 841c7cb..fde46ec 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -57,7 +57,7 @@ char * virNumaGetAutoPlacementAdvice(unsigned short vcpus, unsigned long long balloon) { - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; char *output = NULL; cmd = virCommandNewArgList(NUMAD, "-w", NULL); @@ -71,7 +71,6 @@ virNumaGetAutoPlacementAdvice(unsigned short vcpus, _("Failed to query numad for the " "advisory nodeset")); - virCommandFree(cmd); return output; } #else /* !HAVE_NUMAD */ @@ -252,41 +251,38 @@ int virNumaGetNodeCPUs(int node, virBitmapPtr *cpus) { + VIR_AUTOPTR(virBitmap) cpumap = NULL; VIR_AUTOFREE(unsigned long *) mask = NULL; VIR_AUTOFREE(unsigned long *) allonesmask = NULL; - virBitmapPtr cpumap = NULL; int ncpus = 0; int max_n_cpus = virNumaGetMaxCPUs(); int mask_n_bytes = max_n_cpus / 8; size_t i; - int ret = -1; *cpus = NULL; if (VIR_ALLOC_N(mask, mask_n_bytes / sizeof(*mask)) < 0) - goto cleanup; + return -1; if (VIR_ALLOC_N(allonesmask, mask_n_bytes / sizeof(*mask)) < 0) - goto cleanup; + return -1; memset(allonesmask, 0xff, mask_n_bytes); /* The first time this returns -1, ENOENT if node doesn't exist... */ if (numa_node_to_cpus(node, mask, mask_n_bytes) < 0) { VIR_WARN("NUMA topology for cell %d is not available, ignoring", node); - ret = -2; - goto cleanup; + return -2; } /* second, third... times it returns an all-1's mask */ if (memcmp(mask, allonesmask, mask_n_bytes) == 0) { VIR_DEBUG("NUMA topology for cell %d is invalid, ignoring", node); - ret = -2; - goto cleanup; + return -2; } if (!(cpumap = virBitmapNew(max_n_cpus))) - goto cleanup; + return -1; for (i = 0; i < max_n_cpus; i++) { if (MASK_CPU_ISSET(mask, i)) { @@ -295,14 +291,8 @@ virNumaGetNodeCPUs(int node, } } - *cpus = cpumap; - cpumap = NULL; - ret = ncpus; - - cleanup: - virBitmapFree(cpumap); - - return ret; + VIR_STEAL_PTR(*cpus, cpumap); + return ncpus; } # undef MASK_CPU_ISSET # undef n_bits -- 1.8.3.1

Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in src/util/viralloc.h, define a new wrapper around an existing cleanup function which will be called when a variable declared with VIR_AUTOPTR macro goes out of scope. Also, drop the redundant viralloc.h include, since that has moved from the source module into the header. When a variable of type virPerfPtr is declared using VIR_AUTOPTR, the function virPerfFree will be run automatically on it when it goes out of scope. This commit also adds an intermediate typedef for virPerf type for use with the cleanup macros. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- src/util/virperf.c | 3 +-- src/util/virperf.h | 8 ++++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/util/virperf.c b/src/util/virperf.c index 2c832b3..4537cd0 100644 --- a/src/util/virperf.c +++ b/src/util/virperf.c @@ -26,7 +26,6 @@ #endif #include "virperf.h" -#include "viralloc.h" #include "virerror.h" #include "virlog.h" #include "virfile.h" @@ -61,7 +60,7 @@ struct virPerfEvent { }; typedef struct virPerfEvent *virPerfEventPtr; -struct virPerf { +struct _virPerf { struct virPerfEvent events[VIR_PERF_EVENT_LAST]; }; diff --git a/src/util/virperf.h b/src/util/virperf.h index eee7a03..9d0d5ac 100644 --- a/src/util/virperf.h +++ b/src/util/virperf.h @@ -23,6 +23,7 @@ # define __VIR_PERF_H__ # include "virutil.h" +# include "viralloc.h" /* Some Intel processor families introduced some RDT (Resource Director * Technology) features to monitor or control shared resource based on @@ -62,8 +63,9 @@ typedef enum { VIR_ENUM_DECL(virPerfEvent); -struct virPerf; -typedef struct virPerf *virPerfPtr; +struct _virPerf; +typedef struct _virPerf virPerf; +typedef virPerf *virPerfPtr; virPerfPtr virPerfNew(void); @@ -83,4 +85,6 @@ int virPerfReadEvent(virPerfPtr perf, virPerfEventType type, uint64_t *value); +VIR_DEFINE_AUTOPTR_FUNC(virPerf, virPerfFree) + #endif /* __VIR_PERF_H__ */ -- 1.8.3.1

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@gmail.com> --- src/util/virperf.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/util/virperf.c b/src/util/virperf.c index 4537cd0..1203a6e 100644 --- a/src/util/virperf.c +++ b/src/util/virperf.c @@ -175,12 +175,12 @@ typedef struct virPerfEventAttr *virPerfEventAttrPtr; static int virPerfRdtAttrInit(void) { - char *buf = NULL; + VIR_AUTOFREE(char *) buf = NULL; char *tmp = NULL; unsigned int attr_type = 0; if (virFileReadAllQuiet("/sys/devices/intel_cqm/type", 10, &buf) < 0) - goto error; + return -1; if ((tmp = strchr(buf, '\n'))) *tmp = '\0'; @@ -188,19 +188,14 @@ virPerfRdtAttrInit(void) if (virStrToLong_ui(buf, NULL, 10, &attr_type) < 0) { virReportSystemError(errno, "%s", _("failed to get rdt event type")); - goto error; + return -1; } - VIR_FREE(buf); attrs[VIR_PERF_EVENT_CMT].attrType = attr_type; attrs[VIR_PERF_EVENT_MBMT].attrType = attr_type; attrs[VIR_PERF_EVENT_MBML].attrType = attr_type; return 0; - - error: - VIR_FREE(buf); - return -1; } @@ -209,7 +204,6 @@ virPerfEventEnable(virPerfPtr perf, virPerfEventType type, pid_t pid) { - char *buf = NULL; struct perf_event_attr attr; virPerfEventPtr event = &(perf->events[type]); virPerfEventAttrPtr event_attr = &attrs[type]; @@ -227,6 +221,8 @@ virPerfEventEnable(virPerfPtr perf, } if (type == VIR_PERF_EVENT_CMT) { + VIR_AUTOFREE(char *) buf = NULL; + if (virFileReadAll("/sys/devices/intel_cqm/events/llc_occupancy.scale", 10, &buf) < 0) goto error; @@ -236,8 +232,6 @@ virPerfEventEnable(virPerfPtr perf, _("failed to get cmt scaling factor")); goto error; } - - VIR_FREE(buf); } memset(&attr, 0, sizeof(attr)); @@ -268,7 +262,6 @@ virPerfEventEnable(virPerfPtr perf, error: VIR_FORCE_CLOSE(event->fd); - VIR_FREE(buf); return -1; } -- 1.8.3.1

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@gmail.com> --- src/util/virpidfile.c | 185 ++++++++++++++++---------------------------------- 1 file changed, 59 insertions(+), 126 deletions(-) diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c index 1a85d43..d82bf92 100644 --- a/src/util/virpidfile.c +++ b/src/util/virpidfile.c @@ -97,29 +97,18 @@ int virPidFileWrite(const char *dir, const char *name, pid_t pid) { - int rc; - char *pidfile = NULL; + VIR_AUTOFREE(char *) pidfile = NULL; - if (name == NULL || dir == NULL) { - rc = -EINVAL; - goto cleanup; - } + if (name == NULL || dir == NULL) + return -EINVAL; - if (virFileMakePath(dir) < 0) { - rc = -errno; - goto cleanup; - } + if (virFileMakePath(dir) < 0) + return -errno; - if (!(pidfile = virPidFileBuildPath(dir, name))) { - rc = -ENOMEM; - goto cleanup; - } + if (!(pidfile = virPidFileBuildPath(dir, name))) + return -ENOMEM; - rc = virPidFileWritePath(pidfile, pid); - - cleanup: - VIR_FREE(pidfile); - return rc; + return virPidFileWritePath(pidfile, pid); } @@ -170,25 +159,16 @@ int virPidFileRead(const char *dir, const char *name, pid_t *pid) { - int rc; - char *pidfile = NULL; + VIR_AUTOFREE(char *) pidfile = NULL; *pid = 0; - if (name == NULL || dir == NULL) { - rc = -EINVAL; - goto cleanup; - } + if (name == NULL || dir == NULL) + return -EINVAL; - if (!(pidfile = virPidFileBuildPath(dir, name))) { - rc = -ENOMEM; - goto cleanup; - } + if (!(pidfile = virPidFileBuildPath(dir, name))) + return -ENOMEM; - rc = virPidFileReadPath(pidfile, pid); - - cleanup: - VIR_FREE(pidfile); - return rc; + return virPidFileReadPath(pidfile, pid); } @@ -219,11 +199,11 @@ int virPidFileReadPathIfAlive(const char *path, { int ret; bool isLink; - char *procPath = NULL; - char *procLink = NULL; + VIR_AUTOFREE(char *) procPath = NULL; + VIR_AUTOFREE(char *) procLink = NULL; + VIR_AUTOFREE(char *) resolvedBinPath = NULL; + VIR_AUTOFREE(char *) resolvedProcLink = NULL; size_t procLinkLen; - char *resolvedBinPath = NULL; - char *resolvedProcLink = NULL; const char deletedText[] = " (deleted)"; size_t deletedTextLen = strlen(deletedText); pid_t retPid; @@ -232,7 +212,7 @@ int virPidFileReadPathIfAlive(const char *path, *pid = -1; if ((ret = virPidFileReadPath(path, &retPid)) < 0) - goto cleanup; + return ret; #ifndef WIN32 /* Check that it's still alive. Safe to skip this sanity check on @@ -252,13 +232,12 @@ int virPidFileReadPathIfAlive(const char *path, goto cleanup; } - if (virAsprintf(&procPath, "/proc/%lld/exe", (long long)retPid) < 0) { - ret = -ENOMEM; - goto cleanup; - } + if (virAsprintf(&procPath, "/proc/%lld/exe", (long long)retPid) < 0) + return -ENOMEM; if ((ret = virFileIsLink(procPath)) < 0) - goto cleanup; + return ret; + isLink = ret; if (isLink && virFileLinkPointsTo(procPath, binPath)) { @@ -275,27 +254,21 @@ int virPidFileReadPathIfAlive(const char *path, * "$procpath (deleted)". Read that link, remove the " (deleted)" * part, and see if it has the same canonicalized name as binpath. */ - if (!(procLink = areadlink(procPath))) { - ret = -errno; - goto cleanup; - } + if (!(procLink = areadlink(procPath))) + return -errno; + procLinkLen = strlen(procLink); if (procLinkLen > deletedTextLen) procLink[procLinkLen - deletedTextLen] = 0; if ((ret = virFileResolveAllLinks(binPath, &resolvedBinPath)) < 0) - goto cleanup; + return ret; if ((ret = virFileResolveAllLinks(procLink, &resolvedProcLink)) < 0) - goto cleanup; + return ret; ret = STREQ(resolvedBinPath, resolvedProcLink) ? 0 : -1; cleanup: - VIR_FREE(procPath); - VIR_FREE(procLink); - VIR_FREE(resolvedProcLink); - VIR_FREE(resolvedBinPath); - /* return the originally set pid of -1 unless we proclaim success */ if (ret == 0) *pid = retPid; @@ -326,24 +299,15 @@ int virPidFileReadIfAlive(const char *dir, pid_t *pid, const char *binpath) { - int rc = 0; - char *pidfile = NULL; + VIR_AUTOFREE(char *) pidfile = NULL; - if (name == NULL || dir == NULL) { - rc = -EINVAL; - goto cleanup; - } + if (name == NULL || dir == NULL) + return -EINVAL; - if (!(pidfile = virPidFileBuildPath(dir, name))) { - rc = -ENOMEM; - goto cleanup; - } + if (!(pidfile = virPidFileBuildPath(dir, name))) + return -ENOMEM; - rc = virPidFileReadPathIfAlive(pidfile, pid, binpath); - - cleanup: - VIR_FREE(pidfile); - return rc; + return virPidFileReadPathIfAlive(pidfile, pid, binpath); } @@ -361,24 +325,15 @@ int virPidFileDeletePath(const char *pidfile) int virPidFileDelete(const char *dir, const char *name) { - int rc = 0; - char *pidfile = NULL; + VIR_AUTOFREE(char *) pidfile = NULL; - if (name == NULL || dir == NULL) { - rc = -EINVAL; - goto cleanup; - } + if (name == NULL || dir == NULL) + return -EINVAL; - if (!(pidfile = virPidFileBuildPath(dir, name))) { - rc = -ENOMEM; - goto cleanup; - } + if (!(pidfile = virPidFileBuildPath(dir, name))) + return -ENOMEM; - rc = virPidFileDeletePath(pidfile); - - cleanup: - VIR_FREE(pidfile); - return rc; + return virPidFileDeletePath(pidfile); } int virPidFileAcquirePath(const char *path, @@ -470,24 +425,15 @@ int virPidFileAcquire(const char *dir, bool waitForLock, pid_t pid) { - int rc = 0; - char *pidfile = NULL; + VIR_AUTOFREE(char *) pidfile = NULL; - if (name == NULL || dir == NULL) { - rc = -EINVAL; - goto cleanup; - } + if (name == NULL || dir == NULL) + return -EINVAL; - if (!(pidfile = virPidFileBuildPath(dir, name))) { - rc = -ENOMEM; - goto cleanup; - } + if (!(pidfile = virPidFileBuildPath(dir, name))) + return -ENOMEM; - rc = virPidFileAcquirePath(pidfile, waitForLock, pid); - - cleanup: - VIR_FREE(pidfile); - return rc; + return virPidFileAcquirePath(pidfile, waitForLock, pid); } @@ -518,24 +464,15 @@ int virPidFileRelease(const char *dir, const char *name, int fd) { - int rc = 0; - char *pidfile = NULL; + VIR_AUTOFREE(char *) pidfile = NULL; - if (name == NULL || dir == NULL) { - rc = -EINVAL; - goto cleanup; - } + if (name == NULL || dir == NULL) + return -EINVAL; - if (!(pidfile = virPidFileBuildPath(dir, name))) { - rc = -ENOMEM; - goto cleanup; - } + if (!(pidfile = virPidFileBuildPath(dir, name))) + return -ENOMEM; - rc = virPidFileReleasePath(pidfile, fd); - - cleanup: - VIR_FREE(pidfile); - return rc; + return virPidFileReleasePath(pidfile, fd); } @@ -545,8 +482,7 @@ virPidFileConstructPath(bool privileged, const char *progname, char **pidfile) { - int ret = -1; - char *rundir = NULL; + VIR_AUTOFREE(char *) rundir = NULL; if (privileged) { /* @@ -556,29 +492,26 @@ virPidFileConstructPath(bool privileged, if (!statedir) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("No statedir specified")); - goto cleanup; + return -1; } if (virAsprintf(pidfile, "%s/run/%s.pid", statedir, progname) < 0) - goto cleanup; + return -1; } else { if (!(rundir = virGetUserRuntimeDirectory())) - goto cleanup; + return -1; if (virFileMakePathWithMode(rundir, 0700) < 0) { virReportSystemError(errno, _("Cannot create user runtime directory '%s'"), rundir); - goto cleanup; + return -1; } if (virAsprintf(pidfile, "%s/%s.pid", rundir, progname) < 0) - goto cleanup; + return -1; } - ret = 0; - cleanup: - VIR_FREE(rundir); - return ret; + return 0; } -- 1.8.3.1

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@gmail.com> --- src/util/virprocess.c | 49 ++++++++++++++++++++++++------------------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/src/util/virprocess.c b/src/util/virprocess.c index f92b0dc..1dd14de 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -158,7 +158,7 @@ virProcessAbort(pid_t pid) int saved_errno; int ret; int status; - char *tmp = NULL; + VIR_AUTOFREE(char *) tmp = NULL; if (pid <= 0) return; @@ -199,7 +199,6 @@ virProcessAbort(pid_t pid) VIR_DEBUG("failed to reap child %lld, abandoning it", (long long) pid); cleanup: - VIR_FREE(tmp); errno = saved_errno; } #else @@ -271,11 +270,10 @@ virProcessWait(pid_t pid, int *exitstatus, bool raw) error: { - char *st = virProcessTranslateStatus(status); + VIR_AUTOFREE(char *) st = virProcessTranslateStatus(status); virReportError(VIR_ERR_INTERNAL_ERROR, _("Child process (%lld) unexpected %s"), (long long) pid, NULLSTR(st)); - VIR_FREE(st); } return -1; } @@ -475,7 +473,11 @@ virBitmapPtr virProcessGetAffinity(pid_t pid) { size_t i; +# ifdef CPU_ALLOC cpu_set_t *mask; +# else + VIR_AUTOFREE(cpu_set_t *) mask = NULL; +# endif size_t masklen; size_t ncpus; virBitmapPtr ret = NULL; @@ -504,11 +506,20 @@ virProcessGetAffinity(pid_t pid) if (sched_getaffinity(pid, masklen, mask) < 0) { virReportSystemError(errno, _("cannot get CPU affinity of process %d"), pid); +# ifdef CPU_ALLOC goto cleanup; +# else + return ret; +# endif } if (!(ret = virBitmapNew(ncpus))) - goto cleanup; +# ifdef CPU_ALLOC + + goto cleanup; +# else + return ret; +# endif for (i = 0; i < ncpus; i++) { # ifdef CPU_ALLOC @@ -522,11 +533,7 @@ virProcessGetAffinity(pid_t pid) } cleanup: -# ifdef CPU_ALLOC CPU_FREE(mask); -# else - VIR_FREE(mask); -# endif return ret; } @@ -603,7 +610,7 @@ virProcessGetAffinity(pid_t pid ATTRIBUTE_UNUSED) int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids) { int ret = -1; - char *taskPath = NULL; + VIR_AUTOFREE(char *) taskPath = NULL; DIR *dir = NULL; int value; struct dirent *ent; @@ -636,7 +643,6 @@ int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids) cleanup: VIR_DIR_CLOSE(dir); - VIR_FREE(taskPath); if (ret < 0) VIR_FREE(*pids); return ret; @@ -648,7 +654,6 @@ int virProcessGetNamespaces(pid_t pid, int **fdlist) { int ret = -1; - char *nsfile = NULL; size_t i = 0; const char *ns[] = { "user", "ipc", "uts", "net", "pid", "mnt" }; @@ -656,6 +661,7 @@ int virProcessGetNamespaces(pid_t pid, *fdlist = NULL; for (i = 0; i < ARRAY_CARDINALITY(ns); i++) { + VIR_AUTOFREE(char *) nsfile = NULL; int fd; if (virAsprintf(&nsfile, "/proc/%llu/ns/%s", @@ -671,14 +677,11 @@ int virProcessGetNamespaces(pid_t pid, (*fdlist)[(*nfdlist)-1] = fd; } - - VIR_FREE(nsfile); } ret = 0; cleanup: - VIR_FREE(nsfile); if (ret < 0) { for (i = 0; i < *nfdlist; i++) VIR_FORCE_CLOSE((*fdlist)[i]); @@ -977,8 +980,8 @@ virProcessSetMaxCoreSize(pid_t pid ATTRIBUTE_UNUSED, int virProcessGetStartTime(pid_t pid, unsigned long long *timestamp) { - char *filename = NULL; - char *buf = NULL; + VIR_AUTOFREE(char *) filename = NULL; + VIR_AUTOFREE(char *) buf = NULL; char *tmp; int ret = -1; int len; @@ -1032,8 +1035,6 @@ int virProcessGetStartTime(pid_t pid, cleanup: virStringListFree(tokens); - VIR_FREE(filename); - VIR_FREE(buf); return ret; } #elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) @@ -1080,7 +1081,7 @@ static int virProcessNamespaceHelper(int errfd, virProcessNamespaceCallback cb, void *opaque) { - char *path; + VIR_AUTOFREE(char *) path = NULL; int fd = -1; int ret = -1; @@ -1109,7 +1110,6 @@ static int virProcessNamespaceHelper(int errfd, ignore_value(safewrite(errfd, err->message, len)); } } - VIR_FREE(path); VIR_FORCE_CLOSE(fd); return ret; } @@ -1145,7 +1145,7 @@ virProcessRunInMountNamespace(pid_t pid, VIR_FORCE_CLOSE(errfd[1]); _exit(ret < 0 ? EXIT_CANCELED : ret); } else { - char *buf = NULL; + VIR_AUTOFREE(char *) buf = NULL; int status; VIR_FORCE_CLOSE(errfd[1]); @@ -1159,7 +1159,6 @@ virProcessRunInMountNamespace(pid_t pid, NULLSTR(buf)); } } - VIR_FREE(buf); } cleanup: @@ -1226,7 +1225,7 @@ virProcessNamespaceAvailable(unsigned int ns) int flags = 0; int cpid; char *childStack; - char *stack; + VIR_AUTOFREE(char *)stack = NULL; int stacksize = getpagesize() * 4; if (ns & VIR_PROCESS_NAMESPACE_MNT) @@ -1251,7 +1250,7 @@ virProcessNamespaceAvailable(unsigned int ns) childStack = stack + stacksize; cpid = clone(virProcessDummyChild, childStack, flags, NULL); - VIR_FREE(stack); + if (cpid < 0) { char ebuf[1024] ATTRIBUTE_UNUSED; VIR_DEBUG("clone call returned %s, container support is not enabled", -- 1.8.3.1