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

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

> +void gvir_config_set_xml_error(GError **err,
> +                               GQuark domain, gint code,
> +                               const gchar *format, ...);
>  xmlNodePtr gvir_config_xml_parse(const char *xml,
>                                   const char *root_node,
>                                   GError **err);
> diff --git a/libvirt-gconfig/libvirt-gconfig-helpers.c b/libvirt-gconfig/libvirt-gconfig-helpers.c
> index 7bba594..f4f82e9 100644
> --- a/libvirt-gconfig/libvirt-gconfig-helpers.c
> +++ b/libvirt-gconfig/libvirt-gconfig-helpers.c
> @@ -36,9 +36,9 @@ gvir_config_object_error_quark(void)
>      return g_quark_from_static_string("gvir-config-object");
>  }
>  
> -static GError *gvir_xml_error_new_literal(GQuark domain,
> -                                          gint code,
> -                                          const gchar *message)
> +static GError *gvir_config_xml_error_new_literal(GQuark domain,
> +                                                 gint code,
> +                                                 const gchar *message)
>  {
>      xmlErrorPtr xerr = xmlGetLastError();
>  
> @@ -59,10 +59,10 @@ static GError *gvir_xml_error_new_literal(GQuark domain,
>  }
>  
>  
> -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,
> +                                  ...)
>  {
>      GError *err;
>      va_list args;
> @@ -72,13 +72,34 @@ GError *gvir_xml_error_new(GQuark domain,
>      message = g_strdup_vprintf(format, args);
>      va_end(args);
>  
> -    err = gvir_xml_error_new_literal(domain, code, message);
> +    err = gvir_config_xml_error_new_literal(domain, code, message);
>  
>      g_free(message);
>  
>      return err;
>  }
>  
> +
> +void gvir_config_set_xml_error(GError **err,
> +                               GQuark domain, gint code,
> +                               const gchar *format, ...)
> +{
> +    va_list args;
> +    gchar *message;
> +
> +    if (!err)
> +        return;
> +
> +    va_start(args, format);
> +    message = g_strdup_vprintf(format, args);
> +    va_end(args);
> +
> +    *err = gvir_config_xml_error_new_literal(domain, code, message);
> +
> +    g_free(message);
> +}
> +
> +
>  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

>          return NULL;
>      }
>      if ((!doc->children) || (strcmp((char *)doc->children->name, root_node) != 0)) {
> -        *err = g_error_new(GVIR_CONFIG_OBJECT_ERROR,
> -                           0,
> -                           "XML data has no '%s' node",
> -                           root_node);
> +        g_set_error(err,
> +                    GVIR_CONFIG_OBJECT_ERROR,
> +                    0,
> +                    "XML data has no '%s' node",
> +                    root_node);
>          xmlFreeDoc(doc);
>          return NULL;
>      }
> diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c
> index 5e973d5..3a55387 100644
> --- a/libvirt-gconfig/libvirt-gconfig-object.c
> +++ b/libvirt-gconfig/libvirt-gconfig-object.c
> @@ -202,7 +202,8 @@ void gvir_config_object_validate(GVirConfigObject *config,
>      xmlSetStructuredErrorFunc(NULL, gvir_xml_structured_error_nop);
>  
>      if (!priv->node) {
> -        *err = gvir_xml_error_new(GVIR_CONFIG_OBJECT_ERROR,
> +        gvir_config_set_xml_error(err,
> +                                  GVIR_CONFIG_OBJECT_ERROR,
>                                    0,
>                                    "%s",
>                                    "No XML document associated with this config object");
> @@ -211,7 +212,8 @@ void gvir_config_object_validate(GVirConfigObject *config,
>  
>      rngParser = xmlRelaxNGNewParserCtxt(priv->schema);
>      if (!rngParser) {
> -        *err = gvir_xml_error_new(GVIR_CONFIG_OBJECT_ERROR,
> +        gvir_config_set_xml_error(err,
> +                                  GVIR_CONFIG_OBJECT_ERROR,
>                                    0,
>                                    "Unable to create RNG parser for %s",
>                                    priv->schema);
> @@ -220,7 +222,8 @@ void gvir_config_object_validate(GVirConfigObject *config,
>  
>      rng = xmlRelaxNGParse(rngParser);
>      if (!rng) {
> -        *err = gvir_xml_error_new(GVIR_CONFIG_OBJECT_ERROR,
> +        gvir_config_set_xml_error(err,
> +                                  GVIR_CONFIG_OBJECT_ERROR,
>                                    0,
>                                    "Unable to parse RNG %s",
>                                    priv->schema);
> @@ -231,7 +234,8 @@ void gvir_config_object_validate(GVirConfigObject *config,
>  
>      rngValid = xmlRelaxNGNewValidCtxt(rng);
>      if (!rngValid) {
> -        *err = gvir_xml_error_new(GVIR_CONFIG_OBJECT_ERROR,
> +        gvir_config_set_xml_error(err,
> +                                  GVIR_CONFIG_OBJECT_ERROR,
>                                    0,
>                                    "Unable to create RNG validation context %s",
>                                    priv->schema);
> @@ -240,7 +244,8 @@ void gvir_config_object_validate(GVirConfigObject *config,
>      }
>  
>      if (xmlRelaxNGValidateDoc(rngValid, priv->node->doc) != 0) {
> -        *err = gvir_xml_error_new(GVIR_CONFIG_OBJECT_ERROR,
> +        gvir_config_set_xml_error(err,
> +                                  GVIR_CONFIG_OBJECT_ERROR,
>                                    0,
>                                    "%s",
>                                    "Unable to validate doc");
> diff --git a/libvirt-glib/libvirt-glib-error.c b/libvirt-glib/libvirt-glib-error.c
> index b4d1f93..4a44216 100644
> --- a/libvirt-glib/libvirt-glib-error.c
> +++ b/libvirt-glib/libvirt-glib-error.c
> @@ -126,3 +126,94 @@ GError *gvir_error_new_valist(GQuark domain,
>  
>      return err;
>  }
> +
> +
> +/**
> + * gvir_set_error: (skip)
> + * @error: pointer to error location
> + * @domain: error domain
> + * @code: error code
> + * @format: printf()-style format for error message
> + * @Varargs: parameters for message format
> + *
> + * If @error is NULL this does nothing. Otherwise it
> + * creates a new #GError with the given @domain and @code,
> + * and a message formatted with @format, and stores it
> + * in @error.
> + */
> +void gvir_set_error(GError **error,
> +                    GQuark domain,
> +                    gint code,
> +                    const gchar *format,
> +                    ...)
> +{
> +    va_list args;
> +    gchar *message;
> +
> +    if (!error)
> +        return;
> +
> +    va_start(args, format);
> +    message = g_strdup_vprintf(format, args);
> +    va_end(args);
> +
> +    *error = gvir_error_new_literal(domain, code, message);
> +
> +    g_free(message);
> +}

I'd write it as

    va_start(args, format);
    gvir_set_error_valist(error, domain, code, format, args);
    va_end(args);

Rest of the patch looks good to me,

Christophe

> +
> +
> +/**
> + * gvir_set_error_literal: (skip)
> + * @error: pointer to error location
> + * @domain: error domain
> + * @code: error code
> + * @message: error message
> + *
> + * If @error is NULL this does nothing. Otherwise it
> + * creates a new #GError and stores it in @error; unlike
> + * gvir_set_error(), @message is not a printf()-style
> + * format string. Use this function if @message contains
> + * text you don't have control over, that could include
> + * printf() escape sequences.
> + */
> +void gvir_set_error_literal(GError **error,
> +                            GQuark domain,
> +                            gint code,
> +                            const gchar *message)
> +{
> +    if (!error)
> +        return;
> +
> +    *error = gvir_error_new_literal(domain, code, message);
> +}
> +
> +
> +/**
> + * gvir_set_error_valist: (skip)
> + * @error: pointer to error location
> + * @domain: error domain
> + * @code: error code
> + * @format: printf()-style format for error message
> + * @args: #va_list of parameters for the message format
> + *
> + * If @error is NULL this does nothing. Otherwise it
> + * creates a new #GError with the given @domain and @code,
> + * and a message formatted with @format, and stores it
> + * in @error.
> + */
> +void gvir_set_error_valist(GError **error,
> +                           GQuark domain,
> +                           gint code,
> +                           const gchar *format,
> +                           va_list args)
> +{
> +    gchar *message;
> +    if (!error)
> +        return;
> +
> +    message = g_strdup_vprintf(format, args);
> +    *error = gvir_error_new_literal(domain, code, message);
> +
> +    g_free(message);
> +}
> diff --git a/libvirt-glib/libvirt-glib-error.h b/libvirt-glib/libvirt-glib-error.h
> index cfb5b95..9e44383 100644
> --- a/libvirt-glib/libvirt-glib-error.h
> +++ b/libvirt-glib/libvirt-glib-error.h
> @@ -42,6 +42,24 @@ GError *gvir_error_new_valist(GQuark domain,
>                                const gchar *format,
>                                va_list args);
>  
> +void gvir_set_error(GError **error,
> +                    GQuark domain,
> +                    gint code,
> +                    const gchar *format,
> +                    ...);
> +
> +void gvir_set_error_literal(GError **error,
> +                            GQuark domain,
> +                            gint code,
> +                            const gchar *message);
> +
> +void gvir_set_error_valist(GError **error,
> +                           GQuark domain,
> +                           gint code,
> +                           const gchar *format,
> +                           va_list args);
> +
> +
>  G_END_DECLS
>  
>  #endif /* __LIBVIRT_GLIB_ERROR_H__ */
> diff --git a/libvirt-glib/libvirt-glib.sym b/libvirt-glib/libvirt-glib.sym
> index cd41cb9..289dbee 100644
> --- a/libvirt-glib/libvirt-glib.sym
> +++ b/libvirt-glib/libvirt-glib.sym
> @@ -7,6 +7,9 @@ LIBVIRT_GLIB_0.0.1 {
>  	gvir_error_new;
>  	gvir_error_new_valist;
>  	gvir_error_new_literal;
> +	gvir_set_error;
> +	gvir_set_error_valist;
> +	gvir_set_error_literal;
>  
>    local:
>          *;
> diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c
> index 9d3ab10..50cd860 100644
> --- a/libvirt-gobject/libvirt-gobject-connection.c
> +++ b/libvirt-gobject/libvirt-gobject-connection.c
> @@ -401,21 +401,19 @@ gboolean gvir_connection_open(GVirConnection *conn,
>  
>      g_mutex_lock(priv->lock);
>      if (priv->conn) {
> -        if (err)
> -            *err = g_error_new(GVIR_CONNECTION_ERROR,
> -                               0,
> -                               "Connection %s is already open",
> -                               priv->uri);
> +        g_set_error(err, GVIR_CONNECTION_ERROR,
> +                    0,
> +                    "Connection %s is already open",
> +                    priv->uri);
>          g_mutex_unlock(priv->lock);
>          return FALSE;
>      }
>  
>      if (!(priv->conn = virConnectOpen(priv->uri))) {
> -        if (err)
> -            *err = gvir_error_new(GVIR_CONNECTION_ERROR,
> -                                  0,
> -                                  "Unable to open %s",
> -                                  priv->uri);
> +        gvir_set_error(err, GVIR_CONNECTION_ERROR,
> +                       0,
> +                       "Unable to open %s",
> +                       priv->uri);
>          g_mutex_unlock(priv->lock);
>          return FALSE;
>      }
> @@ -423,10 +421,9 @@ gboolean gvir_connection_open(GVirConnection *conn,
>      if (!priv->uri) {
>          char *uri = virConnectGetURI(priv->conn);
>          if (!uri) {
> -            if (err)
> -                *err = gvir_error_new(GVIR_CONNECTION_ERROR,
> -                                      0,
> -                                      "%s", "Unable to get connection URI");
> +            gvir_set_error(err, GVIR_CONNECTION_ERROR,
> +                           0,
> +                           "%s", "Unable to get connection URI");
>              virConnectClose(priv->conn);
>              priv->conn = NULL;
>              g_mutex_unlock(priv->lock);
> @@ -570,10 +567,9 @@ static gchar ** fetch_list(virConnectPtr vconn,
>      gint i;
>  
>      if ((n = count_func(vconn)) < 0) {
> -        if (err)
> -            *err = gvir_error_new(GVIR_CONNECTION_ERROR,
> -                                  0,
> -                                  "Unable to count %s", name);
> +        gvir_set_error(err, GVIR_CONNECTION_ERROR,
> +                       0,
> +                       "Unable to count %s", name);
>          goto error;
>      }
>  
> @@ -583,10 +579,9 @@ static gchar ** fetch_list(virConnectPtr vconn,
>  
>          lst = g_new(gchar *, n);
>          if ((n = list_func(vconn, lst, n)) < 0) {
> -            if (err)
> -                *err = gvir_error_new(GVIR_CONNECTION_ERROR,
> -                                      0,
> -                                      "Unable to list %s %d", name, n);
> +            gvir_set_error(err, GVIR_CONNECTION_ERROR,
> +                           0,
> +                           "Unable to list %s %d", name, n);
>              goto error;
>          }
>      }
> @@ -623,10 +618,9 @@ gboolean gvir_connection_fetch_domains(GVirConnection *conn,
>  
>      g_mutex_lock(priv->lock);
>      if (!priv->conn) {
> -        if (err)
> -            *err = g_error_new(GVIR_CONNECTION_ERROR,
> -                               0,
> -                               "Connection is not open");
> +        g_set_error(err, GVIR_CONNECTION_ERROR,
> +                    0,
> +                    "Connection is not open");
>          g_mutex_unlock(priv->lock);
>          goto cleanup;
>      }
> @@ -639,10 +633,9 @@ gboolean gvir_connection_fetch_domains(GVirConnection *conn,
>          goto cleanup;
>  
>      if ((nactive = virConnectNumOfDomains(vconn)) < 0) {
> -        if (err)
> -            *err = gvir_error_new(GVIR_CONNECTION_ERROR,
> -                                  0,
> -                                  "Unable to count domains");
> +        gvir_set_error(err, GVIR_CONNECTION_ERROR,
> +                       0,
> +                       "Unable to count domains");
>          goto cleanup;
>      }
>      if (nactive) {
> @@ -651,10 +644,9 @@ gboolean gvir_connection_fetch_domains(GVirConnection *conn,
>  
>          active = g_new(gint, nactive);
>          if ((nactive = virConnectListDomains(vconn, active, nactive)) < 0) {
> -            if (err)
> -                *err = gvir_error_new(GVIR_CONNECTION_ERROR,
> -                                      0,
> -                                      "Unable to list domains");
> +            gvir_set_error(err, GVIR_CONNECTION_ERROR,
> +                           0,
> +                           "Unable to list domains");
>              goto cleanup;
>          }
>      }
> @@ -755,10 +747,9 @@ gboolean gvir_connection_fetch_storage_pools(GVirConnection *conn,
>  
>      g_mutex_lock(priv->lock);
>      if (!priv->conn) {
> -        if (err)
> -            *err = g_error_new(GVIR_CONNECTION_ERROR,
> -                               0,
> -                               "Connection is not open");
> +        g_set_error(err, GVIR_CONNECTION_ERROR,
> +                    0,
> +                    "Connection is not open");
>          g_mutex_unlock(priv->lock);
>          goto cleanup;
>      }
> @@ -1238,10 +1229,9 @@ GVirDomain *gvir_connection_create_domain(GVirConnection *conn,
>      handle = virDomainDefineXML(priv->conn, xml);
>      g_free(xml);
>      if (!handle) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_CONNECTION_ERROR,
> -                                          0,
> -                                          "Failed to create domain");
> +        gvir_set_error_literal(err, GVIR_CONNECTION_ERROR,
> +                               0,
> +                               "Failed to create domain");
>          return NULL;
>      }
>  
> @@ -1286,10 +1276,9 @@ GVirDomain *gvir_connection_start_domain(GVirConnection *conn,
>      handle = virDomainCreateXML(priv->conn, xml, flags);
>      g_free(xml);
>      if (!handle) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_CONNECTION_ERROR,
> -                                          0,
> -                                          "Failed to create domain");
> +        gvir_set_error_literal(err, GVIR_CONNECTION_ERROR,
> +                               0,
> +                               "Failed to create domain");
>          return NULL;
>      }
>  
> @@ -1331,10 +1320,9 @@ GVirStoragePool *gvir_connection_create_storage_pool
>      g_return_val_if_fail(xml != NULL, NULL);
>  
>      if (!(handle = virStoragePoolDefineXML(priv->conn, xml, flags))) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_CONNECTION_ERROR,
> -                                          flags,
> -                                          "Failed to create storage pool");
> +        gvir_set_error_literal(err, GVIR_CONNECTION_ERROR,
> +                               flags,
> +                               "Failed to create storage pool");
>          return NULL;
>      }
>  
> diff --git a/libvirt-gobject/libvirt-gobject-domain-disk.c b/libvirt-gobject/libvirt-gobject-domain-disk.c
> index ab9b29f..f98d816 100644
> --- a/libvirt-gobject/libvirt-gobject-domain-disk.c
> +++ b/libvirt-gobject/libvirt-gobject-domain-disk.c
> @@ -175,10 +175,9 @@ GVirDomainDiskStats *gvir_domain_disk_get_stats(GVirDomainDisk *self, GError **e
>      handle = gvir_domain_device_get_domain_handle(GVIR_DOMAIN_DEVICE(self));
>  
>      if (virDomainBlockStats(handle, priv->path, &stats, sizeof (stats)) < 0) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_DOMAIN_DISK_ERROR,
> -                                          0,
> -                                          "Unable to get domain disk stats");
> +        gvir_set_error_literal(err, GVIR_DOMAIN_DISK_ERROR,
> +                               0,
> +                               "Unable to get domain disk stats");
>          goto end;
>      }
>  
> diff --git a/libvirt-gobject/libvirt-gobject-domain-interface.c b/libvirt-gobject/libvirt-gobject-domain-interface.c
> index 223a663..0917e03 100644
> --- a/libvirt-gobject/libvirt-gobject-domain-interface.c
> +++ b/libvirt-gobject/libvirt-gobject-domain-interface.c
> @@ -175,10 +175,9 @@ GVirDomainInterfaceStats *gvir_domain_interface_get_stats(GVirDomainInterface *s
>      handle = gvir_domain_device_get_domain_handle(GVIR_DOMAIN_DEVICE(self));
>  
>      if (virDomainInterfaceStats(handle, priv->path, &stats, sizeof (stats)) < 0) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_DOMAIN_INTERFACE_ERROR,
> -                                          0,
> -                                          "Unable to get domain interface stats");
> +        gvir_set_error_literal(err, GVIR_DOMAIN_INTERFACE_ERROR,
> +                               0,
> +                               "Unable to get domain interface stats");
>          goto end;
>      }
>  
> diff --git a/libvirt-gobject/libvirt-gobject-domain-snapshot.c b/libvirt-gobject/libvirt-gobject-domain-snapshot.c
> index 36dd15b..e68321d 100644
> --- a/libvirt-gobject/libvirt-gobject-domain-snapshot.c
> +++ b/libvirt-gobject/libvirt-gobject-domain-snapshot.c
> @@ -196,9 +196,9 @@ GVirConfigDomainSnapshot *gvir_domain_snapshot_get_config
>      gchar *xml;
>  
>      if (!(xml = virDomainSnapshotGetXMLDesc(priv->handle, flags))) {
> -        *err = gvir_error_new_literal(GVIR_DOMAIN_SNAPSHOT_ERROR,
> -                                      0,
> -                                      "Unable to get domain_snapshot XML config");
> +        gvir_set_error_literal(err, GVIR_DOMAIN_SNAPSHOT_ERROR,
> +                               0,
> +                               "Unable to get domain_snapshot XML config");
>          return NULL;
>      }
>  
> diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c
> index 56694a8..7bfb0ae 100644
> --- a/libvirt-gobject/libvirt-gobject-domain.c
> +++ b/libvirt-gobject/libvirt-gobject-domain.c
> @@ -279,10 +279,9 @@ gint gvir_domain_get_id(GVirDomain *dom,
>      gint ret;
>  
>      if ((ret = virDomainGetID(priv->handle)) < 0) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR,
> -                                          0,
> -                                          "Unable to get ID for domain");
> +        gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
> +                               0,
> +                               "Unable to get ID for domain");
>      }
>      return ret;
>  }
> @@ -305,10 +304,9 @@ gboolean gvir_domain_start(GVirDomain *dom,
>      else
>          ret = virDomainCreate(priv->handle);
>      if (ret < 0) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR,
> -                                          0,
> -                                          "Unable to start domain");
> +        gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
> +                               0,
> +                               "Unable to start domain");
>          return FALSE;
>      }
>  
> @@ -327,10 +325,9 @@ gboolean gvir_domain_resume(GVirDomain *dom,
>      GVirDomainPrivate *priv = dom->priv;
>  
>      if (virDomainResume(priv->handle) < 0) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR,
> -                                          0,
> -                                          "Unable to resume domain");
> +        gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
> +                               0,
> +                               "Unable to resume domain");
>          return FALSE;
>      }
>  
> @@ -354,10 +351,9 @@ gboolean gvir_domain_stop(GVirDomain *dom,
>      else
>          ret = virDomainDestroy(priv->handle);
>      if (ret < 0) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR,
> -                                          0,
> -                                          "Unable to stop domain");
> +        gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
> +                               0,
> +                               "Unable to stop domain");
>          return FALSE;
>      }
>  
> @@ -381,10 +377,9 @@ gboolean gvir_domain_delete(GVirDomain *dom,
>      else
>          ret = virDomainUndefine(priv->handle);
>      if (ret < 0) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR,
> -                                          0,
> -                                          "Unable to delete domain");
> +        gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
> +                               0,
> +                               "Unable to delete domain");
>          return FALSE;
>      }
>  
> @@ -403,10 +398,9 @@ gboolean gvir_domain_shutdown(GVirDomain *dom,
>      GVirDomainPrivate *priv = dom->priv;
>  
>      if (virDomainShutdown(priv->handle) < 0) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR,
> -                                          0,
> -                                          "Unable to shutdown domain");
> +        gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
> +                               0,
> +                               "Unable to shutdown domain");
>          return FALSE;
>      }
>  
> @@ -425,10 +419,9 @@ gboolean gvir_domain_reboot(GVirDomain *dom,
>      GVirDomainPrivate *priv = dom->priv;
>  
>      if (virDomainReboot(priv->handle, flags) < 0) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR,
> -                                          0,
> -                                          "Unable to reboot domain");
> +        gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
> +                               0,
> +                               "Unable to reboot domain");
>          return FALSE;
>      }
>  
> @@ -449,10 +442,9 @@ GVirConfigDomain *gvir_domain_get_config(GVirDomain *dom,
>      gchar *xml;
>  
>      if (!(xml = virDomainGetXMLDesc(priv->handle, flags))) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR,
> -                                          0,
> -                                          "Unable to get domain XML config");
> +        gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
> +                               0,
> +                               "Unable to get domain XML config");
>          return NULL;
>      }
>  
> @@ -496,10 +488,9 @@ gboolean gvir_domain_set_config(GVirDomain *domain,
>      g_return_val_if_fail(xml != NULL, FALSE);
>  
>      if ((conn = virDomainGetConnect(priv->handle)) == NULL) {
> -        if (err != NULL)
> -            *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR,
> -                                          0,
> -                                          "Failed to get domain connection");
> +        gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
> +                               0,
> +                               "Failed to get domain connection");
>          g_free (xml);
>  
>          return FALSE;
> @@ -509,11 +500,10 @@ gboolean gvir_domain_set_config(GVirDomain *domain,
>      g_free (xml);
>  
>      if (handle == NULL) {
> -        if (err != NULL)
> -            *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR,
> -                                          0,
> -                                          "Failed to set "
> -                                          "domain configuration");
> +        gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
> +                               0,
> +                               "Failed to set "
> +                               "domain configuration");
>          return FALSE;
>      }
>  
> @@ -521,11 +511,10 @@ gboolean gvir_domain_set_config(GVirDomain *domain,
>      virDomainFree(handle);
>  
>      if (g_strcmp0 (uuid, priv->uuid) != 0) {
> -        if (err != NULL)
> -            *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR,
> -                                          0,
> -                                          "Failed to set "
> -                                          "domain configuration");
> +        gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
> +                               0,
> +                               "Failed to set "
> +                               "domain configuration");
>  
>          return FALSE;
>      }
> @@ -546,10 +535,9 @@ GVirDomainInfo *gvir_domain_get_info(GVirDomain *dom,
>      GVirDomainInfo *ret;
>  
>      if (virDomainGetInfo(priv->handle, &info) < 0) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR,
> -                                          0,
> -                                          "Unable to get domain info");
> +        gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
> +                               0,
> +                               "Unable to get domain info");
>          return NULL;
>      }
>  
> @@ -591,10 +579,9 @@ gchar *gvir_domain_screenshot(GVirDomain *dom,
>                                       st,
>                                       monitor_id,
>                                       flags))) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_DOMAIN_ERROR,
> -                                          0,
> -                                          "Unable to take a screenshot");
> +        gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
> +                               0,
> +                               "Unable to take a screenshot");
>          goto end;
>      }
>  end:
> diff --git a/libvirt-gobject/libvirt-gobject-interface.c b/libvirt-gobject/libvirt-gobject-interface.c
> index 1413abf..7e6e9b0 100644
> --- a/libvirt-gobject/libvirt-gobject-interface.c
> +++ b/libvirt-gobject/libvirt-gobject-interface.c
> @@ -185,10 +185,9 @@ GVirConfigInterface *gvir_interface_get_config(GVirInterface *iface,
>      gchar *xml;
>  
>      if (!(xml = virInterfaceGetXMLDesc(priv->handle, flags))) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_INTERFACE_ERROR,
> -                                          0,
> -                                          "Unable to get interface XML config");
> +        gvir_set_error_literal(err, GVIR_INTERFACE_ERROR,
> +                               0,
> +                               "Unable to get interface XML config");
>          return NULL;
>      }
>  
> diff --git a/libvirt-gobject/libvirt-gobject-network-filter.c b/libvirt-gobject/libvirt-gobject-network-filter.c
> index 2ac8236..f0fd024 100644
> --- a/libvirt-gobject/libvirt-gobject-network-filter.c
> +++ b/libvirt-gobject/libvirt-gobject-network-filter.c
> @@ -211,10 +211,9 @@ GVirConfigNetworkFilter *gvir_network_filter_get_config
>      gchar *xml;
>  
>      if (!(xml = virNWFilterGetXMLDesc(priv->handle, flags))) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_NETWORK_FILTER_ERROR,
> -                                          0,
> -                                          "Unable to get network_filter XML config");
> +        gvir_set_error_literal(err, GVIR_NETWORK_FILTER_ERROR,
> +                               0,
> +                               "Unable to get network_filter XML config");
>          return NULL;
>      }
>  
> diff --git a/libvirt-gobject/libvirt-gobject-network.c b/libvirt-gobject/libvirt-gobject-network.c
> index 5f40a77..847c236 100644
> --- a/libvirt-gobject/libvirt-gobject-network.c
> +++ b/libvirt-gobject/libvirt-gobject-network.c
> @@ -207,10 +207,9 @@ GVirConfigNetwork *gvir_network_get_config(GVirNetwork *network,
>      gchar *xml;
>  
>      if (!(xml = virNetworkGetXMLDesc(priv->handle, flags))) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_NETWORK_ERROR,
> -                                          0,
> -                                          "Unable to get network XML config");
> +        gvir_set_error_literal(err, GVIR_NETWORK_ERROR,
> +                               0,
> +                               "Unable to get network XML config");
>          return NULL;
>      }
>  
> diff --git a/libvirt-gobject/libvirt-gobject-node-device.c b/libvirt-gobject/libvirt-gobject-node-device.c
> index e21d1db..59fe84b 100644
> --- a/libvirt-gobject/libvirt-gobject-node-device.c
> +++ b/libvirt-gobject/libvirt-gobject-node-device.c
> @@ -186,10 +186,9 @@ GVirConfigNodeDevice *gvir_node_device_get_config(GVirNodeDevice *device,
>      gchar *xml;
>  
>      if (!(xml = virNodeDeviceGetXMLDesc(priv->handle, flags))) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_NODE_DEVICE_ERROR,
> -                                          0,
> -                                          "Unable to get node_device XML config");
> +        gvir_set_error_literal(err, GVIR_NODE_DEVICE_ERROR,
> +                               0,
> +                               "Unable to get node_device XML config");
>          return NULL;
>      }
>  
> diff --git a/libvirt-gobject/libvirt-gobject-secret.c b/libvirt-gobject/libvirt-gobject-secret.c
> index 7521b73..0c81921 100644
> --- a/libvirt-gobject/libvirt-gobject-secret.c
> +++ b/libvirt-gobject/libvirt-gobject-secret.c
> @@ -197,10 +197,9 @@ GVirConfigSecret *gvir_secret_get_config(GVirSecret *secret,
>      gchar *xml;
>  
>      if (!(xml = virSecretGetXMLDesc(priv->handle, flags))) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_SECRET_ERROR,
> -                                          0,
> -                                          "Unable to get secret XML config");
> +        gvir_set_error_literal(err, GVIR_SECRET_ERROR,
> +                               0,
> +                               "Unable to get secret XML config");
>          return NULL;
>      }
>  
> diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c b/libvirt-gobject/libvirt-gobject-storage-pool.c
> index e267dff..76970db 100644
> --- a/libvirt-gobject/libvirt-gobject-storage-pool.c
> +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c
> @@ -239,10 +239,9 @@ GVirConfigStoragePool *gvir_storage_pool_get_config(GVirStoragePool *pool,
>      gchar *xml;
>  
>      if (!(xml = virStoragePoolGetXMLDesc(priv->handle, flags))) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_STORAGE_POOL_ERROR,
> -                                          0,
> -                                          "Unable to get storage_pool XML config");
> +        gvir_set_error_literal(err, GVIR_STORAGE_POOL_ERROR,
> +                               0,
> +                               "Unable to get storage_pool XML config");
>          return NULL;
>      }
>  
> @@ -297,9 +296,9 @@ static gchar ** fetch_list(virStoragePoolPtr vpool,
>      gint i;
>  
>      if ((n = count_func(vpool)) < 0) {
> -        *err = gvir_error_new(GVIR_STORAGE_POOL_ERROR,
> -                              0,
> -                              "Unable to count %s", name);
> +        gvir_set_error(err, GVIR_STORAGE_POOL_ERROR,
> +                       0,
> +                       "Unable to count %s", name);
>          goto error;
>      }
>  
> @@ -309,9 +308,9 @@ static gchar ** fetch_list(virStoragePoolPtr vpool,
>  
>          lst = g_new(gchar *, n);
>          if ((n = list_func(vpool, lst, n)) < 0) {
> -            *err = gvir_error_new(GVIR_STORAGE_POOL_ERROR,
> -                                  0,
> -                                  "Unable to list %s %d", name, n);
> +            gvir_set_error(err, GVIR_STORAGE_POOL_ERROR,
> +                           0,
> +                           "Unable to list %s %d", name, n);
>              goto error;
>          }
>      }
> @@ -347,10 +346,9 @@ gboolean gvir_storage_pool_refresh(GVirStoragePool *pool,
>      vpool = priv->handle;
>  
>      if (virStoragePoolRefresh(vpool, 0) < 0) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_STORAGE_POOL_ERROR,
> -                                          0,
> -                                          "Unable to refresh storage pool");
> +        gvir_set_error_literal(err, GVIR_STORAGE_POOL_ERROR,
> +                               0,
> +                               "Unable to refresh storage pool");
>          goto cleanup;
>      }
>  
> @@ -539,10 +537,9 @@ GVirStorageVol *gvir_storage_pool_create_volume
>      g_return_val_if_fail(xml != NULL, NULL);
>  
>      if (!(handle = virStorageVolCreateXML(priv->handle, xml, 0))) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_STORAGE_POOL_ERROR,
> -                                          0,
> -                                          "Failed to create volume");
> +        gvir_set_error_literal(err, GVIR_STORAGE_POOL_ERROR,
> +                               0,
> +                               "Failed to create volume");
>          return NULL;
>      }
>  
> @@ -574,10 +571,9 @@ gboolean gvir_storage_pool_build (GVirStoragePool *pool,
>                                    GError **err)
>  {
>      if (virStoragePoolBuild(pool->priv->handle, flags)) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_STORAGE_POOL_ERROR,
> -                                          0,
> -                                          "Failed to build storage pool");
> +        gvir_set_error_literal(err, GVIR_STORAGE_POOL_ERROR,
> +                               0,
> +                               "Failed to build storage pool");
>          return FALSE;
>      }
>  
> @@ -679,10 +675,9 @@ gboolean gvir_storage_pool_start (GVirStoragePool *pool,
>                                    GError **err)
>  {
>      if (virStoragePoolCreate(pool->priv->handle, flags)) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_STORAGE_POOL_ERROR,
> -                                          0,
> -                                          "Failed to start storage pool");
> +        gvir_set_error_literal(err, GVIR_STORAGE_POOL_ERROR,
> +                               0,
> +                               "Failed to start storage pool");
>          return FALSE;
>      }
>  
> diff --git a/libvirt-gobject/libvirt-gobject-storage-vol.c b/libvirt-gobject/libvirt-gobject-storage-vol.c
> index 15ae404..103d402 100644
> --- a/libvirt-gobject/libvirt-gobject-storage-vol.c
> +++ b/libvirt-gobject/libvirt-gobject-storage-vol.c
> @@ -212,10 +212,9 @@ GVirConfigStorageVol *gvir_storage_vol_get_config(GVirStorageVol *vol,
>      gchar *xml;
>  
>      if (!(xml = virStorageVolGetXMLDesc(priv->handle, flags))) {
> -        if (err)
> -            *err = gvir_error_new_literal(GVIR_STORAGE_VOL_ERROR,
> -                                          0,
> -                                          "Unable to get storage_vol XML config");
> +        gvir_set_error_literal(err, GVIR_STORAGE_VOL_ERROR,
> +                               0,
> +                               "Unable to get storage_vol XML config");
>          return NULL;
>      }
>  
> diff --git a/libvirt-gobject/libvirt-gobject-stream.c b/libvirt-gobject/libvirt-gobject-stream.c
> index 7baf515..c1ae765 100644
> --- a/libvirt-gobject/libvirt-gobject-stream.c
> +++ b/libvirt-gobject/libvirt-gobject-stream.c
> @@ -372,10 +372,9 @@ gvir_stream_receive_all(GVirStream *self,
>  
>      r = virStreamRecvAll(self->priv->handle, stream_sink, &helper);
>      if (r < 0) {
> -        if (err != NULL)
> -            *err = gvir_error_new_literal(GVIR_STREAM_ERROR,
> -                                          0,
> -                                          "Unable to perform RecvAll");
> +        gvir_set_error_literal(err, GVIR_STREAM_ERROR,
> +                               0,
> +                               "Unable to perform RecvAll");
>      }
>  
>      return r;
> @@ -476,10 +475,9 @@ gvir_stream_send_all(GVirStream *self,
>  
>      r = virStreamSendAll(self->priv->handle, stream_source, &helper);
>      if (r < 0) {
> -        if (err != NULL)
> -            *err = gvir_error_new_literal(GVIR_STREAM_ERROR,
> -                                          0,
> -                                          "Unable to perform SendAll");
> +        gvir_set_error_literal(err, GVIR_STREAM_ERROR,
> +                               0,
> +                               "Unable to perform SendAll");
>      }
>  
>      return r;
> -- 
> 1.7.6.4
> 
> --
> libvir-list mailing list
> libvir-list redhat com
> https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: pgpMJnRsuRVhP.pgp
Description: PGP signature


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