Re: [libvirt] [PATCHv3 1/4] net-dhcp-leases: Implement the public APIs
On Mon, Sep 16, 2013 at 8:55 PM, Daniel P. Berrange <berrange(a)redhat.com> wrote:
On Mon, Sep 16, 2013 at 08:45:09PM +0530, Nehal J Wani wrote:
> PFA: Diff
>
> On Mon, Sep 16, 2013 at 8:44 PM, Nehal J Wani <nehaljw.kkd1(a)gmail.com> wrote:
> > On Mon, Sep 16, 2013 at 3:22 PM, Daniel P. Berrange <berrange(a)redhat.com>
wrote:
> >>
> >> On Mon, Sep 16, 2013 at 11:19:13AM +0530, Nehal J Wani wrote:
> >> > +int
> >> > +virNetworkGetDHCPLeasesForMAC(virNetworkPtr network,
> >> > + const char *mac,
> >> > + virNetworkDHCPLeasesPtr **leases,
> >> > + unsigned int flags)
> >> > +{
> >> > + virConnectPtr conn;
> >> > + virMacAddr addr;
> >> > +
> >> > + VIR_DEBUG("network=%p, mac=%s, leases=%p, flags=%x",
> >> > + network, mac, leases, flags);
> >> > +
> >> > + virResetLastError();
> >> > +
> >> > + virCheckNonNullArgGoto(network, error);
> >> > + virCheckNonNullArgGoto(mac, error);
> >> > +
> >> > + if (leases)
> >> > + *leases = NULL;
> >> > +
> >> > + if (!VIR_IS_CONNECTED_NETWORK(network)) {
> >> > + virLibNetworkError(VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> >> > + virDispatchError(NULL);
> >> > + return -1;
> >> > + }
> >> > +
> >> > + /* Validate the MAC address */
> >> > + if (mac && virMacAddrParse(mac, &addr) < 0) {
> >> > + virReportInvalidArg(mac,
> >> > + _("Given MAC Address doesn't
comply "
> >> > + "with the standard (IEEE 802)
format in %s"),
> >> > + __FUNCTION__);
> >>
> >> Don't pass __FUNCTION__ in this error message - that is already done
automatically
> >>
> >> > + goto error;
> >> > + }
> >>
> >> 'mac' should be a mandatory parameter here.
> >
> > Attached diff should fix it.
No, it makes it worse - you're now allowing a NULL mac parameter
to be passed to virMacAddrParse which will cause a segv. You need
to be checking for mac == NULL with virCheckNonNullArg
I think you missed this, I am already using virCheckNonNullArg :
+ virConnectPtr conn;
+ virMacAddr addr;
+
+ VIR_DEBUG("network=%p, mac=%s, leases=%p, flags=%x",
+ network, mac, leases, flags);
+
+ virResetLastError();
+
+ virCheckNonNullArgGoto(network, error);
+ virCheckNonNullArgGoto(mac, error);
+
+ if (leases)
+ *leases = NULL;
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 :|
--
Nehal J Wani
UG3, BTech CS+MS(CL)
IIIT-Hyderabad
http://commandlinewani.blogspot.com