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

Re: [libvirt] [libvirt-glib PATCHv2] Fix *_new_from_xml



On Wed, Jul 18, 2012 at 03:39:39PM +0200, Christophe Fergeau wrote:
> For objects with a subtype 'type' attribute, when the _new_from_xml
> function was called, the 'type' attribute was forcefully set to the
> right value rather than checking that the passed-in value matches
> the type of the subclass we are trying to instantiate. This commit
> changes this, and returns NULL when the value of the 'type' attribute
> of the passed-in XML document does not match the expected type.
> ---
>  libvirt-gconfig/libvirt-gconfig-domain-address-pci.c             |    5 ++++-
>  libvirt-gconfig/libvirt-gconfig-domain-address-usb.c             |    5 ++++-
>  libvirt-gconfig/libvirt-gconfig-domain-chardev-source-pty.c      |    5 ++++-
>  libvirt-gconfig/libvirt-gconfig-domain-chardev-source-spicevmc.c |    5 ++++-
>  libvirt-gconfig/libvirt-gconfig-domain-controller-usb.c          |    5 ++++-
>  libvirt-gconfig/libvirt-gconfig-domain-graphics-sdl.c            |    5 +++--
>  libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.c          |    5 +++--
>  libvirt-gconfig/libvirt-gconfig-domain-graphics-vnc.c            |    5 +++--
>  libvirt-gconfig/libvirt-gconfig-domain-interface-bridge.c        |    5 +++--
>  libvirt-gconfig/libvirt-gconfig-domain-interface-network.c       |    5 +++--
>  libvirt-gconfig/libvirt-gconfig-domain-interface-user.c          |    5 +++--
>  libvirt-gconfig/libvirt-gconfig-domain-timer-pit.c               |    5 ++++-
>  libvirt-gconfig/libvirt-gconfig-domain-timer-rtc.c               |    5 ++++-
>  13 files changed, 46 insertions(+), 19 deletions(-)

I've wondered about this before too and never got around to fixing it.
I'm concerned about whether all callers will check for possibility of
returning NULL.  Most glib apps assume _new() calls can't return
NULL, since we abort() on OOM.

Should we instead be using g_return_if_fail() so you get a dianostic,
on the basis that this is programmer mistake ?

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


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