[grilo] operation-options: bypass checks for NULL GValues



commit 3f971c40137c18f364580ef640e054254fd011ca
Author: Victor Toso <victortoso gnome org>
Date:   Wed Oct 6 13:40:57 2021 +0200

    operation-options: bypass checks for NULL GValues
    
    grl_registry_metadata_key_clamp() is a helper to check if a given
    @value is between @min and @max. Only @min and @max should be well
    defined values to be compared with. We can simply bypass when @value
    is NULL (which means, @value is not changed).
    
    This comes with a unit test that shows issue #148 but also highlights
    some other bugs around the code. Those will be discussed in its own
    issue trackers.
    
    Resolves: https://gitlab.gnome.org/GNOME/grilo/-/issues/148

 src/grl-registry.c | 10 ++++++++++
 tests/operations.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)
---
diff --git a/src/grl-registry.c b/src/grl-registry.c
index 9caf9e37..c994ed2f 100644
--- a/src/grl-registry.c
+++ b/src/grl-registry.c
@@ -2289,6 +2289,9 @@ grl_registry_metadata_key_get_limits(GrlRegistry *registry,
   return FALSE;
 }
 
+/* @max and @min are expected to be initialized with G_VALUE_INIT (non null)
+ * Returns TRUE if @value has changed
+ */
 G_GNUC_INTERNAL gboolean
 grl_registry_metadata_key_clamp(GrlRegistry *registry,
                                 GrlKeyID key,
@@ -2298,6 +2301,13 @@ grl_registry_metadata_key_clamp(GrlRegistry *registry,
 {
   const gchar *key_name;
 
+  g_return_val_if_fail (min != NULL, FALSE);
+  g_return_val_if_fail (max != NULL, FALSE);
+
+  if (value == NULL) {
+    return FALSE;
+  }
+
   key_name = key_id_handler_get_name (&registry->priv->key_id_handler, key);
   if (key_name) {
     GParamSpec *key_pspec;
diff --git a/tests/operations.c b/tests/operations.c
index 48f5793e..87cc50c3 100644
--- a/tests/operations.c
+++ b/tests/operations.c
@@ -72,6 +72,59 @@ range_filters_max_min_int(void)
   }
 }
 
+static void
+range_filters_max_min_null(void)
+{
+  GrlOperationOptions *options = grl_operation_options_new (NULL);
+  gboolean ret;
+  GValue value = G_VALUE_INIT;
+  GValue *max, *min;
+
+  g_test_bug ("148");
+
+  /* Before */
+  grl_operation_options_get_key_range_filter (options,
+                                              GRL_METADATA_KEY_ORIENTATION,
+                                              &min,
+                                              &max);
+  /* TODO: this is actually a bug. Should get default min/max for this metadata-key 0/359.
+   * Seems that grilo stores the range in GParamSpec but does not set it in the HashTable
+   * GrlOperationsOptions look at. */
+  g_assert_null(min);
+  g_assert_null(max);
+
+  /* Test MIN */
+  g_value_init (&value, G_TYPE_INT);
+  g_value_set_int (&value, 90);
+  ret = grl_operation_options_set_key_range_filter_value (options,
+                                                          GRL_METADATA_KEY_ORIENTATION,
+                                                          &value,
+                                                          NULL);
+  g_assert_true(ret);
+
+  grl_operation_options_get_key_range_filter (options, GRL_METADATA_KEY_ORIENTATION, &min, &max);
+  g_assert_cmpint(g_value_get_int (min), ==, 90);
+  // TODO: Same bug, as max was not set we receive NULL instead
+  g_assert_null(max);
+  g_value_unset(min);
+
+  /* Test MAX */
+  g_value_set_int (&value, 180);
+  ret = grl_operation_options_set_key_range_filter_value (options,
+                                                          GRL_METADATA_KEY_ORIENTATION,
+                                                          NULL,
+                                                          &value);
+  g_assert_true(ret);
+
+  grl_operation_options_get_key_range_filter (options, GRL_METADATA_KEY_ORIENTATION, &min, &max);
+  /* TODO: This is another bug. When we set max above, the min should not be changed.
+   * g_assert_cmpint(g_value_get_int (min), ==, 90);
+   */
+  g_assert_null(min);
+  g_assert_cmpint(g_value_get_int (max), ==, 180);
+  g_value_unset(max);
+}
+
 int
 main (int argc, char **argv)
 {
@@ -85,6 +138,7 @@ main (int argc, char **argv)
 
   /* registry tests */
   g_test_add_func ("/operation/range-filters/max-min/int", range_filters_max_min_int);
+  g_test_add_func ("/operation/range-filters/max-min/null", range_filters_max_min_null);
 
   return g_test_run ();
 }


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]