Re: [libvirt] [PATCH v1 16/32] util: netdevmacvlan: use VIR_AUTOPTR for aggregate types

On Fri, 3 Aug 2018 at 18:49, Erik Skultety <eskultet(a)redhat.com&gt; wrote: > > On Sat, Jul 28, 2018 at 11:31:31PM +0530, Sukrit Bhatnagar wrote: > > 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/virnetdevmacvlan.c | 28 ++++++++++++---------------- > > 1 file changed, 12 insertions(+), 16 deletions(-) > > > > diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c > > index a2ed65c..d01e5ef 100644 > > --- a/src/util/virnetdevmacvlan.c > > +++ b/src/util/virnetdevmacvlan.c > > @@ -898,19 +898,20 @@ virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname, > > virNetDevVPortProfilePtr virtPortProfile, > > virNetDevVPortProfileOp vmOp) > > { > > - virNetlinkCallbackDataPtr calld = NULL; > > + VIR_AUTOPTR(virNetlinkCallbackData) calld = NULL; > > + virNetlinkCallbackDataPtr temp ATTRIBUTE_UNUSED = NULL; > > > > if (virtPortProfile && virNetlinkEventServiceIsRunning(NETLINK_ROUTE)) { > > if (VIR_ALLOC(calld) < 0) > > - goto error; > > + return -1; > > if (VIR_STRDUP(calld->cr_ifname, ifname) < 0) > > - goto error; > > + return -1; > > if (VIR_ALLOC(calld->virtPortProfile) < 0) > > - goto error; > > + return -1; > > memcpy(calld->virtPortProfile, virtPortProfile, sizeof(*virtPortProfile)); > > virMacAddrSet(&calld->macaddress, macaddress); > > if (VIR_STRDUP(calld->linkdev, linkdev) < 0) > > - goto error; > > + return -1; > > memcpy(calld->vmuuid, vmuuid, sizeof(calld->vmuuid)); > > > > calld->vmOp = vmOp; > > @@ -918,14 +919,12 @@ virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname, > > if (virNetlinkEventAddClient(virNetDevMacVLanVPortProfileCallback, > > virNetDevMacVLanVPortProfileDestroyCallback, > > calld, macaddress, NETLINK_ROUTE) < 0) > > - goto error; > > + return -1; > > } > > > > + VIR_STEAL_PTR(temp, calld); > > + > > return 0; > > - > > - error: > > - virNetlinkCallbackDataFree(calld); > > - return -1; > > } > > ^This is stretching the VIR_AUTO* concept too much, there's no apparent gain > here and should be left as is. But by doing this, are getting rid of goto jumps and error section. That was one of the goals, right?