GVariant: add g_variant_take_ref()

This function implements the following logic:

  if (g_variant_is_floating (value))
    g_variant_ref_sink (value);

which is used for consuming the return value of callbacks that may or
may not return floating references.

This patch also replaces a few instances of the above code with the new
function (GSettings, GDBus) and lifts a long-standing restriction on the
use of floating values as the return value for signal handlers by
improving g_value_take_variant().

https://bugzilla.gnome.org/show_bug.cgi?id=627974

docs/reference/glib/glib-sections.txt

@@ -2992,6 +2992,7 @@ g_variant_unref
 g_variant_ref
 g_variant_ref_sink
 g_variant_is_floating
+g_variant_take_ref
 g_variant_get_type
 g_variant_get_type_string
 g_variant_is_of_type

gio/gdbus-codegen/codegen.py

@@ -2031,8 +2031,7 @@ class CodeGenerator:
                      '          value = _%s_skeleton_handle_get_property (g_dbus_interface_skeleton_get_connection (G_DBUS_INTERFACE_SKELETON (skeleton)), NULL, g_dbus_interface_skeleton_get_object_path (G_DBUS_INTERFACE_SKELETON (skeleton)), "%s", info->name, NULL, skeleton);\n'
                      '          if (value != NULL)\n'
                      '            {\n'
-                     '              if (g_variant_is_floating (value))\n'
-                     '                g_variant_ref_sink (value);\n'
+                     '              g_variant_take_ref (value);\n'
                      '              g_variant_builder_add (&builder, "{sv}", info->name, value);\n'
                      '              g_variant_unref (value);\n'
                      '            }\n'

gio/gdbusconnection.c

@@ -3787,8 +3787,7 @@ invoke_get_property_in_idle_cb (gpointer _data)
     {
       g_assert_no_error (error);
 
-      if (g_variant_is_floating (value))
-        g_variant_ref_sink (value);
+      g_variant_take_ref (value);
       reply = g_dbus_message_new_method_reply (data->message);
       g_dbus_message_set_body (reply, g_variant_new ("(v)", value));
       g_dbus_connection_send_message (data->connection, reply, G_DBUS_SEND_MESSAGE_FLAGS_NONE, NULL, NULL);
@@ -4117,8 +4116,7 @@ invoke_get_all_properties_in_idle_cb (gpointer _data)
       if (value == NULL)
         continue;
 
-      if (g_variant_is_floating (value))
-        g_variant_ref_sink (value);
+      g_variant_take_ref (value);
       g_variant_builder_add (&builder,
                              "{sv}",
                              property_info->name,

gio/gsettings.c

@@ -2604,8 +2604,7 @@ g_settings_binding_property_changed (GObject          *object,
   if ((variant = binding->set_mapping (&value, binding->info.type,
                                        binding->user_data)))
     {
-      if (g_variant_is_floating (variant))
-        g_variant_ref_sink (variant);
+      g_variant_take_ref (variant);
 
       if (!g_settings_type_check (&binding->info, variant))
         {

glib/glib.symbols

@@ -1414,6 +1414,7 @@ g_variant_unref
 g_variant_ref
 g_variant_ref_sink
 g_variant_is_floating
+g_variant_take_ref
 g_variant_n_children
 g_variant_get_child_value
 g_variant_get_size

glib/gvariant-core.c

@@ -696,6 +696,54 @@ g_variant_ref_sink (GVariant *value)
   return value;
 }
 
+/**
+ * g_variant_take_ref:
+ * @value: a #GVariant
+ * @returns: the same @value
+ *
+ * If @value is floating, sink it.  Otherwise, do nothing.
+ *
+ * Typically you want to use g_variant_ref_sink() in order to
+ * automatically do the correct thing with respect to floating or
+ * non-floating references, but there is one specific scenario where
+ * this function is helpful.
+ *
+ * The situation where this function is helpful is when creating an API
+ * that allows the user to provide a callback function that returns a
+ * #GVariant.  We certainly want to allow the user the flexibility to
+ * return a non-floating reference from this callback (for the case
+ * where the value that is being returned already exists).
+ *
+ * At the same time, the style of the #GVariant API makes it likely that
+ * for newly-created #GVariant instances, the user can be saved some
+ * typing if they are allowed to return a #GVariant with a floating
+ * reference.
+ *
+ * Using this function on the return value of the user's callback allows
+ * the user to do whichever is more convenient for them.  The caller
+ * will alway receives exactly one full reference to the value: either
+ * the one that was returned in the first place, or a floating reference
+ * that has been converted to a full reference.
+ *
+ * This function has an odd interaction when combined with
+ * g_variant_ref_sink() running at the same time in another thread on
+ * the same #GVariant instance.  If g_variant_ref_sink() runs first then
+ * the result will be that the floating reference is converted to a hard
+ * reference.  If g_variant_take_ref() runs first then the result will
+ * be that the floating reference is converted to a hard reference and
+ * an additional reference on top of that one is added.  It is best to
+ * avoid this situation.
+ **/
+GVariant *
+g_variant_take_ref (GVariant *value)
+{
+  g_return_val_if_fail (value != NULL, NULL);
+
+  g_atomic_int_and (&value->state, ~STATE_FLOATING);
+
+  return value;
+}
+
 /**
  * g_variant_is_floating:
  * @value: a #GVariant
@@ -705,7 +753,8 @@ g_variant_ref_sink (GVariant *value)
  *
  * This function should only ever be used to assert that a given variant
  * is or is not floating, or for debug purposes. To acquire a reference
- * to a variant that might be floating, always use g_variant_ref_sink().
+ * to a variant that might be floating, always use g_variant_ref_sink()
+ * or g_variant_take_ref().
  *
  * See g_variant_ref_sink() for more information about floating reference
  * counts.

glib/gvariant.h

@@ -60,6 +60,7 @@ void                            g_variant_unref                         (GVarian
 GVariant *                      g_variant_ref                           (GVariant             *value);
 GVariant *                      g_variant_ref_sink                      (GVariant             *value);
 gboolean                        g_variant_is_floating                   (GVariant             *value);
+GVariant *                      g_variant_take_ref                      (GVariant             *value);
 
 const GVariantType *            g_variant_get_type                      (GVariant             *value);
 const gchar *                   g_variant_get_type_string               (GVariant             *value);

gobject/gvaluetypes.c

@@ -1207,11 +1207,9 @@ g_value_set_variant (GValue   *value,
  * the ownership of the caller's reference to @variant;
  * the caller doesn't have to unref it any more (i.e. the reference
  * count of the variant is not increased).
- * 
- * It is a programmer error to pass a floating variant to this function.
- * In particular this means that callbacks in closures, and signal handlers
- * for signals of return type %G_TYPE_VARIANT, must never return floating
- * variants.
+ *
+ * If @variant was floating then its floating reference is converted to
+ * a hard reference.
  *
  * If you want the #GValue to hold its own reference to @variant, use
  * g_value_set_variant() instead.
@@ -1227,11 +1225,13 @@ g_value_take_variant (GValue   *value,
   GVariant *old_variant;
 
   g_return_if_fail (G_VALUE_HOLDS_VARIANT (value));
-  g_return_if_fail (variant == NULL || !g_variant_is_floating (variant));
 
   old_variant = value->data[0].v_pointer;
 
-  value->data[0].v_pointer = variant;
+  if (variant)
+    value->data[0].v_pointer = g_variant_take_ref (variant);
+  else
+    value->data[0].v_pointer = NULL;
 
   if (old_variant)
     g_variant_unref (old_variant);