From 6e723e8b3e5f138f8e0b4fe45c5872a92510d5d3 Mon Sep 17 00:00:00 2001 From: David Zeuthen Date: Wed, 4 Aug 2010 12:24:22 -0400 Subject: [PATCH] GDBusMessage: Assert various things when serializing to a blob We use g_assert() instead of setting the GError because it is a programming error if the GVariant contains invalid data - see commit 5e6f762d61db1a5c64bd1d33e5ba112755106581 for where the last hole in GVariant was closed. So if we can trust GVariant to only contain valid data (ignoring the case where unsafe API such as g_variant_new_from_data() is used), why g_assert() at all with costly g_utf8_validate() checks? Because a) it is relatively inexpensive; and b) it helps find bugs such as the one fixed in commit 5e6f762d61db1a5c64bd1d33e5ba112755106581. If performance is a concern we can play games like introducing environment variables or other machinery to avoid such "costly" checks. I doubt it will ever be an issue. Also replace two "Hmm" TODO item with a static assert - the code that serializes a gdouble into the D-Bus wire format by treating it as a guint64 is indeed correct - endianess needs to be taken into account (see the D-Bus reference implementation for similar code). But we want to make sure that we're indeed using an architecture/compiler where a gdouble takes up 8 bytes - hence the assertion. Signed-off-by: David Zeuthen --- gio/gdbusmessage.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/gio/gdbusmessage.c b/gio/gdbusmessage.c index afdd2a4..4f790ac 100644 --- a/gio/gdbusmessage.c +++ b/gio/gdbusmessage.c @@ -947,7 +947,7 @@ parse_value_from_blob (GMemoryInputStream *mis, v = g_data_input_stream_read_uint64 (dis, NULL, &local_error); if (local_error != NULL) goto fail; - /* TODO: hmm */ + G_STATIC_ASSERT (sizeof (gdouble) == sizeof (guint64)); encoded = (gdouble *) &v; ret = g_variant_new_double (*encoded); } @@ -1690,7 +1690,7 @@ append_value_to_blob (GVariant *value, { guint64 *encoded; gdouble v = g_variant_get_double (value); - /* TODO: hmm */ + G_STATIC_ASSERT (sizeof (gdouble) == sizeof (guint64)); encoded = (guint64 *) &v; g_data_output_stream_put_uint64 (dos, *encoded, NULL, NULL); } @@ -1701,7 +1701,10 @@ append_value_to_blob (GVariant *value, if (value != NULL) { gsize len; - const gchar *v = g_variant_get_string (value, &len); + const gchar *v; + const gchar *end; + v = g_variant_get_string (value, &len); + g_assert (g_utf8_validate (v, -1, &end) && (end == v + len)); g_data_output_stream_put_uint32 (dos, len, NULL, NULL); g_data_output_stream_put_string (dos, v, NULL, NULL); g_data_output_stream_put_byte (dos, '\0', NULL, NULL); @@ -1712,9 +1715,9 @@ append_value_to_blob (GVariant *value, padding_added = ensure_output_padding (mos, dos, 4); if (value != NULL) { - /* TODO: validate object path */ gsize len; const gchar *v = g_variant_get_string (value, &len); + g_assert (g_variant_is_object_path (v)); g_data_output_stream_put_uint32 (dos, len, NULL, NULL); g_data_output_stream_put_string (dos, v, NULL, NULL); g_data_output_stream_put_byte (dos, '\0', NULL, NULL); @@ -1724,9 +1727,9 @@ append_value_to_blob (GVariant *value, { if (value != NULL) { - /* TODO: validate signature (including max len being 255) */ gsize len; const gchar *v = g_variant_get_string (value, &len); + g_assert (g_variant_is_signature (v)); g_data_output_stream_put_byte (dos, len, NULL, NULL); g_data_output_stream_put_string (dos, v, NULL, NULL); g_data_output_stream_put_byte (dos, '\0', NULL, NULL); @@ -1856,7 +1859,6 @@ append_value_to_blob (GVariant *value, const gchar *signature; child = g_variant_get_child_value (value, 0); signature = g_variant_get_type_string (child); - /* TODO: validate signature (including max len being 255) */ g_data_output_stream_put_byte (dos, strlen (signature), NULL, NULL); g_data_output_stream_put_string (dos, signature, NULL, NULL); g_data_output_stream_put_byte (dos, '\0', NULL, NULL); -- 2.7.4