On Mon, Jul 20, 2009 at 01:42:04PM -0400, Laine Stump wrote: > MAC address of a particular interface may change over time, and the > reduced virInterface object (which contains just name and mac) needs > to reflect these changes. > --- > src/datatypes.c | 24 ++++++++++++++++-------- > 1 files changed, 16 insertions(+), 8 deletions(-) > > diff --git a/src/datatypes.c b/src/datatypes.c > index a8bffd2..a0d027c 100644 > --- a/src/datatypes.c > +++ b/src/datatypes.c > @@ -516,9 +516,10 @@ virUnrefNetwork(virNetworkPtr network) { > * @mac: pointer to the mac > * > * Lookup if the interface is already registered for that connection, > - * if yes return a new pointer to it, if no allocate a new structure, > - * and register it in the table. In any case a corresponding call to > - * virUnrefInterface() is needed to not leak data. > + * if yes return a new pointer to it (possibly updating the MAC > + * address), if no allocate a new structure, and register it in the > + * table. In any case a corresponding call to virUnrefInterface() is > + * needed to not leak data. > * > * Returns a pointer to the interface, or NULL in case of failure > */ > @@ -532,11 +533,19 @@ virGetInterface(virConnectPtr conn, const char *name, const char *mac) { > } > virMutexLock(&conn->lock); > > - /* TODO search by MAC first as they are better differentiators */ > - > ret = (virInterfacePtr) virHashLookup(conn->interfaces, name); > - /* TODO check the MAC */ > - if (ret == NULL) { > + > + if (ret != NULL) { > + /* update MAC address if necessary */ > + if ((ret->mac == NULL) || STRNEQ(ret->mac, mac)) { > + VIR_FREE(ret->mac); > + ret->mac = strdup(mac); > + if (ret->mac == NULL) { > + virReportOOMError(conn); > + goto error; > + } > + } > + } else { There's a small edge case there. the 'ret' object you have there is a cached one, whose handled is already in use by other callers on libvirt public API. So although you are reported an OOM to this caller, other users of this cached object have a dangerous instance witha NULL 'mac' field. Easy solution, don't VIR_FREE the existing mac until you have successfully strdup'd the new one Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list