From 992ded39bf75ce08a31e4dc7085ec8d4c9daefa0 Mon Sep 17 00:00:00 2001 From: Krzesimir Nowak Date: Sat, 4 Jun 2016 15:01:31 +0200 Subject: [PATCH] GVariant: Add a G_VARIANT_DICT_INIT macro The macro could be used at initialization time to avoid having an unitialized dict, especially with g_auto variables. The macro tries to be a bit more type-safe by making sure that the asv parameter is actually "GVariant *". https://bugzilla.gnome.org/show_bug.cgi?id=766370 --- glib/gvariant.c | 41 ++++++++++++++++++++++++++++++-------- glib/gvariant.h | 39 +++++++++++++++++++++++++++++++++++- glib/tests/gvariant.c | 46 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 9 deletions(-) diff --git a/glib/gvariant.c b/glib/gvariant.c index 9dca358f6..3bdaa736f 100644 --- a/glib/gvariant.c +++ b/glib/gvariant.c @@ -3796,11 +3796,36 @@ struct heap_dict #define GVSD(d) ((struct stack_dict *) (d)) #define GVHD(d) ((struct heap_dict *) (d)) #define GVSD_MAGIC ((gsize) 2579507750u) +#define GVSD_MAGIC_PARTIAL ((gsize) 3488698669u) #define GVHD_MAGIC ((gsize) 2450270775u) #define is_valid_dict(d) (d != NULL && \ GVSD(d)->magic == GVSD_MAGIC) #define is_valid_heap_dict(d) (GVHD(d)->magic == GVHD_MAGIC) +/* Just to make sure that by adding a union to GVariantDict, we didn't + * accidentally change ABI. */ +G_STATIC_ASSERT (sizeof (GVariantDict) == sizeof (gsize[16])); + +static gboolean +ensure_valid_dict (GVariantDict *dict) +{ + if (is_valid_dict (dict)) + return TRUE; + if (dict->partial_magic == GVSD_MAGIC_PARTIAL) + { + static GVariantDict cleared_dict; + + /* 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_dict.y, dict->y, sizeof cleared_dict.y)) + return FALSE; + + g_variant_dict_init (dict, dict->asv); + } + return is_valid_dict (dict); +} + /** * g_variant_dict_new: * @from_asv: (allow-none): the #GVariant with which to initialise the @@ -3908,7 +3933,7 @@ g_variant_dict_lookup (GVariantDict *dict, GVariant *value; va_list ap; - g_return_val_if_fail (is_valid_dict (dict), FALSE); + g_return_val_if_fail (ensure_valid_dict (dict), FALSE); g_return_val_if_fail (key != NULL, FALSE); g_return_val_if_fail (format_string != NULL, FALSE); @@ -3953,7 +3978,7 @@ g_variant_dict_lookup_value (GVariantDict *dict, { GVariant *result; - g_return_val_if_fail (is_valid_dict (dict), NULL); + g_return_val_if_fail (ensure_valid_dict (dict), NULL); g_return_val_if_fail (key != NULL, NULL); result = g_hash_table_lookup (GVSD(dict)->values, key); @@ -3979,7 +4004,7 @@ gboolean g_variant_dict_contains (GVariantDict *dict, const gchar *key) { - g_return_val_if_fail (is_valid_dict (dict), FALSE); + g_return_val_if_fail (ensure_valid_dict (dict), FALSE); g_return_val_if_fail (key != NULL, FALSE); return g_hash_table_contains (GVSD(dict)->values, key); @@ -4007,7 +4032,7 @@ g_variant_dict_insert (GVariantDict *dict, { va_list ap; - g_return_if_fail (is_valid_dict (dict)); + g_return_if_fail (ensure_valid_dict (dict)); g_return_if_fail (key != NULL); g_return_if_fail (format_string != NULL); @@ -4033,7 +4058,7 @@ g_variant_dict_insert_value (GVariantDict *dict, const gchar *key, GVariant *value) { - g_return_if_fail (is_valid_dict (dict)); + g_return_if_fail (ensure_valid_dict (dict)); g_return_if_fail (key != NULL); g_return_if_fail (value != NULL); @@ -4055,7 +4080,7 @@ gboolean g_variant_dict_remove (GVariantDict *dict, const gchar *key) { - g_return_val_if_fail (is_valid_dict (dict), FALSE); + g_return_val_if_fail (ensure_valid_dict (dict), FALSE); g_return_val_if_fail (key != NULL, FALSE); return g_hash_table_remove (GVSD(dict)->values, key); @@ -4089,7 +4114,7 @@ g_variant_dict_clear (GVariantDict *dict) /* all-zeros case */ return; - g_return_if_fail (is_valid_dict (dict)); + g_return_if_fail (ensure_valid_dict (dict)); g_hash_table_unref (GVSD(dict)->values); GVSD(dict)->values = NULL; @@ -4120,7 +4145,7 @@ g_variant_dict_end (GVariantDict *dict) GHashTableIter iter; gpointer key, value; - g_return_val_if_fail (is_valid_dict (dict), NULL); + g_return_val_if_fail (ensure_valid_dict (dict), NULL); g_variant_builder_init (&builder, G_VARIANT_TYPE_VARDICT); diff --git a/glib/gvariant.h b/glib/gvariant.h index 7d59213c1..ea4546602 100644 --- a/glib/gvariant.h +++ b/glib/gvariant.h @@ -436,9 +436,46 @@ gint g_variant_compare (gconstp typedef struct _GVariantDict GVariantDict; struct _GVariantDict { /*< private >*/ - gsize x[16]; + union + { + struct { + GVariant *asv; + gsize partial_magic; + gsize y[14]; + }; + gsize x[16]; + }; }; +/** + * G_VARIANT_DICT_INIT: + * @asv: (nullable): a GVariant* + * + * A stack-allocated #GVariantDict must be initialized if it is used + * together with g_auto() to avoid warnings or crashes if function + * returns before g_variant_dict_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_dict_init(), but it cannot be assigned to a variable. + * + * The passed @asv has to live long enough for #GVariantDict to gather + * the entries from, as the gathering does not happen in the + * G_VARIANT_DICT_INIT() call, but rather in functions that make sure + * that #GVariantDict is valid. In context where the initialization + * value has to be a constant expression, the only possible value of + * @asv is %NULL. It is still possible to call g_variant_dict_init() + * safely with a different @asv right after the variable was + * initialized with G_VARIANT_DICT_INIT(). + * + * |[ + * g_autoptr(GVariant) variant = get_asv_variant (); + * g_auto(GVariantDict) dict = G_VARIANT_DICT_INIT (variant); + * ]| + * + * Since: 2.50 + */ +#define G_VARIANT_DICT_INIT(asv) { { { asv, 3488698669u, { 0, } } } } + GLIB_AVAILABLE_IN_2_40 GVariantDict * g_variant_dict_new (GVariant *from_asv); diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c index 548973333..dc738e0e1 100644 --- a/glib/tests/gvariant.c +++ b/glib/tests/gvariant.c @@ -4554,6 +4554,51 @@ test_stack_builder_init (void) g_variant_unref (variant); } +static GVariant * +get_asv (void) +{ + GVariantBuilder builder = G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_VARDICT); + + g_variant_builder_add (&builder, "{s@v}", "foo", g_variant_new_variant (g_variant_new_string ("FOO"))); + g_variant_builder_add (&builder, "{s@v}", "bar", g_variant_new_variant (g_variant_new_string ("BAR"))); + + return g_variant_ref_sink (g_variant_builder_end (&builder)); +} + +static void +test_stack_dict_init (void) +{ + GVariant *asv = get_asv (); + GVariantDict dict = G_VARIANT_DICT_INIT (asv); + GVariant *variant; + GVariantIter iter; + gchar *key; + GVariant *value; + + g_variant_dict_insert_value (&dict, "baz", g_variant_new_string ("BAZ")); + g_variant_dict_insert_value (&dict, "quux", g_variant_new_string ("QUUX")); + + variant = g_variant_ref_sink (g_variant_dict_end (&dict)); + g_assert_nonnull (variant); + g_assert (g_variant_type_equal (g_variant_get_type (variant), + G_VARIANT_TYPE_VARDICT)); + g_assert_cmpuint (g_variant_n_children (variant), ==, 4); + + g_variant_iter_init (&iter, variant); + while (g_variant_iter_next (&iter, "{sv}", &key, &value)) + { + gchar *strup = g_ascii_strup (key, -1); + + g_assert_cmpstr (strup, ==, g_variant_get_string (value, NULL)); + g_free (key); + g_free (strup); + g_variant_unref (value); + } + + g_variant_unref (asv); + g_variant_unref (variant); +} + int main (int argc, char **argv) { @@ -4614,5 +4659,6 @@ main (int argc, char **argv) g_test_add_func ("/gvariant/error-quark", test_error_quark); g_test_add_func ("/gvariant/stack-builder-init", test_stack_builder_init); + g_test_add_func ("/gvariant/stack-dict-init", test_stack_dict_init); return g_test_run (); }