[libvirt] [PATCH] Introduce gvir_set_error (and friends) & convert all code

[Daniel P. Berrange]

On Mon, Dec 05, 2011 at 12:59:36PM +0100, Christophe Fergeau wrote:
> On Mon, Dec 05, 2011 at 11:25:31AM +0000, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange" <berrange at redhat.com>
> > 
> > The pattern
> > 
> >     GError **error;
> >     ....
> >     *err = gvir_error_new(...)
> > 
> > is dangerous because 'err' could be NULL, and it is tedious
> > to expect everyone to check. Introduce a new set of APIs
> > for setting errors
> > 
> >     gvir_set_error(err, ...)
> > 
> > and convert all callers to this new pattern
> > ---
> >  libvirt-gconfig/libvirt-gconfig-helpers-private.h  |    7 +-
> >  libvirt-gconfig/libvirt-gconfig-helpers.c          |   48 +++++++---
> >  libvirt-gconfig/libvirt-gconfig-object.c           |   15 ++-
> >  libvirt-glib/libvirt-glib-error.c                  |   91 +++++++++++++++++++
> >  libvirt-glib/libvirt-glib-error.h                  |   18 ++++
> >  libvirt-glib/libvirt-glib.sym                      |    3 +
> >  libvirt-gobject/libvirt-gobject-connection.c       |   88 ++++++++----------
> >  libvirt-gobject/libvirt-gobject-domain-disk.c      |    7 +-
> >  libvirt-gobject/libvirt-gobject-domain-interface.c |    7 +-
> >  libvirt-gobject/libvirt-gobject-domain-snapshot.c  |    6 +-
> >  libvirt-gobject/libvirt-gobject-domain.c           |   95 +++++++++-----------
> >  libvirt-gobject/libvirt-gobject-interface.c        |    7 +-
> >  libvirt-gobject/libvirt-gobject-network-filter.c   |    7 +-
> >  libvirt-gobject/libvirt-gobject-network.c          |    7 +-
> >  libvirt-gobject/libvirt-gobject-node-device.c      |    7 +-
> >  libvirt-gobject/libvirt-gobject-secret.c           |    7 +-
> >  libvirt-gobject/libvirt-gobject-storage-pool.c     |   47 +++++------
> >  libvirt-gobject/libvirt-gobject-storage-vol.c      |    7 +-
> >  libvirt-gobject/libvirt-gobject-stream.c           |   14 ++--
> >  19 files changed, 295 insertions(+), 193 deletions(-)
> > 
> > diff --git a/libvirt-gconfig/libvirt-gconfig-helpers-private.h b/libvirt-gconfig/libvirt-gconfig-helpers-private.h
> > index 6277cbd..087085e 100644
> > --- a/libvirt-gconfig/libvirt-gconfig-helpers-private.h
> > +++ b/libvirt-gconfig/libvirt-gconfig-helpers-private.h
> > @@ -30,8 +30,11 @@
> >  
> >  G_BEGIN_DECLS
> >  
> > -GError *gvir_xml_error_new(GQuark domain, gint code,
> > -                           const gchar *format, ...);
> > +GError *gvir_config_xml_error_new(GQuark domain, gint code,
> > +                                  const gchar *format, ...);
> 
> If we rename these, I'd go with gvir_config_error_new

Ok, changed that.

> >  xmlNodePtr
> >  gvir_config_xml_parse(const char *xml, const char *root_node, GError **err)
> >  {
> > @@ -94,17 +115,18 @@ gvir_config_xml_parse(const char *xml, const char *root_node, GError **err)
> >  
> >      doc = xmlParseMemory(xml, strlen(xml));
> >      if (!doc) {
> > -        *err = gvir_xml_error_new(GVIR_CONFIG_OBJECT_ERROR,
> > +        gvir_config_set_xml_error(err, GVIR_CONFIG_OBJECT_ERROR,
> >                                    0,
> >                                    "%s",
> >                                    "Unable to parse configuration");
> 
> This "%s", msg pattern occurs several time, it will be worth adding a
> _set_error_literal variant some day

Good point, I've introduced that API now, and also a 'va_list' variant
for completeness.


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