From: Ryan Lortie Date: Sat, 29 Nov 2014 20:58:49 +0000 (-0500) Subject: GVariant: clean up serialised data handling API X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=1a3783987be7aa436c4b0789ce019289773d5018;p=platform%2Fupstream%2Fglib.git GVariant: clean up serialised data handling API Add a new pair of internal helpers to gvariant-core.c to provide a unified way to construct and query the content of serialised GVariant instances. Rewrite g_variant_new_from_data() to use this. Move g_variant_new_from_bytes() and g_variant_get_data_as_bytes() out of -core and into gvariant.c, also rewriting them to use the same. Take the time to do some cleanup and make some general improvements in consistency: - move the checks for improperly sized fixed-sized data out of _new_from_bytes() and into the common code so that we do this check on _new_from_data() as well - correctly deal with the case of NULL data in _get_data_as_bytes(). This would have crashed before. Add a test for that. - if the user hands us data with a size of zero then unref and/or destroy-notify things immediately. The idea that every GVariant must have an associated GBytes remains. This could potentially be optimsed a bit further in the future, but the cases where it could be avoided are only a few (errors, zero-size, static stoarge) so let's not pursue that now. --- diff --git a/glib/gvariant-core.c b/glib/gvariant-core.c index 0f68168..1a0c041 100644 --- a/glib/gvariant-core.c +++ b/glib/gvariant-core.c @@ -509,58 +509,6 @@ g_variant_alloc (const GVariantType *type, return value; } -/** - * g_variant_new_from_bytes: - * @type: a #GVariantType - * @bytes: a #GBytes - * @trusted: if the contents of @bytes are trusted - * - * Constructs a new serialised-mode #GVariant instance. This is the - * inner interface for creation of new serialised values that gets - * called from various functions in gvariant.c. - * - * A reference is taken on @bytes. - * - * Returns: (transfer none): a new #GVariant with a floating reference - * - * Since: 2.36 - */ -GVariant * -g_variant_new_from_bytes (const GVariantType *type, - GBytes *bytes, - gboolean trusted) -{ - GVariant *value; - guint alignment; - gsize size; - - value = g_variant_alloc (type, TRUE, trusted); - - value->contents.serialised.bytes = g_bytes_ref (bytes); - - g_variant_type_info_query (value->type_info, - &alignment, &size); - - if (size && g_bytes_get_size (bytes) != size) - { - /* Creating a fixed-sized GVariant with a bytes of the wrong - * size. - * - * We should do the equivalent of pulling a fixed-sized child out - * of a brozen container (ie: data is NULL size is equal to the correct - * fixed size). - */ - value->contents.serialised.data = NULL; - value->size = size; - } - else - { - value->contents.serialised.data = g_bytes_get_data (bytes, &value->size); - } - - return value; -} - /* -- internal -- */ /* < internal > @@ -597,6 +545,63 @@ g_variant_new_from_children (const GVariantType *type, } /* < internal > + * g_variant_new_serialised: + * @type: a #GVariantType + * @bytes: the #GBytes holding @data + * @data: a pointer to the serialised data + * @size: the size of @data, in bytes + * @trusted: %TRUE if @data is trusted + * + * Constructs a new serialised #GVariant instance. This is the inner + * interface for creation of new serialised values that gets called from + * various functions in gvariant.c. + * + * @bytes is consumed by this function. g_bytes_unref() will be called + * on it some time later. + * + * Returns: a new #GVariant with a floating reference + */ +GVariant * +g_variant_new_serialised (const GVariantType *type, + GBytes *bytes, + gconstpointer data, + gsize size, + gboolean trusted) +{ + GVariant *value; + gsize fixed_size; + + value = g_variant_alloc (type, TRUE, trusted); + value->contents.serialised.bytes = bytes; + value->contents.serialised.data = data; + value->size = size; + + g_variant_type_info_query (value->type_info, NULL, &fixed_size); + if G_UNLIKELY (fixed_size && size != fixed_size) + { + /* Creating a fixed-sized GVariant with a bytes of the wrong + * size. + * + * We should do the equivalent of pulling a fixed-sized child out + * of a broken container (ie: data is NULL size is equal to the correct + * fixed size). + * + * This really ought not to happen if the data is trusted... + */ + if (trusted) + g_error ("Attempting to create a trusted GVariant instance out of invalid data"); + + /* We hang on to the GBytes (even though we don't use it anymore) + * because every GVariant must have a GBytes. + */ + value->contents.serialised.data = NULL; + value->size = fixed_size; + } + + return value; +} + +/* < internal > * g_variant_get_type_info: * @value: a #GVariant * @@ -634,6 +639,32 @@ g_variant_is_trusted (GVariant *value) return (value->state & STATE_TRUSTED) != 0; } +/* < internal > + * g_variant_get_serialised: + * @value: a #GVariant + * @bytes: (out) (transfer none): a location to store the #GBytes + * @size: (out): a location to store the size of the returned data + * + * Ensures that @value is in serialised form and returns information + * about it. This is called from various APIs in gvariant.c + * + * Returns: data, of length @size + */ +gconstpointer +g_variant_get_serialised (GVariant *value, + GBytes **bytes, + gsize *size) +{ + g_variant_ensure_serialised (value); + + if (bytes) + *bytes = value->contents.serialised.bytes; + + *size = value->size; + + return value->contents.serialised.data; +} + /* -- public -- */ /** @@ -885,40 +916,6 @@ g_variant_get_data (GVariant *value) return value->contents.serialised.data; } -/** - * g_variant_get_data_as_bytes: - * @value: a #GVariant - * - * Returns a pointer to the serialised form of a #GVariant instance. - * The semantics of this function are exactly the same as - * g_variant_get_data(), except that the returned #GBytes holds - * a reference to the variant data. - * - * Returns: (transfer full): A new #GBytes representing the variant data - * - * Since: 2.36 - */ -GBytes * -g_variant_get_data_as_bytes (GVariant *value) -{ - const gchar *bytes_data; - const gchar *data; - gsize bytes_size; - gsize size; - - g_variant_ensure_serialised (value); - - bytes_data = g_bytes_get_data (value->contents.serialised.bytes, &bytes_size); - data = value->contents.serialised.data; - size = value->size; - - if (data == bytes_data && size == bytes_size) - return g_bytes_ref (value->contents.serialised.bytes); - else - return g_bytes_new_from_bytes (value->contents.serialised.bytes, - data - bytes_data, size); -} - /** * g_variant_n_children: diff --git a/glib/gvariant-core.h b/glib/gvariant-core.h index 034dd43..8e7d6cc 100644 --- a/glib/gvariant-core.h +++ b/glib/gvariant-core.h @@ -30,8 +30,18 @@ GVariant * g_variant_new_from_children (const G gsize n_children, gboolean trusted); +GVariant * g_variant_new_serialised (const GVariantType *type, + GBytes *bytes, + gconstpointer data, + gsize size, + gboolean trusted); + gboolean g_variant_is_trusted (GVariant *value); GVariantTypeInfo * g_variant_get_type_info (GVariant *value); +gconstpointer g_variant_get_serialised (GVariant *value, + GBytes **bytes, + gsize *size); + #endif /* __G_VARIANT_CORE_H__ */ diff --git a/glib/gvariant.c b/glib/gvariant.c index 08421bf..7251d33 100644 --- a/glib/gvariant.c +++ b/glib/gvariant.c @@ -288,14 +288,9 @@ g_variant_new_from_trusted (const GVariantType *type, gconstpointer data, gsize size) { - GVariant *value; - GBytes *bytes; - - bytes = g_bytes_new (data, size); - value = g_variant_new_from_bytes (type, bytes, TRUE); - g_bytes_unref (bytes); + gpointer mydata = g_memdup (data, size); - return value; + return g_variant_new_serialised (type, g_bytes_new_take (mydata, size), mydata, size, TRUE); } /** @@ -5932,21 +5927,99 @@ g_variant_new_from_data (const GVariantType *type, GDestroyNotify notify, gpointer user_data) { - GVariant *value; GBytes *bytes; g_return_val_if_fail (g_variant_type_is_definite (type), NULL); g_return_val_if_fail (data != NULL || size == 0, NULL); + if (size == 0) + { + if (notify) + { + (* notify) (user_data); + notify = NULL; + } + + data = NULL; + } + if (notify) bytes = g_bytes_new_with_free_func (data, size, notify, user_data); else bytes = g_bytes_new_static (data, size); - value = g_variant_new_from_bytes (type, bytes, trusted); - g_bytes_unref (bytes); + return g_variant_new_serialised (type, bytes, data, size, trusted); +} - return value; +/** + * g_variant_new_from_bytes: + * @type: a #GVariantType + * @bytes: a #GBytes + * @trusted: if the contents of @bytes are trusted + * + * Constructs a new serialised-mode #GVariant instance. This is the + * inner interface for creation of new serialised values that gets + * called from various functions in gvariant.c. + * + * A reference is taken on @bytes. + * + * Returns: (transfer none): a new #GVariant with a floating reference + * + * Since: 2.36 + */ +GVariant * +g_variant_new_from_bytes (const GVariantType *type, + GBytes *bytes, + gboolean trusted) +{ + gconstpointer data; + gsize size; + + g_return_val_if_fail (g_variant_type_is_definite (type), NULL); + + data = g_bytes_get_data (bytes, &size); + + return g_variant_new_serialised (type, g_bytes_ref (bytes), data, size, trusted); +} + +/** + * g_variant_get_data_as_bytes: + * @value: a #GVariant + * + * Returns a pointer to the serialised form of a #GVariant instance. + * The semantics of this function are exactly the same as + * g_variant_get_data(), except that the returned #GBytes holds + * a reference to the variant data. + * + * Returns: (transfer full): A new #GBytes representing the variant data + * + * Since: 2.36 + */ +GBytes * +g_variant_get_data_as_bytes (GVariant *value) +{ + gconstpointer data; + GBytes *bytes; + gsize size; + gconstpointer bytes_data; + gsize bytes_size; + + data = g_variant_get_serialised (value, &bytes, &size); + bytes_data = g_bytes_get_data (bytes, &bytes_size); + + /* Try to reuse the GBytes held internally by GVariant, if it exists + * and is covering exactly the correct range. + */ + if (data == bytes_data && size == bytes_size) + return g_bytes_ref (bytes); + + /* See g_variant_get_data() about why it can return NULL... */ + else if (data == NULL) + return g_bytes_new_take (g_malloc0 (size), size); + + /* Otherwise, make a new GBytes with reference to the old. */ + else + return g_bytes_new_with_free_func (data, size, (GDestroyNotify) g_bytes_unref, g_bytes_ref (bytes)); } /* Epilogue {{{1 */ diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c index b68c583..d42bdfb 100644 --- a/glib/tests/gvariant.c +++ b/glib/tests/gvariant.c @@ -4497,10 +4497,13 @@ test_gbytes (void) { GVariant *a; GVariant *tuple; + GVariant *b; + GVariant *c; GBytes *bytes; GBytes *bytes2; const guint8 values[5] = { 1, 2, 3, 4, 5 }; const guint8 *elts; + gchar *tmp; gsize n_elts; gint i; @@ -4531,6 +4534,41 @@ test_gbytes (void) g_bytes_unref (bytes2); g_variant_unref (a); g_variant_unref (tuple); + + /* Feed in some non-normal data... make sure it's aligned. + * + * Here we have an array of three elements. The first and last are + * normal ints ('iiii') and array-of-bytes data ('ayay'). The middle + * element is zero-bytes wide, which will present a problem when + * fetching the fixed-size integer out of it. + * */ + tmp = g_strdup ("iiiiayayiiiisayay\x08\x08\x10"); + bytes = g_bytes_new_take (tmp, strlen (tmp)); + a = g_variant_new_from_bytes (G_VARIANT_TYPE ("a(iay)"), bytes, FALSE); + g_bytes_unref (bytes); + + /* The middle tuple is zero bytes */ + b = g_variant_get_child_value (a, 1); + g_assert_cmpint (g_variant_get_size (b), ==, 0); + + /* But we're going to pull a 4-byte child out of it... */ + c = g_variant_get_child_value (b, 0); + g_assert_cmpint (g_variant_get_size (c), ==, 4); + + /* g_variant_get_data() is allowed to fail in this case. + * NB: if someone finds a way to avoid this then that's fine too... + */ + g_assert (g_variant_get_data (c) == NULL); + + /* but since it's four bytes, it ought to have data... */ + bytes = g_variant_get_data_as_bytes (c); + g_assert_cmpint (g_bytes_get_size (bytes), ==, 4); + g_assert (memcmp (g_bytes_get_data (bytes, NULL), "\0\0\0\0", 4) == 0); + g_bytes_unref (bytes); + + g_variant_unref (c); + g_variant_unref (b); + g_variant_unref (a); } typedef struct {