Re: Function completion for GVariant maybe types?

[Ryan Lortie <desrt desrt ca>]

hi Markus,

On Mon, 2011-12-05 at 22:30 +0100, Markus Elfring wrote:
> How do you think about the following update suggestion?

See comments below.

> +GVariant *
> +g_variant_new_nothing_from_type (GVariant const * value)

We don't use 'const' in this way with GVariant (although I considered
doing so).

> +  if (value == NULL)
> + 	return g_variant_new ("ms", NULL);

Why arbitrarily "ms"?

> +  else
> +    {
> +	  GVariantType* type = g_variant_get_type (value);
> +
> +      if (g_variant_type_is_maybe (type))
> +        {
> +		  GVariant* contained = g_variant_get_child_value (value, 0);
This could fail -- the maybe value may not contain a value.

> +		  type = g_variant_get_type (contained);
> +		  g_variant_unref(contained);

It is far more efficient to just call g_variant_type_element() on the
original type, as Simon suggested.  The following line is also
theoretically invalid since 'type' belongs to the 'contained' GVariant
that you just freed (with the current implementation this happens to be
safe, but no promises that this won't change in the future).

> +		  return g_variant_new_maybe (type, NULL);
> +        }
> +      else
> + 		  return g_variant_new_maybe (type, value);
> +    }

This function pretty much makes no sense to me:

  - if you pass in NULL, return a Nothing of (arbitrary) type "ms"

  - if you pass in a GVariant with a maybe type:

    - if the maybe contains no value, crash

    - if the maybe contains a value, return the NULL of the same type

  - if you pass in a GVariant with a non-maybe type, return the same
    value as a nullable type

Even correcting the bugs, I can't imagine any situation in which this
function would be useful.  Can you please explain, at a higher level,
what you are trying to accomplish?

Cheers