From cc4de801c9643b62c5566d4d9dc3439ca801a4e6 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 4 May 2018 10:13:13 +0100 Subject: [PATCH] gobject: Reimplement g_param_values_cmp() for GParamSpecVariant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existing implementation was completely incorrect (despite the fix in commit 566e64a66) — it always compared GVariants by pointer, rather than by value. Reimplement it to compare them by value where possible, depending on their type. The core of this implementation is g_variant_compare(). See the documentation and tests for further details of the new sort order. This adds documentation and tests. Signed-off-by: Philip Withnall https://bugzilla.gnome.org/show_bug.cgi?id=795735 --- gobject/gparamspecs.c | 28 ++++++++- gobject/gparamspecs.h | 6 ++ tests/gobject/paramspec-test.c | 104 ++++++++++++++++++++++++++++++++- 3 files changed, 134 insertions(+), 4 deletions(-) diff --git a/gobject/gparamspecs.c b/gobject/gparamspecs.c index 8c285fa1c..d76e12f55 100644 --- a/gobject/gparamspecs.c +++ b/gobject/gparamspecs.c @@ -1147,6 +1147,20 @@ param_variant_validate (GParamSpec *pspec, return FALSE; } +/* g_variant_compare() can only be used with scalar types. */ +static gboolean +variant_is_incomparable (GVariant *v) +{ + GVariantClass v_class = g_variant_classify (v); + + return (v_class == G_VARIANT_CLASS_HANDLE || + v_class == G_VARIANT_CLASS_VARIANT || + v_class == G_VARIANT_CLASS_MAYBE|| + v_class == G_VARIANT_CLASS_ARRAY || + v_class == G_VARIANT_CLASS_TUPLE || + v_class == G_VARIANT_CLASS_DICT_ENTRY); +} + static gint param_variant_values_cmp (GParamSpec *pspec, const GValue *value1, @@ -1155,7 +1169,19 @@ param_variant_values_cmp (GParamSpec *pspec, GVariant *v1 = value1->data[0].v_pointer; GVariant *v2 = value2->data[0].v_pointer; - return v1 < v2 ? -1 : v2 > v1; + if (v1 == NULL && v2 == NULL) + return 0; + else if (v1 == NULL && v2 != NULL) + return -1; + else if (v1 != NULL && v2 == NULL) + return 1; + + if (!g_variant_type_equal (g_variant_get_type (v1), g_variant_get_type (v2)) || + variant_is_incomparable (v1) || + variant_is_incomparable (v2)) + return g_variant_equal (v1, v2) ? 0 : (v1 < v2 ? -1 : 1); + + return g_variant_compare (v1, v2); } /* --- type initialization --- */ diff --git a/gobject/gparamspecs.h b/gobject/gparamspecs.h index e2bb62130..26045a368 100644 --- a/gobject/gparamspecs.h +++ b/gobject/gparamspecs.h @@ -962,6 +962,12 @@ struct _GParamSpecGType * * A #GParamSpec derived structure that contains the meta data for #GVariant properties. * + * When comparing values with g_param_values_cmp(), scalar values with the same + * type will be compared with g_variant_compare(). Other non-%NULL variants will + * be checked for equality with g_variant_equal(), and their sort order is + * otherwise undefined. %NULL is ordered before non-%NULL variants. Two %NULL + * values compare equal. + * * Since: 2.26 */ struct _GParamSpecVariant diff --git a/tests/gobject/paramspec-test.c b/tests/gobject/paramspec-test.c index 02a964bfa..a794df338 100644 --- a/tests/gobject/paramspec-test.c +++ b/tests/gobject/paramspec-test.c @@ -228,6 +228,10 @@ test_param_spec_variant (void) { GParamSpec *pspec; GValue value = G_VALUE_INIT; + GValue value2 = G_VALUE_INIT; + GValue value3 = G_VALUE_INIT; + GValue value4 = G_VALUE_INIT; + GValue value5 = G_VALUE_INIT; gboolean modified; pspec = g_param_spec_variant ("variant", "nick", "blurb", @@ -238,21 +242,114 @@ test_param_spec_variant (void) g_value_init (&value, G_TYPE_VARIANT); g_value_set_variant (&value, g_variant_new_int32 (42)); - g_assert (g_param_value_defaults (pspec, &value)); + g_value_init (&value2, G_TYPE_VARIANT); + g_value_set_variant (&value2, g_variant_new_int32 (43)); + + g_value_init (&value3, G_TYPE_VARIANT); + g_value_set_variant (&value3, g_variant_new_int16 (42)); + + g_value_init (&value4, G_TYPE_VARIANT); + g_value_set_variant (&value4, g_variant_new_parsed ("[@u 15, @u 10]")); + + g_value_init (&value5, G_TYPE_VARIANT); + g_value_set_variant (&value5, NULL); + + g_assert_true (g_param_value_defaults (pspec, &value)); + g_assert_false (g_param_value_defaults (pspec, &value2)); + g_assert_false (g_param_value_defaults (pspec, &value3)); + g_assert_false (g_param_value_defaults (pspec, &value4)); + g_assert_false (g_param_value_defaults (pspec, &value5)); modified = g_param_value_validate (pspec, &value); - g_assert (!modified); + g_assert_false (modified); g_value_reset (&value); g_value_set_variant (&value, g_variant_new_uint32 (41)); modified = g_param_value_validate (pspec, &value); - g_assert (modified); + g_assert_true (modified); g_assert_cmpint (g_variant_get_int32 (g_value_get_variant (&value)), ==, 42); g_value_unset (&value); + g_value_unset (&value5); + g_value_unset (&value4); + g_value_unset (&value3); + g_value_unset (&value2); + g_param_spec_unref (pspec); } +/* Test g_param_values_cmp() for #GParamSpecVariant. */ +static void +test_param_spec_variant_cmp (void) +{ + const struct + { + const GVariantType *pspec_type; + const gchar *v1; + enum + { + LESS_THAN = -1, + EQUAL = 0, + GREATER_THAN = 1, + NOT_EQUAL, + } expected_result; + const gchar *v2; + } + vectors[] = + { + { G_VARIANT_TYPE ("i"), "@i 1", LESS_THAN, "@i 2" }, + { G_VARIANT_TYPE ("i"), "@i 2", EQUAL, "@i 2" }, + { G_VARIANT_TYPE ("i"), "@i 3", GREATER_THAN, "@i 2" }, + { G_VARIANT_TYPE ("i"), NULL, LESS_THAN, "@i 2" }, + { G_VARIANT_TYPE ("i"), NULL, EQUAL, NULL }, + { G_VARIANT_TYPE ("i"), "@i 1", GREATER_THAN, NULL }, + { G_VARIANT_TYPE ("i"), "@u 1", LESS_THAN, "@u 2" }, + { G_VARIANT_TYPE ("i"), "@as ['hi']", NOT_EQUAL, "@u 2" }, + { G_VARIANT_TYPE ("i"), "@as ['hi']", NOT_EQUAL, "@as ['there']" }, + { G_VARIANT_TYPE ("i"), "@as ['hi']", EQUAL, "@as ['hi']" }, + }; + gsize i; + + for (i = 0; i < G_N_ELEMENTS (vectors); i++) + { + GParamSpec *pspec; + GValue v1 = G_VALUE_INIT; + GValue v2 = G_VALUE_INIT; + gint cmp; + + pspec = g_param_spec_variant ("variant", "nick", "blurb", + vectors[i].pspec_type, + NULL, + G_PARAM_READWRITE); + + g_value_init (&v1, G_TYPE_VARIANT); + g_value_set_variant (&v1, (vectors[i].v1 != NULL) ? g_variant_new_parsed (vectors[i].v1) : NULL); + + g_value_init (&v2, G_TYPE_VARIANT); + g_value_set_variant (&v2, (vectors[i].v2 != NULL) ? g_variant_new_parsed (vectors[i].v2) : NULL); + + cmp = g_param_values_cmp (pspec, &v1, &v2); + + switch (vectors[i].expected_result) + { + case LESS_THAN: + case EQUAL: + case GREATER_THAN: + g_assert_cmpint (cmp, ==, vectors[i].expected_result); + break; + case NOT_EQUAL: + g_assert_cmpint (cmp, !=, 0); + break; + default: + g_assert_not_reached (); + } + + g_value_unset (&v2); + g_value_unset (&v1); + g_param_spec_unref (pspec); + } +} + int main (int argc, char *argv[]) { @@ -263,6 +360,7 @@ main (int argc, char *argv[]) g_test_add_func ("/paramspec/override", test_param_spec_override); g_test_add_func ("/paramspec/gtype", test_param_spec_gtype); g_test_add_func ("/paramspec/variant", test_param_spec_variant); + g_test_add_func ("/paramspec/variant/cmp", test_param_spec_variant_cmp); return g_test_run (); }