[libvirt] [PATCHv3 1/4] net-dhcp-leases: Implement the public APIs

[Daniel P. Berrange]

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 at gmail.com> wrote:
> > On Mon, Sep 16, 2013 at 3:22 PM, Daniel P. Berrange <berrange at 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

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 :|