[libvirt] [PATCH v2 00/35] use GNU C's cleanup attribute in src/util (batch III)
This third series of patches also modifies a few files in src/util to use VIR_AUTOFREE and VIR_AUTOPTR for automatic freeing of memory and get rid of some VIR_FREE macro invocations and *Free function calls. Sukrit Bhatnagar (35): util: iscsi: use VIR_AUTOFREE instead of VIR_FREE for scalar types util: iscsi: use VIR_AUTOPTR for aggregate types util: netlink: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC util: netlink: use VIR_AUTOFREE instead of VIR_FREE for scalar types util: netlink: use VIR_AUTOPTR for aggregate types util: netdevbridge: use VIR_AUTOFREE instead of VIR_FREE for scalar types util: netdevbridge: use VIR_AUTOPTR for aggregate types util: macaddr: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC util: netdev: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC util: netdev: use VIR_AUTOFREE instead of VIR_FREE for scalar types util: netdev: use VIR_AUTOPTR for aggregate types util: socketaddr: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC util: socketaddr: use VIR_AUTOFREE instead of VIR_FREE for scalar types util: socketaddr: use VIR_AUTOPTR for aggregate types util: netdevip: define virNetDevIPAddrFree for use with cleanup macros util: netdevip: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC util: netdevip: use VIR_AUTOFREE instead of VIR_FREE for scalar types util: netdevip: use VIR_AUTOPTR for aggregate types util: netdevmacvlan: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC util: netdevmacvlan: use VIR_AUTOFREE instead of VIR_FREE for scalar types util: netdevmacvlan: use VIR_AUTOPTR for aggregate types util: netdevopenvswitch: use VIR_AUTOFREE instead of VIR_FREE for scalar types util: netdevopenvswitch: use VIR_AUTOPTR for aggregate types util: netdevtap: use VIR_AUTOFREE instead of VIR_FREE for scalar types util: netdevveth: use VIR_AUTOFREE instead of VIR_FREE for scalar types util: netdevveth: use VIR_AUTOPTR for aggregate types util: numa: use VIR_AUTOFREE instead of VIR_FREE for scalar types util: numa: use VIR_AUTOPTR for aggregate types util: perf: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC util: perf: use VIR_AUTOFREE instead of VIR_FREE for scalar types util: pidfile: use VIR_AUTOFREE instead of VIR_FREE for scalar types util: process: use VIR_AUTOFREE instead of VIR_FREE for scalar types util: process: use VIR_AUTOPTR for aggregate types util: qemu: use VIR_AUTOFREE instead of VIR_FREE for scalar types util: qemu: use VIR_AUTOPTR for aggregate types src/util/viriscsi.c | 140 ++++------ src/util/virmacaddr.c | 6 + src/util/virmacaddr.h | 4 + src/util/virnetdev.c | 604 ++++++++++++++++------------------------ src/util/virnetdev.h | 4 + src/util/virnetdevbridge.c | 81 ++---- src/util/virnetdevip.c | 227 ++++++--------- src/util/virnetdevip.h | 5 + src/util/virnetdevmacvlan.c | 22 +- src/util/virnetdevopenvswitch.c | 98 +++---- src/util/virnetdevtap.c | 11 +- src/util/virnetdevveth.c | 32 +-- src/util/virnetlink.c | 118 ++++---- src/util/virnetlink.h | 5 + src/util/virnuma.c | 103 +++---- src/util/virperf.c | 20 +- src/util/virperf.h | 8 +- src/util/virpidfile.c | 186 ++++--------- src/util/virprocess.c | 58 ++-- src/util/virqemu.c | 31 +-- src/util/virsocketaddr.c | 113 ++++---- src/util/virsocketaddr.h | 9 +- 22 files changed, 730 insertions(+), 1155 deletions(-) -- 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> Reviewed-by: Erik Skultety <eskultet@redhat.com> --- src/util/viriscsi.c | 44 +++++++++++++++++--------------------------- 1 file changed, 17 insertions(+), 27 deletions(-) diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c index f00aeb5..0c2d8bb 100644 --- a/src/util/viriscsi.c +++ b/src/util/viriscsi.c @@ -88,8 +88,8 @@ virISCSIGetSession(const char *devpath, .session = NULL, .devpath = devpath, }; - char *error = NULL; int exitstatus = 0; + VIR_AUTOFREE(char *) error = NULL; virCommandPtr cmd = virCommandNewArgList(ISCSIADM, "--mode", "session", NULL); @@ -109,7 +109,6 @@ virISCSIGetSession(const char *devpath, NULLSTR(error)); cleanup: - VIR_FREE(error); virCommandFree(cmd); return cbdata.session; } @@ -125,12 +124,11 @@ virStorageBackendIQNFound(const char *initiatoriqn, char **ifacename) { int ret = IQN_ERROR; - char *outbuf = NULL; char *line = NULL; - char *iface = NULL; - char *iqn = NULL; virCommandPtr cmd = virCommandNewArgList(ISCSIADM, "--mode", "iface", NULL); + VIR_AUTOFREE(char *) outbuf = NULL; + VIR_AUTOFREE(char *) iqn = NULL; *ifacename = NULL; @@ -150,13 +148,13 @@ virStorageBackendIQNFound(const char *initiatoriqn, char *newline; char *next; size_t i; + VIR_AUTOFREE(char *) iface = NULL; if (!(newline = strchr(line, '\n'))) break; *newline = '\0'; - VIR_FREE(iface); VIR_FREE(iqn); /* Find the first space, copy everything up to that point into @@ -197,9 +195,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,8 +211,9 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn, char **ifacename) { int ret = -1, exitstatus = -1; - char *temp_ifacename; virCommandPtr cmd = NULL; + VIR_AUTOFREE(char *) iface_name = NULL; + VIR_AUTOFREE(char *) temp_ifacename = NULL; if (virAsprintf(&temp_ifacename, "libvirt-iface-%08llx", @@ -263,23 +259,23 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn, } /* 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; } else { VIR_DEBUG("Interface '%s' with IQN '%s' was created successfully", - *ifacename, initiatoriqn); + iface_name, initiatoriqn); } - ret = 0; + VIR_STEAL_PTR(*ifacename, iface_name); + + virCommandFree(cmd); + return 0; cleanup: virCommandFree(cmd); - VIR_FREE(temp_ifacename); - if (ret != 0) - VIR_FREE(*ifacename); return ret; } @@ -299,7 +295,7 @@ virISCSIConnection(const char *portal, NULL }; virCommandPtr cmd; - char *ifacename = NULL; + VIR_AUTOFREE(char *) ifacename = NULL; cmd = virCommandNewArgs(baseargv); virCommandAddArgSet(cmd, extraargv); @@ -339,7 +335,6 @@ virISCSIConnection(const char *portal, cleanup: virCommandFree(cmd); - VIR_FREE(ifacename); return ret; } @@ -390,15 +385,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 +491,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 +514,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 | 100 +++++++++++++++++++--------------------------------- 1 file changed, 36 insertions(+), 64 deletions(-) diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c index 0c2d8bb..8c272b5 100644 --- a/src/util/viriscsi.c +++ b/src/util/viriscsi.c @@ -89,10 +89,10 @@ virISCSIGetSession(const char *devpath, .devpath = devpath, }; int exitstatus = 0; + VIR_AUTOPTR(virCommand) cmd = virCommandNewArgList(ISCSIADM, "--mode", + "session", NULL); VIR_AUTOFREE(char *) error = NULL; - virCommandPtr cmd = virCommandNewArgList(ISCSIADM, "--mode", - "session", NULL); virCommandSetErrorBuffer(cmd, &error); if (virCommandRunRegex(cmd, @@ -101,15 +101,13 @@ virISCSIGetSession(const char *devpath, vars, virISCSIExtractSession, &cbdata, NULL, &exitstatus) < 0) - goto cleanup; + return cbdata.session; if (cbdata.session == NULL && !probe) virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find iscsiadm session: %s"), NULLSTR(error)); - cleanup: - virCommandFree(cmd); return cbdata.session; } @@ -125,8 +123,8 @@ virStorageBackendIQNFound(const char *initiatoriqn, { int ret = IQN_ERROR; char *line = NULL; - virCommandPtr cmd = virCommandNewArgList(ISCSIADM, - "--mode", "iface", NULL); + VIR_AUTOPTR(virCommand) cmd = virCommandNewArgList(ISCSIADM, + "--mode", "iface", NULL); VIR_AUTOFREE(char *) outbuf = NULL; VIR_AUTOFREE(char *) iqn = NULL; @@ -195,7 +193,6 @@ virStorageBackendIQNFound(const char *initiatoriqn, if (ret == IQN_MISSING) VIR_DEBUG("Could not find interface with IQN '%s'", iqn); - virCommandFree(cmd); return ret; error: @@ -210,8 +207,8 @@ static int virStorageBackendCreateIfaceIQN(const char *initiatoriqn, char **ifacename) { - int ret = -1, exitstatus = -1; - virCommandPtr cmd = NULL; + int exitstatus = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; VIR_AUTOFREE(char *) iface_name = NULL; VIR_AUTOFREE(char *) temp_ifacename = NULL; @@ -236,7 +233,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); @@ -255,7 +252,7 @@ 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. */ @@ -263,7 +260,7 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn, 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", iface_name, initiatoriqn); @@ -271,12 +268,7 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn, VIR_STEAL_PTR(*ifacename, iface_name); - virCommandFree(cmd); return 0; - - cleanup: - virCommandFree(cmd); - return ret; } @@ -286,7 +278,6 @@ virISCSIConnection(const char *portal, const char *target, const char **extraargv) { - int ret = -1; const char *const baseargv[] = { ISCSIADM, "--mode", "node", @@ -294,7 +285,7 @@ virISCSIConnection(const char *portal, "--targetname", target, NULL }; - virCommandPtr cmd; + VIR_AUTOPTR(virCommand) cmd = NULL; VIR_AUTOFREE(char *) ifacename = NULL; cmd = virCommandNewArgs(baseargv); @@ -307,7 +298,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 @@ -318,25 +309,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; } @@ -363,14 +349,12 @@ virISCSIConnectionLogout(const char *portal, int virISCSIRescanLUNs(const char *session) { - virCommandPtr cmd = virCommandNewArgList(ISCSIADM, - "--mode", "session", - "-r", session, - "-R", - NULL); - int ret = virCommandRun(cmd, NULL); - virCommandFree(cmd); - return ret; + VIR_AUTOPTR(virCommand) cmd = virCommandNewArgList(ISCSIADM, + "--mode", "session", + "-r", session, + "-R", + NULL); + return virCommandRun(cmd, NULL); } @@ -420,12 +404,11 @@ virISCSIScanTargetsInternal(const char *portal, int vars[] = { 2 }; struct virISCSITargetList list; size_t i; - int ret = -1; - virCommandPtr cmd = virCommandNewArgList(ISCSIADM, - "--mode", "discovery", - "--type", "sendtargets", - "--portal", portal, - NULL); + VIR_AUTOPTR(virCommand) cmd = virCommandNewArgList(ISCSIADM, + "--mode", "discovery", + "--type", "sendtargets", + "--portal", portal, + NULL); if (!persist) { virCommandAddArgList(cmd, @@ -447,7 +430,7 @@ virISCSIScanTargetsInternal(const char *portal, vars, virISCSIGetTargets, &list, NULL, NULL) < 0) - goto cleanup; + return -1; if (ntargetsret && targetsret) { *ntargetsret = list.ntargets; @@ -458,10 +441,7 @@ virISCSIScanTargetsInternal(const char *portal, VIR_FREE(list.targets); } - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; + return 0; } @@ -539,9 +519,8 @@ int virISCSINodeNew(const char *portal, const char *target) { - virCommandPtr cmd = NULL; int status; - int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; cmd = virCommandNewArgList(ISCSIADM, "--mode", "node", @@ -554,20 +533,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; } @@ -577,9 +553,8 @@ virISCSINodeUpdate(const char *portal, const char *name, const char *value) { - virCommandPtr cmd = NULL; int status; - int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; cmd = virCommandNewArgList(ISCSIADM, "--mode", "node", @@ -595,11 +570,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
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 pointers to virNlMsg and virNetlinkHandle types are declared using VIR_AUTOPTR, the functions nlmsg_free and virNetlinkFree, respectively, will be run automatically on them when they go out of scope. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- src/util/virnetlink.c | 3 ++- src/util/virnetlink.h | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index fa1ba3e..1c1eac7 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" @@ -72,6 +71,8 @@ typedef struct nl_handle virNetlinkHandle; typedef struct nl_sock virNetlinkHandle; # endif +VIR_DEFINE_AUTOPTR_FUNC(virNetlinkHandle, virNetlinkFree) + typedef struct _virNetlinkEventSrvPrivate virNetlinkEventSrvPrivate; typedef virNetlinkEventSrvPrivate *virNetlinkEventSrvPrivatePtr; struct _virNetlinkEventSrvPrivate { diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h index 2a9de0a..647f589 100644 --- a/src/util/virnetlink.h +++ b/src/util/virnetlink.h @@ -44,6 +44,9 @@ struct nlmsghdr; # endif /* __linux__ */ +typedef struct nl_msg virNlMsg; +typedef virNlMsg *virNlMsgPtr; + 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
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/virnetlink.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index 1c1eac7..aa3a86d 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -346,7 +346,6 @@ virNetlinkDumpCommand(struct nl_msg *nl_msg, int ret = -1; bool end = false; int len = 0; - struct nlmsghdr *resp = NULL; struct nlmsghdr *msg = NULL; struct sockaddr_nl nladdr = { @@ -361,6 +360,8 @@ virNetlinkDumpCommand(struct nl_msg *nl_msg, goto cleanup; 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)) { @@ -374,13 +375,11 @@ virNetlinkDumpCommand(struct nl_msg *nl_msg, if (callback(msg, opaque) < 0) goto cleanup; } - VIR_FREE(resp); } ret = 0; cleanup: - VIR_FREE(resp); virNetlinkFree(nlhandle); return ret; } @@ -410,7 +409,6 @@ virNetlinkDumpLink(const char *ifname, int ifindex, uint32_t src_pid, uint32_t dst_pid) { int rc = -1; - struct nlmsghdr *resp = NULL; struct nlmsgerr *err; struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC, @@ -418,6 +416,9 @@ virNetlinkDumpLink(const char *ifname, int ifindex, }; unsigned int recvbuflen; struct nl_msg *nl_msg; + VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; + + *nlData = NULL; if (ifname && ifindex <= 0 && virNetDevGetIndex(ifname, &ifindex) < 0) return -1; @@ -485,12 +486,12 @@ virNetlinkDumpLink(const char *ifname, int ifindex, default: goto malformed_resp; } + + VIR_STEAL_PTR(*nlData, resp); rc = 0; + cleanup: nlmsg_free(nl_msg); - if (rc < 0) - VIR_FREE(resp); - *nlData = resp; return rc; malformed_resp: @@ -524,11 +525,11 @@ int virNetlinkDelLink(const char *ifname, virNetlinkDelLinkFallback fallback) { 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; + VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; nl_msg = nlmsg_alloc_simple(RTM_DELLINK, NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL); @@ -579,7 +580,6 @@ virNetlinkDelLink(const char *ifname, virNetlinkDelLinkFallback fallback) rc = 0; cleanup: nlmsg_free(nl_msg); - VIR_FREE(resp); return rc; malformed_resp: @@ -612,13 +612,15 @@ int virNetlinkGetNeighbor(void **nlData, uint32_t src_pid, uint32_t dst_pid) { int rc = -1; - struct nlmsghdr *resp = NULL; struct nlmsgerr *err; struct ndmsg ndinfo = { .ndm_family = AF_UNSPEC, }; unsigned int recvbuflen; struct nl_msg *nl_msg; + VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; + + *nlData = NULL; nl_msg = nlmsg_alloc_simple(RTM_GETNEIGH, NLM_F_DUMP | NLM_F_REQUEST); if (!nl_msg) { @@ -656,13 +658,12 @@ virNetlinkGetNeighbor(void **nlData, uint32_t src_pid, uint32_t dst_pid) default: goto malformed_resp; } + + VIR_STEAL_PTR(*nlData, resp); rc = recvbuflen; cleanup: nlmsg_free(nl_msg); - if (rc < 0) - VIR_FREE(resp); - *nlData = resp; return rc; malformed_resp: @@ -768,12 +769,12 @@ virNetlinkEventCallback(int watch, void *opaque) { virNetlinkEventSrvPrivatePtr srv = opaque; - struct nlmsghdr *msg; struct sockaddr_nl peer; struct ucred *creds = NULL; size_t i; int length; bool handled = false; + VIR_AUTOFREE(struct nlmsghdr *) msg = NULL; length = nl_recv(srv->netlinknh, &peer, (unsigned char **)&msg, &creds); @@ -803,7 +804,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_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 | 88 +++++++++++++++++++-------------------------------- 1 file changed, 33 insertions(+), 55 deletions(-) diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index aa3a86d..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]; - virNetlinkHandle *nlhandle = NULL; int len = 0; + VIR_AUTOPTR(virNetlinkHandle) nlhandle = NULL; + + *respbuflen = 0; memset(fds, 0, sizeof(fds)); @@ -324,16 +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; - virNetlinkFree(nlhandle); - return ret; + cleanup: + *resp = NULL; + return -1; } int @@ -343,7 +340,6 @@ 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 *msg = NULL; @@ -353,11 +349,11 @@ virNetlinkDumpCommand(struct nl_msg *nl_msg, .nl_pid = dst_pid, .nl_groups = 0, }; - virNetlinkHandle *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; @@ -370,18 +366,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; } } - ret = 0; - - cleanup: - virNetlinkFree(nlhandle); - return ret; + return 0; } /** @@ -415,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; @@ -456,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; @@ -471,7 +463,7 @@ virNetlinkDumpLink(const char *ifname, int ifindex, virReportSystemError(-err->error, _("error dumping %s (%d) interface"), ifname, ifindex); - goto cleanup; + return -1; } break; @@ -488,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; } @@ -524,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, @@ -546,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) @@ -558,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; @@ -577,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; } /** @@ -611,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; @@ -634,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; @@ -648,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; @@ -660,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
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 | 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@gmail.com> --- 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
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
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 variables of type virNetDevRxFilterPtr and virNetDevMcastEntryPtr are declared using VIR_AUTOPTR, the functions virNetDevRxFilterFree and virNetDevMcastEntryFree, respectively, will be run automatically on them when they go out of scope. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- src/util/virnetdev.c | 9 ++++++++- src/util/virnetdev.h | 4 ++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 0777eca..9eca786 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -29,7 +29,6 @@ #include "virfile.h" #include "virerror.h" #include "vircommand.h" -#include "viralloc.h" #include "virpci.h" #include "virlog.h" #include "virstring.h" @@ -120,6 +119,14 @@ struct _virNetDevMcastEntry { virMacAddr macaddr; }; +static void +virNetDevMcastEntryFree(virNetDevMcastEntryPtr entry) +{ + VIR_FREE(entry); +} + +VIR_DEFINE_AUTOPTR_FUNC(virNetDevMcastEntry, virNetDevMcastEntryFree) + typedef struct _virNetDevMcastList virNetDevMcastList; typedef virNetDevMcastList *virNetDevMcastListPtr; struct _virNetDevMcastList { diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index 71eaf45..8860ea1 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -30,6 +30,7 @@ # include "virmacaddr.h" # include "virpci.h" # include "virnetdevvlan.h" +# include "viralloc.h" # ifdef HAVE_STRUCT_IFREQ typedef struct ifreq virIfreq; @@ -313,4 +314,7 @@ int virNetDevSysfsFile(char **pf_sysfs_device_link, int virNetDevRunEthernetScript(const char *ifname, const char *script) ATTRIBUTE_NOINLINE; + +VIR_DEFINE_AUTOPTR_FUNC(virNetDevRxFilter, virNetDevRxFilterFree) + #endif /* __VIR_NETDEV_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 | 328 +++++++++++++++++++-------------------------------- 1 file changed, 119 insertions(+), 209 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 9eca786..5651f6f 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,10 +2750,10 @@ static int virNetDevGetMcastList(const char *ifname, virNetDevMcastListPtr mcast) { char *cur = NULL; - char *buf = NULL; char *next = NULL; int ret = -1, len; virNetDevMcastEntryPtr entry = NULL; + VIR_AUTOFREE(char *) buf = NULL; mcast->entries = NULL; mcast->nentries = 0; @@ -2866,7 +2786,6 @@ static int virNetDevGetMcastList(const char *ifname, ret = 0; cleanup: - VIR_FREE(buf); VIR_FREE(entry); return ret; @@ -3006,13 +2925,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; @@ -3028,6 +2945,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 +2956,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,11 +3119,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))) { @@ -3245,7 +3159,6 @@ virNetDevGetFamilyId(const char *family_name) cleanup: nlmsg_free(nl_msg); - VIR_FREE(resp); return family_id; } @@ -3265,16 +3178,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; @@ -3333,8 +3246,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 +3286,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 +3296,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 | 267 +++++++++++++++++++++------------------------------ 1 file changed, 110 insertions(+), 157 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 5651f6f..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; } @@ -2751,8 +2732,8 @@ static int virNetDevGetMcastList(const char *ifname, { char *cur = NULL; char *next = NULL; - int ret = -1, len; - virNetDevMcastEntryPtr entry = NULL; + int len; + VIR_AUTOPTR(virNetDevMcastEntry) entry = NULL; VIR_AUTOFREE(char *) buf = NULL; mcast->entries = NULL; @@ -2760,35 +2741,31 @@ static int virNetDevGetMcastList(const char *ifname, /* 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(entry); - - return ret; + return 0; } @@ -2867,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; @@ -2898,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) @@ -3118,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; @@ -3140,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]); } @@ -3177,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; @@ -3224,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] && @@ -3241,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 @@ -3501,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, "")) @@ -3516,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
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> Reviewed-by: Erik Skultety <eskultet@redhat.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> Reviewed-by: Erik Skultety <eskultet@redhat.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> Reviewed-by: Erik Skultety <eskultet@redhat.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..3e5ea64 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; size_t i; - int ret = -1; + VIR_AUTOPTR(virBuffer) buf = NULL; + + 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
Define a free function for virNetDevIPAddrPtr type for consistency and extensibility. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- src/util/virnetdevip.c | 6 ++++++ src/util/virnetdevip.h | 1 + 2 files changed, 7 insertions(+) diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c index bf98ed8..7197d07 100644 --- a/src/util/virnetdevip.c +++ b/src/util/virnetdevip.c @@ -1129,3 +1129,9 @@ virNetDevIPInfoAddToDev(const char *ifname, cleanup: return ret; } + +void +virNetDevIPAddrFree(virNetDevIPAddrPtr ip) +{ + VIR_FREE(ip); +} diff --git a/src/util/virnetdevip.h b/src/util/virnetdevip.h index 6b509ea..dfc978d 100644 --- a/src/util/virnetdevip.h +++ b/src/util/virnetdevip.h @@ -84,6 +84,7 @@ int virNetDevIPAddrGet(const char *ifname, virSocketAddrPtr addr) int virNetDevIPWaitDadFinish(virSocketAddrPtr *addrs, size_t count) ATTRIBUTE_NONNULL(1); bool virNetDevIPCheckIPv6Forwarding(void); +void virNetDevIPAddrFree(virNetDevIPAddrPtr ip); /* virNetDevIPRoute object */ void virNetDevIPRouteFree(virNetDevIPRoutePtr def); -- 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 variables of type virNetDevIPAddrPtr and virNetDevIPRoutePtr are declared using VIR_AUTOPTR, the functions virNetDevIPAddrFree and virNetDevIPRouteFree, respectively, will be run automatically on them when they go out of scope. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- src/util/virnetdevip.c | 1 - src/util/virnetdevip.h | 4 ++++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c index 7197d07..fdb0b74 100644 --- a/src/util/virnetdevip.c +++ b/src/util/virnetdevip.c @@ -27,7 +27,6 @@ #include "virnetlink.h" #include "virfile.h" #include "virerror.h" -#include "viralloc.h" #include "virlog.h" #include "virstring.h" #include "virutil.h" diff --git a/src/util/virnetdevip.h b/src/util/virnetdevip.h index dfc978d..9cfd27c 100644 --- a/src/util/virnetdevip.h +++ b/src/util/virnetdevip.h @@ -24,6 +24,7 @@ # define __VIR_NETDEVIP_H__ # include "virsocketaddr.h" +# include "viralloc.h" typedef struct _virNetDevIPAddr virNetDevIPAddr; typedef virNetDevIPAddr *virNetDevIPAddrPtr; @@ -98,4 +99,7 @@ void virNetDevIPInfoClear(virNetDevIPInfoPtr ip); int virNetDevIPInfoAddToDev(const char *ifname, virNetDevIPInfo const *ipInfo); +VIR_DEFINE_AUTOPTR_FUNC(virNetDevIPAddr, virNetDevIPAddrFree) +VIR_DEFINE_AUTOPTR_FUNC(virNetDevIPRoute, virNetDevIPRouteFree) + #endif /* __VIR_NETDEVIP_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/virnetdevip.c | 97 ++++++++++++++++++-------------------------------- 1 file changed, 34 insertions(+), 63 deletions(-) diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c index fdb0b74..78c6dcf 100644 --- a/src/util/virnetdevip.c +++ b/src/util/virnetdevip.c @@ -171,11 +171,11 @@ virNetDevIPAddrAdd(const char *ifname, virSocketAddr *broadcast = NULL; int ret = -1; struct nl_msg *nlmsg = NULL; - struct nlmsghdr *resp = NULL; unsigned int recvbuflen; - char *ipStr = NULL; - char *peerStr = NULL; - char *bcastStr = NULL; + VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; + VIR_AUTOFREE(char *) ipStr = NULL; + VIR_AUTOFREE(char *) peerStr = NULL; + VIR_AUTOFREE(char *) bcastStr = NULL; ipStr = virSocketAddrFormat(addr); if (peer && VIR_SOCKET_ADDR_VALID(peer)) @@ -225,11 +225,7 @@ virNetDevIPAddrAdd(const char *ifname, ret = 0; cleanup: - VIR_FREE(ipStr); - VIR_FREE(peerStr); - VIR_FREE(bcastStr); nlmsg_free(nlmsg); - VIR_FREE(resp); VIR_FREE(broadcast); return ret; } @@ -252,8 +248,8 @@ virNetDevIPAddrDel(const char *ifname, { int ret = -1; struct nl_msg *nlmsg = NULL; - struct nlmsghdr *resp = NULL; unsigned int recvbuflen; + VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; if (!(nlmsg = virNetDevCreateNetlinkAddressMessage(RTM_DELADDR, ifname, addr, prefix, @@ -273,7 +269,6 @@ virNetDevIPAddrDel(const char *ifname, ret = 0; cleanup: nlmsg_free(nlmsg); - VIR_FREE(resp); return ret; } @@ -309,8 +304,8 @@ virNetDevIPRouteAdd(const char *ifname, int errCode; virSocketAddr defaultAddr; virSocketAddrPtr actualAddr; - char *toStr = NULL; - char *viaStr = NULL; + VIR_AUTOFREE(char *) toStr = NULL; + VIR_AUTOFREE(char *) viaStr = NULL; actualAddr = addr; @@ -383,8 +378,6 @@ virNetDevIPRouteAdd(const char *ifname, ret = 0; cleanup: - VIR_FREE(toStr); - VIR_FREE(viaStr); nlmsg_free(nlmsg); return ret; @@ -452,7 +445,6 @@ virNetDevIPWaitDadFinish(virSocketAddrPtr *addrs, size_t count) { struct nl_msg *nlmsg = NULL; struct ifaddrmsg ifa; - struct nlmsghdr *resp = NULL; unsigned int recvbuflen; int ret = -1; bool dad = true; @@ -475,6 +467,8 @@ virNetDevIPWaitDadFinish(virSocketAddrPtr *addrs, size_t count) /* Periodically query netlink until DAD finishes on all known addresses. */ while (dad && time(NULL) < max_time) { + VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; + if (virNetlinkCommand(nlmsg, &resp, &recvbuflen, 0, 0, NETLINK_ROUTE, 0) < 0) goto cleanup; @@ -489,8 +483,6 @@ virNetDevIPWaitDadFinish(virSocketAddrPtr *addrs, size_t count) dad = virNetDevIPParseDadStatus(resp, recvbuflen, addrs, count); if (dad) usleep(1000 * 10); - - VIR_FREE(resp); } /* Check timeout. */ if (dad) { @@ -502,7 +494,6 @@ virNetDevIPWaitDadFinish(virSocketAddrPtr *addrs, size_t count) } cleanup: - VIR_FREE(resp); nlmsg_free(nlmsg); return ret; } @@ -510,22 +501,18 @@ virNetDevIPWaitDadFinish(virSocketAddrPtr *addrs, size_t count) static int virNetDevIPGetAcceptRA(const char *ifname) { - char *path = NULL; - char *buf = NULL; char *suffix; int accept_ra = -1; + VIR_AUTOFREE(char *) path = NULL; + VIR_AUTOFREE(char *) buf = NULL; if (virAsprintf(&path, "/proc/sys/net/ipv6/conf/%s/accept_ra", ifname ? ifname : "all") < 0) - goto cleanup; + return -1; if ((virFileReadAll(path, 512, &buf) < 0) || (virStrToLong_i(buf, &suffix, 10, &accept_ra) < 0)) - goto cleanup; - - cleanup: - VIR_FREE(path); - VIR_FREE(buf); + return accept_ra; return accept_ra; } @@ -545,17 +532,16 @@ virNetDevIPCheckIPv6ForwardingCallback(const struct nlmsghdr *resp, struct rtmsg *rtmsg = NLMSG_DATA(resp); int accept_ra = -1; struct rtattr *rta; - char *ifname = NULL; struct virNetDevIPCheckIPv6ForwardingData *data = opaque; - int ret = 0; int len = RTM_PAYLOAD(resp); int oif = -1; size_t i; bool hasDevice; + VIR_AUTOFREE(char *) ifname = NULL; /* Ignore messages other than route ones */ if (resp->nlmsg_type != RTM_NEWROUTE) - return ret; + return 0; /* Extract a device ID attribute */ VIR_WARNINGS_NO_CAST_ALIGN @@ -566,21 +552,20 @@ virNetDevIPCheckIPv6ForwardingCallback(const struct nlmsghdr *resp, /* Should never happen: netlink message would be broken */ if (ifname) { - char *ifname2 = virNetDevGetName(oif); + VIR_AUTOFREE(char *) ifname2 = virNetDevGetName(oif); VIR_WARN("Single route has unexpected 2nd interface " "- '%s' and '%s'", ifname, ifname2); - VIR_FREE(ifname2); break; } if (!(ifname = virNetDevGetName(oif))) - goto error; + return -1; } } /* No need to do anything else for non RA routes */ if (rtmsg->rtm_protocol != RTPROT_RA) - goto cleanup; + return 0; data->hasRARoutes = true; @@ -595,15 +580,9 @@ virNetDevIPCheckIPv6ForwardingCallback(const struct nlmsghdr *resp, } if (accept_ra != 2 && !hasDevice && VIR_APPEND_ELEMENT(data->devices, data->ndevices, ifname) < 0) - goto error; + return -1; - cleanup: - VIR_FREE(ifname); - return ret; - - error: - ret = -1; - goto cleanup; + return 0; } bool @@ -687,9 +666,11 @@ virNetDevIPAddrAdd(const char *ifname, unsigned int prefix) { virCommandPtr cmd = NULL; - char *addrstr = NULL, *bcaststr = NULL, *peerstr = NULL; virSocketAddr broadcast; int ret = -1; + VIR_AUTOFREE(char *) addrstr = NULL; + VIR_AUTOFREE(char *) bcaststr = NULL; + VIR_AUTOFREE(char *) peerstr = NULL; if (!(addrstr = virSocketAddrFormat(addr))) goto cleanup; @@ -733,9 +714,6 @@ virNetDevIPAddrAdd(const char *ifname, ret = 0; cleanup: - VIR_FREE(addrstr); - VIR_FREE(bcaststr); - VIR_FREE(peerstr); virCommandFree(cmd); return ret; } @@ -747,8 +725,8 @@ virNetDevIPAddrDel(const char *ifname, unsigned int prefix) { virCommandPtr cmd = NULL; - char *addrstr; int ret = -1; + VIR_AUTOFREE(char *) addrstr = NULL; if (!(addrstr = virSocketAddrFormat(addr))) goto cleanup; @@ -773,7 +751,6 @@ virNetDevIPAddrDel(const char *ifname, ret = 0; cleanup: - VIR_FREE(addrstr); virCommandFree(cmd); return ret; } @@ -786,9 +763,10 @@ virNetDevIPRouteAdd(const char *ifname, virSocketAddrPtr gateway, unsigned int metric) { - virCommandPtr cmd = NULL; - char *addrstr = NULL, *gatewaystr = NULL; int ret = -1; + virCommandPtr cmd = NULL; + VIR_AUTOFREE(char *) addrstr = NULL; + VIR_AUTOFREE(char *) gatewaystr = NULL; if (!(addrstr = virSocketAddrFormat(addr))) goto cleanup; @@ -806,8 +784,6 @@ virNetDevIPRouteAdd(const char *ifname, ret = 0; cleanup: - VIR_FREE(addrstr); - VIR_FREE(gatewaystr); virCommandFree(cmd); return ret; } @@ -1083,7 +1059,6 @@ int virNetDevIPInfoAddToDev(const char *ifname, virNetDevIPInfo const *ipInfo) { - int ret = -1; size_t i; int prefix; @@ -1093,16 +1068,15 @@ virNetDevIPInfoAddToDev(const char *ifname, if ((prefix = virSocketAddrGetIPPrefix(&ip->address, NULL, ip->prefix)) < 0) { - char *ipStr = virSocketAddrFormat(&ip->address); + VIR_AUTOFREE(char *) ipStr = virSocketAddrFormat(&ip->address); virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to determine prefix for IP address '%s'"), NULLSTR(ipStr)); - VIR_FREE(ipStr); - goto cleanup; + return -1; } if (virNetDevIPAddrAdd(ifname, &ip->address, &ip->peer, prefix) < 0) - goto cleanup; + return -1; } /* add all routes */ @@ -1110,23 +1084,20 @@ virNetDevIPInfoAddToDev(const char *ifname, virNetDevIPRoutePtr route = ipInfo->routes[i]; if ((prefix = virNetDevIPRouteGetPrefix(route)) < 0) { - char *ipStr = virSocketAddrFormat(&route->address); + VIR_AUTOFREE(char *) ipStr = virSocketAddrFormat(&route->address); virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to determine prefix for route with destination '%s'"), NULLSTR(ipStr)); - VIR_FREE(ipStr); - goto cleanup; + return -1; } if (virNetDevIPRouteAdd(ifname, &route->address, prefix, &route->gateway, virNetDevIPRouteGetMetric(route)) < 0) - goto cleanup; + return -1; } - ret = 0; - cleanup: - return ret; + return 0; } void -- 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/virnetdevip.c | 125 +++++++++++++++++++------------------------------ 1 file changed, 48 insertions(+), 77 deletions(-) diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c index 78c6dcf..df19d16 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,12 +476,10 @@ 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 @@ -588,7 +572,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 +580,7 @@ virNetDevIPCheckIPv6Forwarding(void) .devices = NULL, .ndevices = 0 }; + VIR_AUTOPTR(virNlMsg) nlmsg = NULL; /* Prepare the request message */ @@ -650,7 +634,6 @@ virNetDevIPCheckIPv6Forwarding(void) } cleanup: - nlmsg_free(nlmsg); for (i = 0; i < data.ndevices; i++) VIR_FREE(data.devices[i]); return valid; @@ -665,24 +648,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 +692,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 +703,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 +725,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 +738,14 @@ virNetDevIPRouteAdd(const char *ifname, virSocketAddrPtr gateway, unsigned int metric) { - int ret = -1; - virCommandPtr cmd = NULL; + 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 +754,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
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> Reviewed-by: Erik Skultety <eskultet@redhat.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> Reviewed-by: Erik Skultety <eskultet@redhat.com> --- src/util/virnetdevmacvlan.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index a2ed65c..100507f 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -1184,9 +1184,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 +1194,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 | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index d1c5cf4..eff6b0f 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,10 +335,10 @@ virNetDevOpenvswitchInterfaceStats(const char *ifname, virDomainInterfaceStatsPtr stats) { virCommandPtr cmd = NULL; - char *output; char *tmp; bool gotStats = false; int ret = -1; + VIR_AUTOFREE(char *) output = NULL; /* Just ensure the interface exists in ovs */ cmd = virCommandNew(OVSVSCTL); @@ -399,7 +395,6 @@ virNetDevOpenvswitchInterfaceStats(const char *ifname, ret = 0; cleanup: - VIR_FREE(output); virCommandFree(cmd); return ret; } @@ -481,7 +476,7 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path, size_t ntokens = 0; int status; int ret = -1; - char *ovs_timeout = NULL; + VIR_AUTOFREE(char *) ovs_timeout = NULL; /* Openvswitch vhostuser path are hardcoded to * /<runstatedir>/openvswitch/<ifname> @@ -513,7 +508,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 | 80 ++++++++++++++--------------------------- 1 file changed, 27 insertions(+), 53 deletions(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index eff6b0f..d34fb6f 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; } @@ -470,12 +449,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; VIR_AUTOFREE(char *) ovs_timeout = NULL; /* Openvswitch vhostuser path are hardcoded to @@ -507,7 +486,6 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path, cleanup: virStringListFreeCount(tokens, ntokens); - virCommandFree(cmd); return ret; } @@ -523,8 +501,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); @@ -535,16 +512,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> Reviewed-by: Erik Skultety <eskultet@redhat.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> Reviewed-by: Erik Skultety <eskultet@redhat.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> Reviewed-by: Erik Skultety <eskultet@redhat.com> --- src/util/virnetdevveth.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/src/util/virnetdevveth.c b/src/util/virnetdevveth.c index 8c1a7f3..d2232ac 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,21 @@ int virNetDevVethCreate(char** veth1, char** veth2) */ int virNetDevVethDelete(const char *veth) { - virCommandPtr cmd = virCommandNewArgList("ip", "link", "del", veth, NULL); int status; - int ret = -1; + VIR_AUTOPTR(virCommand) cmd = virCommandNewArgList("ip", "link", "del", veth, NULL); 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
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 | 75 +++++++++++++++++++++--------------------------------- 1 file changed, 29 insertions(+), 46 deletions(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 784db0a..6c6be1c 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,25 +564,24 @@ virNumaGetHugePageInfo(int node, unsigned long long *page_avail, unsigned long long *page_free) { - int ret = -1; - char *path = NULL; - char *buf = NULL; char *end; + VIR_AUTOFREE(char *) buf = NULL; + VIR_AUTOFREE(char *) path = NULL; if (page_avail) { 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); @@ -593,25 +590,21 @@ virNumaGetHugePageInfo(int node, if (page_free) { 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 6c6be1c..c1457be 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -57,8 +57,8 @@ char * virNumaGetAutoPlacementAdvice(unsigned short vcpus, unsigned long long balloon) { - virCommandPtr cmd = NULL; char *output = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; cmd = virCommandNewArgList(NUMAD, "-w", NULL); virCommandAddArgFormat(cmd, "%d:%llu", vcpus, @@ -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> Reviewed-by: Erik Skultety <eskultet@redhat.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> Reviewed-by: Erik Skultety <eskultet@redhat.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..23f7703 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; char *tmp = NULL; unsigned int attr_type = 0; + VIR_AUTOFREE(char *) buf = NULL; 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> Reviewed-by: Erik Skultety <eskultet@redhat.com> --- src/util/virpidfile.c | 186 ++++++++++++++++---------------------------------- 1 file changed, 60 insertions(+), 126 deletions(-) diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c index 1a85d43..999bccb 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,17 @@ 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,20 +200,20 @@ int virPidFileReadPathIfAlive(const char *path, { int ret; bool isLink; - char *procPath = NULL; - char *procLink = NULL; size_t procLinkLen; - char *resolvedBinPath = NULL; - char *resolvedProcLink = NULL; const char deletedText[] = " (deleted)"; size_t deletedTextLen = strlen(deletedText); pid_t retPid; + VIR_AUTOFREE(char *) procPath = NULL; + VIR_AUTOFREE(char *) procLink = NULL; + VIR_AUTOFREE(char *) resolvedBinPath = NULL; + VIR_AUTOFREE(char *) resolvedProcLink = NULL; /* only set this at the very end on success */ *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 +233,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 +255,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 +300,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 +326,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 +426,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 +465,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 +483,7 @@ virPidFileConstructPath(bool privileged, const char *progname, char **pidfile) { - int ret = -1; - char *rundir = NULL; + VIR_AUTOFREE(char *) rundir = NULL; if (privileged) { /* @@ -556,29 +493,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 | 39 ++++++++++++++------------------------- 1 file changed, 14 insertions(+), 25 deletions(-) diff --git a/src/util/virprocess.c b/src/util/virprocess.c index f92b0dc..215e05d 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 @@ -238,6 +237,7 @@ virProcessWait(pid_t pid, int *exitstatus, bool raw) { int ret; int status; + VIR_AUTOFREE(char *) st = NULL; if (pid <= 0) { if (pid != -1) @@ -270,13 +270,10 @@ virProcessWait(pid_t pid, int *exitstatus, bool raw) return 0; error: - { - char *st = virProcessTranslateStatus(status); - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Child process (%lld) unexpected %s"), - (long long) pid, NULLSTR(st)); - VIR_FREE(st); - } + st = virProcessTranslateStatus(status); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Child process (%lld) unexpected %s"), + (long long) pid, NULLSTR(st)); return -1; } @@ -603,7 +600,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 +633,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 +644,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 +651,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 +667,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,12 +970,12 @@ virProcessSetMaxCoreSize(pid_t pid ATTRIBUTE_UNUSED, int virProcessGetStartTime(pid_t pid, unsigned long long *timestamp) { - char *filename = NULL; - char *buf = NULL; char *tmp; int ret = -1; int len; char **tokens = NULL; + VIR_AUTOFREE(char *) filename = NULL; + VIR_AUTOFREE(char *) buf = NULL; if (virAsprintf(&filename, "/proc/%llu/stat", (long long) pid) < 0) return -1; @@ -1032,8 +1025,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 +1071,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 +1100,6 @@ static int virProcessNamespaceHelper(int errfd, ignore_value(safewrite(errfd, err->message, len)); } } - VIR_FREE(path); VIR_FORCE_CLOSE(fd); return ret; } @@ -1145,7 +1135,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 +1149,6 @@ virProcessRunInMountNamespace(pid_t pid, NULLSTR(buf)); } } - VIR_FREE(buf); } cleanup: @@ -1226,7 +1215,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 +1240,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
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> Reviewed-by: Erik Skultety <eskultet@redhat.com> --- src/util/virprocess.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 215e05d..1fbbe0c 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -971,9 +971,8 @@ int virProcessGetStartTime(pid_t pid, unsigned long long *timestamp) { char *tmp; - int ret = -1; int len; - char **tokens = NULL; + VIR_AUTOPTR(virString) tokens = NULL; VIR_AUTOFREE(char *) filename = NULL; VIR_AUTOFREE(char *) buf = NULL; @@ -981,7 +980,7 @@ int virProcessGetStartTime(pid_t pid, return -1; if ((len = virFileReadAll(filename, 1024, &buf)) < 0) - goto cleanup; + return -1; /* start time is the token at index 19 after the '(process name)' entry - since only this * field can contain the ')' character, search backwards for this to avoid malicious @@ -992,14 +991,14 @@ int virProcessGetStartTime(pid_t pid, virReportError(VIR_ERR_INTERNAL_ERROR, _("Cannot find start time in %s"), filename); - goto cleanup; + return -1; } tmp += 2; /* skip ') ' */ if ((tmp - buf) >= len) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Cannot find start time in %s"), filename); - goto cleanup; + return -1; } tokens = virStringSplit(tmp, " ", 0); @@ -1008,7 +1007,7 @@ int virProcessGetStartTime(pid_t pid, virReportError(VIR_ERR_INTERNAL_ERROR, _("Cannot find start time in %s"), filename); - goto cleanup; + return -1; } if (virStrToLong_ull(tokens[19], @@ -1018,14 +1017,10 @@ int virProcessGetStartTime(pid_t pid, virReportError(VIR_ERR_INTERNAL_ERROR, _("Cannot parse start time %s in %s"), tokens[19], filename); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - virStringListFree(tokens); - return ret; + return 0; } #elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) int virProcessGetStartTime(pid_t pid, -- 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> Reviewed-by: Erik Skultety <eskultet@redhat.com> --- src/util/virqemu.c | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/src/util/virqemu.c b/src/util/virqemu.c index 30b8dc1..4089b8e 100644 --- a/src/util/virqemu.c +++ b/src/util/virqemu.c @@ -85,29 +85,22 @@ virQEMUBuildCommandLineJSONArrayNumbered(const char *key, virBufferPtr buf) { virJSONValuePtr member; - char *prefix = NULL; size_t i; - int ret = 0; for (i = 0; i < virJSONValueArraySize(array); i++) { + VIR_AUTOFREE(char *) prefix = NULL; member = virJSONValueArrayGet((virJSONValuePtr) array, i); if (virAsprintf(&prefix, "%s.%zu", key, i) < 0) - goto cleanup; + return 0; if (virQEMUBuildCommandLineJSONRecurse(prefix, member, buf, virQEMUBuildCommandLineJSONArrayNumbered, true) < 0) - goto cleanup; - - VIR_FREE(prefix); + return 0; } - ret = 0; - - cleanup: - VIR_FREE(prefix); - return ret; + return 0; } @@ -118,23 +111,18 @@ virQEMUBuildCommandLineJSONIterate(const char *key, void *opaque) { struct virQEMUCommandLineJSONIteratorData *data = opaque; - char *tmpkey = NULL; - int ret = -1; if (data->prefix) { + VIR_AUTOFREE(char *) tmpkey = NULL; if (virAsprintf(&tmpkey, "%s.%s", data->prefix, key) < 0) return -1; - ret = virQEMUBuildCommandLineJSONRecurse(tmpkey, value, data->buf, + return virQEMUBuildCommandLineJSONRecurse(tmpkey, value, data->buf, data->arrayFunc, false); - - VIR_FREE(tmpkey); } else { - ret = virQEMUBuildCommandLineJSONRecurse(key, value, data->buf, + return virQEMUBuildCommandLineJSONRecurse(key, value, data->buf, data->arrayFunc, false); } - - 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/virqemu.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/util/virqemu.c b/src/util/virqemu.c index 4089b8e..bb88f49 100644 --- a/src/util/virqemu.c +++ b/src/util/virqemu.c @@ -56,7 +56,7 @@ virQEMUBuildCommandLineJSONArrayBitmap(const char *key, { ssize_t pos = -1; ssize_t end; - virBitmapPtr bitmap = NULL; + VIR_AUTOPTR(virBitmap) bitmap = NULL; if (virJSONValueGetArrayAsBitmap(array, &bitmap) < 0) return -1; @@ -73,8 +73,6 @@ virQEMUBuildCommandLineJSONArrayBitmap(const char *key, } } - virBitmapFree(bitmap); - return 0; } @@ -282,6 +280,7 @@ virQEMUBuildDriveCommandlineFromJSON(virJSONValuePtr srcdef) cleanup: virBufferFreeAndReset(&buf); return ret; + } -- 1.8.3.1
On Mon, Aug 06, 2018 at 02:13:27AM +0530, Sukrit Bhatnagar wrote:
This third series of patches also modifies a few files in src/util to use VIR_AUTOFREE and VIR_AUTOPTR for automatic freeing of memory and get rid of some VIR_FREE macro invocations and *Free function calls.
This will clearly not apply on top the current master since most of the patches were already merged in some (updated) form, thus any other necessary changes/adjustments need to be covered by a follow-up series built on top of the current master. Also, there's something wrong with your system time, since git send-email overwrote the time stamp of the patches to 5th of August, it just looks weird on the list. Erik
participants (2)
-
Erik Skultety -
Sukrit Bhatnagar