[glib/wip/gvariant-kdbus: 2/6] GVariant: add internal tree-form locking helper

[Ryan Lortie <desrt src gnome org>]

commit ffdea05e4a71d6e3213be6b108693eb75af6fb05
Author: Ryan Lortie <desrt desrt ca>
Date:   Fri Nov 28 15:42:34 2014 -0500

    GVariant: add internal tree-form locking helper
    
    Many of the core GVariant operations have two modes: one for tree-form
    and one for serialised.
    
    Once a GVariant is in serialised form it will always be serialised, so
    it is safe to simply check for that and proceed with the operation in
    that case.
    
    A tree-form GVariant instance always has a chance of being implicitly
    serialised, however, so we have to take locks when performing operations
    on these.
    
    Write a helper function that reliably checks if the instance is in
    tree-form, locking it if it is.  Rewrite some of the other functions to
    use this helper.  In some cases this simplifies the code and in others
    it reduces locking.

 glib/gvariant-core.c |  126 ++++++++++++++++++++++++++++---------------------
 1 files changed, 72 insertions(+), 54 deletions(-)
---
diff --git a/glib/gvariant-core.c b/glib/gvariant-core.c
index ae95db7..19732c1 100644
--- a/glib/gvariant-core.c
+++ b/glib/gvariant-core.c
@@ -249,6 +249,31 @@ g_variant_release_children (GVariant *value)
   g_free (value->contents.tree.children);
 }
 
+/* < private >
+ * g_variant_lock_in_tree_form:
+ * @value: a #GVariant
+ *
+ * Locks @value if it is in tree form.
+ *
+ * Returns: %TRUE if @value is now in tree form with the lock acquired
+ */
+static gboolean
+g_variant_lock_in_tree_form (GVariant *value)
+{
+  if (g_atomic_int_get (&value->state) & STATE_SERIALISED)
+    return FALSE;
+
+  g_variant_lock (value);
+
+  if (value->state & STATE_SERIALISED)
+    {
+      g_variant_unlock (value);
+      return FALSE;
+    }
+
+  return TRUE;
+}
+
 /* This begins the main body of the recursive serialiser.
  *
  * There are 3 functions here that work as a team with the serialiser to
@@ -871,9 +896,12 @@ g_variant_n_children (GVariant *value)
 {
   gsize n_children;
 
-  g_variant_lock (value);
-
-  if (value->state & STATE_SERIALISED)
+  if (g_variant_lock_in_tree_form (value))
+    {
+      n_children = value->contents.tree.n_children;
+      g_variant_unlock (value);
+    }
+  else
     {
       GVariantSerialised serialised = {
         value->type_info,
@@ -883,10 +911,6 @@ g_variant_n_children (GVariant *value)
 
       n_children = g_variant_serialised_n_children (serialised);
     }
-  else
-    n_children = value->contents.tree.n_children;
-
-  g_variant_unlock (value);
 
   return n_children;
 }
@@ -917,52 +941,43 @@ GVariant *
 g_variant_get_child_value (GVariant *value,
                            gsize     index_)
 {
+  GVariant *child;
+
   g_return_val_if_fail (index_ < g_variant_n_children (value), NULL);
 
-  if (~g_atomic_int_get (&value->state) & STATE_SERIALISED)
+  if (g_variant_lock_in_tree_form (value))
     {
-      g_variant_lock (value);
-
-      if (~value->state & STATE_SERIALISED)
-        {
-          GVariant *child;
-
-          child = g_variant_ref (value->contents.tree.children[index_]);
-          g_variant_unlock (value);
-
-          return child;
-        }
 
+      child = g_variant_ref (value->contents.tree.children[index_]);
       g_variant_unlock (value);
     }
+  else
+    {
+      GVariantSerialised serialised = {
+        value->type_info,
+        (gpointer) value->contents.serialised.data,
+        value->size
+      };
+      GVariantSerialised s_child;
 
-  {
-    GVariantSerialised serialised = {
-      value->type_info,
-      (gpointer) value->contents.serialised.data,
-      value->size
-    };
-    GVariantSerialised s_child;
-    GVariant *child;
-
-    /* get the serialiser to extract the serialised data for the child
-     * from the serialised data for the container
-     */
-    s_child = g_variant_serialised_get_child (serialised, index_);
-
-    /* create a new serialised instance out of it */
-    child = g_slice_new (GVariant);
-    child->type_info = s_child.type_info;
-    child->state = (value->state & STATE_TRUSTED) |
-                   STATE_SERIALISED;
-    child->size = s_child.size;
-    child->ref_count = 1;
-    child->contents.serialised.bytes =
-      g_bytes_ref (value->contents.serialised.bytes);
-    child->contents.serialised.data = s_child.data;
-
-    return child;
-  }
+      /* get the serialiser to extract the serialised data for the child
+       * from the serialised data for the container
+       */
+      s_child = g_variant_serialised_get_child (serialised, index_);
+
+      /* create a new serialised instance out of it */
+      child = g_slice_new (GVariant);
+      child->type_info = s_child.type_info;
+      child->state = (value->state & STATE_TRUSTED) |
+                     STATE_SERIALISED;
+      child->size = s_child.size;
+      child->ref_count = 1;
+      child->contents.serialised.bytes =
+        g_bytes_ref (value->contents.serialised.bytes);
+      child->contents.serialised.data = s_child.data;
+    }
+
+  return child;
 }
 
 /**
@@ -989,19 +1004,18 @@ void
 g_variant_store (GVariant *value,
                  gpointer  data)
 {
-  g_variant_lock (value);
-
-  if (value->state & STATE_SERIALISED)
+  if (g_variant_lock_in_tree_form (value))
+    {
+      g_variant_serialise (value, data);
+      g_variant_unlock (value);
+    }
+  else
     {
       if (value->contents.serialised.data != NULL)
         memcpy (data, value->contents.serialised.data, value->size);
       else
         memset (data, 0, value->size);
     }
-  else
-    g_variant_serialise (value, data);
-
-  g_variant_unlock (value);
 }
 
 /**
@@ -1026,9 +1040,13 @@ g_variant_store (GVariant *value,
 gboolean
 g_variant_is_normal_form (GVariant *value)
 {
-  if (value->state & STATE_TRUSTED)
+  if (g_atomic_int_get (&value->state) & STATE_TRUSTED)
     return TRUE;
 
+  /* We always take the lock here because we expect to find that the
+   * value is in normal form and in that case, we need to update the
+   * state, which requires holding the lock.
+   */
   g_variant_lock (value);
 
   if (value->state & STATE_SERIALISED)