[libvirt] [PATCH 11/11] Delete udevFreeIfaceDef function in udev interface driver

[Daniel P. Berrange]

On Fri, May 03, 2013 at 10:44:24AM +0200, Ján Tomko wrote:
> On 05/02/2013 02:03 PM, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange" <berrange at redhat.com>
> > 
> > The udevFreeIfaceDef function in the udev interface driver
> > just duplicates code from virInterfaceDefFree. Delete it
> > and call the standard API instead.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> > ---
> >  src/interface/interface_backend_udev.c | 45 ++--------------------------------
> >  1 file changed, 2 insertions(+), 43 deletions(-)
> > 
> > diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c
> > index 79c57ee..0384ef5 100644
> > --- a/src/interface/interface_backend_udev.c
> > +++ b/src/interface/interface_backend_udev.c
> > @@ -542,44 +542,6 @@ udevBridgeScanDirFilter(const struct dirent *entry)
> >      return 1;
> >  }
> >  
> > -/**
> > - * Frees any memory allocated by udevGetIfaceDef()
> > - *
> > - * @param ifacedef - interface to free and cleanup
> > - */
> > -static void
> > -udevFreeIfaceDef(virInterfaceDef *ifacedef)
> > -{
> ...
> > -
> > -    if (ifacedef->type == VIR_INTERFACE_TYPE_VLAN) {
> > -        VIR_FREE(ifacedef->data.vlan.devname);
> > -    }
> 
> virInterfaceDefFree also frees data.vlan.tag, which would result in freeing an
> invalid pointer, because it's created by a pretty ugly way in udevGetIfaceDefVlan:
> 
>     vlan_parent_dev = strdup(name);
>     if (!vlan_parent_dev) {
>         virReportOOMError();
>         goto cleanup;
>     }
> 
>     /* Find the DEVICE.VID again */
>     vid = strrchr(vlan_parent_dev, '.');
>     if (!vid) {
>         virReportError(VIR_ERR_INTERNAL_ERROR,
>                 _("failed to find the VID for the VLAN device '%s'"),
>                 name);
>         goto cleanup;
>         }
> 
>     /* Replace the dot with a NULL so we can have the device and VID */
>     vid[0] = '\0';
>     vid++;
> 
>     /* Set the VLAN specifics */
>     ifacedef->data.vlan.tag = vid;
>     ifacedef->data.vlan.devname = vlan_parent_dev;

My god that code must die a horrible death. It is currently leaking
memory by doing this.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|