On Sat, Jul 28, 2018 at 11:31:18PM +0530, Sukrit Bhatnagar wrote:
By making use of GNU C's cleanup attribute handled by the VIR_AUTOFREE macro for declaring scalar variables, majority of the VIR_FREE calls can be dropped, which in turn leads to getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- src/util/virnetdevbridge.c | 45 +++++++++++++++------------------------------ 1 file changed, 15 insertions(+), 30 deletions(-)
diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index e46ac35..bf30d7c 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -126,8 +126,7 @@ static int virNetDevBridgeSet(const char *brname, int fd, /* control socket */ struct ifreq *ifr) /* pre-filled bridge name */ { - char *path = NULL; - int ret = -1; + VIR_AUTOFREE(char *) path = NULL;
if (virAsprintf(&path, SYSFS_NET_DIR "%s/bridge/%s", brname, paramname) < 0) return -1; @@ -138,7 +137,7 @@ static int virNetDevBridgeSet(const char *brname, if (virFileWriteStr(path, valuestr, 0) < 0) { virReportSystemError(errno, _("Unable to set bridge %s %s"), brname, paramname); - goto cleanup; + return -1; } } else { unsigned long paramid; @@ -149,21 +148,18 @@ static int virNetDevBridgeSet(const char *brname, } else { virReportSystemError(EINVAL, _("Unable to set bridge %s %s"), brname, paramname); - goto cleanup; + return -1; } unsigned long args[] = { paramid, value, 0, 0 }; ifr->ifr_data = (char*)&args; if (ioctl(fd, SIOCDEVPRIVATE, ifr) < 0) { virReportSystemError(errno, _("Unable to set bridge %s %s"), brname, paramname); - goto cleanup; + return -1; } }
- ret = 0; - cleanup: - VIR_FREE(path); - return ret; + return 0; }
@@ -171,7 +167,7 @@ static int virNetDevBridgeGet(const char *brname, const char *paramname, /* sysfs param name */ unsigned long *value) /* current value */ { - char *path = NULL; + VIR_AUTOFREE(char *) path = NULL;
Referring to my response to previous patch, I'll move ^this at the end of the "declare" block (there are a few identical spots across the patch). ...
malformed_resp: @@ -1069,7 +1055,7 @@ virNetDevBridgeFDBAddDel(const virMacAddr *mac, const char *ifname, unsigned int flags, bool isAdd) { int ret = -1; - struct nlmsghdr *resp = NULL; + VIR_AUTOFREE(struct nlmsghdr *) resp = NULL; struct nlmsgerr *err; unsigned int recvbuflen; struct nl_msg *nl_msg;
So, I believe ^this external type can easily be turned into an autoclean variant.
@@ -1142,7 +1128,6 @@ virNetDevBridgeFDBAddDel(const virMacAddr *mac, const char *ifname, ret = 0; cleanup: nlmsg_free(nl_msg);
So that ^this would be done automatically. Otherwise it the patch looks fine. Erik