[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 21/27] network: add public APIs for network port object



On Tue, Jan 08, 2019 at 05:16:32PM +0100, Michal Privoznik wrote:
> On 12/24/18 3:59 PM, Daniel P. Berrangé wrote:
> > Introduce a new virNetworPort object that will present an attachment to
> > a virtual network from a VM.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange redhat com>
> > ---
> >  include/libvirt/libvirt-network.h |  49 +++++
> >  include/libvirt/virterror.h       |   3 +
> >  src/datatypes.c                   |  60 +++++
> >  src/datatypes.h                   |  41 ++++
> >  src/driver-network.h              |  27 +++
> >  src/libvirt-network.c             | 351 ++++++++++++++++++++++++++++++
> >  src/libvirt_private.syms          |   2 +
> >  src/libvirt_public.syms           |  14 ++
> >  src/util/virerror.c               |   9 +
> >  9 files changed, 556 insertions(+)
> > 
> 
> 
> > +/**
> > + * virNetworkPortFree:
> > + * @port: a network port object
> > + *
> > + * Free the network port object.
> > + * The data structure is freed and should not be used thereafter.
> > + *
> > + * Returns 0 in case of success and -1 in case of failure.
> > + */
> > +int
> > +virNetworkPortFree(virNetworkPortPtr port)
> > +{
> > +    VIR_DEBUG("port=%p", port);
> > +
> > +    virResetLastError();
> > +
> > +    virCheckNetworkPortReturn(port, -1);
> > +
> > +    virObjectUnref(port);
> > +    return 0;
> 
> Don't we want to make this accept NULL? I know we don't do it for some
> other public free functions, but that was a mistake we can't fix (in
> fact I think we could because one can argue that relying on
> virDomainFree() returning -1 is a broken code anyway).

AFAIK, all our other public API object free functions forbid NULL,
so I'd rather keep consistency, rather than have this be an exception.

The rejection of NULL as an error is done by the virCheckNetworkPortReturn
method which does a virObjectIsClass call. This was always somewhat
sketchy as really the only way that could  fail is if you passed an
invalid pointer to libvirt APIs. IOW at best it is accessing unrelated
memory, at worst it could access freed memory.  That said while these
checks are never going to be 100% reliable, they have been useful in
debugging problems related to the language bindings in particular.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]