[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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 redhat com> 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 gmail com>
> > ---
> >  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?

>
> >
> >
> > @@ -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 redhat com>
>
>
> >      }
> >
> > --
> > 1.8.3.1
> >
> > --
> > libvir-list mailing list
> > libvir-list redhat com
> > https://www.redhat.com/mailman/listinfo/libvir-list


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]