[libvirt] [libvirt-glib] Check arguments to public GVirConfigObject methods

[Daniel P. Berrange]

On Mon, May 21, 2012 at 02:14:19PM +0200, Christophe Fergeau wrote:
> GVirConfigObject public methods don't have g_return_if_fail checks
> on their arguments. It happens that GNOME Boxes tries to call
> gvir_config_object_to_xml with a NULL GVirConfigObject causing a
> crash https://bugzilla.gnome.org/show_bug.cgi?id=676308
> While GNOME Boxes needs to be fixed not to pass an invalid value
> to gvir_config_object_to_xml, it's better if libvirt-gconfig sanity
> checks the argument it is passed by the caller before using them.
> ---
>  libvirt-gconfig/libvirt-gconfig-object.c |   14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c
> index 8286bcb..1474393 100644
> --- a/libvirt-gconfig/libvirt-gconfig-object.c
> +++ b/libvirt-gconfig/libvirt-gconfig-object.c
> @@ -191,11 +191,16 @@ static void gvir_config_object_init(GVirConfigObject *conn)
>  void gvir_config_object_validate(GVirConfigObject *config,
>                                   GError **err)
>  {
> -    GVirConfigObjectPrivate *priv = config->priv;
> +    GVirConfigObjectPrivate *priv;
>      xmlRelaxNGParserCtxtPtr rngParser = NULL;
>      xmlRelaxNGPtr rng = NULL;
>      xmlRelaxNGValidCtxtPtr rngValid = NULL;
>  
> +    g_return_if_fail(GVIR_CONFIG_IS_OBJECT(config));
> +    g_return_if_fail(err == NULL || *err == NULL);
> +
> +    priv = config->priv;
> +
>      xmlSetGenericErrorFunc(NULL, gvir_xml_generic_error_nop);
>      xmlSetStructuredErrorFunc(NULL, gvir_xml_structured_error_nop);
>  
> @@ -256,13 +261,16 @@ void gvir_config_object_validate(GVirConfigObject *config,
>  
>  gchar *gvir_config_object_to_xml(GVirConfigObject *config)
>  {
> +    g_return_val_if_fail(GVIR_CONFIG_IS_OBJECT(config), NULL);
> +
>      return gvir_config_xml_node_to_string(config->priv->node);
>  }
>  
>  const gchar *gvir_config_object_get_schema(GVirConfigObject *config)
>  {
> -    GVirConfigObjectPrivate *priv = config->priv;
> -    return priv->schema;
> +    g_return_val_if_fail(GVIR_CONFIG_IS_OBJECT(config), NULL);
> +
> +    return config->priv->schema;
>  }
>  
>  /* FIXME: will we always have one xmlNode per GConfig object? */

ACK


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