[glib: 3/5] gobject: Add g_{param_spec, signal}_is_valid_name() functions

[Philip Withnall <pwithnall src gnome org>]

commit 13d1697b67aa6740f58156675f7fca04d6a3a7e5
Author: Jehan <jehan girinstud io>
Date:   Mon Dec 23 18:36:21 2019 +0100

    gobject: Add g_{param_spec,signal}_is_valid_name() functions
    
    Making this validation code public allows projects to validate a
    GParamSpec name before creating it. While hard-coded GParamSpec don't
    need this, we can't afford crashing the main program for dynamically
    generated GParamSpec from user-created data.
    
    In such case, we will need to validate the param names and return errors
    instead of trying to create a GParamSpec with invalid names.
    
    Includes modifications from Philip Withnall and Emmanuele Bassi to
    rearrange the new function addition and split it into one function for
    GParamSpecs and one for GSignals.

 docs/reference/gobject/gobject-sections.txt |  2 ++
 gobject/gparam.c                            | 28 +++++++++++++-----
 gobject/gparam.h                            |  3 ++
 gobject/gsignal.c                           | 44 +++++++++++++----------------
 gobject/gsignal.h                           |  2 ++
 gobject/tests/param.c                       | 29 ++++++++++++++++++-
 gobject/tests/signals.c                     | 29 ++++++++++++++++++-
 7 files changed, 104 insertions(+), 33 deletions(-)
---
diff --git a/docs/reference/gobject/gobject-sections.txt b/docs/reference/gobject/gobject-sections.txt
index bed38e4c5..f8b4c89e7 100644
--- a/docs/reference/gobject/gobject-sections.txt
+++ b/docs/reference/gobject/gobject-sections.txt
@@ -521,6 +521,7 @@ g_param_value_defaults
 g_param_value_validate
 g_param_value_convert
 g_param_values_cmp
+g_param_spec_is_valid_name
 g_param_spec_get_name
 g_param_spec_get_name_quark
 g_param_spec_get_nick
@@ -859,6 +860,7 @@ g_signal_override_class_handler
 g_signal_chain_from_overridden_handler
 g_signal_add_emission_hook
 g_signal_remove_emission_hook
+g_signal_is_valid_name
 g_signal_parse_name
 g_signal_get_invocation_hint
 g_signal_type_cclosure_new
diff --git a/gobject/gparam.c b/gobject/gparam.c
index b336125e4..82199478c 100644
--- a/gobject/gparam.c
+++ b/gobject/gparam.c
@@ -381,20 +381,34 @@ is_canonical (const gchar *key)
   return (strchr (key, '_') == NULL);
 }
 
