On Mon, Sep 16, 2013 at 8:55 PM, Daniel P. Berrange <berrange@xxxxxxxxxx> 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@xxxxxxxxx> wrote:
> > > On Mon, Sep 16, 2013 at 3:22 PM, Daniel P. Berrange <berrange@xxxxxxxxxx> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list