[libvirt] [PATCH v2] Delete udevFreeIfaceDef function in udev interface driver

From: "Daniel P. Berrange" <berrange@redhat.com> The udevFreeIfaceDef function in the udev interface driver just duplicates code from virInterfaceDefFree. Delete it and call the standard API instead. Fix the udevGetIfaceDefVlan method so that it doesn't store pointers to the middle of a malloc'd memory area. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/interface/interface_backend_udev.c | 80 +++++++--------------------------- 1 file changed, 16 insertions(+), 64 deletions(-) diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c index 7daad16..9b708fa 100644 --- a/src/interface/interface_backend_udev.c +++ b/src/interface/interface_backend_udev.c @@ -543,44 +543,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) -{ - int i; - - if (!ifacedef) - return; - - if (ifacedef->type == VIR_INTERFACE_TYPE_BOND) { - VIR_FREE(ifacedef->data.bond.target); - for (i = 0; i < ifacedef->data.bond.nbItf; i++) { - udevFreeIfaceDef(ifacedef->data.bond.itf[i]); - } - VIR_FREE(ifacedef->data.bond.itf); - } - - if (ifacedef->type == VIR_INTERFACE_TYPE_BRIDGE) { - VIR_FREE(ifacedef->data.bridge.delay); - for (i = 0; i < ifacedef->data.bridge.nbItf; i++) { - udevFreeIfaceDef(ifacedef->data.bridge.itf[i]); - } - VIR_FREE(ifacedef->data.bridge.itf); - } - - if (ifacedef->type == VIR_INTERFACE_TYPE_VLAN) { - VIR_FREE(ifacedef->data.vlan.devname); - } - - VIR_FREE(ifacedef->mac); - VIR_FREE(ifacedef->name); - - VIR_FREE(ifacedef); -} static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) @@ -935,36 +897,29 @@ udevGetIfaceDefVlan(struct udev *udev ATTRIBUTE_UNUSED, const char *name, virInterfaceDef *ifacedef) { - char *vid; - char *vlan_parent_dev = NULL; - - vlan_parent_dev = strdup(name); - if (!vlan_parent_dev) { - virReportOOMError(); - goto cleanup; - } + const char *vid; /* Find the DEVICE.VID again */ - vid = strrchr(vlan_parent_dev, '.'); + vid = strrchr(name, '.'); 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++; + _("failed to find the VID for the VLAN device '%s'"), + name); + return -1; + } /* Set the VLAN specifics */ - ifacedef->data.vlan.tag = vid; - ifacedef->data.vlan.devname = vlan_parent_dev; + if (!(ifacedef->data.vlan.tag = strdup(vid + 1))) + goto no_memory; + if (!(ifacedef->data.vlan.devname = strndup(name, + (vid - name)))) + goto no_memory; return 0; -cleanup: - VIR_FREE(vlan_parent_dev); +no_memory: + VIR_FREE(ifacedef->data.vlan.tag); + VIR_FREE(ifacedef->data.vlan.devname); return -1; } @@ -1081,7 +1036,7 @@ udevGetIfaceDef(struct udev *udev, const char *name) cleanup: udev_device_unref(dev); - udevFreeIfaceDef(ifacedef); + virInterfaceDefFree(ifacedef); return NULL; } @@ -1102,15 +1057,12 @@ udevInterfaceGetXMLDesc(virInterfacePtr ifinfo, */ ifacedef = udevGetIfaceDef(udev, ifinfo->name); - /* We've already printed by it happened */ if (!ifacedef) goto err; - /* Convert our interface to XML */ xmlstr = virInterfaceDefFormat(ifacedef); - /* Recursively free our interface structures and free the children too */ - udevFreeIfaceDef(ifacedef); + virInterfaceDefFree(ifacedef); err: /* decrement our udev ptr */ -- 1.8.1.4

On 05/08/2013 03:45 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The udevFreeIfaceDef function in the udev interface driver just duplicates code from virInterfaceDefFree. Delete it and call the standard API instead.
Fix the udevGetIfaceDefVlan method so that it doesn't store pointers to the middle of a malloc'd memory area.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> ---
/* Set the VLAN specifics */ - ifacedef->data.vlan.tag = vid; - ifacedef->data.vlan.devname = vlan_parent_dev; + if (!(ifacedef->data.vlan.tag = strdup(vid + 1))) + goto no_memory;
VIR_STRDUP exists in the tree now, if you'd like to start using it (instead of making Michal touch up yet another spot when he finally turns on the syntax-check rule at the end of his series).
+ if (!(ifacedef->data.vlan.devname = strndup(name, + (vid - name)))) + goto no_memory;
Same for VIR_STRNDUP.
return 0;
-cleanup: - VIR_FREE(vlan_parent_dev); +no_memory: + VIR_FREE(ifacedef->data.vlan.tag); + VIR_FREE(ifacedef->data.vlan.devname);
Also, by using VIR_STRDUP, I wouldn't have to call you on your missing virReportOOMError() :) ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Daniel P. Berrange
-
Eric Blake