-static gboolean
-is_valid_property_name (const gchar *key)
+/**
+ * g_param_spec_is_valid_name:
+ * @name: the canonical name of the property
+ *
+ * Validate a property name for a #GParamSpec. This can be useful for
+ * dynamically-generated properties which need to be validated at run-time
+ * before actually trying to create them.
+ *
+ * See [canonical parameter names][canonical-parameter-names] for details of
+ * the rules for valid names.
+ *
+ * Returns: %TRUE if @name is a valid property name, %FALSE otherwise.
+ * Since: 2.66
+ */
+gboolean
+g_param_spec_is_valid_name (const gchar *name)
 {
   const gchar *p;
 
   /* First character must be a letter. */
-  if ((key[0] < 'A' || key[0] > 'Z') &&
-      (key[0] < 'a' || key[0] > 'z'))
+  if ((name[0] < 'A' || name[0] > 'Z') &&
+      (name[0] < 'a' || name[0] > 'z'))
     return FALSE;
 
-  for (p = key; *p != 0; p++)
+  for (p = name; *p != 0; p++)
     {
       const gchar c = *p;
-      
+
       if (c != '-' && c != '_' &&
           (c < '0' || c > '9') &&
           (c < 'A' || c > 'Z') &&
@@ -439,7 +453,7 @@ g_param_spec_internal (GType        param_type,
   
   g_return_val_if_fail (G_TYPE_IS_PARAM (param_type) && param_type != G_TYPE_PARAM, NULL);
   g_return_val_if_fail (name != NULL, NULL);
-  g_return_val_if_fail (is_valid_property_name (name), NULL);
+  g_return_val_if_fail (g_param_spec_is_valid_name (name), NULL);
   g_return_val_if_fail (!(flags & G_PARAM_STATIC_NAME) || is_canonical (name), NULL);
   
   pspec = (gpointer) g_type_create_instance (param_type);
diff --git a/gobject/gparam.h b/gobject/gparam.h
index b6a554653..7294ed515 100644
--- a/gobject/gparam.h
+++ b/gobject/gparam.h
@@ -395,6 +395,9 @@ GLIB_AVAILABLE_IN_ALL
 GType  g_param_type_register_static    (const gchar              *name,
                                         const GParamSpecTypeInfo *pspec_info);
 
+GLIB_AVAILABLE_IN_2_66
+gboolean g_param_spec_is_valid_name    (const gchar              *name);
+
 /* For registering builting types */
 GType  _g_param_type_register_static_constant (const gchar              *name,
                                               const GParamSpecTypeInfo *pspec_info,
diff --git a/gobject/gsignal.c b/gobject/gsignal.c
index 2b6b0d05b..64603c291 100644
--- a/gobject/gsignal.c
+++ b/gobject/gsignal.c
@@ -367,33 +367,29 @@ is_canonical (const gchar *key)
   return (strchr (key, '_') == NULL);
 }
 
-static gboolean
-is_valid_signal_name (const gchar *key)
+/**
+ * g_signal_is_valid_name:
+ * @name: the canonical name of the signal
+ *
+ * Validate a signal name. This can be useful for dynamically-generated signals
+ * which need to be validated at run-time before actually trying to create them.
+ *
+ * See [canonical parameter names][canonical-parameter-names] for details of
+ * the rules for valid names. The rules for signal names are the same as those
+ * for property names.
+ *
+ * Returns: %TRUE if @name is a valid signal name, %FALSE otherwise.
+ * Since: 2.66
+ */
+gboolean
+g_signal_is_valid_name (const gchar *name)
 {
-  const gchar *p;
-
   /* FIXME: We allow this, against our own documentation (the leading `-` is
    * invalid), because GTK has historically used this. */
-  if (g_str_equal (key, "-gtk-private-changed"))
+  if (g_str_equal (name, "-gtk-private-changed"))
     return TRUE;
 
-  /* First character must be a letter. */
-  if ((key[0] < 'A' || key[0] > 'Z') &&
-      (key[0] < 'a' || key[0] > 'z'))
-    return FALSE;
-
-  for (p = key; *p != 0; p++)
-    {
-      const gchar c = *p;
-
-      if (c != '-' && c != '_' &&
-          (c < '0' || c > '9') &&
-          (c < 'A' || c > 'Z') &&
-          (c < 'a' || c > 'z'))
-        return FALSE;
-    }
-
-  return TRUE;
+  return g_param_spec_is_valid_name (name);
 }
 
 static inline guint
@@ -1325,7 +1321,7 @@ g_signal_lookup (const gchar *name,
       if (!g_type_name (itype))
        g_warning (G_STRLOC ": unable to look up signal \"%s\" for invalid type id '%"G_GSIZE_FORMAT"'",
                   name, itype);
-      else if (!is_valid_signal_name (name))
+      else if (!g_signal_is_valid_name (name))
         g_warning (G_STRLOC ": unable to look up invalid signal name \"%s\" on type '%s'",
                    name, g_type_name (itype));
     }
@@ -1712,7 +1708,7 @@ g_signal_newv (const gchar       *signal_name,
   GSignalCVaMarshaller va_marshaller;
   
   g_return_val_if_fail (signal_name != NULL, 0);
-  g_return_val_if_fail (is_valid_signal_name (signal_name), 0);
+  g_return_val_if_fail (g_signal_is_valid_name (signal_name), 0);
   g_return_val_if_fail (G_TYPE_IS_INSTANTIATABLE (itype) || G_TYPE_IS_INTERFACE (itype), 0);
   if (n_params)
     g_return_val_if_fail (param_types != NULL, 0);
diff --git a/gobject/gsignal.h b/gobject/gsignal.h
index 59f94d59a..5cc2b6dfa 100644
--- a/gobject/gsignal.h
+++ b/gobject/gsignal.h
@@ -338,6 +338,8 @@ void                  g_signal_query        (guint               signal_id,
 GLIB_AVAILABLE_IN_ALL
 guint*                g_signal_list_ids     (GType               itype,
                                             guint              *n_ids);
+GLIB_AVAILABLE_IN_2_66
+gboolean              g_signal_is_valid_name (const gchar      *name);
 GLIB_AVAILABLE_IN_ALL
 gboolean             g_signal_parse_name   (const gchar        *detailed_signal,
                                             GType               itype,
diff --git a/gobject/tests/param.c b/gobject/tests/param.c
index 93c3f4b94..44faef10c 100644
--- a/gobject/tests/param.c
+++ b/gobject/tests/param.c
@@ -126,7 +126,7 @@ test_param_invalid_name (gconstpointer test_data)
 
   g_test_trap_subprocess (NULL, 0, 0);
   g_test_trap_assert_failed ();
-  g_test_trap_assert_stderr ("*CRITICAL*is_valid_property_name (name)*");
+  g_test_trap_assert_stderr ("*CRITICAL*g_param_spec_is_valid_name (name)*");
 }
 
 static void
@@ -846,6 +846,32 @@ test_param_default (void)
   g_param_spec_unref (param);
 }
 
+static void
+test_param_is_valid_name (void)
+{
+  const gchar *valid_names[] =
+    {
+      "property",
+      "i",
+      "multiple-segments",
+      "segment0-SEGMENT1",
+      "using_underscores",
+    };
+  const gchar *invalid_names[] =
+    {
+      "",
+      "7zip",
+      "my_int:hello",
+    };
+  gsize i;
+
+  for (i = 0; i < G_N_ELEMENTS (valid_names); i++)
+    g_assert_true (g_param_spec_is_valid_name (valid_names[i]));
+
+  for (i = 0; i < G_N_ELEMENTS (invalid_names); i++)
+    g_assert_false (g_param_spec_is_valid_name (invalid_names[i]));
+}
+
 int
 main (int argc, char *argv[])
 {
@@ -881,6 +907,7 @@ main (int argc, char *argv[])
 
   g_test_add_func ("/value/transform", test_value_transform);
   g_test_add_func ("/param/default", test_param_default);
+  g_test_add_func ("/param/is-valid-name", test_param_is_valid_name);
 
   return g_test_run ();
 }
diff --git a/gobject/tests/signals.c b/gobject/tests/signals.c
index ae8bd9dc2..08b54d0fa 100644
--- a/gobject/tests/signals.c
+++ b/gobject/tests/signals.c
@@ -1449,7 +1449,33 @@ test_signals_invalid_name (gconstpointer test_data)
 
   g_test_trap_subprocess (NULL, 0, 0);
   g_test_trap_assert_failed ();
-  g_test_trap_assert_stderr ("*CRITICAL*is_valid_signal_name (signal_name)*");
+  g_test_trap_assert_stderr ("*CRITICAL*g_signal_is_valid_name (signal_name)*");
+}
+
+static void
+test_signal_is_valid_name (void)
+{
+  const gchar *valid_names[] =
+    {
+      "signal",
+      "i",
+      "multiple-segments",
+      "segment0-SEGMENT1",
+      "using_underscores",
+    };
+  const gchar *invalid_names[] =
+    {
+      "",
+      "7zip",
+      "my_int:hello",
+    };
+  gsize i;
+
+  for (i = 0; i < G_N_ELEMENTS (valid_names); i++)
+    g_assert_true (g_signal_is_valid_name (valid_names[i]));
+
+  for (i = 0; i < G_N_ELEMENTS (invalid_names); i++)
+    g_assert_false (g_signal_is_valid_name (invalid_names[i]));
 }
 
 /* --- */
@@ -1485,6 +1511,7 @@ main (int argc,
   g_test_add_data_func ("/gobject/signals/invalid-name/colon", "my_int:hello", test_signals_invalid_name);
   g_test_add_data_func ("/gobject/signals/invalid-name/first-char", "7zip", test_signals_invalid_name);
   g_test_add_data_func ("/gobject/signals/invalid-name/empty", "", test_signals_invalid_name);
+  g_test_add_func ("/gobject/signals/is-valid-name", test_signal_is_valid_name);
 
   return g_test_run ();
 }