On Tue, Jan 31, 2012 at 12:00:20PM +0100, Christophe Fergeau wrote:
gvir_storage_vol_get_name and gvir_storage_vol_get_path currently call g_error when an error occurs. Since g_error trigger a coredump, calling it in a library is harmful. Replace this with proper GError error reporting. --- libvirt-gobject/libvirt-gobject-storage-pool.c | 10 +++++++--- libvirt-gobject/libvirt-gobject-storage-vol.c | 14 ++++++++++---- libvirt-gobject/libvirt-gobject-storage-vol.h | 4 ++-- 3 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c b/libvirt-gobject/libvirt-gobject-storage-pool.c index bf25641..a88699e 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.c +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c @@ -528,6 +528,7 @@ GVirStorageVol *gvir_storage_pool_create_volume const gchar *xml; virStorageVolPtr handle; GVirStoragePoolPrivate *priv = pool->priv; + const char *name;
xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(conf));
@@ -545,11 +546,14 @@ GVirStorageVol *gvir_storage_pool_create_volume volume = GVIR_STORAGE_VOL(g_object_new(GVIR_TYPE_STORAGE_VOL, "handle", handle, NULL)); + name = gvir_storage_vol_get_name(volume, err); + if (name == NULL) { + g_object_unref(G_OBJECT(volume)); + return NULL; + }
g_mutex_lock(priv->lock); - g_hash_table_insert(priv->volumes, - g_strdup(gvir_storage_vol_get_name(volume)), - volume); + g_hash_table_insert(priv->volumes, g_strdup(name), volume); g_mutex_unlock(priv->lock);
return g_object_ref(volume); diff --git a/libvirt-gobject/libvirt-gobject-storage-vol.c b/libvirt-gobject/libvirt-gobject-storage-vol.c index 5b18877..c427e42 100644 --- a/libvirt-gobject/libvirt-gobject-storage-vol.c +++ b/libvirt-gobject/libvirt-gobject-storage-vol.c @@ -172,25 +172,31 @@ gvir_storage_vol_info_free(GVirStorageVolInfo *info) G_DEFINE_BOXED_TYPE(GVirStorageVolInfo, gvir_storage_vol_info, gvir_storage_vol_info_copy, gvir_storage_vol_info_free)
-const gchar *gvir_storage_vol_get_name(GVirStorageVol *vol) +const gchar *gvir_storage_vol_get_name(GVirStorageVol *vol, GError **error) { GVirStorageVolPrivate *priv = vol->priv; const char *name;
if (!(name = virStorageVolGetName(priv->handle))) { - g_error("Failed to get storage_vol name on %p", priv->handle); + gvir_set_error(error, GVIR_STORAGE_VOL_ERROR, 0, + "Failed to get storage_vol name on %p", + priv->handle); + return NULL; }
return name; }
-const gchar *gvir_storage_vol_get_path(GVirStorageVol *vol) +const gchar *gvir_storage_vol_get_path(GVirStorageVol *vol, GError **error) { GVirStorageVolPrivate *priv = vol->priv; const char *path;
if (!(path = virStorageVolGetPath(priv->handle))) { - g_error("Failed to get storage_vol path on %p", priv->handle); + gvir_set_error(error, GVIR_STORAGE_VOL_ERROR, 0, + "Failed to get storage_vol path on %p", + priv->handle); + return NULL; }
return path; diff --git a/libvirt-gobject/libvirt-gobject-storage-vol.h b/libvirt-gobject/libvirt-gobject-storage-vol.h index db63865..fd2be80 100644 --- a/libvirt-gobject/libvirt-gobject-storage-vol.h +++ b/libvirt-gobject/libvirt-gobject-storage-vol.h @@ -77,8 +77,8 @@ GType gvir_storage_vol_get_type(void); GType gvir_storage_vol_info_get_type(void); GType gvir_storage_vol_handle_get_type(void);
-const gchar *gvir_storage_vol_get_name(GVirStorageVol *vol); -const gchar *gvir_storage_vol_get_path(GVirStorageVol *vol); +const gchar *gvir_storage_vol_get_name(GVirStorageVol *vol, GError **error); +const gchar *gvir_storage_vol_get_path(GVirStorageVol *vol, GError **error);
gboolean gvir_storage_vol_delete(GVirStorageVol *vol, guint flags,
ACK for the change to get_path, but we don't need todo this for get_name. The virStorageVolGetName() method will always succeed, unless passed an invalid pointer, since it is not involving any RPC. It merely returns the static string in the virStorageVolPtr struct. 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 :|