GVariant: refactor locking a bit more

[platform/upstream/glib.git] / glib / gvariant-core.c

diff --git a/glib/gvariant-core.c b/glib/gvariant-core.c
index 8ea4269..4ab4238 100644 (file)
--- 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
@@ -376,15 +401,11 @@ g_variant_fill_gvs (GVariantSerialised *serialised,
  * that size and serialises the instance into the buffer.  The
  * 'children' array is then released and the instance is set to
  * serialised form based on the contents of the buffer.
- *
- * The current thread must hold the lock on @value.
  */
 static void
 g_variant_ensure_serialised (GVariant *value)
 {
-  g_assert (value->state & STATE_LOCKED);
-
-  if (~value->state & STATE_SERIALISED)
+  if (g_variant_lock_in_tree_form (value))
     {
       GBytes *bytes;
       gpointer data;
@@ -398,6 +419,8 @@ g_variant_ensure_serialised (GVariant *value)
       value->contents.serialised.data = g_bytes_get_data (bytes, NULL);
       value->contents.serialised.bytes = bytes;
       value->state |= STATE_SERIALISED;
+
+      g_variant_unlock (value);
     }
 }
 
@@ -801,9 +824,7 @@ g_variant_get_size (GVariant *value)
 gconstpointer
 g_variant_get_data (GVariant *value)
 {
-  g_variant_lock (value);
   g_variant_ensure_serialised (value);
-  g_variant_unlock (value);
 
   return value->contents.serialised.data;
 }
@@ -829,9 +850,7 @@ g_variant_get_data_as_bytes (GVariant *value)
   gsize bytes_size;
   gsize size;
 
-  g_variant_lock (value);
   g_variant_ensure_serialised (value);
-  g_variant_unlock (value);
 
   bytes_data = g_bytes_get_data (value->contents.serialised.bytes, &bytes_size);
   data = value->contents.serialised.data;
@@ -870,9 +889,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,
@@ -882,10 +904,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;
 }
@@ -916,52 +934,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;
 }
 
 /**
@@ -988,19 +997,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);
 }
 
 /**
@@ -1025,9 +1033,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)