Add another usage for VIR_AUTOFREE macro which was left in the commit ec3e878, thereby dropping a VIR_FREE call and and a cleanup section. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/viriscsi.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c index c805ffc..cf07968 100644 --- a/src/util/viriscsi.c +++ b/src/util/viriscsi.c @@ -208,9 +208,10 @@ static int virStorageBackendCreateIfaceIQN(const char *initiatoriqn, char **ifacename) { - int ret = -1, exitstatus = -1; + int exitstatus = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; + VIR_AUTOFREE(char *) iface_name = NULL; VIR_AUTOFREE(char *) temp_ifacename = NULL; - VIR_AUTOPTR(virCommand) cmd = NULL; if (virAsprintf(&temp_ifacename, "libvirt-iface-%08llx", @@ -233,7 +234,7 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn, virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to run command '%s' to create new iscsi interface"), ISCSIADM); - goto cleanup; + return -1; } virCommandFree(cmd); @@ -252,26 +253,23 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn, virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to run command '%s' to update iscsi interface with IQN '%s'"), ISCSIADM, initiatoriqn); - goto cleanup; + return -1; } /* Check again to make sure the interface was created. */ - if (virStorageBackendIQNFound(initiatoriqn, ifacename) != IQN_FOUND) { + if (virStorageBackendIQNFound(initiatoriqn, &iface_name) != IQN_FOUND) { VIR_DEBUG("Failed to find interface '%s' with IQN '%s' " "after attempting to create it", &temp_ifacename[0], initiatoriqn); - goto cleanup; + return -1; } else { VIR_DEBUG("Interface '%s' with IQN '%s' was created successfully", - *ifacename, initiatoriqn); + iface_name, initiatoriqn); } - ret = 0; + VIR_STEAL_PTR(*ifacename, iface_name); - cleanup: - if (ret != 0) - VIR_FREE(*ifacename); - 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 typedefs virNlMsg to struct nl_msg type for use with the cleanup macros. When a variable of type virNlMsg * is declared using VIR_AUTOPTR, the function nlmsg_free will be run automatically on it when it goes out of scope. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virnetlink.c | 1 - src/util/virnetlink.h | 5 +++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index 162efe6..ecf62c9 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -38,7 +38,6 @@ #include "virnetlink.h" #include "virnetdev.h" #include "virlog.h" -#include "viralloc.h" #include "virthread.h" #include "virmacaddr.h" #include "virerror.h" diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h index 2a9de0a..8ebeab8 100644 --- a/src/util/virnetlink.h +++ b/src/util/virnetlink.h @@ -22,6 +22,7 @@ # include "internal.h" # include "virmacaddr.h" +# include "viralloc.h" # if defined(__linux__) && defined(HAVE_LIBNL) @@ -44,6 +45,8 @@ struct nlmsghdr; # endif /* __linux__ */ +typedef struct nl_msg virNlMsg; + int virNetlinkStartup(void); void virNetlinkShutdown(void); @@ -123,4 +126,6 @@ int virNetlinkEventAddClient(virNetlinkEventHandleCallback handleCB, int virNetlinkEventRemoveClient(int watch, const virMacAddr *macaddr, unsigned int protocol); +VIR_DEFINE_AUTOPTR_FUNC(virNlMsg, nlmsg_free) + #endif /* __VIR_NETLINK_H__ */ -- 1.8.3.1

On 08/13/2018 07:42 AM, Erik Skultety wrote: The freebsd builds are not very happy, for example from... https://ci.centos.org/view/libvirt/job/libvirt-master-build/systems=libvi... One gets: ... CC util/libvirt_nss_la-viratomic.lo In file included from ../../src/network/bridge_driver.c:63: ../../src/util/virnetlink.h:129:40: error: use of undeclared identifier 'nlmsg_free' VIR_DEFINE_AUTOPTR_FUNC(virNetlinkMsg, nlmsg_free) ^ 1 error generated. John

