By making use of GNU C's cleanup attribute handled by the VIR_AUTOFREE macro for declaring scalar variables, majority of the VIR_FREE calls can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- 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

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(a)gmail.com&gt; --- 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_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virnetlink.c | 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_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virnetdevbridge.c | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index fe3697b..089da78 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -417,12 +417,11 @@ virNetDevBridgeCreate(const char *brname) { /* use a netlink RTM_NEWLINK message to create the bridge */ const char *type = "bridge"; - int rc = -1; struct nlmsgerr *err; struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC }; unsigned int recvbuflen; - struct nl_msg *nl_msg; struct nlattr *linkinfo; + VIR_AUTOPTR(virNlMsg) nl_msg = NULL; VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; nl_msg = nlmsg_alloc_simple(RTM_NEWLINK, @@ -444,7 +443,7 @@ virNetDevBridgeCreate(const char *brname) if (virNetlinkCommand(nl_msg, &resp, &recvbuflen, 0, 0, NETLINK_ROUTE, 0) < 0) { - goto cleanup; + return -1; } if (recvbuflen < NLMSG_LENGTH(0) || resp == NULL) @@ -462,15 +461,14 @@ virNetDevBridgeCreate(const char *brname) /* fallback to ioctl if netlink doesn't support creating * bridges */ - rc = virNetDevBridgeCreateWithIoctl(brname); - goto cleanup; + return virNetDevBridgeCreateWithIoctl(brname); } # endif virReportSystemError(-err->error, _("error creating bridge interface %s"), brname); - goto cleanup; + return -1; } break; @@ -480,19 +478,16 @@ virNetDevBridgeCreate(const char *brname) goto malformed_resp; } - rc = 0; - cleanup: - nlmsg_free(nl_msg); - return rc; + return 0; malformed_resp: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed netlink response message")); - goto cleanup; + return -1; buffer_too_small: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("allocated netlink buffer is too small")); - goto cleanup; + return -1; } @@ -1055,11 +1050,10 @@ static int virNetDevBridgeFDBAddDel(const virMacAddr *mac, const char *ifname, unsigned int flags, bool isAdd) { - int ret = -1; struct nlmsgerr *err; unsigned int recvbuflen; - struct nl_msg *nl_msg; struct ndmsg ndm = { .ndm_family = PF_BRIDGE, .ndm_state = NUD_NOARP }; + VIR_AUTOPTR(virNlMsg) nl_msg = NULL; VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; if (virNetDevGetIndex(ifname, &ndm.ndm_ifindex) < 0) @@ -1103,7 +1097,7 @@ virNetDevBridgeFDBAddDel(const virMacAddr *mac, const char *ifname, if (virNetlinkCommand(nl_msg, &resp, &recvbuflen, 0, 0, NETLINK_ROUTE, 0) < 0) { - goto cleanup; + return -1; } if (recvbuflen < NLMSG_LENGTH(0) || resp == NULL) goto malformed_resp; @@ -1116,7 +1110,7 @@ virNetDevBridgeFDBAddDel(const virMacAddr *mac, const char *ifname, if (err->error) { virReportSystemError(-err->error, _("error adding fdb entry for %s"), ifname); - goto cleanup; + return -1; } break; case NLMSG_DONE: @@ -1126,20 +1120,17 @@ virNetDevBridgeFDBAddDel(const virMacAddr *mac, const char *ifname, goto malformed_resp; } - ret = 0; - cleanup: - nlmsg_free(nl_msg); - return ret; + return 0; malformed_resp: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed netlink response message")); - goto cleanup; + return -1; buffer_too_small: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("allocated netlink buffer is too small")); - goto cleanup; + return -1; } -- 1.8.3.1

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(a)gmail.com&gt; --- 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_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virnetdev.c | 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

By making use of GNU C's cleanup attribute handled by the VIR_AUTOFREE macro for declaring scalar variables, majority of the VIR_FREE calls can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- 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

Define a free function for virNetDevIPAddrPtr type for consistency and extensibility. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- 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

By making use of GNU C's cleanup attribute handled by the VIR_AUTOFREE macro for declaring scalar variables, majority of the VIR_FREE calls can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/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

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(a)gmail.com&gt; --- 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_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- 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_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virnetdevopenvswitch.c | 80 ++++++++++++++--------------------------- 1 file changed, 27 insertions(+), 53 deletions(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 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(a)gmail.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- 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_AUTOFREE macro for declaring scalar variables, majority of the VIR_FREE calls can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/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

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(a)gmail.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- 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(a)gmail.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- 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_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- 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_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virqemu.c | 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