Merge branch 'signal-underscores' into 'master'

Signal name handling improvements

See merge request GNOME/glib!1224

gobject/gbinding.c

@@ -423,6 +423,53 @@ g_binding_finalize (GObject *gobject)
   G_OBJECT_CLASS (g_binding_parent_class)->finalize (gobject);
 }
 
+/* @key must have already been validated with is_valid()
+ * Modifies @key in place. */
+static void
+canonicalize_key (gchar *key)
+{
+  gchar *p;
+
+  for (p = key; *p != 0; p++)
+    {
+      gchar c = *p;
+
+      if (c == '_')
+        *p = '-';
+    }
+}
+
+/* @key must have already been validated with is_valid() */
+static gboolean
+is_canonical (const gchar *key)
+{
+  return (strchr (key, '_') == NULL);
+}
+
+static gboolean
+is_valid_property_name (const gchar *key)
+{
+  const gchar *p;
+
+  /* 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;
+}
+
 static void
 g_binding_set_property (GObject      *gobject,
                         guint         prop_id,
@@ -437,17 +484,35 @@ g_binding_set_property (GObject      *gobject,
       binding->source = g_value_get_object (value);
       break;
 
-    case PROP_SOURCE_PROPERTY:
-      binding->source_property = g_intern_string (g_value_get_string (value));
-      break;
-
     case PROP_TARGET:
       binding->target = g_value_get_object (value);
       break;
 
+    case PROP_SOURCE_PROPERTY:
     case PROP_TARGET_PROPERTY:
-      binding->target_property = g_intern_string (g_value_get_string (value));
+      {
-      break;
+        gchar *name_copy = NULL;
+        const gchar *name = g_value_get_string (value);
+        const gchar **dest;
+
+        /* Ensure the name we intern is canonical. */
+        if (!is_canonical (name))
+          {
+            name_copy = g_value_dup_string (value);
+            canonicalize_key (name_copy);
+            name = name_copy;
+          }
+
+        if (prop_id == PROP_SOURCE_PROPERTY)
+          dest = &binding->source_property;
+        else
+          dest = &binding->target_property;
+
+        *dest = g_intern_string (name);
+
+        g_free (name_copy);
+        break;
+      }
 
     case PROP_FLAGS:
       binding->flags = g_value_get_flags (value);
