Re: [libvirt-glib 01/23] Add helpers in libvirt-gconfig-helpers.[ch]

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

 



On Mon, Nov 07, 2011 at 02:52:24PM +0100, Christophe Fergeau wrote:
> Hi,
> 
> On Tue, Oct 18, 2011 at 12:06:50PM +0100, Daniel P. Berrange wrote:
> > On Fri, Oct 07, 2011 at 11:40:46AM +0200, Christophe Fergeau wrote:
> > > These helper functions will be useful later on.
> > > ---
> > >  examples/Makefile.am                      |    1 +
> > >  libvirt-gconfig/Makefile.am               |    3 +
> > >  libvirt-gconfig/libvirt-gconfig-helpers.c |  179 +++++++++++++++++++++++++++++
> > >  libvirt-gconfig/libvirt-gconfig-helpers.h |   48 ++++++++
> > >  libvirt-gconfig/libvirt-gconfig-object.c  |   52 ---------
> > >  libvirt-gconfig/libvirt-gconfig.h         |    2 +
> > >  libvirt-gconfig/libvirt-gconfig.sym       |    1 +
> > >  libvirt-gobject/Makefile.am               |    2 +
> > >  8 files changed, 236 insertions(+), 52 deletions(-)
> > >  create mode 100644 libvirt-gconfig/libvirt-gconfig-helpers.c
> > >  create mode 100644 libvirt-gconfig/libvirt-gconfig-helpers.h
> > > 
> > > diff --git a/libvirt-gconfig/libvirt-gconfig-helpers.h b/libvirt-gconfig/libvirt-gconfig-helpers.h
> > > new file mode 100644
> > > index 0000000..1bdf2d0
> > > --- /dev/null
> > > +++ b/libvirt-gconfig/libvirt-gconfig-helpers.h
> > > @@ -0,0 +1,48 @@
> > 
> > > +
> > > +GQuark gvir_config_object_error_quark(void);
> > > +#define GVIR_CONFIG_OBJECT_ERROR gvir_config_object_error_quark()
> > > +
> > > +GError *gvir_xml_error_new(GQuark domain, gint code,
> > > +                           const gchar *format, ...);
> > > +xmlNodePtr gvir_config_xml_parse(const char *xml,
> > > +                                 const char *root_node,
> > > +                                 GError **err);
> > > +xmlNode * gvir_config_xml_get_element (xmlNode *node, ...);
> > > +xmlChar * gvir_config_xml_get_child_element_content (xmlNode    *node,
> > > +                                                     const char *child_name);
> > > +char *gvir_config_xml_get_child_element_content_glib (xmlNode    *node,
> > > +                                                      const char *child_name);
> > 
> > 
> > > diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym
> > > index 2d09c37..24407c3 100644
> > > --- a/libvirt-gconfig/libvirt-gconfig.sym
> > > +++ b/libvirt-gconfig/libvirt-gconfig.sym
> > > @@ -22,6 +22,7 @@ LIBVIRT_GOBJECT_0.0.1 {
> > >  	gvir_config_node_device_new;
> > >  
> > >  	gvir_config_object_get_type;
> > > +	gvir_config_object_error_quark;
> > 
> > 
> > Don't we want to also export the rest of those new APIs in
> > gconfig-helpers.h ?
> > 
> > 
> > ACK, if you tell me why they're not required, or if you add them
> > before pushing the patch.
> 
> At this point, I'm not sure what kind of low-level API we want to make
> available to the library user (by low-level, I mean letting people directly
> poke at the xmlNode wrapped in a GVirConfigObject instance). That's why I
> chose not to export them for now, and to make them public if/once they are
> needed.
> If you think they should be exported, I'll make the change.

Personally I'd rather we didn't expose libxml2 data types in our public
API, since it feels like an admission of defeat in our object model.
I just mentioned the  .syms file, because I saw the symbols included
in the libvirt-gconfig/libvirt-gconfig-helpers.h header which presumably
will end up in /usr/include.

So if we want to avoid exposing libxml2 types in our API, we should
ensure the header file in question doesn't get installed in /usr/include
too.

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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]