[glib] gsettings: Fix leaks and assertion on range binding failures

[Philip Withnall <pwithnall src gnome org>]

commit fbbad525a5344a0c19f75470218f55b652143b5b
Author: Christophe Fergeau <cfergeau redhat com>
Date:   Thu Mar 29 13:29:21 2018 +0200

    gsettings: Fix leaks and assertion on range binding failures
    
    When using g_settings_bind(), if a range binding triggers a range check
    failure, g_settings_binding_property_changed() will return early, but it
    won't cleanup properly causing some leaks. The binding will also still
    be marked as 'running', which causes an assertion failure when trying to
    free it:
    "g_settings_binding_free: assertion failed: (!binding->running)"
    
    Signed-off-by: Christophe Fergeau <cfergeau redhat com>
    
    https://bugzilla.gnome.org/show_bug.cgi?id=794805

 gio/gsettings.c                         | 22 +++++++++++-----
 gio/tests/gsettings.c                   | 46 +++++++++++++++++++++++++++++++++
 gio/tests/org.gtk.test.gschema.xml.orig |  4 +++
 3 files changed, 66 insertions(+), 6 deletions(-)
---
diff --git a/gio/gsettings.c b/gio/gsettings.c
index f05471c61..87b38bcbb 100644
--- a/gio/gsettings.c
+++ b/gio/gsettings.c
@@ -2682,6 +2682,7 @@ g_settings_binding_property_changed (GObject          *object,
   GSettingsBinding *binding = user_data;
   GValue value = G_VALUE_INIT;
   GVariant *variant;
+  gboolean valid = TRUE;
 
   g_assert (object == binding->object);
   g_assert (pspec == binding->property);
@@ -2700,24 +2701,33 @@ g_settings_binding_property_changed (GObject          *object,
 
       if (!g_settings_schema_key_type_check (&binding->key, variant))
         {
+          gchar *type_str;
+          type_str = g_variant_type_dup_string (binding->key.type);
           g_critical ("binding mapping function for key '%s' returned "
                       "GVariant of type '%s' when type '%s' was requested",
                       binding->key.name, g_variant_get_type_string (variant),
-                      g_variant_type_dup_string (binding->key.type));
-          return;
+                      type_str);
+          g_free (type_str);
+          valid = FALSE;
         }
 
-      if (!g_settings_schema_key_range_check (&binding->key, variant))
+      if (valid && !g_settings_schema_key_range_check (&binding->key, variant))
         {
+          gchar *variant_str;
+          variant_str = g_variant_print (variant, TRUE);
           g_critical ("GObject property '%s' on a '%s' object is out of "
                       "schema-specified range for key '%s' of '%s': %s",
                       binding->property->name, g_type_name (binding->property->owner_type),
                       binding->key.name, g_settings_schema_get_id (binding->key.schema),
-                      g_variant_print (variant, TRUE));
-          return;
+                      variant_str);
+          g_free (variant_str);
+          valid = FALSE;
         }
 
-      g_settings_write_to_backend (binding->settings, &binding->key, variant);
+      if (valid)
+        {
+          g_settings_write_to_backend (binding->settings, &binding->key, variant);
+        }
       g_variant_unref (variant);
     }
   g_value_unset (&value);
diff --git a/gio/tests/gsettings.c b/gio/tests/gsettings.c
index 2be4122fe..2f056ee95 100644
--- a/gio/tests/gsettings.c
+++ b/gio/tests/gsettings.c
@@ -1324,6 +1324,33 @@ test_simple_binding (void)
   g_object_get (obj, "flags", &i, NULL);
   g_assert_cmpint (i, ==, TEST_FLAGS_MOURNING | TEST_FLAGS_WALKING);
 
+  g_settings_bind (settings, "uint", obj, "uint", G_SETTINGS_BIND_DEFAULT);
+
+  g_object_set (obj, "uint", 12345, NULL);
+  g_assert_cmpuint (g_settings_get_uint (settings, "uint"), ==, 12345);
+
+  g_settings_set_uint (settings, "uint", 54321);
+  u = 1111;
+  g_object_get (obj, "uint", &u, NULL);
+  g_assert_cmpuint (u, ==, 54321);
+
+  g_settings_bind (settings, "range", obj, "uint", G_SETTINGS_BIND_DEFAULT);
+  g_object_set (obj, "uint", 22, NULL);
+  u = 1111;
+  g_assert_cmpuint (g_settings_get_uint (settings, "range"), ==, 22);
+  g_object_get (obj, "uint", &u, NULL);
+  g_assert_cmpuint (u, ==, 22);
+
+  g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
+                         "* is out of schema-specified range for*");
+  g_object_set (obj, "uint", 45, NULL);
+  g_test_assert_expected_messages ();
+  u = 1111;
+  g_object_get (obj, "uint", &u, NULL);
+  g_assert_cmpuint (g_settings_get_uint (settings, "range"), ==, 22);
+  /* The value of the object is currently not reset back to its initial value
+  g_assert_cmpuint (u, ==, 22); */
+
   g_object_unref (obj);
   g_object_unref (settings);
 }
@@ -1472,6 +1499,14 @@ bool_to_string (const GValue       *value,
     return g_variant_new_string ("false");
 }
 
+static GVariant *
+bool_to_bool (const GValue       *value,
+              const GVariantType *expected_type,
+              gpointer            user_data)
+{
+  return g_variant_new_boolean (g_value_get_boolean (value));
+}
+
 /* Test custom bindings.
  * Translate strings to booleans and back
  */
@@ -1508,6 +1543,17 @@ test_custom_binding (void)
   g_assert_cmpstr (s, ==, "true");
   g_free (s);
 
+  g_settings_bind_with_mapping (settings, "string",
+                                obj, "bool",
+                                G_SETTINGS_BIND_DEFAULT,
+                                string_to_bool, bool_to_bool,
+                                NULL, NULL);
+  g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
+                         "*binding mapping function for key 'string' returned"
+                         " GVariant of type 'b' when type 's' was requested*");
+  g_object_set (obj, "bool", FALSE, NULL);
+  g_test_assert_expected_messages ();
+
   g_object_unref (obj);
   g_object_unref (settings);
 }
diff --git a/gio/tests/org.gtk.test.gschema.xml.orig b/gio/tests/org.gtk.test.gschema.xml.orig
index c07558335..3c9d7b779 100644
--- a/gio/tests/org.gtk.test.gschema.xml.orig
+++ b/gio/tests/org.gtk.test.gschema.xml.orig
@@ -131,6 +131,10 @@
     <key name="flags" flags="org.gtk.test.TestFlags">
       <default>['mourning', 'laughing']</default>
     </key>
+    <key name="range" type='u'>
+      <default>33</default>
+      <range min="2" max="44"/>
+    </key>
   </schema>
 
   <schema id='org.gtk.test.enums' path='/tests/enums/'>