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

Re: [libvirt] [libvirt-glib 01/37] Add gvir_config_object_new_child helper



Hey,

On Fri, Nov 11, 2011 at 03:28:34PM +0100, Marc-André Lureau wrote:
> The functions declaration should be put in -private.h . Marking
> definition G_GNUC_INTERNAL would also make it more obvious.
> 
> It would also less confuse gir-scanner which doesn't know about libxml types.

At this point, I'm undecided whether these functions should be private or
not. Having some way to directly manipulate the XML data can be useful when
you want to generate some config data not yet handled by libvirt-gconfig,
but on the other hand, it makes it really easy to shoot yourself in the
foot.
I'm fine with marking them private for now.

Christophe

> 
> On Thu, Nov 10, 2011 at 9:33 PM, Christophe Fergeau <cfergeau redhat com> wrote:
> > This allows us to factor the code to add an XML node to a config
> > object.
> > ---
> >  libvirt-gconfig/libvirt-gconfig-object.c |   72 ++++++++++++++++++------------
> >  libvirt-gconfig/libvirt-gconfig-object.h |    4 ++
> >  libvirt-gconfig/libvirt-gconfig.h        |    2 +-
> >  3 files changed, 49 insertions(+), 29 deletions(-)
> >
> > diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c
> > index fbdbedd..598ac0c 100644
> > --- a/libvirt-gconfig/libvirt-gconfig-object.c
> > +++ b/libvirt-gconfig/libvirt-gconfig-object.c
> > @@ -294,35 +294,62 @@ char *gvir_config_object_get_node_content(GVirConfigObject *object,
> >     return gvir_config_xml_get_child_element_content_glib(node, node_name);
> >  }
> >
> > -/* FIXME: if there are multiple nodes with the same name, this function
> > - * won't behave as expected. Should we get rid of the duplicated node names
> > - * here?
> > - */
> > -void gvir_config_object_set_node_content(GVirConfigObject *object,
> > -                                         const char *node_name,
> > -                                         const char *value)
> > +void
> > +gvir_config_object_set_child(GVirConfigObject *object, xmlNodePtr child)
> >  {
> >     xmlNodePtr parent_node;
> >     xmlNodePtr old_node;
> > -    xmlNodePtr new_node;
> > -    xmlChar *encoded_name;
> >
> >     parent_node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(object));
> > -    encoded_name = xmlEncodeEntitiesReentrant(parent_node->doc,
> > -                                              (xmlChar *)value);
> > -    new_node = xmlNewDocNode(parent_node->doc, NULL,
> > -                             (xmlChar *)node_name, encoded_name);
> > -    xmlFree(encoded_name);
> > +    g_return_if_fail (parent_node != NULL);
> >
> > -    old_node = gvir_config_xml_get_element(parent_node, node_name, NULL);
> > +    old_node = gvir_config_xml_get_element(parent_node, child->name, NULL);
> > +    /* FIXME: should we make sure there are no multiple occurrences
> > +     * of this node?
> > +     */
> >     if (old_node) {
> > -        old_node = xmlReplaceNode(old_node, new_node);
> > +        old_node = xmlReplaceNode(old_node, child);
> >         xmlFreeNode(old_node);
> >     } else {
> > -        xmlAddChild(parent_node, new_node);
> > +        xmlAddChild(parent_node, child);
> >     }
> >  }
> >
> > +xmlNodePtr
> > +gvir_config_object_new_child(GVirConfigObject *object, const char *child_name)
> > +{
> > +    xmlNodePtr new_node;
> > +
> > +    new_node = xmlNewDocNode(NULL, NULL, (xmlChar *)child_name, NULL);
> > +    gvir_config_object_set_child(object, new_node);
> > +    return new_node;
> > +}
> > +
> > +void gvir_config_object_set_node_content(GVirConfigObject *object,
> > +                                         const char *node_name,
> > +                                         const char *value)
> > +{
> > +    xmlNodePtr node;
> > +    xmlChar *encoded_data;
> > +
> > +    node = gvir_config_object_new_child(object, node_name);
> > +    g_return_if_fail(node != NULL);
> > +    encoded_data = xmlEncodeEntitiesReentrant(node->doc,
> > +                                              (xmlChar *)value);
> > +    xmlNodeSetContent(node, encoded_data);
> > +    xmlFree(encoded_data);
> > +}
> > +
> > +void gvir_config_object_set_node_content_uint64(GVirConfigObject *object,
> > +                                                const char *node_name,
> > +                                                guint64 value)
> > +{
> > +    char *str;
> > +    str = g_strdup_printf("%"G_GUINT64_FORMAT, value);
> > +    gvir_config_object_set_node_content(object, node_name, str);
> > +    g_free(str);
> > +}
> > +
> >  /* FIXME: how to notify of errors/node not found? */
> >  guint64 gvir_config_object_get_node_content_uint64(GVirConfigObject *object,
> >                                                    const char *node_name)
> > @@ -345,17 +372,6 @@ guint64 gvir_config_object_get_node_content_uint64(GVirConfigObject *object,
> >     return value;
> >  }
> >
> > -
> > -void gvir_config_object_set_node_content_uint64(GVirConfigObject *object,
> > -                                                const char *node_name,
> > -                                                guint64 value)
> > -{
> > -    char *str;
> > -    str = g_strdup_printf("%"G_GUINT64_FORMAT, value);
> > -    gvir_config_object_set_node_content(object, node_name, str);
> > -    g_free(str);
> > -}
> > -
> >  GVirConfigObject *gvir_config_object_new_from_xml(GType type,
> >                                                   const char *root_name,
> >                                                   const char *schema,
> > diff --git a/libvirt-gconfig/libvirt-gconfig-object.h b/libvirt-gconfig/libvirt-gconfig-object.h
> > index 52e4525..8e67b92 100644
> > --- a/libvirt-gconfig/libvirt-gconfig-object.h
> > +++ b/libvirt-gconfig/libvirt-gconfig-object.h
> > @@ -78,6 +78,10 @@ char *gvir_config_object_get_node_content(GVirConfigObject *object,
> >                                           const char *node_name);
> >  guint64 gvir_config_object_get_node_content_uint64(GVirConfigObject *object,
> >                                                    const char *node_name);
> > +xmlNodePtr gvir_config_object_new_child(GVirConfigObject *object,
> > +                                        const char *child_name);
> > +void gvir_config_object_set_child(GVirConfigObject *object,
> > +                                  xmlNodePtr child);
> >  void gvir_config_object_set_node_content(GVirConfigObject *object,
> >                                          const char *node_name,
> >                                          const char *value);
> > diff --git a/libvirt-gconfig/libvirt-gconfig.h b/libvirt-gconfig/libvirt-gconfig.h
> > index fdc78a4..4e23f0d 100644
> > --- a/libvirt-gconfig/libvirt-gconfig.h
> > +++ b/libvirt-gconfig/libvirt-gconfig.h
> > @@ -26,11 +26,11 @@
> >  #include <glib-object.h>
> >  #include <libxml/tree.h>
> >
> > -#include <libvirt-gconfig/libvirt-gconfig-helpers.h>
> >  #include <libvirt-gconfig/libvirt-gconfig-object.h>
> >  #include <libvirt-gconfig/libvirt-gconfig-capabilities.h>
> >  #include <libvirt-gconfig/libvirt-gconfig-domain.h>
> >  #include <libvirt-gconfig/libvirt-gconfig-domain-snapshot.h>
> > +#include <libvirt-gconfig/libvirt-gconfig-helpers.h>
> >  #include <libvirt-gconfig/libvirt-gconfig-interface.h>
> >  #include <libvirt-gconfig/libvirt-gconfig-network.h>
> >  #include <libvirt-gconfig/libvirt-gconfig-node-device.h>
> > --
> > 1.7.7
> >
> > --
> > libvir-list mailing list
> > libvir-list redhat com
> > https://www.redhat.com/mailman/listinfo/libvir-list
> >
> 
> 
> 
> -- 
> Marc-André Lureau

Attachment: pgpzXnWJRqnDy.pgp
Description: PGP signature


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