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

[Nehal J Wani]

On Mon, Sep 16, 2013 at 8:55 PM, Daniel P. Berrange <berrange at 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 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
>

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