@@ -474,7 +539,8 @@ g_binding_get_property (GObject    *gobject,
       break;
 
     case PROP_SOURCE_PROPERTY:
-      g_value_set_string (value, binding->source_property);
+      /* @source_property is interned, so we don’t need to take a copy */
+      g_value_set_static_string (value, binding->source_property);
       break;
 
     case PROP_TARGET:
@@ -482,7 +548,8 @@ g_binding_get_property (GObject    *gobject,
       break;
 
     case PROP_TARGET_PROPERTY:
-      g_value_set_string (value, binding->target_property);
+      /* @target_property is interned, so we don’t need to take a copy */
+      g_value_set_static_string (value, binding->target_property);
       break;
 
     case PROP_FLAGS:
@@ -606,7 +673,10 @@ g_binding_class_init (GBindingClass *klass)
    * GBinding:source-property:
    *
    * The name of the property of #GBinding:source that should be used
-   * as the source of the binding
+   * as the source of the binding.
+   *
+   * This should be in [canonical form][canonical-parameter-names] to get the
+   * best performance.
    *
    * Since: 2.26
    */
@@ -622,7 +692,10 @@ g_binding_class_init (GBindingClass *klass)
    * GBinding:target-property:
    *
    * The name of the property of #GBinding:target that should be used
-   * as the target of the binding
+   * as the target of the binding.
+   *
+   * This should be in [canonical form][canonical-parameter-names] to get the
+   * best performance.
    *
    * Since: 2.26
    */
@@ -835,8 +908,10 @@ g_object_bind_property_full (gpointer               source,
 
   g_return_val_if_fail (G_IS_OBJECT (source), NULL);
   g_return_val_if_fail (source_property != NULL, NULL);
+  g_return_val_if_fail (is_valid_property_name (source_property), NULL);
   g_return_val_if_fail (G_IS_OBJECT (target), NULL);
   g_return_val_if_fail (target_property != NULL, NULL);
+  g_return_val_if_fail (is_valid_property_name (target_property), NULL);
 
   if (source == target && g_strcmp0 (source_property, target_property) == 0)
     {

gobject/gparam.c

@@ -41,11 +41,14 @@
  *
  * ## Parameter names # {#canonical-parameter-names}
  *
- * Parameter names need to start with a letter (a-z or A-Z).
+ * A property name consists of segments consisting of ASCII letters and
- * Subsequent characters can be letters, numbers or a '-'.
+ * digits, separated by either the `-` or `_` character. The first
- * All other characters are replaced by a '-' during construction.
+ * character of a property name must be a letter. These are the same rules as
- * The result of this replacement is called the canonical name of
+ * for signal naming (see g_signal_new()).
- * the parameter.
+ *
+ * When creating and looking up a #GParamSpec, either separator can be
+ * used, but they cannot be mixed. Using `-` is considerably more
+ * efficient, and is the ‘canonical form’. Using `_` is discouraged.
  */
 
 
@@ -355,6 +358,8 @@ g_param_spec_get_blurb (GParamSpec *pspec)
   return NULL;
 }
 
+/* @key must have already been validated with is_valid()
+ * Modifies @key in place. */
 static void
 canonicalize_key (gchar *key)
 {
@@ -364,28 +369,37 @@ canonicalize_key (gchar *key)
     {
       gchar c = *p;
       
-      if (c != '-' &&
+      if (c == '_')
-	  (c < '0' || c > '9') &&
+        *p = '-';
-	  (c < 'A' || c > 'Z') &&
-	  (c < 'a' || c > 'z'))
-	*p = '-';
     }
 }
 
+/* @key must have already been validated with is_valid() */
 static gboolean
 is_canonical (const gchar *key)
+{
+  return (strchr (key, '_') == NULL);
+}
+
+static gboolean
+is_valid_property_name (const gchar *key)
 {
   const gchar *p;
 
+  /* 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++)
     {
-      gchar c = *p;
+      const gchar c = *p;
       
-      if (c != '-' &&
+      if (c != '-' && c != '_' &&
-	  (c < '0' || c > '9') &&
+          (c < '0' || c > '9') &&
-	  (c < 'A' || c > 'Z') &&
+          (c < 'A' || c > 'Z') &&
-	  (c < 'a' || c > 'z'))
+          (c < 'a' || c > 'z'))
-	return FALSE;
+        return FALSE;
     }
 
   return TRUE;
@@ -401,15 +415,9 @@ is_canonical (const gchar *key)
  *
  * Creates a new #GParamSpec instance.
  *
- * A property name consists of segments consisting of ASCII letters and
+ * See [canonical parameter names][canonical-parameter-names] for details of
- * digits, separated by either the '-' or '_' character. The first
+ * the rules for @name. Names which violate these rules lead to undefined
- * character of a property name must be a letter. Names which violate these
+ * behaviour.
- * rules lead to undefined behaviour.
- *
- * When creating and looking up a #GParamSpec, either separator can be
- * used, but they cannot be mixed. Using '-' is considerably more
- * efficient and in fact required when using property names as detail
- * strings for signals.
  *
  * Beyond the name, #GParamSpecs have two more descriptive
  * strings associated with them, the @nick, which should be suitable
@@ -431,7 +439,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 ((name[0] >= 'A' && name[0] <= 'Z') || (name[0] >= 'a' && name[0] <= 'z'), NULL);
+  g_return_val_if_fail (is_valid_property_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);

gobject/gsignal.c

@@ -98,6 +98,10 @@
  * detail part of the signal specification upon connection) serves as a
  * wildcard and matches any detail argument passed in to emission.
  *
+ * While the @detail argument is typically used to pass an object property name
+ * (as with #GObject::notify), no specific format is mandated for the detail
+ * string, other than that it must be non-empty.
+ *
  * ## Memory management of signal handlers # {#signal-memory-management}
  *
  * If you are connecting handlers to signals and using a #GObject instance as
@@ -145,8 +149,8 @@ typedef enum
 
 
 /* --- prototypes --- */
-static inline guint		signal_id_lookup	(GQuark		  quark,
+static inline guint   signal_id_lookup  (const gchar *name,
-							 GType		  itype);
+                                         GType        itype);
 static	      void		signal_destroy_R	(SignalNode	 *signal_node);
 static inline HandlerList*	handler_list_ensure	(guint		  signal_id,
 							 gpointer	  instance);
@@ -340,14 +344,68 @@ LOOKUP_SIGNAL_NODE (guint signal_id)
 
 
 /* --- functions --- */
-static inline guint
+/* @key must have already been validated with is_valid()
-signal_id_lookup (GQuark quark,
+ * Modifies @key in place. */
-		  GType  itype)
+static void
+canonicalize_key (gchar *key)
 {
+  gchar *p;
+
+  for (p = key; *p != 0; p++)
+    {
+      gchar c = *p;
+
+      if (c == '_')
+        *p = '-';
+    }
+}
+
+/* @key must have already been validated with is_valid() */
+static gboolean
+is_canonical (const gchar *key)
+{
+  return (strchr (key, '_') == NULL);
+}
+
+static gboolean
+is_valid_signal_name (const gchar *key)
+{
+  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"))
+    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;
+}
+
+static inline guint
+signal_id_lookup (const gchar *name,
+                  GType  itype)
+{
+  GQuark quark;
   GType *ifaces, type = itype;
   SignalKey key;
   guint n_ifaces;
 
+  quark = g_quark_try_string (name);
   key.quark = quark;
 
   /* try looking up signals for this type and its ancestors */
@@ -381,7 +439,22 @@ signal_id_lookup (GQuark quark,
 	}
     }
   g_free (ifaces);
-  
+
+  /* If the @name is non-canonical, try again. This is the slow path — people
+   * should use canonical names in their queries if they want performance. */
+  if (!is_canonical (name))
+    {
+      guint signal_id;
+      gchar *name_copy = g_strdup (name);
+      canonicalize_key (name_copy);
+
+      signal_id = signal_id_lookup (name_copy, itype);
+
+      g_free (name_copy);
+
+      return signal_id;
+    }
+
   return 0;
 }
 
@@ -1080,7 +1153,7 @@ signal_parse_name (const gchar *name,
   
   if (!colon)
     {
-      signal_id = signal_id_lookup (g_quark_try_string (name), itype);
+      signal_id = signal_id_lookup (name, itype);
       if (signal_id && detail_p)
 	*detail_p = 0;
     }
@@ -1089,11 +1162,14 @@ signal_parse_name (const gchar *name,
       gchar buffer[32];
       guint l = colon - name;
       
+      if (colon[2] == '\0')
+        return 0;
+
       if (l < 32)
 	{
 	  memcpy (buffer, name, l);
 	  buffer[l] = 0;
-	  signal_id = signal_id_lookup (g_quark_try_string (buffer), itype);
+	  signal_id = signal_id_lookup (buffer, itype);
 	}
       else
 	{
@@ -1101,12 +1177,12 @@ signal_parse_name (const gchar *name,
 	  
 	  memcpy (signal, name, l);
 	  signal[l] = 0;
-	  signal_id = signal_id_lookup (g_quark_try_string (signal), itype);
+	  signal_id = signal_id_lookup (signal, itype);
 	  g_free (signal);
 	}
       
       if (signal_id && detail_p)
-	*detail_p = colon[2] ? (force_quark ? g_quark_from_string : g_quark_try_string) (colon + 2) : 0;
+        *detail_p = (force_quark ? g_quark_from_string : g_quark_try_string) (colon + 2);
     }
   else
     signal_id = 0;
@@ -1241,7 +1317,7 @@ g_signal_lookup (const gchar *name,
   g_return_val_if_fail (G_TYPE_IS_INSTANTIATABLE (itype) || G_TYPE_IS_INTERFACE (itype), 0);
   
   SIGNAL_LOCK ();
-  signal_id = signal_id_lookup (g_quark_try_string (name), itype);
+  signal_id = signal_id_lookup (name, itype);
   SIGNAL_UNLOCK ();
   if (!signal_id)
     {
@@ -1255,6 +1331,9 @@ g_signal_lookup (const gchar *name,
       else if (!g_type_class_peek (itype))
 	g_warning (G_STRLOC ": unable to look up signal \"%s\" of unloaded type '%s'",
 		   name, g_type_name (itype));
+      else if (!is_valid_signal_name (name))
+        g_warning (G_STRLOC ": unable to look up invalid signal name \"%s\" on type '%s'",
+                   name, g_type_name (itype));
     }
   
   return signal_id;
@@ -1291,13 +1370,7 @@ g_signal_list_ids (GType  itype,
   for (i = 0; i < n_nodes; i++)
     if (keys[i].itype == itype)
       {
-	const gchar *name = g_quark_to_string (keys[i].quark);
+        g_array_append_val (result, keys[i].signal_id);
-	
-	/* Signal names with "_" in them are aliases to the same
-	 * name with "-" instead of "_".
-	 */
-	if (!strchr (name, '_'))
-	  g_array_append_val (result, keys[i].signal_id);
       }
   *n_ids = result->len;
   SIGNAL_UNLOCK ();
@@ -1403,12 +1476,14 @@ g_signal_query (guint         signal_id,
  * Creates a new signal. (This is usually done in the class initializer.)
  *
  * A signal name consists of segments consisting of ASCII letters and
- * digits, separated by either the '-' or '_' character. The first
+ * digits, separated by either the `-` or `_` character. The first
  * character of a signal name must be a letter. Names which violate these
- * rules lead to undefined behaviour of the GSignal system.
+ * rules lead to undefined behaviour. These are the same rules as for property
+ * naming (see g_param_spec_internal()).
  *
  * When registering a signal and looking up a signal, either separator can
- * be used, but they cannot be mixed.
+ * be used, but they cannot be mixed. Using `-` is considerably more efficient.
+ * Using `_` is discouraged.
  *
  * If 0 is used for @class_offset subclasses cannot override the class handler
  * in their class_init method by doing super_class->signal_handler = my_signal_handler.
@@ -1634,7 +1709,8 @@ g_signal_newv (const gchar       *signal_name,
                guint              n_params,
                GType		 *param_types)
 {
-  gchar *name;
+  const gchar *name;
+  gchar *signal_name_copy = NULL;
   guint signal_id, i;
   SignalNode *node;
   GSignalCMarshaller builtin_c_marshaller;
@@ -1642,6 +1718,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_TYPE_IS_INSTANTIATABLE (itype) || G_TYPE_IS_INTERFACE (itype), 0);
   if (n_params)
     g_return_val_if_fail (param_types != NULL, 0);
@@ -1651,12 +1728,20 @@ g_signal_newv (const gchar       *signal_name,
   if (!accumulator)
     g_return_val_if_fail (accu_data == NULL, 0);
 
-  name = g_strdup (signal_name);
+  if (!is_canonical (signal_name))
-  g_strdelimit (name, G_STR_DELIMITERS ":^", '_');  /* FIXME do character checks like for types */
+    {
+      signal_name_copy = g_strdup (signal_name);
+      canonicalize_key (signal_name_copy);
+      name = signal_name_copy;
+    }
+  else
+    {
+      name = signal_name;
+    }
   
   SIGNAL_LOCK ();
   
-  signal_id = signal_id_lookup (g_quark_try_string (name), itype);
+  signal_id = signal_id_lookup (name, itype);
   node = LOOKUP_SIGNAL_NODE (signal_id);
   if (node && !node->destroyed)
     {
@@ -1664,7 +1749,7 @@ g_signal_newv (const gchar       *signal_name,
                  name,
                  type_debug_name (node->itype),
                  G_TYPE_IS_INTERFACE (node->itype) ? "interface" : "class ancestry");
-      g_free (name);
+      g_free (signal_name_copy);
       SIGNAL_UNLOCK ();
       return 0;
     }
@@ -1674,7 +1759,7 @@ g_signal_newv (const gchar       *signal_name,
                  name,
                  type_debug_name (itype),
                  type_debug_name (node->itype));
-      g_free (name);
+      g_free (signal_name_copy);
       SIGNAL_UNLOCK ();
       return 0;
     }
@@ -1683,7 +1768,7 @@ g_signal_newv (const gchar       *signal_name,
       {
 	g_warning (G_STRLOC ": parameter %d of type '%s' for signal \"%s::%s\" is not a value type",
 		   i + 1, type_debug_name (param_types[i]), type_debug_name (itype), name);
-	g_free (name);
+	g_free (signal_name_copy);
 	SIGNAL_UNLOCK ();
 	return 0;
       }
@@ -1691,7 +1776,7 @@ g_signal_newv (const gchar       *signal_name,
     {
       g_warning (G_STRLOC ": return value of type '%s' for signal \"%s::%s\" is not a value type",
 		 type_debug_name (return_type), type_debug_name (itype), name);
-      g_free (name);
+      g_free (signal_name_copy);
       SIGNAL_UNLOCK ();
       return 0;
     }
@@ -1700,7 +1785,7 @@ g_signal_newv (const gchar       *signal_name,
     {
       g_warning (G_STRLOC ": signal \"%s::%s\" has return type '%s' and is only G_SIGNAL_RUN_FIRST",
 		 type_debug_name (itype), name, type_debug_name (return_type));
-      g_free (name);
+      g_free (signal_name_copy);
       SIGNAL_UNLOCK ();
       return 0;
     }
@@ -1716,12 +1801,8 @@ g_signal_newv (const gchar       *signal_name,
       g_signal_nodes = g_renew (SignalNode*, g_signal_nodes, g_n_signal_nodes);
       g_signal_nodes[signal_id] = node;
       node->itype = itype;
-      node->name = name;
       key.itype = itype;
-      key.quark = g_quark_from_string (node->name);
       key.signal_id = signal_id;
-      g_signal_key_bsa = g_bsearch_array_insert (g_signal_key_bsa, &g_signal_key_bconfig, &key);
-      g_strdelimit (name, "_", '-');
       node->name = g_intern_string (name);
       key.quark = g_quark_from_string (name);
       g_signal_key_bsa = g_bsearch_array_insert (g_signal_key_bsa, &g_signal_key_bconfig, &key);
@@ -1809,7 +1890,7 @@ g_signal_newv (const gchar       *signal_name,
 
   SIGNAL_UNLOCK ();
 
-  g_free (name);
+  g_free (signal_name_copy);
 
   return signal_id;
 }

gobject/gsignal.h

@@ -160,11 +160,11 @@ typedef enum
 /**
  * GSignalMatchType:
  * @G_SIGNAL_MATCH_ID: The signal id must be equal.
- * @G_SIGNAL_MATCH_DETAIL: The signal detail be equal.
+ * @G_SIGNAL_MATCH_DETAIL: The signal detail must be equal.
  * @G_SIGNAL_MATCH_CLOSURE: The closure must be the same.
  * @G_SIGNAL_MATCH_FUNC: The C closure callback must be the same.
  * @G_SIGNAL_MATCH_DATA: The closure data must be the same.
- * @G_SIGNAL_MATCH_UNBLOCKED: Only unblocked signals may matched.
+ * @G_SIGNAL_MATCH_UNBLOCKED: Only unblocked signals may be matched.
  * 
  * The match types specify what g_signal_handlers_block_matched(),
  * g_signal_handlers_unblock_matched() and g_signal_handlers_disconnect_matched()

gobject/tests/binding.c

@@ -8,7 +8,7 @@ typedef struct _BindingSource
 
   gint foo;
   gint bar;
-  gdouble value;
+  gdouble double_value;
   gboolean toggle;
 } BindingSource;
 
@@ -23,7 +23,7 @@ enum
 
   PROP_SOURCE_FOO,
   PROP_SOURCE_BAR,
-  PROP_SOURCE_VALUE,
+  PROP_SOURCE_DOUBLE_VALUE,
   PROP_SOURCE_TOGGLE
 };
 
@@ -48,8 +48,8 @@ binding_source_set_property (GObject      *gobject,
       source->bar = g_value_get_int (value);
       break;
 
-    case PROP_SOURCE_VALUE:
+    case PROP_SOURCE_DOUBLE_VALUE:
-      source->value = g_value_get_double (value);
+      source->double_value = g_value_get_double (value);
       break;
 
     case PROP_SOURCE_TOGGLE:
@@ -79,8 +79,8 @@ binding_source_get_property (GObject    *gobject,
       g_value_set_int (value, source->bar);
       break;
 
-    case PROP_SOURCE_VALUE:
+    case PROP_SOURCE_DOUBLE_VALUE:
-      g_value_set_double (value, source->value);
+      g_value_set_double (value, source->double_value);
       break;
 
     case PROP_SOURCE_TOGGLE:
@@ -110,8 +110,8 @@ binding_source_class_init (BindingSourceClass *klass)
                                                      -1, 100,
                                                      0,
                                                      G_PARAM_READWRITE));
-  g_object_class_install_property (gobject_class, PROP_SOURCE_VALUE,
+  g_object_class_install_property (gobject_class, PROP_SOURCE_DOUBLE_VALUE,
-                                   g_param_spec_double ("value", "Value", "Value",
+                                   g_param_spec_double ("double-value", "Value", "Value",
                                                         -100.0, 200.0,
                                                         0.0,
                                                         G_PARAM_READWRITE));
@@ -131,7 +131,7 @@ typedef struct _BindingTarget
   GObject parent_instance;
 
   gint bar;
-  gdouble value;
+  gdouble double_value;
   gboolean toggle;
 } BindingTarget;
 
@@ -145,7 +145,7 @@ enum
   PROP_TARGET_0,
 
   PROP_TARGET_BAR,
-  PROP_TARGET_VALUE,
+  PROP_TARGET_DOUBLE_VALUE,
   PROP_TARGET_TOGGLE
 };
 
@@ -166,8 +166,8 @@ binding_target_set_property (GObject      *gobject,
       target->bar = g_value_get_int (value);
       break;
 
-    case PROP_TARGET_VALUE:
+    case PROP_TARGET_DOUBLE_VALUE:
-      target->value = g_value_get_double (value);
+      target->double_value = g_value_get_double (value);
       break;
 
     case PROP_TARGET_TOGGLE:
@@ -193,8 +193,8 @@ binding_target_get_property (GObject    *gobject,
       g_value_set_int (value, target->bar);
       break;
 
-    case PROP_TARGET_VALUE:
+    case PROP_TARGET_DOUBLE_VALUE:
-      g_value_set_double (value, target->value);
+      g_value_set_double (value, target->double_value);
       break;
 
     case PROP_TARGET_TOGGLE:
@@ -219,8 +219,8 @@ binding_target_class_init (BindingTargetClass *klass)
                                                      -1, 100,
                                                      0,
                                                      G_PARAM_READWRITE));
-  g_object_class_install_property (gobject_class, PROP_TARGET_VALUE,
+  g_object_class_install_property (gobject_class, PROP_TARGET_DOUBLE_VALUE,
-                                   g_param_spec_double ("value", "Value", "Value",
+                                   g_param_spec_double ("double-value", "Value", "Value",
                                                         -100.0, 200.0,
                                                         0.0,
                                                         G_PARAM_READWRITE));
@@ -243,8 +243,8 @@ celsius_to_fahrenheit (GBinding     *binding,
 {
   gdouble celsius, fahrenheit;
 
-  g_assert (G_VALUE_HOLDS (from_value, G_TYPE_DOUBLE));
+  g_assert_true (G_VALUE_HOLDS (from_value, G_TYPE_DOUBLE));
-  g_assert (G_VALUE_HOLDS (to_value, G_TYPE_DOUBLE));
+  g_assert_true (G_VALUE_HOLDS (to_value, G_TYPE_DOUBLE));
 
   celsius = g_value_get_double (from_value);
   fahrenheit = (9 * celsius / 5) + 32.0;
@@ -265,8 +265,8 @@ fahrenheit_to_celsius (GBinding     *binding,
 {
   gdouble celsius, fahrenheit;
 
-  g_assert (G_VALUE_HOLDS (from_value, G_TYPE_DOUBLE));
+  g_assert_true (G_VALUE_HOLDS (from_value, G_TYPE_DOUBLE));
-  g_assert (G_VALUE_HOLDS (to_value, G_TYPE_DOUBLE));
+  g_assert_true (G_VALUE_HOLDS (to_value, G_TYPE_DOUBLE));
 
   fahrenheit = g_value_get_double (from_value);
   celsius = 5 * (fahrenheit - 32.0) / 9;
@@ -291,8 +291,8 @@ binding_default (void)
                                     G_BINDING_DEFAULT);
 
   g_object_add_weak_pointer (G_OBJECT (binding), (gpointer *) &binding);
-  g_assert ((BindingSource *) g_binding_get_source (binding) == source);
+  g_assert_true ((BindingSource *) g_binding_get_source (binding) == source);
-  g_assert ((BindingTarget *) g_binding_get_target (binding) == target);
+  g_assert_true ((BindingTarget *) g_binding_get_target (binding) == target);
   g_assert_cmpstr (g_binding_get_source_property (binding), ==, "foo");
   g_assert_cmpstr (g_binding_get_target_property (binding), ==, "bar");
   g_assert_cmpint (g_binding_get_flags (binding), ==, G_BINDING_DEFAULT);
@@ -310,7 +310,38 @@ binding_default (void)
 
   g_object_unref (source);
   g_object_unref (target);
-  g_assert (binding == NULL);
+  g_assert_null (binding);
+}
+
+static void
+binding_canonicalisation (void)
+{
+  BindingSource *source = g_object_new (binding_source_get_type (), NULL);
+  BindingTarget *target = g_object_new (binding_target_get_type (), NULL);
+  GBinding *binding;
+
+  g_test_summary ("Test that bindings set up with non-canonical property names work");
+
+  binding = g_object_bind_property (source, "double_value",
+                                    target, "double_value",
+                                    G_BINDING_DEFAULT);
+
+  g_object_add_weak_pointer (G_OBJECT (binding), (gpointer *) &binding);
+  g_assert_true ((BindingSource *) g_binding_get_source (binding) == source);
+  g_assert_true ((BindingTarget *) g_binding_get_target (binding) == target);
+  g_assert_cmpstr (g_binding_get_source_property (binding), ==, "double-value");
+  g_assert_cmpstr (g_binding_get_target_property (binding), ==, "double-value");
+  g_assert_cmpint (g_binding_get_flags (binding), ==, G_BINDING_DEFAULT);
+
+  g_object_set (source, "double-value", 24.0, NULL);
+  g_assert_cmpfloat (target->double_value, ==, source->double_value);
+
+  g_object_set (target, "double-value", 69.0, NULL);
+  g_assert_cmpfloat (source->double_value, !=, target->double_value);
+
+  g_object_unref (target);
+  g_object_unref (source);
+  g_assert_null (binding);
 }
 
 static void
@@ -338,7 +369,7 @@ binding_bidirectional (void)
 
   g_object_unref (source);
   g_object_unref (target);
-  g_assert (binding == NULL);
+  g_assert_null (binding);
 }
 
 static void
@@ -360,7 +391,7 @@ binding_transform_default (void)
   GBindingFlags flags;
 
   binding = g_object_bind_property (source, "foo",
-                                    target, "value",
+                                    target, "double-value",
                                     G_BINDING_BIDIRECTIONAL);
 
   g_object_add_weak_pointer (G_OBJECT (binding), (gpointer *) &binding);
@@ -372,10 +403,10 @@ binding_transform_default (void)
                 "target-property", &trg_prop,
                 "flags", &flags,
                 NULL);
-  g_assert (src == source);
+  g_assert_true (src == source);
-  g_assert (trg == target);
+  g_assert_true (trg == target);
   g_assert_cmpstr (src_prop, ==, "foo");
-  g_assert_cmpstr (trg_prop, ==, "value");
+  g_assert_cmpstr (trg_prop, ==, "double-value");
   g_assert_cmpint (flags, ==, G_BINDING_BIDIRECTIONAL);
   g_object_unref (src);
   g_object_unref (trg);
@@ -383,14 +414,14 @@ binding_transform_default (void)
   g_free (trg_prop);
 
   g_object_set (source, "foo", 24, NULL);
-  g_assert_cmpfloat (target->value, ==, 24.0);
+  g_assert_cmpfloat (target->double_value, ==, 24.0);
 
-  g_object_set (target, "value", 69.0, NULL);
+  g_object_set (target, "double-value", 69.0, NULL);
   g_assert_cmpint (source->foo, ==, 69);
 
   g_object_unref (target);
   g_object_unref (source);
-  g_assert (binding == NULL);
+  g_assert_null (binding);
 }
 
 static void
@@ -401,23 +432,23 @@ binding_transform (void)
   GBinding *binding G_GNUC_UNUSED;
   gboolean unused_data = FALSE;
 
-  binding = g_object_bind_property_full (source, "value",
+  binding = g_object_bind_property_full (source, "double-value",
-                                         target, "value",
+                                         target, "double-value",
                                          G_BINDING_BIDIRECTIONAL,
                                          celsius_to_fahrenheit,
                                          fahrenheit_to_celsius,
                                          &unused_data, data_free);
 
-  g_object_set (source, "value", 24.0, NULL);
+  g_object_set (source, "double-value", 24.0, NULL);
-  g_assert_cmpfloat (target->value, ==, ((9 * 24.0 / 5) + 32.0));
+  g_assert_cmpfloat (target->double_value, ==, ((9 * 24.0 / 5) + 32.0));
 
-  g_object_set (target, "value", 69.0, NULL);
+  g_object_set (target, "double-value", 69.0, NULL);
-  g_assert_cmpfloat (source->value, ==, (5 * (69.0 - 32.0) / 9));
+  g_assert_cmpfloat (source->double_value, ==, (5 * (69.0 - 32.0) / 9));
 
   g_object_unref (source);
   g_object_unref (target);
 
-  g_assert (unused_data);
+  g_assert_true (unused_data);
 }
 
 static void
@@ -433,23 +464,23 @@ binding_transform_closure (void)
 
   f2c_clos = g_cclosure_new (G_CALLBACK (fahrenheit_to_celsius), &unused_data_2, (GClosureNotify) data_free);
 
-  binding = g_object_bind_property_with_closures (source, "value",
+  binding = g_object_bind_property_with_closures (source, "double-value",
-                                                  target, "value",
+                                                  target, "double-value",
                                                   G_BINDING_BIDIRECTIONAL,
                                                   c2f_clos,
                                                   f2c_clos);
 
-  g_object_set (source, "value", 24.0, NULL);
+  g_object_set (source, "double-value", 24.0, NULL);
-  g_assert_cmpfloat (target->value, ==, ((9 * 24.0 / 5) + 32.0));
+  g_assert_cmpfloat (target->double_value, ==, ((9 * 24.0 / 5) + 32.0));
 
-  g_object_set (target, "value", 69.0, NULL);
+  g_object_set (target, "double-value", 69.0, NULL);
-  g_assert_cmpfloat (source->value, ==, (5 * (69.0 - 32.0) / 9));
+  g_assert_cmpfloat (source->double_value, ==, (5 * (69.0 - 32.0) / 9));
 
   g_object_unref (source);
   g_object_unref (target);
 
-  g_assert (unused_data_1);
+  g_assert_true (unused_data_1);
-  g_assert (unused_data_2);
+  g_assert_true (unused_data_2);
 }
 
 static void
@@ -477,9 +508,9 @@ binding_chain (void)
 
   /* unbind A -> B and B -> C */
   g_object_unref (binding_1);
-  g_assert (binding_1 == NULL);
+  g_assert_null (binding_1);
   g_object_unref (binding_2);
-  g_assert (binding_2 == NULL);
+  g_assert_null (binding_2);
 
   /* bind A -> C directly */
   binding_2 = g_object_bind_property (a, "foo", c, "foo", G_BINDING_BIDIRECTIONAL);
@@ -544,16 +575,16 @@ binding_invert_boolean (void)
                                     target, "toggle",
                                     G_BINDING_BIDIRECTIONAL | G_BINDING_INVERT_BOOLEAN);
 
-  g_assert (source->toggle);
+  g_assert_true (source->toggle);
-  g_assert (!target->toggle);
+  g_assert_false (target->toggle);
 
   g_object_set (source, "toggle", FALSE, NULL);
-  g_assert (!source->toggle);
+  g_assert_false (source->toggle);
-  g_assert (target->toggle);
+  g_assert_true (target->toggle);
 
   g_object_set (target, "toggle", FALSE, NULL);
-  g_assert (source->toggle);
+  g_assert_true (source->toggle);
-  g_assert (!target->toggle);
+  g_assert_false (target->toggle);
 
   g_object_unref (binding);
   g_object_unref (source);
@@ -582,7 +613,7 @@ binding_same_object (void)
   g_assert_cmpint (source->bar, ==, 30);
 
   g_object_unref (source);
-  g_assert (binding == NULL);
+  g_assert_null (binding);
 }
 
 static void
@@ -604,7 +635,7 @@ binding_unbind (void)
   g_assert_cmpint (source->foo, !=, target->bar);
 
   g_binding_unbind (binding);
-  g_assert (binding == NULL);
+  g_assert_null (binding);
 
   g_object_set (source, "foo", 0, NULL);
   g_assert_cmpint (source->foo, !=, target->bar);
@@ -621,7 +652,7 @@ binding_unbind (void)
   g_object_add_weak_pointer (G_OBJECT (binding), (gpointer *) &binding);
 
   g_binding_unbind (binding);
-  g_assert (binding == NULL);
+  g_assert_null (binding);
 
   g_object_unref (source);
 }
@@ -711,19 +742,19 @@ binding_fail (void)
   GBinding *binding;
 
   /* double -> boolean is not supported */
-  binding = g_object_bind_property (source, "value",
+  binding = g_object_bind_property (source, "double-value",
                                     target, "toggle",
                                     G_BINDING_DEFAULT);
   g_object_add_weak_pointer (G_OBJECT (binding), (gpointer *) &binding);
 
   g_test_expect_message ("GLib-GObject", G_LOG_LEVEL_WARNING,
                          "*Unable to convert*double*boolean*");
-  g_object_set (source, "value", 1.0, NULL);
+  g_object_set (source, "double-value", 1.0, NULL);
   g_test_assert_expected_messages ();
 
   g_object_unref (source);
   g_object_unref (target);
-  g_assert (binding == NULL);
+  g_assert_null (binding);
 }
 
 int
@@ -734,6 +765,7 @@ main (int argc, char *argv[])
   g_test_bug_base ("https://gitlab.gnome.org/GNOME/glib/issues/");
 
   g_test_add_func ("/binding/default", binding_default);
+  g_test_add_func ("/binding/canonicalisation", binding_canonicalisation);
   g_test_add_func ("/binding/bidirectional", binding_bidirectional);
   g_test_add_func ("/binding/transform", binding_transform);
   g_test_add_func ("/binding/transform-default", binding_transform_default);

gobject/tests/param.c

@@ -10,19 +10,19 @@ test_param_value (void)
   GValue value = G_VALUE_INIT;
 
   g_value_init (&value, G_TYPE_PARAM);
-  g_assert (G_VALUE_HOLDS_PARAM (&value));
+  g_assert_true (G_VALUE_HOLDS_PARAM (&value));
 
   p = g_param_spec_int ("my-int", "My Int", "Blurb", 0, 20, 10, G_PARAM_READWRITE);
 
   g_value_take_param (&value, p);
   p2 = g_value_get_param (&value);
-  g_assert (p2 == p);
+  g_assert_true (p2 == p);
 
   pp = g_param_spec_uint ("my-uint", "My UInt", "Blurb", 0, 10, 5, G_PARAM_READWRITE);
   g_value_set_param (&value, pp);
 
   p2 = g_value_dup_param (&value);
-  g_assert (p2 == pp); /* param specs use ref/unref for copy/free */
+  g_assert_true (p2 == pp); /* param specs use ref/unref for copy/free */
   g_param_spec_unref (p2);
 
   g_value_unset (&value);
@@ -57,7 +57,7 @@ test_param_qdata (void)
   g_assert_cmpint (destroy_count, ==, 1);
   g_assert_cmpstr (g_param_spec_steal_qdata (p, q), ==, "blabla");
   g_assert_cmpint (destroy_count, ==, 1);
-  g_assert (g_param_spec_get_qdata (p, q) == NULL);
+  g_assert_null (g_param_spec_get_qdata (p, q));
 
   g_param_spec_ref_sink (p);
 
@@ -74,12 +74,12 @@ test_param_validate (void)
 
   g_value_init (&value, G_TYPE_INT);
   g_value_set_int (&value, 100);
-  g_assert (!g_param_value_defaults (p, &value));
+  g_assert_false (g_param_value_defaults (p, &value));
-  g_assert (g_param_value_validate (p, &value));
+  g_assert_true (g_param_value_validate (p, &value));
   g_assert_cmpint (g_value_get_int (&value), ==, 20);
 
   g_param_value_set_default (p, &value);
-  g_assert (g_param_value_defaults (p, &value));
+  g_assert_true (g_param_value_defaults (p, &value));
   g_assert_cmpint (g_value_get_int (&value), ==, 10);
 
   g_param_spec_unref (p);
@@ -91,9 +91,9 @@ test_param_strings (void)
   GParamSpec *p;
 
   /* test canonicalization */
-  p = g_param_spec_int ("my_int:bla", "My Int", "Blurb", 0, 20, 10, G_PARAM_READWRITE);
+  p = g_param_spec_int ("my_int", "My Int", "Blurb", 0, 20, 10, G_PARAM_READWRITE);
 
-  g_assert_cmpstr (g_param_spec_get_name (p), ==, "my-int-bla");
+  g_assert_cmpstr (g_param_spec_get_name (p), ==, "my-int");
   g_assert_cmpstr (g_param_spec_get_nick (p), ==, "My Int");
   g_assert_cmpstr (g_param_spec_get_blurb (p), ==, "Blurb");
 
@@ -104,11 +104,31 @@ test_param_strings (void)
 
   g_assert_cmpstr (g_param_spec_get_name (p), ==, "my-int");
   g_assert_cmpstr (g_param_spec_get_nick (p), ==, "my-int");
-  g_assert (g_param_spec_get_blurb (p) == NULL);
+  g_assert_null (g_param_spec_get_blurb (p));
 
   g_param_spec_unref (p);
 }
 
+static void
+test_param_invalid_name (gconstpointer test_data)
+{
+  const gchar *invalid_name = test_data;
+
+  g_test_summary ("Test that properties cannot be created with invalid names");
+
+  if (g_test_subprocess ())
+    {
+      GParamSpec *p;
+      p = g_param_spec_int (invalid_name, "My Int", "Blurb", 0, 20, 10, G_PARAM_READWRITE);
+      g_param_spec_unref (p);
+      return;
+    }
+
+  g_test_trap_subprocess (NULL, 0, 0);
+  g_test_trap_assert_failed ();
+  g_test_trap_assert_stderr ("*CRITICAL*is_valid_property_name (name)*");
+}
+
 static void
 test_param_convert (void)
 {
@@ -123,10 +143,10 @@ test_param_convert (void)
   g_value_init (&v2, G_TYPE_INT);
   g_value_set_int (&v2, -4);
 
-  g_assert (!g_param_value_convert (p, &v1, &v2, TRUE));
+  g_assert_false (g_param_value_convert (p, &v1, &v2, TRUE));
   g_assert_cmpint (g_value_get_int (&v2), ==, -4);
 
-  g_assert (g_param_value_convert (p, &v1, &v2, FALSE));
+  g_assert_true (g_param_value_convert (p, &v1, &v2, FALSE));
   g_assert_cmpint (g_value_get_int (&v2), ==, 20);
 
   g_param_spec_unref (p);
@@ -139,11 +159,11 @@ test_value_transform (void)
   GValue dest = G_VALUE_INIT;
 
 #define CHECK_INT_CONVERSION(type, getter, value)                       \
-  g_assert (g_value_type_transformable (G_TYPE_INT, type));             \
+  g_assert_true (g_value_type_transformable (G_TYPE_INT, type));        \
   g_value_init (&src, G_TYPE_INT);                                      \
   g_value_init (&dest, type);                                           \
   g_value_set_int (&src, value);                                        \
-  g_assert (g_value_transform (&src, &dest));                           \
+  g_assert_true (g_value_transform (&src, &dest));                      \
   g_assert_cmpint (g_value_get_##getter (&dest), ==, value);            \
   g_value_unset (&src);                                                 \
   g_value_unset (&dest);
@@ -171,11 +191,11 @@ test_value_transform (void)
   CHECK_INT_CONVERSION(G_TYPE_DOUBLE, double, 12345678)
 
 #define CHECK_UINT_CONVERSION(type, getter, value)                      \
-  g_assert (g_value_type_transformable (G_TYPE_UINT, type));            \
+  g_assert_true (g_value_type_transformable (G_TYPE_UINT, type));       \
   g_value_init (&src, G_TYPE_UINT);                                     \
   g_value_init (&dest, type);                                           \
   g_value_set_uint (&src, value);                                       \
-  g_assert (g_value_transform (&src, &dest));                           \
+  g_assert_true (g_value_transform (&src, &dest));                      \
   g_assert_cmpuint (g_value_get_##getter (&dest), ==, value);           \
   g_value_unset (&src);                                                 \
   g_value_unset (&dest);
@@ -196,11 +216,11 @@ test_value_transform (void)
   CHECK_UINT_CONVERSION(G_TYPE_DOUBLE, double, 12345678)
 
 #define CHECK_LONG_CONVERSION(type, getter, value)                      \
-  g_assert (g_value_type_transformable (G_TYPE_LONG, type));            \
+  g_assert_true (g_value_type_transformable (G_TYPE_LONG, type));       \
   g_value_init (&src, G_TYPE_LONG);                                     \
   g_value_init (&dest, type);                                           \
   g_value_set_long (&src, value);                                       \
-  g_assert (g_value_transform (&src, &dest));                           \
+  g_assert_true (g_value_transform (&src, &dest));                      \
   g_assert_cmpint (g_value_get_##getter (&dest), ==, value);            \
   g_value_unset (&src);                                                 \
   g_value_unset (&dest);
@@ -221,11 +241,11 @@ test_value_transform (void)
   CHECK_LONG_CONVERSION(G_TYPE_DOUBLE, double, 12345678)
 
 #define CHECK_ULONG_CONVERSION(type, getter, value)                     \
-  g_assert (g_value_type_transformable (G_TYPE_ULONG, type));           \
+  g_assert_true (g_value_type_transformable (G_TYPE_ULONG, type));      \
   g_value_init (&src, G_TYPE_ULONG);                                    \
   g_value_init (&dest, type);                                           \
   g_value_set_ulong (&src, value);                                      \
-  g_assert (g_value_transform (&src, &dest));                           \
+  g_assert_true (g_value_transform (&src, &dest));                      \
   g_assert_cmpuint (g_value_get_##getter (&dest), ==, value);           \
   g_value_unset (&src);                                                 \
   g_value_unset (&dest);
@@ -246,11 +266,11 @@ test_value_transform (void)
   CHECK_ULONG_CONVERSION(G_TYPE_DOUBLE, double, 12345678)
 
 #define CHECK_INT64_CONVERSION(type, getter, value)                     \
-  g_assert (g_value_type_transformable (G_TYPE_INT64, type));           \
+  g_assert_true (g_value_type_transformable (G_TYPE_INT64, type));      \
   g_value_init (&src, G_TYPE_INT64);                                    \
   g_value_init (&dest, type);                                           \
   g_value_set_int64 (&src, value);                                      \
-  g_assert (g_value_transform (&src, &dest));                           \
+  g_assert_true (g_value_transform (&src, &dest));                      \
   g_assert_cmpint (g_value_get_##getter (&dest), ==, value);            \
   g_value_unset (&src);                                                 \
   g_value_unset (&dest);
@@ -271,11 +291,11 @@ test_value_transform (void)
   CHECK_INT64_CONVERSION(G_TYPE_DOUBLE, double, 12345678)
 
 #define CHECK_UINT64_CONVERSION(type, getter, value)                    \
-  g_assert (g_value_type_transformable (G_TYPE_UINT64, type));          \
+  g_assert_true (g_value_type_transformable (G_TYPE_UINT64, type));     \
   g_value_init (&src, G_TYPE_UINT64);                                   \
   g_value_init (&dest, type);                                           \
   g_value_set_uint64 (&src, value);                                     \
-  g_assert (g_value_transform (&src, &dest));                           \
+  g_assert_true (g_value_transform (&src, &dest));                      \
   g_assert_cmpuint (g_value_get_##getter (&dest), ==, value);           \
   g_value_unset (&src);                                                 \
   g_value_unset (&dest);
@@ -296,11 +316,11 @@ test_value_transform (void)
   CHECK_UINT64_CONVERSION(G_TYPE_DOUBLE, double, 12345678)
 
 #define CHECK_FLOAT_CONVERSION(type, getter, value)                    \
-  g_assert (g_value_type_transformable (G_TYPE_FLOAT, type));          \
+  g_assert_true (g_value_type_transformable (G_TYPE_FLOAT, type));     \
   g_value_init (&src, G_TYPE_FLOAT);                                   \
   g_value_init (&dest, type);                                          \
   g_value_set_float (&src, value);                                     \
-  g_assert (g_value_transform (&src, &dest));                          \
+  g_assert_true (g_value_transform (&src, &dest));                     \
   g_assert_cmpfloat (g_value_get_##getter (&dest), ==, value);         \
   g_value_unset (&src);                                                \
   g_value_unset (&dest);
@@ -321,11 +341,11 @@ test_value_transform (void)
   CHECK_FLOAT_CONVERSION(G_TYPE_DOUBLE, double, 12345678)
 
 #define CHECK_DOUBLE_CONVERSION(type, getter, value)                    \
-  g_assert (g_value_type_transformable (G_TYPE_DOUBLE, type));          \
+  g_assert_true (g_value_type_transformable (G_TYPE_DOUBLE, type));     \
   g_value_init (&src, G_TYPE_DOUBLE);                                   \
   g_value_init (&dest, type);                                           \
   g_value_set_double (&src, value);                                     \
-  g_assert (g_value_transform (&src, &dest));                           \
+  g_assert_true (g_value_transform (&src, &dest));                      \
   g_assert_cmpfloat (g_value_get_##getter (&dest), ==, value);          \
   g_value_unset (&src);                                                 \
   g_value_unset (&dest);
@@ -346,14 +366,14 @@ test_value_transform (void)
   CHECK_DOUBLE_CONVERSION(G_TYPE_DOUBLE, double, 12345678)
 
 #define CHECK_BOOLEAN_CONVERSION(type, setter, value)                   \
-  g_assert (g_value_type_transformable (type, G_TYPE_BOOLEAN));         \
+  g_assert_true (g_value_type_transformable (type, G_TYPE_BOOLEAN));    \
   g_value_init (&src, type);                                            \
   g_value_init (&dest, G_TYPE_BOOLEAN);                                 \
   g_value_set_##setter (&src, value);                                   \
-  g_assert (g_value_transform (&src, &dest));                           \
+  g_assert_true (g_value_transform (&src, &dest));                      \
   g_assert_cmpint (g_value_get_boolean (&dest), ==, TRUE);              \
   g_value_set_##setter (&src, 0);                                       \
-  g_assert (g_value_transform (&src, &dest));                           \
+  g_assert_true (g_value_transform (&src, &dest));                      \
   g_assert_cmpint (g_value_get_boolean (&dest), ==, FALSE);             \
   g_value_unset (&src);                                                 \
   g_value_unset (&dest);
@@ -366,11 +386,11 @@ test_value_transform (void)
   CHECK_BOOLEAN_CONVERSION(G_TYPE_UINT64, uint64, 12345678)
 
 #define CHECK_STRING_CONVERSION(int_type, setter, int_value)            \
-  g_assert (g_value_type_transformable (int_type, G_TYPE_STRING));      \
+  g_assert_true (g_value_type_transformable (int_type, G_TYPE_STRING)); \
   g_value_init (&src, int_type);                                        \
   g_value_init (&dest, G_TYPE_STRING);                                  \
   g_value_set_##setter (&src, int_value);                               \
-  g_assert (g_value_transform (&src, &dest));                           \
+  g_assert_true (g_value_transform (&src, &dest));                      \
   g_assert_cmpstr (g_value_get_string (&dest), ==, #int_value);         \
   g_value_unset (&src);                                                 \
   g_value_unset (&dest);
@@ -384,12 +404,12 @@ test_value_transform (void)
   CHECK_STRING_CONVERSION(G_TYPE_FLOAT, float, 0.500000)
   CHECK_STRING_CONVERSION(G_TYPE_DOUBLE, double, -1.234567)
 
-  g_assert (!g_value_type_transformable (G_TYPE_STRING, G_TYPE_CHAR));
+  g_assert_false (g_value_type_transformable (G_TYPE_STRING, G_TYPE_CHAR));
   g_value_init (&src, G_TYPE_STRING);
   g_value_init (&dest, G_TYPE_CHAR);
   g_value_set_static_string (&src, "bla");
   g_value_set_schar (&dest, 'c');
-  g_assert (!g_value_transform (&src, &dest));
+  g_assert_false (g_value_transform (&src, &dest));
   g_assert_cmpint (g_value_get_schar (&dest), ==, 'c');
   g_value_unset (&src);
   g_value_unset (&dest);
@@ -757,7 +777,7 @@ test_param_implement (void)
               {
               case 0:
                 /* make sure the other table agrees */
-                g_assert (valid_impl_types[change_this_type * 16 + change_this_flag][use_this_type] == 0);
+                g_assert_cmpint (valid_impl_types[change_this_type * 16 + change_this_flag][use_this_type], ==, 0);
                 g_test_trap_assert_failed ();
                 g_test_trap_assert_stderr ("*Interface property does not exist*");
                 continue;
@@ -820,7 +840,7 @@ test_param_default (void)
   param = g_param_spec_int ("my-int", "My Int", "Blurb", 0, 20, 10, G_PARAM_READWRITE);
   def = g_param_spec_get_default_value (param);
 
-  g_assert (G_VALUE_HOLDS (def, G_TYPE_INT));
+  g_assert_true (G_VALUE_HOLDS (def, G_TYPE_INT));
   g_assert_cmpint (g_value_get_int (def), ==, 10);
 
   g_param_spec_unref (param);
@@ -836,6 +856,9 @@ main (int argc, char *argv[])
 
   g_test_add_func ("/param/value", test_param_value);
   g_test_add_func ("/param/strings", test_param_strings);
+  g_test_add_data_func ("/param/invalid-name/colon", "my_int:hello", test_param_invalid_name);
+  g_test_add_data_func ("/param/invalid-name/first-char", "7zip", test_param_invalid_name);
+  g_test_add_data_func ("/param/invalid-name/empty", "", test_param_invalid_name);
   g_test_add_func ("/param/qdata", test_param_qdata);
   g_test_add_func ("/param/validate", test_param_validate);
   g_test_add_func ("/param/convert", test_param_convert);

gobject/tests/signals.c

@@ -180,7 +180,16 @@ test_class_init (TestClass *klass)
                 NULL,
                 G_TYPE_NONE,
                 0);
-  simple2_id = g_signal_new ("simple-2",
+  g_signal_new ("simple-detailed",
+                G_TYPE_FROM_CLASS (klass),
+                G_SIGNAL_RUN_LAST | G_SIGNAL_DETAILED,
+                0,
+                NULL, NULL,
+                NULL,
+                G_TYPE_NONE,
+                0);
+  /* Deliberately install this one in non-canonical form to check that’s handled correctly: */
+  simple2_id = g_signal_new ("simple_2",
                 G_TYPE_FROM_CLASS (klass),
                 G_SIGNAL_RUN_LAST | G_SIGNAL_NO_RECURSE,
                 0,
@@ -455,16 +464,16 @@ test_variant_signal (void)
 
   v = g_variant_new_boolean (TRUE);
   g_variant_ref (v);
-  g_assert (g_variant_is_floating (v));
+  g_assert_true (g_variant_is_floating (v));
   g_signal_emit_by_name (test, "variant-changed-no-slot", v);
-  g_assert (!g_variant_is_floating (v));
+  g_assert_false (g_variant_is_floating (v));
   g_variant_unref (v);
 
   v = g_variant_new_boolean (TRUE);
   g_variant_ref (v);
-  g_assert (g_variant_is_floating (v));
+  g_assert_true (g_variant_is_floating (v));
   g_signal_emit_by_name (test, "variant-changed", v);
-  g_assert (!g_variant_is_floating (v));
+  g_assert_false (g_variant_is_floating (v));
   g_variant_unref (v);
 
   g_object_unref (test);
@@ -485,7 +494,7 @@ on_generic_marshaller_1 (Test *obj,
   g_assert_cmpint (v_uchar, ==, 43);
   g_assert_cmpint (v_int, ==, 4096);
   g_assert_cmpint (v_long, ==, 8192);
-  g_assert (v_pointer == NULL);
+  g_assert_null (v_pointer);
   g_assert_cmpfloat (v_double, >, 0.0);
   g_assert_cmpfloat (v_double, <, 1.0);
   g_assert_cmpfloat (v_float, >, 5.0);
@@ -754,7 +763,7 @@ custom_marshaller_callback (Test                  *test,
 {
   GSignalInvocationHint *ihint;
 
-  g_assert (hint != &dont_use_this);
+  g_assert_true (hint != &dont_use_this);
 
   ihint = g_signal_get_invocation_hint (test);
 
@@ -801,7 +810,7 @@ all_types_handler (Test *test, int i, gboolean b, char c, guchar uc, guint ui, g
   g_assert_cmpstr (str, ==, "Test");
   g_assert_cmpstr (g_param_spec_get_nick (param), ==, "nick");
   g_assert_cmpstr (g_bytes_get_data (bytes, NULL), ==, "Blah");
-  g_assert (ptr == &enum_type);
+  g_assert_true (ptr == &enum_type);
   g_assert_cmpuint (g_variant_get_uint16 (var), == , 99);
   g_assert_cmpint (i64, ==, G_MAXINT64 - 1234);
   g_assert_cmpuint (ui64, ==, G_MAXUINT64 - 123456);
@@ -810,7 +819,7 @@ all_types_handler (Test *test, int i, gboolean b, char c, guchar uc, guint ui, g
 static void
 all_types_handler_cb (Test *test, int i, gboolean b, char c, guchar uc, guint ui, glong l, gulong ul, MyEnum e, guint f, float fl, double db, char *str, GParamSpec *param, GBytes *bytes, gpointer ptr, Test *obj, GVariant *var, gint64 i64, guint64 ui64, gpointer user_data)
 {
-  g_assert (user_data == &flags_type);
+  g_assert_true (user_data == &flags_type);
   all_types_handler (test, i, b, c, uc, ui, l, ul, e, f, fl, db, str, param, bytes, ptr, obj, var, i64, ui64);
 }
 
@@ -1064,6 +1073,7 @@ test_introspection (void)
   gint i;
   const gchar *names[] = {
     "simple",
+    "simple-detailed",
     "simple-2",
     "generic-marshaller-1",
     "generic-marshaller-2",
@@ -1091,15 +1101,15 @@ test_introspection (void)
   for (i = 0; i < n_ids; i++)
     {
       name = g_signal_name (ids[i]);
-      g_assert (in_set (name, names));
+      g_assert_true (in_set (name, names));
     }
 
   g_signal_query (simple_id, &query);
   g_assert_cmpuint (query.signal_id, ==, simple_id);
   g_assert_cmpstr (query.signal_name, ==, "simple");
-  g_assert (query.itype == test_get_type ());
+  g_assert_true (query.itype == test_get_type ());
-  g_assert (query.signal_flags == G_SIGNAL_RUN_LAST);
+  g_assert_cmpint (query.signal_flags, ==, G_SIGNAL_RUN_LAST);
-  g_assert (query.return_type == G_TYPE_NONE);
+  g_assert_cmpint (query.return_type, ==, G_TYPE_NONE);
   g_assert_cmpuint (query.n_params, ==, 0);
 
   g_free (ids);
@@ -1129,7 +1139,7 @@ test_block_handler (void)
 
   handler = g_signal_handler_find (test1, G_SIGNAL_MATCH_ID, simple_id, 0, NULL, NULL, NULL);
 
-  g_assert (handler == handler1);
+  g_assert_true (handler == handler1);
 
   g_assert_cmpint (count1, ==, 0);
   g_assert_cmpint (count2, ==, 0);
@@ -1238,7 +1248,7 @@ test_signal_disconnect_wrong_object (void)
   g_test_assert_expected_messages ();
 
   /* it's still connected */
-  g_assert (g_signal_handler_is_connected (object, signal_id));
+  g_assert_true (g_signal_handler_is_connected (object, signal_id));
 
   g_object_unref (object);
   g_object_unref (object2);
@@ -1276,6 +1286,172 @@ test_clear_signal_handler (void)
   g_object_unref (test_obj);
 }
 
+static void
+test_lookup (void)
+{
+  GTypeClass *test_class;
+  guint signal_id, saved_signal_id;
+
+  g_test_summary ("Test that g_signal_lookup() works with a variety of inputs.");
+
+  test_class = g_type_class_ref (test_get_type ());
+
+  signal_id = g_signal_lookup ("all-types", test_get_type ());
+  g_assert_cmpint (signal_id, !=, 0);
+
+  saved_signal_id = signal_id;
+
+  /* Try with a non-canonical name. */
+  signal_id = g_signal_lookup ("all_types", test_get_type ());
+  g_assert_cmpint (signal_id, ==, saved_signal_id);
+
+  /* Looking up a non-existent signal should return nothing. */
+  g_assert_cmpint (g_signal_lookup ("nope", test_get_type ()), ==, 0);
+
+  g_type_class_unref (test_class);
+}
+
+static void
+test_lookup_invalid (void)
+{
+  g_test_summary ("Test that g_signal_lookup() emits a warning if looking up an invalid signal name.");
+
+  if (g_test_subprocess ())
+    {
+      GTypeClass *test_class;
+      guint signal_id;
+
+      test_class = g_type_class_ref (test_get_type ());
+
+      signal_id = g_signal_lookup ("", test_get_type ());
+      g_assert_cmpint (signal_id, ==, 0);
+
+      g_type_class_unref (test_class);
+      return;
+    }
+
+  g_test_trap_subprocess (NULL, 0, 0);
+  g_test_trap_assert_failed ();
+  g_test_trap_assert_stderr ("*WARNING*unable to look up invalid signal name*");
+}
+
+static void
+test_parse_name (void)
+{
+  GTypeClass *test_class;
+  guint signal_id, saved_signal_id;
+  gboolean retval;
+  GQuark detail, saved_detail;
+
+  g_test_summary ("Test that g_signal_parse_name() works with a variety of inputs.");
+
+  test_class = g_type_class_ref (test_get_type ());
+
+  /* Simple test. */
+  retval = g_signal_parse_name ("simple-detailed", test_get_type (), &signal_id, &detail, TRUE);
+  g_assert_true (retval);
+  g_assert_cmpint (signal_id, !=, 0);
+  g_assert_cmpint (detail, ==, 0);
+
+  saved_signal_id = signal_id;
+
+  /* Simple test with detail. */
+  retval = g_signal_parse_name ("simple-detailed::a-detail", test_get_type (), &signal_id, &detail, TRUE);
+  g_assert_true (retval);
+  g_assert_cmpint (signal_id, ==, saved_signal_id);
+  g_assert_cmpint (detail, !=, 0);
+
+  saved_detail = detail;
+
+  /* Simple test with the same detail again. */
+  retval = g_signal_parse_name ("simple-detailed::a-detail", test_get_type (), &signal_id, &detail, FALSE);
+  g_assert_true (retval);
+  g_assert_cmpint (signal_id, ==, saved_signal_id);
+  g_assert_cmpint (detail, ==, saved_detail);
+
+  /* Simple test with a new detail. */
+  retval = g_signal_parse_name ("simple-detailed::another-detail", test_get_type (), &signal_id, &detail, FALSE);
+  g_assert_true (retval);
+  g_assert_cmpint (signal_id, ==, saved_signal_id);
+  g_assert_cmpint (detail, ==, 0);  /* we didn’t force the quark */
+
+  /* Canonicalisation shouldn’t affect the results. */
+  retval = g_signal_parse_name ("simple_detailed::a-detail", test_get_type (), &signal_id, &detail, FALSE);
+  g_assert_true (retval);
+  g_assert_cmpint (signal_id, ==, saved_signal_id);
+  g_assert_cmpint (detail, ==, saved_detail);
+
+  /* Details don’t have to look like property names. */
+  retval = g_signal_parse_name ("simple-detailed::hello::world", test_get_type (), &signal_id, &detail, TRUE);
+  g_assert_true (retval);
+  g_assert_cmpint (signal_id, ==, saved_signal_id);
+  g_assert_cmpint (detail, !=, 0);
+
+  /* Trying to parse a detail for a signal which isn’t %G_SIGNAL_DETAILED should fail. */
+  retval = g_signal_parse_name ("all-types::a-detail", test_get_type (), &signal_id, &detail, FALSE);
+  g_assert_false (retval);
+
+  g_type_class_unref (test_class);
+}
+
+static void
+test_parse_name_invalid (void)
+{
+  GTypeClass *test_class;
+  gsize i;
+  guint signal_id;
+  GQuark detail;
+  const gchar *vectors[] =
+    {
+      "",
+      "7zip",
+      "invalid:signal",
+      "simple-detailed::",
+      "simple-detailed:",
+      ":",
+      "::",
+      ":valid-detail",
+      "::valid-detail",
+    };
+
+  g_test_summary ("Test that g_signal_parse_name() ignores a variety of invalid inputs.");
+
+  test_class = g_type_class_ref (test_get_type ());
+
+  for (i = 0; i < G_N_ELEMENTS (vectors); i++)
+    {
+      g_test_message ("Parser input: %s", vectors[i]);
+      g_assert_false (g_signal_parse_name (vectors[i], test_get_type (), &signal_id, &detail, TRUE));
+    }
+
+  g_type_class_unref (test_class);
+}
+
+static void
+test_signals_invalid_name (gconstpointer test_data)
+{
+  const gchar *signal_name = test_data;
+
+  g_test_summary ("Check that g_signal_new() rejects invalid signal names.");
+
+  if (g_test_subprocess ())
+    {
+      g_signal_new (signal_name,
+                    test_get_type (),
+                    G_SIGNAL_RUN_LAST | G_SIGNAL_NO_RECURSE,
+                    0,
+                    NULL, NULL,
+                    NULL,
+                    G_TYPE_NONE,
+                    0);
+      return;
+    }
+
+  g_test_trap_subprocess (NULL, 0, 0);
+  g_test_trap_assert_failed ();
+  g_test_trap_assert_stderr ("*CRITICAL*is_valid_signal_name (signal_name)*");
+}
+
 /* --- */
 
 int
@@ -1302,6 +1478,13 @@ main (int argc,
   g_test_add_func ("/gobject/signals/invocation-hint", test_invocation_hint);
   g_test_add_func ("/gobject/signals/test-disconnection-wrong-object", test_signal_disconnect_wrong_object);
   g_test_add_func ("/gobject/signals/clear-signal-handler", test_clear_signal_handler);
+  g_test_add_func ("/gobject/signals/lookup", test_lookup);
+  g_test_add_func ("/gobject/signals/lookup/invalid", test_lookup_invalid);
+  g_test_add_func ("/gobject/signals/parse-name", test_parse_name);
+  g_test_add_func ("/gobject/signals/parse-name/invalid", test_parse_name_invalid);
+  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);
 
   return g_test_run ();
 }