On Fri, Aug 03, 2018 at 10:56:40PM +0530, Sukrit Bhatnagar wrote:
> On Fri, 3 Aug 2018 at 18:49, Erik Skultety <eskultet@xxxxxxxxxx> 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@xxxxxxxxx>
> > > ---
> > > 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?
Yes, it was the goal, but we can't do it blindly everywhere just for the sake
of replacement, for this specific case, we didn't really gain anything, because
we traded one error label containing a single *Free call for one extra
virNetlinkCallbackDataPtr variable just to be able to do a VIR_STEAL_PTR
(which again, should be mainly used for caller-provided pointers),
virNetlinkCallbackDataPtr typedefs and VIR_DEFINE_AUTOPTR_FUNC for a single use
case.
Erik
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
