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@xxxxxxxxxx> 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
Attachment:
pgpbuV82fUgIP.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list