On Thu, Aug 09, 2018 at 09:42:11AM +0530, Sukrit Bhatnagar wrote: unnecessary... unnecessary... The rest is fine, I'll adjust before merging. Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt;

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(a)gmail.com&gt; --- src/util/virnetlink.c | 72 ++++++++++++++++++++------------------------------- 1 file changed, 28 insertions(+), 44 deletions(-) diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index fcdc09d..66e80e2 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -297,15 +297,16 @@ int virNetlinkCommand(struct nl_msg *nl_msg, uint32_t src_pid, uint32_t dst_pid, unsigned int protocol, unsigned int groups) { - int ret = -1; struct sockaddr_nl nladdr = { .nl_family = AF_NETLINK, .nl_pid = dst_pid, .nl_groups = 0, }; struct pollfd fds[1]; - VIR_AUTOPTR(virNetlinkHandle) nlhandle = NULL; int len = 0; + VIR_AUTOPTR(virNetlinkHandle) nlhandle = NULL; + + *respbuflen = 0; memset(fds, 0, sizeof(fds)); @@ -324,15 +325,12 @@ int virNetlinkCommand(struct nl_msg *nl_msg, goto cleanup; } - ret = 0; *respbuflen = len; - cleanup: - if (ret < 0) { - *resp = NULL; - *respbuflen = 0; - } + return 0; - return ret; + cleanup: + *resp = NULL; + return -1; } int @@ -409,7 +407,7 @@ virNetlinkDumpLink(const char *ifname, int ifindex, .ifi_index = ifindex }; unsigned int recvbuflen; - struct nl_msg *nl_msg; + VIR_AUTOPTR(virNlMsg) nl_msg = NULL; VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; *nlData = NULL; @@ -450,7 +448,7 @@ virNetlinkDumpLink(const char *ifname, int ifindex, if (virNetlinkCommand(nl_msg, &resp, &recvbuflen, src_pid, dst_pid, NETLINK_ROUTE, 0) < 0) - goto cleanup; + return -1; if (recvbuflen < NLMSG_LENGTH(0) || resp == NULL) goto malformed_resp; @@ -465,7 +463,7 @@ virNetlinkDumpLink(const char *ifname, int ifindex, virReportSystemError(-err->error, _("error dumping %s (%d) interface"), ifname, ifindex); - goto cleanup; + return -1; } break; @@ -482,21 +480,17 @@ virNetlinkDumpLink(const char *ifname, int ifindex, } VIR_STEAL_PTR(*nlData, resp); - rc = 0; - - cleanup: - nlmsg_free(nl_msg); - return rc; + return 0; malformed_resp: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed netlink response message")); - goto cleanup; + return rc; buffer_too_small: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("allocated netlink buffer is too small")); - goto cleanup; + return rc; } @@ -518,11 +512,10 @@ virNetlinkDumpLink(const char *ifname, int ifindex, int virNetlinkDelLink(const char *ifname, virNetlinkDelLinkFallback fallback) { - int rc = -1; struct nlmsgerr *err; struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC }; unsigned int recvbuflen; - struct nl_msg *nl_msg; + VIR_AUTOPTR(virNlMsg) nl_msg = NULL; VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; nl_msg = nlmsg_alloc_simple(RTM_DELLINK, @@ -540,7 +533,7 @@ virNetlinkDelLink(const char *ifname, virNetlinkDelLinkFallback fallback) if (virNetlinkCommand(nl_msg, &resp, &recvbuflen, 0, 0, NETLINK_ROUTE, 0) < 0) { - goto cleanup; + return -1; } if (recvbuflen < NLMSG_LENGTH(0) || resp == NULL) @@ -552,15 +545,14 @@ virNetlinkDelLink(const char *ifname, virNetlinkDelLinkFallback fallback) if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err))) goto malformed_resp; - if (-err->error == EOPNOTSUPP && fallback) { - rc = fallback(ifname); - goto cleanup; - } + if (-err->error == EOPNOTSUPP && fallback) + return fallback(ifname); + if (err->error) { virReportSystemError(-err->error, _("error destroying network device %s"), ifname); - goto cleanup; + return -1; } break; @@ -571,20 +563,17 @@ virNetlinkDelLink(const char *ifname, virNetlinkDelLinkFallback fallback) goto malformed_resp; } - rc = 0; - cleanup: - nlmsg_free(nl_msg); - return rc; + return 0; malformed_resp: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed netlink response message")); - goto cleanup; + return -1; buffer_too_small: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("allocated netlink buffer is too small")); - goto cleanup; + return -1; } /** @@ -605,13 +594,12 @@ virNetlinkDelLink(const char *ifname, virNetlinkDelLinkFallback fallback) int virNetlinkGetNeighbor(void **nlData, uint32_t src_pid, uint32_t dst_pid) { - int rc = -1; struct nlmsgerr *err; struct ndmsg ndinfo = { .ndm_family = AF_UNSPEC, }; unsigned int recvbuflen; - struct nl_msg *nl_msg; + VIR_AUTOPTR(virNlMsg) nl_msg = NULL; VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; *nlData = NULL; @@ -628,7 +616,7 @@ virNetlinkGetNeighbor(void **nlData, uint32_t src_pid, uint32_t dst_pid) if (virNetlinkCommand(nl_msg, &resp, &recvbuflen, src_pid, dst_pid, NETLINK_ROUTE, 0) < 0) - goto cleanup; + return -1; if (recvbuflen < NLMSG_LENGTH(0) || resp == NULL) goto malformed_resp; @@ -642,7 +630,7 @@ virNetlinkGetNeighbor(void **nlData, uint32_t src_pid, uint32_t dst_pid) if (err->error) { virReportSystemError(-err->error, "%s", _("error dumping")); - goto cleanup; + return -1; } break; @@ -654,21 +642,17 @@ virNetlinkGetNeighbor(void **nlData, uint32_t src_pid, uint32_t dst_pid) } VIR_STEAL_PTR(*nlData, resp); - rc = recvbuflen; - - cleanup: - nlmsg_free(nl_msg); - return rc; + return recvbuflen; malformed_resp: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed netlink response message")); - goto cleanup; + return -1; buffer_too_small: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("allocated netlink buffer is too small")); - goto cleanup; + return -1; } int -- 1.8.3.1

On Mon, 13 Aug 2018 at 18:10, Erik Skultety <eskultet(a)redhat.com&gt; wrote: We need *respbuflen to be 0 if -1 is returned. So if this initialization is removed, we need to add `*respbuflen = 0;` in the cleanup section below.

By making use of GNU C's cleanup attribute handled by the VIR_AUTOFREE macro for declaring scalar variables, majority of the VIR_FREE calls can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virnetdevbridge.c | 46 ++++++++++++++++------------------------------ 1 file changed, 16 insertions(+), 30 deletions(-) diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index e46ac35..fe3697b 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,16 +167,17 @@ static int virNetDevBridgeGet(const char *brname, const char *paramname, /* sysfs param name */ unsigned long *value) /* current value */ { - char *path = NULL; int ret = -1; int fd = -1; struct ifreq ifr; + VIR_AUTOFREE(char *) path = NULL; if (virAsprintf(&path, SYSFS_NET_DIR "%s/bridge/%s", brname, paramname) < 0) 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 +186,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 +216,6 @@ static int virNetDevBridgeGet(const char *brname, ret = 0; cleanup: VIR_FORCE_CLOSE(fd); - VIR_FREE(path); return ret; } #endif /* __linux__ */ @@ -233,9 +227,9 @@ virNetDevBridgePortSet(const char *brname, const char *paramname, unsigned long value) { - char *path = NULL; char valuestr[INT_BUFSIZE_BOUND(value)]; int ret = -1; + VIR_AUTOFREE(char *) path = NULL; snprintf(valuestr, sizeof(valuestr), "%lu", value); @@ -254,7 +248,6 @@ virNetDevBridgePortSet(const char *brname, brname, ifname, paramname, valuestr); } - VIR_FREE(path); return ret; } @@ -265,29 +258,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,12 +418,12 @@ 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; struct nlmsgerr *err; struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC }; unsigned int recvbuflen; struct nl_msg *nl_msg; struct nlattr *linkinfo; + VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; nl_msg = nlmsg_alloc_simple(RTM_NEWLINK, NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL); @@ -495,7 +483,6 @@ virNetDevBridgeCreate(const char *brname) rc = 0; cleanup: nlmsg_free(nl_msg); - VIR_FREE(resp); return rc; malformed_resp: @@ -1069,11 +1056,11 @@ virNetDevBridgeFDBAddDel(const virMacAddr *mac, const char *ifname, unsigned int flags, bool isAdd) { int ret = -1; - struct nlmsghdr *resp = NULL; struct nlmsgerr *err; unsigned int recvbuflen; struct nl_msg *nl_msg; struct ndmsg ndm = { .ndm_family = PF_BRIDGE, .ndm_state = NUD_NOARP }; + VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; if (virNetDevGetIndex(ifname, &ndm.ndm_ifindex) < 0) return -1; @@ -1142,7 +1129,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

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(a)gmail.com&gt; --- src/util/virnetdevbridge.c | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index fe3697b..089da78 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -417,12 +417,11 @@ virNetDevBridgeCreate(const char *brname) { /* use a netlink RTM_NEWLINK message to create the bridge */ const char *type = "bridge"; - int rc = -1; struct nlmsgerr *err; struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC }; unsigned int recvbuflen; - struct nl_msg *nl_msg; struct nlattr *linkinfo; + VIR_AUTOPTR(virNlMsg) nl_msg = NULL; VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; nl_msg = nlmsg_alloc_simple(RTM_NEWLINK, @@ -444,7 +443,7 @@ virNetDevBridgeCreate(const char *brname) if (virNetlinkCommand(nl_msg, &resp, &recvbuflen, 0, 0, NETLINK_ROUTE, 0) < 0) { - goto cleanup; + return -1; } if (recvbuflen < NLMSG_LENGTH(0) || resp == NULL) @@ -462,15 +461,14 @@ virNetDevBridgeCreate(const char *brname) /* fallback to ioctl if netlink doesn't support creating * bridges */ - rc = virNetDevBridgeCreateWithIoctl(brname); - goto cleanup; + return virNetDevBridgeCreateWithIoctl(brname); } # endif virReportSystemError(-err->error, _("error creating bridge interface %s"), brname); - goto cleanup; + return -1; } break; @@ -480,19 +478,16 @@ virNetDevBridgeCreate(const char *brname) goto malformed_resp; } - rc = 0; - cleanup: - nlmsg_free(nl_msg); - return rc; + return 0; malformed_resp: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed netlink response message")); - goto cleanup; + return -1; buffer_too_small: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("allocated netlink buffer is too small")); - goto cleanup; + return -1; } @@ -1055,11 +1050,10 @@ static int virNetDevBridgeFDBAddDel(const virMacAddr *mac, const char *ifname, unsigned int flags, bool isAdd) { - int ret = -1; struct nlmsgerr *err; unsigned int recvbuflen; - struct nl_msg *nl_msg; struct ndmsg ndm = { .ndm_family = PF_BRIDGE, .ndm_state = NUD_NOARP }; + VIR_AUTOPTR(virNlMsg) nl_msg = NULL; VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; if (virNetDevGetIndex(ifname, &ndm.ndm_ifindex) < 0) @@ -1103,7 +1097,7 @@ virNetDevBridgeFDBAddDel(const virMacAddr *mac, const char *ifname, if (virNetlinkCommand(nl_msg, &resp, &recvbuflen, 0, 0, NETLINK_ROUTE, 0) < 0) { - goto cleanup; + return -1; } if (recvbuflen < NLMSG_LENGTH(0) || resp == NULL) goto malformed_resp; @@ -1116,7 +1110,7 @@ virNetDevBridgeFDBAddDel(const virMacAddr *mac, const char *ifname, if (err->error) { virReportSystemError(-err->error, _("error adding fdb entry for %s"), ifname); - goto cleanup; + return -1; } break; case NLMSG_DONE: @@ -1126,20 +1120,17 @@ virNetDevBridgeFDBAddDel(const virMacAddr *mac, const char *ifname, goto malformed_resp; } - ret = 0; - cleanup: - nlmsg_free(nl_msg); - return ret; + return 0; malformed_resp: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed netlink response message")); - goto cleanup; + return -1; buffer_too_small: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("allocated netlink buffer is too small")); - goto cleanup; + 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(a)gmail.com&gt; --- src/util/virnetdev.c | 343 +++++++++++++++++++-------------------------------- 1 file changed, 125 insertions(+), 218 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 8eac419..edb7393 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -535,18 +535,17 @@ int virNetDevSetMTUFromDevice(const char *ifname, */ int virNetDevSetNamespace(const char *ifname, pid_t pidInNs) { - int ret = -1; - char *pid = NULL; - char *phy = NULL; - char *phy_path = NULL; int len; + VIR_AUTOFREE(char *) pid = NULL; + VIR_AUTOFREE(char *) phy = NULL; + VIR_AUTOFREE(char *) phy_path = NULL; if (virAsprintf(&pid, "%lld", (long long) pidInNs) == -1) return -1; /* The 802.11 wireless devices only move together with their PHY. */ if (virNetDevSysfsFile(&phy_path, ifname, "phy80211/name") < 0) - goto cleanup; + return -1; if ((len = virFileReadAllQuiet(phy_path, 1024, &phy)) <= 0) { /* Not a wireless device. */ @@ -556,7 +555,7 @@ int virNetDevSetNamespace(const char *ifname, pid_t pidInNs) argv[5] = pid; if (virRun(argv, NULL) < 0) - goto cleanup; + return -1; } else { const char *argv[] = { @@ -569,15 +568,10 @@ int virNetDevSetNamespace(const char *ifname, pid_t pidInNs) argv[2] = phy; argv[5] = pid; if (virRun(argv, NULL) < 0) - goto cleanup; + return -1; } - ret = 0; - cleanup: - VIR_FREE(phy_path); - VIR_FREE(phy); - VIR_FREE(pid); - return ret; + return 0; } #if defined(SIOCSIFNAME) && defined(HAVE_STRUCT_IFREQ) @@ -969,25 +963,21 @@ int virNetDevGetIndex(const char *ifname ATTRIBUTE_UNUSED, int virNetDevGetMaster(const char *ifname, char **master) { - int ret = -1; - void *nlData = NULL; struct nlattr *tb[IFLA_MAX + 1] = {NULL, }; + VIR_AUTOFREE(void *) nlData = NULL; *master = NULL; if (virNetlinkDumpLink(ifname, -1, &nlData, tb, 0, 0) < 0) - goto cleanup; + return -1; if (tb[IFLA_MASTER]) { if (!(*master = virNetDevGetName(*(int *)RTA_DATA(tb[IFLA_MASTER])))) - goto cleanup; + return -1; } VIR_DEBUG("IFLA_MASTER for %s is %s", ifname, *master ? *master : "(none)"); - ret = 0; - cleanup: - VIR_FREE(nlData); - return ret; + return 0; } @@ -1168,39 +1158,33 @@ virNetDevSysfsDeviceFile(char **pf_sysfs_device_link, const char *ifname, static bool virNetDevIsPCIDevice(const char *devpath) { - char *subsys_link = NULL; - char *abs_path = NULL; char *subsys = NULL; - bool ret = false; + VIR_AUTOFREE(char *) subsys_link = NULL; + VIR_AUTOFREE(char *) abs_path = NULL; if (virAsprintf(&subsys_link, "%s/subsystem", devpath) < 0) return false; if (!virFileExists(subsys_link)) - goto cleanup; + return false; if (virFileResolveLink(subsys_link, &abs_path) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to resolve device subsystem symlink %s"), subsys_link); - goto cleanup; + return false; } subsys = last_component(abs_path); - ret = STRPREFIX(subsys, "pci"); - - cleanup: - VIR_FREE(subsys_link); - VIR_FREE(abs_path); - return ret; + return STRPREFIX(subsys, "pci"); } static virPCIDevicePtr virNetDevGetPCIDevice(const char *devName) { - char *vfSysfsDevicePath = NULL; virPCIDeviceAddressPtr vfPCIAddr = NULL; virPCIDevicePtr vfPCIDevice = NULL; + VIR_AUTOFREE(char *) vfSysfsDevicePath = NULL; if (virNetDevSysfsFile(&vfSysfsDevicePath, devName, "device") < 0) goto cleanup; @@ -1216,7 +1200,6 @@ virNetDevGetPCIDevice(const char *devName) vfPCIAddr->slot, vfPCIAddr->function); cleanup: - VIR_FREE(vfSysfsDevicePath); VIR_FREE(vfPCIAddr); return vfPCIDevice; @@ -1241,25 +1224,20 @@ int virNetDevGetPhysPortID(const char *ifname, char **physPortID) { - int ret = -1; - char *physPortIDFile = NULL; + VIR_AUTOFREE(char *) physPortIDFile = NULL; *physPortID = NULL; if (virNetDevSysfsFile(&physPortIDFile, ifname, "phys_port_id") < 0) - goto cleanup; + return -1; /* a failure to read just means the driver doesn't support - * phys_port_id, so set success now and ignore the return from - * virFileReadAllQuiet(). + * phys_port_id, so ignore the return from virFileReadAllQuiet() + * and return success. */ - ret = 0; - ignore_value(virFileReadAllQuiet(physPortIDFile, 1024, physPortID)); - cleanup: - VIR_FREE(physPortIDFile); - return ret; + return 0; } @@ -1280,67 +1258,61 @@ virNetDevGetVirtualFunctions(const char *pfname, size_t *n_vfname, unsigned int *max_vfs) { - int ret = -1; size_t i; - char *pf_sysfs_device_link = NULL; - char *pci_sysfs_device_link = NULL; - char *pciConfigAddr = NULL; - char *pfPhysPortID = NULL; + VIR_AUTOFREE(char *) pf_sysfs_device_link = NULL; + VIR_AUTOFREE(char *) pci_sysfs_device_link = NULL; + VIR_AUTOFREE(char *) pciConfigAddr = NULL; + VIR_AUTOFREE(char *) pfPhysPortID = NULL; + VIR_AUTOFREE(char **) tmpVfname = NULL; + VIR_AUTOFREE(virPCIDeviceAddressPtr *) tmpVirtFns = NULL; *virt_fns = NULL; *n_vfname = 0; *max_vfs = 0; if (virNetDevGetPhysPortID(pfname, &pfPhysPortID) < 0) - goto cleanup; + return -1; if (virNetDevSysfsFile(&pf_sysfs_device_link, pfname, "device") < 0) - goto cleanup; + return -1; - if (virPCIGetVirtualFunctions(pf_sysfs_device_link, virt_fns, - n_vfname, max_vfs) < 0) - goto cleanup; + if (virPCIGetVirtualFunctions(pf_sysfs_device_link, &tmpVirtFns, + n_vfname, max_vfs) < 0) { + return -1; + } - if (VIR_ALLOC_N(*vfname, *n_vfname) < 0) - goto cleanup; + if (VIR_ALLOC_N(tmpVfname, *n_vfname) < 0) + return -1; for (i = 0; i < *n_vfname; i++) { - if (virPCIGetAddrString((*virt_fns)[i]->domain, - (*virt_fns)[i]->bus, - (*virt_fns)[i]->slot, - (*virt_fns)[i]->function, + if (virPCIGetAddrString(tmpVirtFns[i]->domain, + tmpVirtFns[i]->bus, + tmpVirtFns[i]->slot, + tmpVirtFns[i]->function, &pciConfigAddr) < 0) { virReportSystemError(ENOSYS, "%s", _("Failed to get PCI Config Address String")); - goto cleanup; + return -1; } if (virPCIGetSysfsFile(pciConfigAddr, &pci_sysfs_device_link) < 0) { virReportSystemError(ENOSYS, "%s", _("Failed to get PCI SYSFS file")); - goto cleanup; + return -1; } if (virPCIGetNetName(pci_sysfs_device_link, 0, - pfPhysPortID, &((*vfname)[i])) < 0) { - goto cleanup; + pfPhysPortID, &tmpVfname[i]) < 0) { + return -1; } - if (!(*vfname)[i]) + if (!tmpVfname[i]) VIR_INFO("VF does not have an interface name"); } - ret = 0; + VIR_STEAL_PTR(*vfname, tmpVfname); + VIR_STEAL_PTR(*virt_fns, tmpVirtFns); - cleanup: - if (ret < 0) { - VIR_FREE(*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; + return 0; } /** @@ -1355,17 +1327,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 +1350,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 +1378,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 +1399,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 +1426,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 +1453,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,30 +1470,24 @@ 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 *) tmpPfname = NULL; *pfname = NULL; - if (virNetDevGetPhysicalFunction(vfname, pfname) < 0) - return ret; + if (virNetDevGetPhysicalFunction(vfname, &tmpPfname) < 0) + return -1; - if (virNetDevSysfsFile(&pf_sysfs_path, *pfname, "device") < 0) - goto cleanup; + if (virNetDevSysfsFile(&pf_sysfs_path, tmpPfname, "device") < 0) + return -1; if (virNetDevSysfsFile(&vf_sysfs_path, vfname, "device") < 0) - goto cleanup; + return -1; - ret = virPCIGetVirtualFunctionIndex(pf_sysfs_path, vf_sysfs_path, vf); + VIR_STEAL_PTR(*pfname, tmpPfname); - cleanup: - if (ret < 0) - VIR_FREE(*pfname); - - VIR_FREE(vf_sysfs_path); - VIR_FREE(pf_sysfs_path); - - return ret; + return virPCIGetVirtualFunctionIndex(pf_sysfs_path, vf_sysfs_path, vf); } #else /* !__linux__ */ @@ -1657,7 +1599,6 @@ virNetDevSetVfConfig(const char *ifname, int vf, { int rc = -1; char macstr[VIR_MAC_STRING_BUFLEN]; - struct nlmsghdr *resp = NULL; struct nlmsgerr *err; unsigned int recvbuflen = 0; struct nl_msg *nl_msg; @@ -1666,6 +1607,7 @@ virNetDevSetVfConfig(const char *ifname, int vf, .ifi_family = AF_UNSPEC, .ifi_index = -1, }; + VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; if (!macaddr && vlanid < 0) return -1; @@ -1769,7 +1711,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 +1784,15 @@ virNetDevGetVfConfig(const char *ifname, int vf, virMacAddrPtr mac, int *vlanid) { int rc = -1; - void *nlData = NULL; struct nlattr *tb[IFLA_MAX + 1] = {NULL, }; int ifindex = -1; + VIR_AUTOFREE(void *) nlData = NULL; 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,14 +1851,14 @@ virNetDevSaveNetConfig(const char *linkdev, int vf, { int ret = -1; const char *pfDevName = NULL; - char *pfDevOrig = NULL; - char *vfDevOrig = NULL; virMacAddr oldMAC; char MACStr[VIR_MAC_STRING_BUFLEN]; int oldVlanTag = -1; - char *filePath = NULL; - char *fileStr = NULL; virJSONValuePtr configJSON = NULL; + VIR_AUTOFREE(char *) pfDevOrig = NULL; + VIR_AUTOFREE(char *) vfDevOrig = NULL; + VIR_AUTOFREE(char *) filePath = NULL; + VIR_AUTOFREE(char *) fileStr = NULL; if (vf >= 0) { /* linkdev is the PF */ @@ -2030,10 +1967,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,14 +2002,14 @@ 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; virJSONValuePtr configJSON = NULL; const char *MACStr = NULL; const char *adminMACStr = NULL; int vlanTag = -1; + VIR_AUTOFREE(char *) pfDevOrig = NULL; + VIR_AUTOFREE(char *) vfDevOrig = NULL; + VIR_AUTOFREE(char *) filePath = NULL; + VIR_AUTOFREE(char *) fileStr = NULL; *adminMAC = NULL; *vlan = NULL; @@ -2245,10 +2178,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,10 +2211,10 @@ 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; int vlanTag = -1; virPCIDevicePtr vfPCIDevice = NULL; + VIR_AUTOFREE(char *) pfDevOrig = NULL; + VIR_AUTOFREE(char *) vfDevOrig = NULL; if (vf >= 0) { /* linkdev is the PF */ @@ -2462,8 +2391,6 @@ virNetDevSetNetConfig(const char *linkdev, int vf, ret = 0; cleanup: - VIR_FREE(pfDevOrig); - VIR_FREE(vfDevOrig); virPCIDeviceFree(vfPCIDevice); return ret; } @@ -2543,28 +2470,27 @@ int virNetDevGetLinkInfo(const char *ifname, virNetDevIfLinkPtr lnk) { - int ret = -1; - char *path = NULL; - char *buf = NULL; char *tmp; int tmp_state; unsigned int tmp_speed; + VIR_AUTOFREE(char *) path = NULL; + VIR_AUTOFREE(char *) buf = NULL; 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 +2501,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 +2512,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 +2537,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,45 +2750,41 @@ static int virNetDevGetMcastList(const char *ifname, virNetDevMcastListPtr mcast) { char *cur = NULL; - char *buf = NULL; char *next = NULL; - int ret = -1, len; + int len; VIR_AUTOPTR(virNetDevMcastEntry) entry = NULL; + VIR_AUTOFREE(char *) buf = NULL; mcast->entries = NULL; mcast->nentries = 0; /* Read entire multicast table into memory */ if ((len = virFileReadAll(PROC_NET_DEV_MCAST, MAX_MCAST_SIZE, &buf)) <= 0) - goto cleanup; + return -1; cur = buf; while (cur) { if (!entry && VIR_ALLOC(entry) < 0) - goto cleanup; + return -1; next = strchr(cur, '\n'); if (next) next++; if (virNetDevParseMcast(cur, entry)) - goto cleanup; + return -1; /* Only return global multicast MAC addresses for * specified interface */ if (entry->global && STREQ(ifname, entry->name)) { if (VIR_APPEND_ELEMENT(mcast->entries, mcast->nentries, entry)) - goto cleanup; + return -1; } else { memset(entry, 0, sizeof(virNetDevMcastEntry)); } cur = next && ((next - buf) < len) ? next : NULL; } - ret = 0; - cleanup: - VIR_FREE(buf); - - return ret; + return 0; } @@ -3005,13 +2921,11 @@ 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; DIR *dirp = NULL; struct dirent *dp; int ret = -1; + VIR_AUTOFREE(char *) eth_devpath = NULL; + VIR_AUTOFREE(char *) eth_res_buf = NULL; if (!virFileExists(SYSFS_INFINIBAND_DIR)) return 0; @@ -3027,6 +2941,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; @@ -3035,17 +2952,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; } @@ -3204,11 +3115,11 @@ static uint32_t virNetDevGetFamilyId(const char *family_name) { struct nl_msg *nl_msg = NULL; - struct nlmsghdr *resp = NULL; struct genlmsghdr* gmsgh = NULL; struct nlattr *tb[CTRL_ATTR_MAX + 1] = {NULL, }; unsigned int recvbuflen; uint32_t family_id = 0; + VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; if (!(nl_msg = nlmsg_alloc_simple(GENL_ID_CTRL, NLM_F_REQUEST | NLM_F_ACK))) { @@ -3244,7 +3155,6 @@ virNetDevGetFamilyId(const char *family_name) cleanup: nlmsg_free(nl_msg); - VIR_FREE(resp); return family_id; } @@ -3264,16 +3174,16 @@ virNetDevSwitchdevFeature(const char *ifname, virBitmapPtr *out) { struct nl_msg *nl_msg = NULL; - 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; int is_vf = -1; int ret = -1; uint32_t family_id; + VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; + VIR_AUTOFREE(char *) pfname = NULL; if ((family_id = virNetDevGetFamilyId(DEVLINK_GENL_NAME)) <= 0) return ret; @@ -3332,8 +3242,6 @@ virNetDevSwitchdevFeature(const char *ifname, cleanup: nlmsg_free(nl_msg); virPCIDeviceFree(pci_device_ptr); - VIR_FREE(resp); - VIR_FREE(pfname); return ret; } # else @@ -3374,7 +3282,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) @@ -3384,7 +3292,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(a)gmail.com&gt; --- src/util/virnetdev.c | 249 +++++++++++++++++++++------------------------------ 1 file changed, 103 insertions(+), 146 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index edb7393..d5aa94c 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1182,27 +1182,21 @@ virNetDevIsPCIDevice(const char *devpath) static virPCIDevicePtr virNetDevGetPCIDevice(const char *devName) { - virPCIDeviceAddressPtr vfPCIAddr = NULL; - virPCIDevicePtr vfPCIDevice = NULL; + VIR_AUTOPTR(virPCIDeviceAddress) vfPCIAddr = NULL; VIR_AUTOFREE(char *) vfSysfsDevicePath = NULL; if (virNetDevSysfsFile(&vfSysfsDevicePath, devName, "device") < 0) - goto cleanup; + return NULL; if (!virNetDevIsPCIDevice(vfSysfsDevicePath)) - goto cleanup; + return NULL; vfPCIAddr = virPCIGetDeviceAddressFromSysfsLink(vfSysfsDevicePath); if (!vfPCIAddr) - goto cleanup; + return NULL; - vfPCIDevice = virPCIDeviceNew(vfPCIAddr->domain, vfPCIAddr->bus, - vfPCIAddr->slot, vfPCIAddr->function); - - cleanup: - VIR_FREE(vfPCIAddr); - - return vfPCIDevice; + return virPCIDeviceNew(vfPCIAddr->domain, vfPCIAddr->bus, + vfPCIAddr->slot, vfPCIAddr->function); } @@ -1601,12 +1595,12 @@ virNetDevSetVfConfig(const char *ifname, int vf, char macstr[VIR_MAC_STRING_BUFLEN]; struct nlmsgerr *err; unsigned int recvbuflen = 0; - struct nl_msg *nl_msg; struct nlattr *vfinfolist, *vfinfo; struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC, .ifi_index = -1, }; + VIR_AUTOPTR(virNlMsg) nl_msg = NULL; VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; if (!macaddr && vlanid < 0) @@ -1710,7 +1704,6 @@ virNetDevSetVfConfig(const char *ifname, int vf, macaddr ? virMacAddrFormat(macaddr, macstr) : "(unchanged)", vlanid, rc < 0 ? "Fail" : "Success"); - nlmsg_free(nl_msg); return rc; malformed_resp: @@ -1849,12 +1842,11 @@ virNetDevSaveNetConfig(const char *linkdev, int vf, const char *stateDir, bool saveVlan) { - int ret = -1; const char *pfDevName = NULL; virMacAddr oldMAC; char MACStr[VIR_MAC_STRING_BUFLEN]; int oldVlanTag = -1; - virJSONValuePtr configJSON = NULL; + VIR_AUTOPTR(virJSONValue) configJSON = NULL; VIR_AUTOFREE(char *) pfDevOrig = NULL; VIR_AUTOFREE(char *) vfDevOrig = NULL; VIR_AUTOFREE(char *) filePath = NULL; @@ -1866,7 +1858,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; @@ -1878,12 +1870,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) { @@ -1901,7 +1893,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, @@ -1910,12 +1902,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 @@ -1924,11 +1916,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, @@ -1936,39 +1928,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; } @@ -2000,12 +1989,14 @@ virNetDevReadNetConfig(const char *linkdev, int vf, virNetDevVlanPtr *vlan, virMacAddrPtr *MAC) { - int ret = -1; const char *pfDevName = NULL; - virJSONValuePtr configJSON = NULL; const char *MACStr = NULL; const char *adminMACStr = NULL; int vlanTag = -1; + VIR_AUTOPTR(virMacAddr) tmpAdminMAC = NULL; + VIR_AUTOPTR(virNetDevVlan) tmpVlan = NULL; + VIR_AUTOPTR(virMacAddr) tmpMAC = NULL; + VIR_AUTOPTR(virJSONValue) configJSON = NULL; VIR_AUTOFREE(char *) pfDevOrig = NULL; VIR_AUTOFREE(char *) vfDevOrig = NULL; VIR_AUTOFREE(char *) filePath = NULL; @@ -2021,7 +2012,7 @@ virNetDevReadNetConfig(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; @@ -2032,12 +2023,12 @@ virNetDevReadNetConfig(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 there is a PF, it's now in pfDevName, and linkdev is either @@ -2047,7 +2038,7 @@ virNetDevReadNetConfig(const char *linkdev, int vf, if (pfDevName) { if (virAsprintf(&filePath, "%s/%s_vf%d", stateDir, pfDevName, vf) < 0) - goto cleanup; + return -1; if (linkdev && !virFileExists(filePath)) { /* the device may have been stored in a file named for the @@ -2062,19 +2053,18 @@ virNetDevReadNetConfig(const char *linkdev, int vf, if (!pfDevName) { if (virAsprintf(&filePath, "%s/%s", stateDir, linkdev) < 0) - goto cleanup; + return -1; } if (!virFileExists(filePath)) { /* having no file to read is not necessarily an error, so we * just return success, but with MAC, adminMAC, and vlan set to NULL */ - ret = 0; - goto cleanup; + return 0; } if (virFileReadAll(filePath, 128, &fileStr) < 0) - goto cleanup; + return -1; if (strchr("0123456789abcdefABCDEF", fileStr[0])) { const char *vlanStr = NULL; @@ -2096,7 +2086,7 @@ virNetDevReadNetConfig(const char *linkdev, int vf, virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse vlan tag '%s' from file '%s'"), vlanStr, filePath); - goto cleanup; + return -1; } } else { /* if there is only one line, it is MAC */ @@ -2112,7 +2102,7 @@ virNetDevReadNetConfig(const char *linkdev, int vf, _("invalid json in net device saved " "config file '%s': '%.60s'"), filePath, fileStr); - goto cleanup; + return -1; } MACStr = virJSONValueObjectGetString(configJSON, @@ -2129,57 +2119,52 @@ virNetDevReadNetConfig(const char *linkdev, int vf, "has unexpected contents, missing both " "'MAC' and 'adminMAC': '%.60s'"), filePath, fileStr); - goto cleanup; + return -1; } } if (MACStr) { - if (VIR_ALLOC(*MAC) < 0) - goto cleanup; + if (VIR_ALLOC(tmpMAC) < 0) + return -1; - if (virMacAddrParse(MACStr, *MAC) < 0) { + if (virMacAddrParse(MACStr, tmpMAC) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse MAC address '%s' from file '%s'"), MACStr, filePath); - goto cleanup; + return -1; } } if (adminMACStr) { - if (VIR_ALLOC(*adminMAC) < 0) - goto cleanup; + if (VIR_ALLOC(tmpAdminMAC) < 0) + return -1; - if (virMacAddrParse(adminMACStr, *adminMAC) < 0) { + if (virMacAddrParse(adminMACStr, tmpAdminMAC) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse MAC address '%s' from file '%s'"), adminMACStr, filePath); - goto cleanup; + return -1; } } if (vlanTag != -1) { /* construct a simple virNetDevVlan object with a single tag */ - if (VIR_ALLOC(*vlan) < 0) - goto cleanup; - if (VIR_ALLOC((*vlan)->tag) < 0) - goto cleanup; - (*vlan)->nTags = 1; - (*vlan)->tag[0] = vlanTag; + if (VIR_ALLOC(tmpVlan) < 0) + return -1; + if (VIR_ALLOC(tmpVlan->tag) < 0) + return -1; + tmpVlan->nTags = 1; + tmpVlan->tag[0] = vlanTag; } /* we won't need the file again */ ignore_value(unlink(filePath)); - ret = 0; - cleanup: - if (ret < 0) { - VIR_FREE(*adminMAC); - VIR_FREE(*MAC); - VIR_FREE(*vlan); - } + VIR_STEAL_PTR(*adminMAC, tmpAdminMAC); + VIR_STEAL_PTR(*MAC, tmpMAC); + VIR_STEAL_PTR(*vlan, tmpVlan); - virJSONValueFree(configJSON); - return ret; + return 0; } @@ -2208,11 +2193,10 @@ virNetDevSetNetConfig(const char *linkdev, int vf, const virMacAddr *MAC, bool setVlan) { - int ret = -1; char MACStr[VIR_MAC_STRING_BUFLEN]; const char *pfDevName = NULL; int vlanTag = -1; - virPCIDevicePtr vfPCIDevice = NULL; + VIR_AUTOPTR(virPCIDevice) vfPCIDevice = NULL; VIR_AUTOFREE(char *) pfDevOrig = NULL; VIR_AUTOFREE(char *) vfDevOrig = NULL; @@ -2222,7 +2206,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; @@ -2233,12 +2217,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; } @@ -2250,14 +2234,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 { @@ -2266,14 +2250,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]; @@ -2291,7 +2275,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); @@ -2302,7 +2286,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 @@ -2316,18 +2300,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 @@ -2379,20 +2363,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; } @@ -2863,30 +2844,29 @@ virNetDevRxFilterFree(virNetDevRxFilterPtr filter) int virNetDevGetRxFilter(const char *ifname, virNetDevRxFilterPtr *filter) { - int ret = -1; bool receive = false; - virNetDevRxFilterPtr fil = virNetDevRxFilterNew(); + VIR_AUTOPTR(virNetDevRxFilter) fil = virNetDevRxFilterNew(); 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; @@ -2894,15 +2874,9 @@ int virNetDevGetRxFilter(const char *ifname, fil->multicast.mode = VIR_NETDEV_RX_FILTER_MODE_NONE; } - ret = 0; - cleanup: - if (ret < 0) { - virNetDevRxFilterFree(fil); - fil = NULL; - } + VIR_STEAL_PTR(*filter, fil); - *filter = fil; - return ret; + return 0; } #if defined(SIOCETHTOOL) && defined(HAVE_STRUCT_IFREQ) @@ -3114,21 +3088,20 @@ virNetDevPutExtraHeader(struct nlmsghdr *nlh, static uint32_t virNetDevGetFamilyId(const char *family_name) { - struct nl_msg *nl_msg = NULL; struct genlmsghdr* gmsgh = NULL; struct nlattr *tb[CTRL_ATTR_MAX + 1] = {NULL, }; unsigned int recvbuflen; - uint32_t family_id = 0; + VIR_AUTOPTR(virNlMsg) nl_msg = NULL; VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; if (!(nl_msg = nlmsg_alloc_simple(GENL_ID_CTRL, NLM_F_REQUEST | NLM_F_ACK))) { virReportOOMError(); - goto cleanup; + return 0; } if (!(gmsgh = virNetDevPutExtraHeader(nlmsg_hdr(nl_msg), sizeof(struct genlmsghdr)))) - goto cleanup; + return 0; gmsgh->cmd = CTRL_CMD_GETFAMILY; gmsgh->version = DEVLINK_GENL_VERSION; @@ -3136,26 +3109,22 @@ virNetDevGetFamilyId(const char *family_name) if (nla_put_string(nl_msg, CTRL_ATTR_FAMILY_NAME, family_name) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("allocated netlink buffer is too small")); - goto cleanup; + return 0; } if (virNetlinkCommand(nl_msg, &resp, &recvbuflen, 0, 0, NETLINK_GENERIC, 0) < 0) - goto cleanup; + return 0; if (nlmsg_parse(resp, sizeof(struct nlmsghdr), tb, CTRL_ATTR_MAX, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed netlink response message")); - goto cleanup; + return 0; } if (tb[CTRL_ATTR_FAMILY_ID] == NULL) - goto cleanup; + return 0; - family_id = *(uint32_t *)RTA_DATA(tb[CTRL_ATTR_FAMILY_ID]); - - cleanup: - nlmsg_free(nl_msg); - return family_id; + return *(uint32_t *)RTA_DATA(tb[CTRL_ATTR_FAMILY_ID]); } @@ -3173,43 +3142,40 @@ static int virNetDevSwitchdevFeature(const char *ifname, virBitmapPtr *out) { - struct nl_msg *nl_msg = 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; int is_vf = -1; - int ret = -1; uint32_t family_id; + VIR_AUTOPTR(virNlMsg) nl_msg = NULL; + VIR_AUTOPTR(virPCIDevice) pci_device_ptr = NULL; VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; VIR_AUTOFREE(char *) pfname = NULL; if ((family_id = virNetDevGetFamilyId(DEVLINK_GENL_NAME)) <= 0) - return ret; + return -1; if ((is_vf = virNetDevIsVirtualFunction(ifname)) < 0) - return ret; + return -1; if (is_vf == 1 && virNetDevGetPhysicalFunction(ifname, &pfname) < 0) - goto cleanup; + return -1; pci_device_ptr = pfname ? virNetDevGetPCIDevice(pfname) : virNetDevGetPCIDevice(ifname); /* No PCI device, then no feature bit to check/add */ - if (pci_device_ptr == NULL) { - ret = 0; - goto cleanup; - } + if (pci_device_ptr == NULL) + return 0; if (!(nl_msg = nlmsg_alloc_simple(family_id, NLM_F_REQUEST | NLM_F_ACK))) { virReportOOMError(); - goto cleanup; + return -1; } if (!(gmsgh = virNetDevPutExtraHeader(nlmsg_hdr(nl_msg), sizeof(struct genlmsghdr)))) - goto cleanup; + return -1; gmsgh->cmd = DEVLINK_CMD_ESWITCH_GET; gmsgh->version = DEVLINK_GENL_VERSION; @@ -3220,16 +3186,16 @@ virNetDevSwitchdevFeature(const char *ifname, nla_put(nl_msg, DEVLINK_ATTR_DEV_NAME, strlen(pci_name)+1, pci_name) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("allocated netlink buffer is too small")); - goto cleanup; + return -1; } if (virNetlinkCommand(nl_msg, &resp, &recvbuflen, 0, 0, NETLINK_GENERIC, 0) < 0) - goto cleanup; + return -1; if (nlmsg_parse(resp, sizeof(struct genlmsghdr), tb, DEVLINK_ATTR_MAX, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed netlink response message")); - goto cleanup; + return -1; } if (tb[DEVLINK_ATTR_ESWITCH_MODE] && @@ -3237,12 +3203,7 @@ virNetDevSwitchdevFeature(const char *ifname, ignore_value(virBitmapSetBit(*out, VIR_NET_DEV_FEAT_SWITCHDEV)); } - ret = 0; - - cleanup: - nlmsg_free(nl_msg); - virPCIDeviceFree(pci_device_ptr); - return ret; + return 0; } # else static int @@ -3497,8 +3458,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, "")) @@ -3512,8 +3472,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

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(a)gmail.com&gt; --- src/util/virnetdevip.c | 130 +++++++++++++++++++------------------------------ 1 file changed, 51 insertions(+), 79 deletions(-) diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c index c6d6175..4a38a54 100644 --- a/src/util/virnetdevip.c +++ b/src/util/virnetdevip.c @@ -168,10 +168,9 @@ virNetDevIPAddrAdd(const char *ifname, virSocketAddr *peer, unsigned int prefix) { - virSocketAddr *broadcast = NULL; - int ret = -1; - struct nl_msg *nlmsg = NULL; unsigned int recvbuflen; + VIR_AUTOPTR(virNlMsg) nlmsg = NULL; + VIR_AUTOPTR(virSocketAddr) broadcast = NULL; VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; VIR_AUTOFREE(char *) ipStr = NULL; VIR_AUTOFREE(char *) peerStr = NULL; @@ -186,13 +185,13 @@ virNetDevIPAddrAdd(const char *ifname, !(peer && VIR_SOCKET_ADDR_VALID(peer))) { /* compute a broadcast address if this is IPv4 */ if (VIR_ALLOC(broadcast) < 0) - goto cleanup; + return -1; if (virSocketAddrBroadcastByPrefix(addr, prefix, broadcast) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to determine broadcast address for '%s/%d'"), - ipStr, prefix); - goto cleanup; + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to determine broadcast address for '%s/%d'"), + ipStr, prefix); + return -1; } bcastStr = virSocketAddrFormat(broadcast); } @@ -206,11 +205,11 @@ virNetDevIPAddrAdd(const char *ifname, if (!(nlmsg = virNetDevCreateNetlinkAddressMessage(RTM_NEWADDR, ifname, addr, prefix, broadcast, peer))) - goto cleanup; + return -1; if (virNetlinkCommand(nlmsg, &resp, &recvbuflen, 0, 0, NETLINK_ROUTE, 0) < 0) - goto cleanup; + return -1; if (virNetlinkGetErrorCode(resp, recvbuflen) < 0) { @@ -220,14 +219,10 @@ virNetDevIPAddrAdd(const char *ifname, peerStr ? " peer " : "", peerStr ? peerStr : "", bcastStr ? " bcast " : "", bcastStr ? bcastStr : "", ifname); - goto cleanup; + return -1; } - ret = 0; - cleanup: - nlmsg_free(nlmsg); - VIR_FREE(broadcast); - return ret; + return 0; } @@ -246,30 +241,26 @@ virNetDevIPAddrDel(const char *ifname, virSocketAddr *addr, unsigned int prefix) { - int ret = -1; - struct nl_msg *nlmsg = NULL; unsigned int recvbuflen; + VIR_AUTOPTR(virNlMsg) nlmsg = NULL; VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; if (!(nlmsg = virNetDevCreateNetlinkAddressMessage(RTM_DELADDR, ifname, addr, prefix, NULL, NULL))) - goto cleanup; + return -1; if (virNetlinkCommand(nlmsg, &resp, &recvbuflen, 0, 0, NETLINK_ROUTE, 0) < 0) - goto cleanup; + return -1; if (virNetlinkGetErrorCode(resp, recvbuflen) < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, _("Error removing IP address from %s"), ifname); - goto cleanup; + return -1; } - ret = 0; - cleanup: - nlmsg_free(nlmsg); - return ret; + return 0; } @@ -292,8 +283,6 @@ virNetDevIPRouteAdd(const char *ifname, virSocketAddrPtr gateway, unsigned int metric) { - int ret = -1; - struct nl_msg *nlmsg = NULL; struct nlmsghdr *resp = NULL; unsigned int recvbuflen; unsigned int ifindex; @@ -304,6 +293,7 @@ virNetDevIPRouteAdd(const char *ifname, int errCode; virSocketAddr defaultAddr; virSocketAddrPtr actualAddr; + VIR_AUTOPTR(virNlMsg) nlmsg = NULL; VIR_AUTOFREE(char *) toStr = NULL; VIR_AUTOFREE(char *) viaStr = NULL; @@ -315,10 +305,10 @@ virNetDevIPRouteAdd(const char *ifname, int family = VIR_SOCKET_ADDR_FAMILY(gateway); if (family == AF_INET) { if (virSocketAddrParseIPv4(&defaultAddr, VIR_SOCKET_ADDR_IPV4_ALL) < 0) - goto cleanup; + return -1; } else { if (virSocketAddrParseIPv6(&defaultAddr, VIR_SOCKET_ADDR_IPV6_ALL) < 0) - goto cleanup; + return -1; } actualAddr = &defaultAddr; @@ -330,17 +320,17 @@ virNetDevIPRouteAdd(const char *ifname, if (virNetDevGetIPAddressBinary(actualAddr, &addrData, &addrDataLen) < 0 || virNetDevGetIPAddressBinary(gateway, &gatewayData, &addrDataLen) < 0) - goto cleanup; + return -1; /* Get the interface index */ if ((ifindex = if_nametoindex(ifname)) == 0) - goto cleanup; + return -1; if (!(nlmsg = nlmsg_alloc_simple(RTM_NEWROUTE, NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL))) { virReportOOMError(); - goto cleanup; + return -1; } memset(&rtmsg, 0, sizeof(rtmsg)); @@ -369,22 +359,19 @@ virNetDevIPRouteAdd(const char *ifname, if (virNetlinkCommand(nlmsg, &resp, &recvbuflen, 0, 0, NETLINK_ROUTE, 0) < 0) - goto cleanup; + return -1; if ((errCode = virNetlinkGetErrorCode(resp, recvbuflen)) < 0) { virReportSystemError(errCode, _("Error adding route to %s"), ifname); - goto cleanup; + return -1; } - ret = 0; - cleanup: - nlmsg_free(nlmsg); - return ret; + return 0; buffer_too_small: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("allocated netlink buffer is too small")); - goto cleanup; + return -1; } @@ -443,12 +430,11 @@ virNetDevIPParseDadStatus(struct nlmsghdr *nlh, int len, int virNetDevIPWaitDadFinish(virSocketAddrPtr *addrs, size_t count) { - struct nl_msg *nlmsg = NULL; struct ifaddrmsg ifa; unsigned int recvbuflen; - int ret = -1; bool dad = true; time_t max_time = time(NULL) + VIR_DAD_WAIT_TIMEOUT; + VIR_AUTOPTR(virNlMsg) nlmsg = NULL; if (!(nlmsg = nlmsg_alloc_simple(RTM_GETADDR, NLM_F_REQUEST | NLM_F_DUMP))) { @@ -462,7 +448,7 @@ virNetDevIPWaitDadFinish(virSocketAddrPtr *addrs, size_t count) if (nlmsg_append(nlmsg, &ifa, sizeof(ifa), NLMSG_ALIGNTO) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("allocated netlink buffer is too small")); - goto cleanup; + return -1; } /* Periodically query netlink until DAD finishes on all known addresses. */ @@ -471,12 +457,12 @@ virNetDevIPWaitDadFinish(virSocketAddrPtr *addrs, size_t count) if (virNetlinkCommand(nlmsg, &resp, &recvbuflen, 0, 0, NETLINK_ROUTE, 0) < 0) - goto cleanup; + return -1; if (virNetlinkGetErrorCode(resp, recvbuflen) < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, "%s", _("error reading DAD state information")); - goto cleanup; + return -1; } /* Parse response. */ @@ -490,21 +476,19 @@ virNetDevIPWaitDadFinish(virSocketAddrPtr *addrs, size_t count) _("Duplicate Address Detection " "not finished in %d seconds"), VIR_DAD_WAIT_TIMEOUT); } else { - ret = 0; + return 0; } - cleanup: - nlmsg_free(nlmsg); - return ret; + return -1; } static int virNetDevIPGetAcceptRA(const char *ifname) { + char *suffix; + int accept_ra = -1; VIR_AUTOFREE(char *) path = NULL; VIR_AUTOFREE(char *) buf = NULL; - char *suffix; - int accept_ra = -1; if (virAsprintf(&path, "/proc/sys/net/ipv6/conf/%s/accept_ra", ifname ? ifname : "all") < 0) @@ -553,6 +537,7 @@ virNetDevIPCheckIPv6ForwardingCallback(const struct nlmsghdr *resp, /* Should never happen: netlink message would be broken */ if (ifname) { VIR_AUTOFREE(char *) ifname2 = virNetDevGetName(oif); + VIR_WARN("Single route has unexpected 2nd interface " "- '%s' and '%s'", ifname, ifname2); break; @@ -588,7 +573,6 @@ virNetDevIPCheckIPv6ForwardingCallback(const struct nlmsghdr *resp, bool virNetDevIPCheckIPv6Forwarding(void) { - struct nl_msg *nlmsg = NULL; bool valid = false; struct rtgenmsg genmsg; size_t i; @@ -597,6 +581,7 @@ virNetDevIPCheckIPv6Forwarding(void) .devices = NULL, .ndevices = 0 }; + VIR_AUTOPTR(virNlMsg) nlmsg = NULL; /* Prepare the request message */ @@ -650,7 +635,6 @@ virNetDevIPCheckIPv6Forwarding(void) } cleanup: - nlmsg_free(nlmsg); for (i = 0; i < data.ndevices; i++) VIR_FREE(data.devices[i]); return valid; @@ -665,24 +649,23 @@ virNetDevIPAddrAdd(const char *ifname, virSocketAddr *peer, unsigned int prefix) { - virCommandPtr cmd = NULL; virSocketAddr broadcast; - int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; VIR_AUTOFREE(char *) addrstr = NULL; VIR_AUTOFREE(char *) bcaststr = NULL; VIR_AUTOFREE(char *) peerstr = NULL; if (!(addrstr = virSocketAddrFormat(addr))) - goto cleanup; + return -1; if (peer && VIR_SOCKET_ADDR_VALID(peer) && !(peerstr = virSocketAddrFormat(peer))) - goto cleanup; + return -1; /* format up a broadcast address if this is IPv4 */ if (!peerstr && ((VIR_SOCKET_ADDR_IS_FAMILY(addr, AF_INET)) && ((virSocketAddrBroadcastByPrefix(addr, prefix, &broadcast) < 0) || !(bcaststr = virSocketAddrFormat(&broadcast))))) { - goto cleanup; + return -1; } # ifdef IFCONFIG_PATH @@ -710,12 +693,9 @@ virNetDevIPAddrAdd(const char *ifname, # endif if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return -1; - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; + return 0; } @@ -724,12 +704,11 @@ virNetDevIPAddrDel(const char *ifname, virSocketAddr *addr, unsigned int prefix) { - virCommandPtr cmd = NULL; - int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; VIR_AUTOFREE(char *) addrstr = NULL; if (!(addrstr = virSocketAddrFormat(addr))) - goto cleanup; + return -1; # ifdef IFCONFIG_PATH cmd = virCommandNew(IFCONFIG_PATH); virCommandAddArg(cmd, ifname); @@ -747,12 +726,9 @@ virNetDevIPAddrDel(const char *ifname, # endif if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return -1; - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; + return 0; } @@ -763,15 +739,14 @@ virNetDevIPRouteAdd(const char *ifname, virSocketAddrPtr gateway, unsigned int metric) { - virCommandPtr cmd = NULL; - int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; VIR_AUTOFREE(char *) addrstr = NULL; VIR_AUTOFREE(char *) gatewaystr = NULL; if (!(addrstr = virSocketAddrFormat(addr))) - goto cleanup; + return -1; if (!(gatewaystr = virSocketAddrFormat(gateway))) - goto cleanup; + return -1; cmd = virCommandNew(IP_PATH); virCommandAddArgList(cmd, "route", "add", NULL); virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix); @@ -780,12 +755,9 @@ virNetDevIPRouteAdd(const char *ifname, virCommandAddArgFormat(cmd, "%u", metric); if (virCommandRun(cmd, NULL) < 0) - 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_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(a)gmail.com&gt; --- src/util/virnetdevopenvswitch.c | 80 ++++++++++++++--------------------------- 1 file changed, 27 insertions(+), 53 deletions(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 9a9435f..a5de541 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -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; size_t len; - int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; 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,10 +318,9 @@ int virNetDevOpenvswitchInterfaceStats(const char *ifname, virDomainInterfaceStatsPtr stats) { - virCommandPtr cmd = NULL; char *tmp; bool gotStats = false; - int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; VIR_AUTOFREE(char *) output = NULL; /* Just ensure the interface exists in ovs */ @@ -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,12 +446,12 @@ int virNetDevOpenvswitchGetVhostuserIfname(const char *path, char **ifname) { - virCommandPtr cmd = NULL; char *tmpIfname = NULL; char **tokens = NULL; size_t ntokens = 0; int status; int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; /* Openvswitch vhostuser path are hardcoded to * /<runstatedir>/openvswitch/<ifname> @@ -503,7 +482,6 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path, cleanup: virStringListFreeCount(tokens, ntokens); - virCommandFree(cmd); return ret; } @@ -519,8 +497,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); @@ -531,16 +508,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_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(a)gmail.com&gt; --- src/util/virqemu.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/virqemu.c b/src/util/virqemu.c index bc78853..1cd2b93 100644 --- a/src/util/virqemu.c +++ b/src/util/virqemu.c @@ -281,6 +281,7 @@ virQEMUBuildDriveCommandlineFromJSON(virJSONValuePtr srcdef) cleanup: virBufferFreeAndReset(&buf); return ret; + } -- 1.8.3.1

My bad. You can ignore this patch. On Thu, 9 Aug 2018 at 18:18, Erik Skultety <eskultet(a)redhat.com&gt; wrote: