[libvirt] [libvirt-glib 11/37] Implement gvir_config_os_set_os_type

[Christophe Fergeau]

On Fri, Nov 11, 2011 at 03:58:40PM +0100, Marc-André Lureau wrote:
> On Thu, Nov 10, 2011 at 9:33 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> > ---
> >  libvirt-gconfig/libvirt-gconfig-os.c |   14 ++++++++++++++
> >  libvirt-gconfig/libvirt-gconfig-os.h |    6 ++++++
> >  libvirt-gconfig/libvirt-gconfig.sym  |    1 +
> >  3 files changed, 21 insertions(+), 0 deletions(-)
> >
> > diff --git a/libvirt-gconfig/libvirt-gconfig-os.c b/libvirt-gconfig/libvirt-gconfig-os.c
> > index b47e859..d4a04ae 100644
> > --- a/libvirt-gconfig/libvirt-gconfig-os.c
> > +++ b/libvirt-gconfig/libvirt-gconfig-os.c
> > @@ -27,6 +27,7 @@
> >  #include <libxml/tree.h>
> >
> >  #include "libvirt-gconfig/libvirt-gconfig.h"
> > +#include "libvirt-gconfig/libvirt-gconfig-helpers-private.h"
> >
> >  extern gboolean debugFlag;
> >
> > @@ -78,3 +79,16 @@ GVirConfigOs *gvir_config_os_new_from_xml(const gchar *xml, GError **error)
> >                                              NULL, xml, error);
> >     return GVIR_CONFIG_OS(object);
> >  }
> > +
> > +void gvir_config_os_set_os_type(GVirConfigOs *os, GVirConfigOsType type)
> > +{
> > +    xmlNodePtr node;
> > +    const char *type_str;
> > +
> > +    node = gvir_config_object_new_child(GVIR_CONFIG_OBJECT(os), "type");
> > +    if (node == NULL)
> > +        return;
> 
> I would use a couple of g_return_if_fail so that we get warnings of
> something going wrong instead of silently returning.
> 
> Same in previous code actually.

Ok, I'll make a pass over this once I have addressed the other comments.
> 
> > +    type_str = gvir_config_genum_get_nick(GVIR_TYPE_CONFIG_OS_TYPE, type);
> > +    if (type_str != NULL)
> > +        xmlNodeSetContent(node, (xmlChar*)type_str);
> > +}
> 
> Perhaps the warning for invalid enum could be put in
> gvir_config_genum_get_nick().

What do you mean exactly? Having a warning in genum_get_nick and drop the
!= NULL test? Or only to add the warning in genum_get_nick, and not add a
warning here? I can easily add a g_warn_if_reached() in genum_get_nick

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20111115/f181b9cf/attachment-0001.sig>