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

On Mon, Dec 05, 2011 at 11:25:31AM +0000, Daniel P. Berrange wrote: > From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; > > 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

> 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