[glib] GVariant: Add a G_VARIANT_BUILDER_INIT macro

[Matthias Clasen <matthiasc src gnome org>]

commit e1c640f81934ad3b908354ace1fda7344c368c85
Author: Krzesimir Nowak <qdlacz gmail com>
Date:   Fri May 13 17:55:30 2016 +0200

    GVariant: Add a G_VARIANT_BUILDER_INIT macro
    
    The macro could be used at initialization time to avoid having an
    unitialized builder, especially with g_auto variables.
    
    The macro tries to be a bit more type-safe by making sure that the
    variant_type parameter is actually "const GVariantType
    *". Unfortunately I have no idea how to make it possible to also pass
    a "const gchar *" parameter without warning.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=766370

 docs/reference/glib/glib-sections.txt |    1 +
 glib/gvariant.c                       |   37 +++++++++++++++++++++++++++-----
 glib/gvariant.h                       |   34 +++++++++++++++++++++++++++++-
 glib/tests/gvariant.c                 |   22 +++++++++++++++++++
 4 files changed, 87 insertions(+), 7 deletions(-)
---
diff --git a/docs/reference/glib/glib-sections.txt b/docs/reference/glib/glib-sections.txt
index 4636036..577118a 100644
--- a/docs/reference/glib/glib-sections.txt
+++ b/docs/reference/glib/glib-sections.txt
@@ -3329,6 +3329,7 @@ g_variant_iter_next
 g_variant_iter_loop
 
 <SUBSECTION>
+G_VARIANT_BUILDER_INIT
 GVariantBuilder
 g_variant_builder_unref
 g_variant_builder_ref
diff --git a/glib/gvariant.c b/glib/gvariant.c
index ba34ff8..9dca358 100644
--- a/glib/gvariant.c
+++ b/glib/gvariant.c
@@ -3169,11 +3169,36 @@ struct heap_builder
 #define GVSB(b)                  ((struct stack_builder *) (b))
 #define GVHB(b)                  ((struct heap_builder *) (b))
 #define GVSB_MAGIC               ((gsize) 1033660112u)
+#define GVSB_MAGIC_PARTIAL       ((gsize) 2942751021u)
 #define GVHB_MAGIC               ((gsize) 3087242682u)
 #define is_valid_builder(b)      (b != NULL && \
                                   GVSB(b)->magic == GVSB_MAGIC)
 #define is_valid_heap_builder(b) (GVHB(b)->magic == GVHB_MAGIC)
 
+/* Just to make sure that by adding a union to GVariantBuilder, we
+ * didn't accidentally change ABI. */
+G_STATIC_ASSERT (sizeof (GVariantBuilder) == sizeof (gsize[16]));
+
+static gboolean
+ensure_valid_builder (GVariantBuilder *builder)
+{
+  if (is_valid_builder (builder))
+    return TRUE;
+  if (builder->partial_magic == GVSB_MAGIC_PARTIAL)
+    {
+      static GVariantBuilder cleared_builder;
+
+      /* Make sure that only first two fields were set and the rest is
+       * zeroed to avoid messing up the builder that had parent
+       * address equal to GVSB_MAGIC_PARTIAL. */
+      if (memcmp (cleared_builder.y, builder->y, sizeof cleared_builder.y))
+        return FALSE;
+
+      g_variant_builder_init (builder, builder->type);
+    }
+  return is_valid_builder (builder);
+}
+
 /**
  * g_variant_builder_new:
  * @type: a container type
@@ -3283,10 +3308,10 @@ g_variant_builder_clear (GVariantBuilder *builder)
   gsize i;
 
   if (GVSB(builder)->magic == 0)
-    /* all-zeros case */
+    /* all-zeros or partial case */
     return;
 
-  g_return_if_fail (is_valid_builder (builder));
+  g_return_if_fail (ensure_valid_builder (builder));
 
   g_variant_type_free (GVSB(builder)->type);
 
