[glib] GVariant: add g_variant_take_ref()

[Ryan Lortie <ryanl src gnome org>]

commit 58c247e51bfc7d7ff4ed3b351ba4fdab0f012bac
Author: Ryan Lortie <desrt desrt ca>
Date:   Mon Jul 11 14:27:53 2011 +0200

    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 |    1 +
 gio/gdbus-codegen/codegen.py          |    3 +-
 gio/gdbusconnection.c                 |    6 +--
 gio/gsettings.c                       |    3 +-
 glib/glib.symbols                     |    1 +
 glib/gvariant-core.c                  |   51 ++++++++++++++++++++++++++++++++-
 glib/gvariant.h                       |    1 +
 gobject/gvaluetypes.c                 |   14 ++++----
 8 files changed, 64 insertions(+), 16 deletions(-)
---
diff --git a/docs/reference/glib/glib-sections.txt b/docs/reference/glib/glib-sections.txt
index 0a129ad..0eed0a7 100644
--- a/docs/reference/glib/glib-sections.txt
+++ b/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
diff --git a/gio/gdbus-codegen/codegen.py b/gio/gdbus-codegen/codegen.py
index c4112e5..6a2d620 100644
--- a/gio/gdbus-codegen/codegen.py
+++ b/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'
diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c
index 951bb2d..37e9b80 100644
--- a/gio/gdbusconnection.c
+++ b/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,
diff --git a/gio/gsettings.c b/gio/gsettings.c
index fc7e074..24dc5ca 100644
--- a/gio/gsettings.c
+++ b/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))
         {
diff --git a/glib/glib.symbols b/glib/glib.symbols
index a0c902f..f82c6dd 100644
--- a/glib/glib.symbols
+++ b/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
diff --git a/glib/gvariant-core.c b/glib/gvariant-core.c
index 2111561..e967701 100644
--- a/glib/gvariant-core.c
+++ b/glib/gvariant-core.c
@@ -697,6 +697,54 @@ g_variant_ref_sink (GVariant *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
  * @returns: whether @value is floating
@@ -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.
diff --git a/glib/gvariant.h b/glib/gvariant.h
index c894614..f4f49bc 100644
--- a/glib/gvariant.h
+++ b/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);
diff --git a/gobject/gvaluetypes.c b/gobject/gvaluetypes.c
index bff6592..c870445 100644
--- a/gobject/gvaluetypes.c
+++ b/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);