Merge branch 'value-interned' into 'master'

GValue: Add interned string support

Closes #2109

See merge request GNOME/glib!1497

docs/reference/gobject/gobject-sections.txt

@@ -684,6 +684,8 @@ G_IS_PARAM_SPEC_STRING
 G_PARAM_SPEC_STRING
 G_VALUE_HOLDS_STRING
 G_TYPE_PARAM_STRING
+G_VALUE_IS_INTERNED_STRING
+G_VALUE_INTERNED_STRING
 GParamSpecString
 gchararray
 g_param_spec_string
@@ -693,6 +695,7 @@ g_value_take_string
 g_value_set_string_take_ownership
 g_value_get_string
 g_value_dup_string
+g_value_set_interned_string
 
 <SUBSECTION GParamSpec>
 G_IS_PARAM_SPEC_PARAM

gobject/gbinding.c

@@ -540,7 +540,7 @@ g_binding_get_property (GObject    *gobject,
 
     case PROP_SOURCE_PROPERTY:
       /* @source_property is interned, so we don’t need to take a copy */
-      g_value_set_static_string (value, binding->source_property);
+      g_value_set_interned_string (value, binding->source_property);
       break;
 
     case PROP_TARGET:
@@ -549,7 +549,7 @@ g_binding_get_property (GObject    *gobject,
 
     case PROP_TARGET_PROPERTY:
       /* @target_property is interned, so we don’t need to take a copy */
-      g_value_set_static_string (value, binding->target_property);
+      g_value_set_interned_string (value, binding->target_property);
       break;
 
     case PROP_FLAGS:

gobject/gvalue.h

@@ -175,6 +175,16 @@ void	g_value_register_transform_func	(GType		 src_type,
  */
 #define G_VALUE_NOCOPY_CONTENTS (1 << 27)
 
+/**
+ * G_VALUE_INTERNED_STRING:
+ *
+ * For string values, indicates that the string contained is canonical and will
+ * exist for the duration of the process. See g_value_set_interned_string().
+ *
+ * Since: 2.66
+ */
+#define G_VALUE_INTERNED_STRING (1 << 28) GLIB_AVAILABLE_MACRO_IN_2_66
+
 /**
  * G_VALUE_INIT:
  *

gobject/gvaluetypes.c

@@ -271,7 +271,16 @@ static void
 value_copy_string (const GValue *src_value,
 		   GValue	*dest_value)
 {
-  dest_value->data[0].v_pointer = g_strdup (src_value->data[0].v_pointer);
+  if (src_value->data[1].v_uint & G_VALUE_NOCOPY_CONTENTS)
+    {
+      dest_value->data[0].v_pointer = src_value->data[0].v_pointer;
+      dest_value->data[1].v_uint = src_value->data[1].v_uint;
+    }
+  else
+    {
+      dest_value->data[0].v_pointer = g_strdup (src_value->data[0].v_pointer);
+      dest_value->data[1].v_uint = src_value->data[1].v_uint;
+    }
 }
 
 static gchar*
@@ -1053,6 +1062,9 @@ g_value_set_string (GValue	*value,
  * Set the contents of a %G_TYPE_STRING #GValue to @v_string.
  * The string is assumed to be static, and is thus not duplicated
  * when setting the #GValue.
+ *
+ * If the the string is a canonical string, using g_value_set_interned_string()
+ * is more appropriate.
  */
 void
 g_value_set_static_string (GValue      *value,
@@ -1066,6 +1078,29 @@ g_value_set_static_string (GValue      *value,
   value->data[0].v_pointer = (gchar*) v_string;
 }
 
+/**
+ * g_value_set_interned_string:
+ * @value: a valid #GValue of type %G_TYPE_STRING
+ * @v_string: (nullable): static string to be set
+ *
+ * Set the contents of a %G_TYPE_STRING #GValue to @v_string.  The string is
+ * assumed to be static and interned (canonical, for example from
+ * g_intern_string()), and is thus not duplicated when setting the #GValue.
+ *
+ * Since: 2.66
+ */
+void
+g_value_set_interned_string (GValue *value,
+                             const gchar *v_string)
+{
+  g_return_if_fail (G_VALUE_HOLDS_STRING (value));
+
+  if (!(value->data[1].v_uint & G_VALUE_NOCOPY_CONTENTS))
+    g_free (value->data[0].v_pointer);
+  value->data[1].v_uint = G_VALUE_NOCOPY_CONTENTS | G_VALUE_INTERNED_STRING;
+  value->data[0].v_pointer = (gchar *) v_string;
+}
+
 /**
  * g_value_set_string_take_ownership:
  * @value: a valid #GValue of type %G_TYPE_STRING

gobject/gvaluetypes.h

@@ -136,6 +136,19 @@ G_BEGIN_DECLS
  * Returns: %TRUE on success.
  */
 #define G_VALUE_HOLDS_STRING(value)	 (G_TYPE_CHECK_VALUE_TYPE ((value), G_TYPE_STRING))
+/**
+ * G_VALUE_IS_INTERNED_STRING:
+ * @value: a valid #GValue structure
+ *
+ * Checks whether @value contains a string which is canonical.
+ *
+ * Returns: %TRUE if the value contains a string in its canonical
+ * representation, as returned by g_intern_string(). See also
+ * g_value_set_interned_string().
+ *
+ * Since: 2.66
+ */
+#define G_VALUE_IS_INTERNED_STRING(value) (G_VALUE_HOLDS_STRING (value) && ((value)->data[1].v_uint & G_VALUE_INTERNED_STRING)) GLIB_AVAILABLE_MACRO_IN_2_66
 /**
  * G_VALUE_HOLDS_POINTER:
  * @value: a valid #GValue structure
@@ -241,6 +254,9 @@ void		      g_value_set_string	(GValue	      *value,
 GLIB_AVAILABLE_IN_ALL
 void		      g_value_set_static_string (GValue	      *value,
 						 const gchar  *v_string);
+GLIB_AVAILABLE_IN_2_66
+void		      g_value_set_interned_string (GValue      *value,
+						   const gchar  *v_string);
 GLIB_AVAILABLE_IN_ALL
 const gchar *         g_value_get_string	(const GValue *value);
 GLIB_AVAILABLE_IN_ALL

gobject/tests/value.c

@@ -31,6 +31,184 @@ test_value_basic (void)
   g_assert_false (G_VALUE_HOLDS_INT (&value));
 }
 
+static void
+test_value_string (void)
+{
+  const gchar *static1 = "static1";
+  const gchar *static2 = "static2";
+  const gchar *storedstr;
+  const gchar *copystr;
+  gchar *str1, *str2;
+  GValue value = G_VALUE_INIT;
+  GValue copy = G_VALUE_INIT;
+
+  g_test_summary ("Test that G_TYPE_STRING GValue copy properly");
+
+  /*
+   * Regular strings (ownership not passed)
+   */
+
+  /* Create a regular string gvalue and make sure it copies the provided string */
+  g_value_init (&value, G_TYPE_STRING);
+  g_assert_true (G_VALUE_HOLDS_STRING (&value));
+
+  /* The string contents should be empty at this point */
+  storedstr = g_value_get_string (&value);
+  g_assert_true (storedstr == NULL);
+
+  g_value_set_string (&value, static1);
+  /* The contents should be a copy of the same string */
+  storedstr = g_value_get_string (&value);
+  g_assert_true (storedstr != static1);
+  g_assert_cmpstr (storedstr, ==, static1);
+  /* Check g_value_dup_string() provides a copy */
+  str1 = g_value_dup_string (&value);
+  g_assert_true (storedstr != str1);
+  g_assert_cmpstr (str1, ==, static1);
+  g_free (str1);
+
+  /* Copying a regular string gvalue should copy the contents */
+  g_value_init (&copy, G_TYPE_STRING);
+  g_value_copy (&value, &copy);
+  copystr = g_value_get_string (&copy);
+  g_assert_true (copystr != storedstr);
+  g_assert_cmpstr (copystr, ==, static1);
+  g_value_unset (&copy);
+
+  /* Setting a new string should change the contents */
+  g_value_set_string (&value, static2);
+  /* The contents should be a copy of that *new* string */
+  storedstr = g_value_get_string (&value);
+  g_assert_true (storedstr != static2);
+  g_assert_cmpstr (storedstr, ==, static2);
+
+  /* Setting a static string over that should also change it (test for
+   * coverage and valgrind) */
+  g_value_set_static_string (&value, static1);
+  storedstr = g_value_get_string (&value);
+  g_assert_true (storedstr != static2);
+  g_assert_cmpstr (storedstr, ==, static1);
+
+  /* Giving a string directly (ownership passed) should replace the content */
+  str2 = g_strdup (static2);
+  g_value_take_string (&value, str2);
+  storedstr = g_value_get_string (&value);
+  g_assert_true (storedstr != static2);
+  g_assert_cmpstr (storedstr, ==, str2);
+
+  g_value_unset (&value);
+
+  /*
+   * Regular strings (ownership passed)
+   */
+
+  g_value_init (&value, G_TYPE_STRING);
+  g_assert_true (G_VALUE_HOLDS_STRING (&value));
+  str1 = g_strdup (static1);
+  g_value_take_string (&value, str1);
+  /* The contents should be the string we provided */
+  storedstr = g_value_get_string (&value);
+  g_assert_true (storedstr == str1);
+  /* But g_value_dup_string() should provide a copy */
+  str2 = g_value_dup_string (&value);
+  g_assert_true (storedstr != str2);
+  g_assert_cmpstr (str2, ==, static1);
+  g_free (str2);
+
+  /* Copying a regular string gvalue (even with ownership passed) should copy
+   * the contents */
+  g_value_init (&copy, G_TYPE_STRING);
+  g_value_copy (&value, &copy);
+  copystr = g_value_get_string (&copy);
+  g_assert_true (copystr != storedstr);
+  g_assert_cmpstr (copystr, ==, static1);
+  g_value_unset (&copy);
+
+  /* Setting a new regular string should change the contents */
+  g_value_set_string (&value, static2);
+  /* The contents should be a copy of that *new* string */
+  storedstr = g_value_get_string (&value);
+  g_assert_true (storedstr != static2);
+  g_assert_cmpstr (storedstr, ==, static2);
+
+  g_value_unset (&value);
+
+  /*
+   * Static strings
+   */
+  g_value_init (&value, G_TYPE_STRING);
+  g_assert_true (G_VALUE_HOLDS_STRING (&value));
+  g_value_set_static_string (&value, static1);
+  /* The contents should be the string we provided */
+  storedstr = g_value_get_string (&value);
+  g_assert_true (storedstr == static1);
+  /* But g_value_dup_string() should provide a copy */
+  str2 = g_value_dup_string (&value);
+  g_assert_true (storedstr != str2);
+  g_assert_cmpstr (str2, ==, static1);
+  g_free (str2);
+
+  /* Copying a static string gvalue should *not* copy the contents */
+  g_value_init (&copy, G_TYPE_STRING);
+  g_value_copy (&value, &copy);
+  copystr = g_value_get_string (&copy);
+  g_assert_true (copystr == static1);
+  g_value_unset (&copy);
+
+  /* Setting a new string should change the contents */
+  g_value_set_static_string (&value, static2);
+  /* The contents should be a copy of that *new* string */
+  storedstr = g_value_get_string (&value);
+  g_assert_true (storedstr != static1);
+  g_assert_cmpstr (storedstr, ==, static2);
+
+  g_value_unset (&value);
+
+  /*
+   * Interned/Canonical strings
+   */
+  static1 = g_intern_static_string (static1);
+  g_value_init (&value, G_TYPE_STRING);
+  g_assert_true (G_VALUE_HOLDS_STRING (&value));
+  g_value_set_interned_string (&value, static1);
+  g_assert_true (G_VALUE_IS_INTERNED_STRING (&value));
+  /* The contents should be the string we provided */
+  storedstr = g_value_get_string (&value);
+  g_assert_true (storedstr == static1);
+  /* But g_value_dup_string() should provide a copy */
+  str2 = g_value_dup_string (&value);
+  g_assert_true (storedstr != str2);
+  g_assert_cmpstr (str2, ==, static1);
+  g_free (str2);
+
+  /* Copying an interned string gvalue should *not* copy the contents
+   * and should still be an interned string */
+  g_value_init (&copy, G_TYPE_STRING);
+  g_value_copy (&value, &copy);
+  g_assert_true (G_VALUE_IS_INTERNED_STRING (&copy));
+  copystr = g_value_get_string (&copy);
+  g_assert_true (copystr == static1);
+  g_value_unset (&copy);
+
+  /* Setting a new interned string should change the contents */
+  static2 = g_intern_static_string (static2);
+  g_value_set_interned_string (&value, static2);
+  g_assert_true (G_VALUE_IS_INTERNED_STRING (&value));
+  /* The contents should be the interned string */
+  storedstr = g_value_get_string (&value);
+  g_assert_cmpstr (storedstr, ==, static2);
+
+  /* Setting a new regular string should change the contents */
+  g_value_set_string (&value, static2);
+  g_assert_false (G_VALUE_IS_INTERNED_STRING (&value));
+  /* The contents should be a copy of that *new* string */
+  storedstr = g_value_get_string (&value);
+  g_assert_true (storedstr != static2);
+  g_assert_cmpstr (storedstr, ==, static2);
+
+  g_value_unset (&value);
+}
+
 static gint
 cmpint (gconstpointer a, gconstpointer b)
 {
@@ -89,6 +267,7 @@ main (int argc, char *argv[])
   g_test_init (&argc, &argv, NULL);
 
   g_test_add_func ("/value/basic", test_value_basic);
+  g_test_add_func ("/value/string", test_value_string);
   g_test_add_func ("/value/array/basic", test_valuearray_basic);
 
   return g_test_run ();