On Mon, Mar 16, 2015 at 08:42:18AM -0400, John Ferlan wrote: > > > On 03/11/2015 07:09 AM, Daniel P. Berrange wrote: > > From: Nehal J Wani <nehaljw.kkd1@xxxxxxxxx> > > > > Define helper function virDomainInterfaceFree, which allows > > the upper layer application to free the domain interface object > > conveniently. > > > > The API is going to provide multiple methods by flags, e.g. > > * Query guest agent > > * Parse DHCP lease file > > > > include/libvirt/libvirt-domain.h > > * Define virDomainInterfaceAddresses, virDomainInterfaceFree > > * Define structs virDomainInterface, virDomainIPAddress > > > > src/driver-hypervisor.h: > > * Define domainInterfaceAddresses > > > > src/libvirt-domain.c: > > * Implement virDomainInterfaceAddresses > > * Implement virDomainInterfaceFree > > > > src/libvirt_public.syms: > > * Export the new symbols > > > > Signed-off-by: Nehal J Wani <nehaljw.kkd1@xxxxxxxxx> > > --- > > include/libvirt/libvirt-domain.h | 32 ++++++++++ > > src/driver-hypervisor.h | 6 ++ > > src/libvirt-domain.c | 123 +++++++++++++++++++++++++++++++++++++++ > > src/libvirt_public.syms | 2 + > > 4 files changed, 163 insertions(+) > > > > + * for (i = 0; i < ifaces_count; i++) > > + * virDomainInterfaceFree(ifaces[i]); > > + * free(ifaces); > > + * > > + * Returns the number of interfaces on success, -1 in case of error. > > + */ > > +int > > +virDomainInterfaceAddresses(virDomainPtr dom, > > + virDomainInterfacePtr **ifaces, > > + unsigned int source, > > + unsigned int flags) > > +{ > > + virConnectPtr conn; > > + > > + VIR_DOMAIN_DEBUG(dom, "ifaces=%p, source=%d, flags=%x", ifaces, source, flags); > > + > > + virResetLastError(); > > + > > + conn = dom->conn; > > If 'dom' is NULL, then there's a pointer deref (considering check below) > > > + > > + if (ifaces) > > + *ifaces = NULL; > > + virCheckDomainReturn(dom, -1); > > + virCheckNonNullArgGoto(ifaces, error); > > + virCheckReadOnlyGoto(conn->flags, error); > > I think I ran into this when I was adding the libvirt-perl code - since > this is a get interface - does it really matter if it's readonly? In this case it matters because we need to block access to the QEMU guest agent. > > Also, I'd move both conn = dom->conn and "if (ifaces) *ifaces = NULL;" > to here I'll just delete the 'conn' var - it is fairly pointless and just causes problems. Regards, 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list