On Thu, Aug 02, 2018 at 11:03:16PM +0530, Sukrit Bhatnagar wrote: > On Thu, 2 Aug 2018 at 19:33, Erik Skultety <eskultet@xxxxxxxxxx> wrote: > > > > 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@xxxxxxxxx> > > > --- > > > 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. > > We would need to pass the nlmsg_free function in the cleanup attribute somehow. > So, shall we not use the VIR_AUTO macros and declare it with cleanup attribute > explicitly, or create a new type and a new Free wrapper for it to use > with out macros? Why do you need a new Free wrapper? Naturally, you'd need a "single-token" type name, something like virNetlinkNlMsg which you'd put into src/util/virnetlink.h along with the VIR_DEFINE_AUTOPTR_FUNC(virNetlinkNlMsg, nlmsg_free) and there you go, you can use VIR_AUTOPTR with it. Erik -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list