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

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.

> > > @@ -1184,9 +1183,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 +1193,6 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname, > > ignore_value(virNetDevSetNetConfig(linkdev, -1, > adminMAC, vlan, MAC, !!vlan)); > - VIR_FREE(MAC); > - VIR_FREE(adminMAC); > - virNetDevVlanFree(vlan); > } To ^this hunk: Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; > } > > -- > 1.8.3.1 > > -- > libvir-list mailing list > libvir-list(a)redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list