@@ -3449,7 +3474,7 @@ void
 g_variant_builder_add_value (GVariantBuilder *builder,
                              GVariant        *value)
 {
-  g_return_if_fail (is_valid_builder (builder));
+  g_return_if_fail (ensure_valid_builder (builder));
   g_return_if_fail (GVSB(builder)->offset < GVSB(builder)->max_items);
   g_return_if_fail (!GVSB(builder)->expected_type ||
                     g_variant_is_of_type (value,
@@ -3500,7 +3525,7 @@ g_variant_builder_open (GVariantBuilder    *builder,
 {
   GVariantBuilder *parent;
 
-  g_return_if_fail (is_valid_builder (builder));
+  g_return_if_fail (ensure_valid_builder (builder));
   g_return_if_fail (GVSB(builder)->offset < GVSB(builder)->max_items);
   g_return_if_fail (!GVSB(builder)->expected_type ||
                     g_variant_type_is_subtype_of (type,
@@ -3546,7 +3571,7 @@ g_variant_builder_close (GVariantBuilder *builder)
 {
   GVariantBuilder *parent;
 
-  g_return_if_fail (is_valid_builder (builder));
+  g_return_if_fail (ensure_valid_builder (builder));
   g_return_if_fail (GVSB(builder)->parent != NULL);
 
   parent = GVSB(builder)->parent;
@@ -3614,7 +3639,7 @@ g_variant_builder_end (GVariantBuilder *builder)
   GVariantType *my_type;
   GVariant *value;
 
-  g_return_val_if_fail (is_valid_builder (builder), NULL);
+  g_return_val_if_fail (ensure_valid_builder (builder), NULL);
   g_return_val_if_fail (GVSB(builder)->offset >= GVSB(builder)->min_items,
                         NULL);
   g_return_val_if_fail (!GVSB(builder)->uniform_item_types ||
diff --git a/glib/gvariant.h b/glib/gvariant.h
index fa0fee1..7d59213 100644
--- a/glib/gvariant.h
+++ b/glib/gvariant.h
@@ -297,7 +297,15 @@ gboolean                        g_variant_iter_loop                     (GVarian
 typedef struct _GVariantBuilder GVariantBuilder;
 struct _GVariantBuilder {
   /*< private >*/
-  gsize x[16];
+  union
+  {
+    struct {
+      gsize partial_magic;
+      const GVariantType *type;
+      gsize y[14];
+    };
+    gsize x[16];
+  };
 };
 
 typedef enum
@@ -329,6 +337,30 @@ GQuark                          g_variant_parser_get_error_quark        (void);
 GLIB_AVAILABLE_IN_ALL
 GQuark                          g_variant_parse_error_quark             (void);
 
+/**
+ * G_VARIANT_BUILDER_INIT:
+ * @variant_type: a const GVariantType*
+ *
+ * A stack-allocated #GVariantBuilder must be initialized if it is
+ * used together with g_auto() to avoid warnings or crashes if
+ * function returns before g_variant_builder_init() is called on the
+ * builder.  This macro can be used as initializer instead of an
+ * explicit zeroing a variable when declaring it and a following
+ * g_variant_builder_init(), but it cannot be assigned to a variable.
+ *
+ * The passed @variant_type should be a static GVariantType to avoid
+ * lifetime issues, as copying the @variant_type does not happen in
+ * the G_VARIANT_BUILDER_INIT() call, but rather in functions that
+ * make sure that #GVariantBuilder is valid.
+ *
+ * |[
+ *   g_auto(GVariantBuilder) builder = G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_BYTESTRING);
+ * ]|
+ *
+ * Since: 2.50
+ */
+#define G_VARIANT_BUILDER_INIT(variant_type) { { { 2942751021u, variant_type, { 0, } } } }
+
 GLIB_AVAILABLE_IN_ALL
 GVariantBuilder *               g_variant_builder_new                   (const GVariantType   *type);
 GLIB_AVAILABLE_IN_ALL
diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c
index 889b28b..5489733 100644
--- a/glib/tests/gvariant.c
+++ b/glib/tests/gvariant.c
@@ -4533,6 +4533,27 @@ G_GNUC_BEGIN_IGNORE_DEPRECATIONS
 G_GNUC_END_IGNORE_DEPRECATIONS
 }
 
+static void
+test_stack_builder_init (void)
+{
+  GVariantBuilder builder = G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_BYTESTRING);
+  GVariant *variant;
+
+  g_variant_builder_add_value (&builder, g_variant_new_byte ('g'));
+  g_variant_builder_add_value (&builder, g_variant_new_byte ('l'));
+  g_variant_builder_add_value (&builder, g_variant_new_byte ('i'));
+  g_variant_builder_add_value (&builder, g_variant_new_byte ('b'));
+  g_variant_builder_add_value (&builder, g_variant_new_byte ('\0'));
+
+  variant = g_variant_ref_sink (g_variant_builder_end (&builder));
+  g_assert_nonnull (variant);
+  g_assert (g_variant_type_equal (g_variant_get_type (variant),
+                                  G_VARIANT_TYPE_BYTESTRING));
+  g_assert_cmpuint (g_variant_n_children (variant), ==, 5);
+  g_assert_cmpstr (g_variant_get_bytestring (variant), ==, "glib");
+  g_variant_unref (variant);
+}
+
 int
 main (int argc, char **argv)
 {
@@ -4592,5 +4613,6 @@ main (int argc, char **argv)
   g_test_add_func ("/gvariant/print-context", test_print_context);
   g_test_add_func ("/gvariant/error-quark", test_error_quark);
 
+  g_test_add_func ("/gvariant/stack-builder-init", test_stack_builder_init);
   return g_test_run ();
 }