From c6040e8d49d235f2071fa995e9f8fbeb28556862 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 10 Oct 2011 11:20:31 +0100 Subject: [PATCH 1/2] Promote DBusBasicValue and DBus8ByteStruct to be API In practice, D-Bus bindings end up reinventing DBusBasicValue anyway, so it might as well be API. Also stop claiming that all basic-typed values are guaranteed to fit in 8 bytes - this is not true if your platform has more than 8-byte pointers (I'm not aware of any such platform now, but let's not rule it out). Bug: https://bugs.freedesktop.org/show_bug.cgi?id=11191 Signed-off-by: Simon McVittie --- dbus/dbus-marshal-basic.c | 13 +++++++++-- dbus/dbus-marshal-basic.h | 34 --------------------------------- dbus/dbus-marshal-recursive-util.c | 8 +++--- dbus/dbus-message.c | 16 +++++++------- dbus/dbus-types.h | 37 ++++++++++++++++++++++++++++++++++++ 5 files changed, 59 insertions(+), 49 deletions(-) diff --git a/dbus/dbus-marshal-basic.c b/dbus/dbus-marshal-basic.c index b139853..2534b61 100644 --- a/dbus/dbus-marshal-basic.c +++ b/dbus/dbus-marshal-basic.c @@ -65,6 +65,13 @@ _DBUS_STATIC_ASSERT (sizeof (dbus_uint64_t) == 8); _DBUS_ASSERT_ALIGNMENT (dbus_uint64_t, <=, 8); #endif +_DBUS_STATIC_ASSERT (sizeof (DBusBasicValue) >= 8); +/* The alignment of a DBusBasicValue might conceivably be > 8 because of the + * pointer, so we don't assert about it */ + +_DBUS_STATIC_ASSERT (sizeof (DBus8ByteStruct) == 8); +_DBUS_ASSERT_ALIGNMENT (DBus8ByteStruct, <=, 8); + /** * @defgroup DBusMarshal marshaling and unmarshaling * @ingroup DBusInternals @@ -119,7 +126,7 @@ pack_8_octets (DBusBasicValue value, else *((dbus_uint64_t*)(data)) = DBUS_UINT64_TO_BE (value.u64); #else - *(DBus8ByteStruct*)data = value.u64; + *(DBus8ByteStruct*)data = value.eight; swap_8_octets ((DBusBasicValue*)data, byte_order); #endif } @@ -169,7 +176,7 @@ swap_8_octets (DBusBasicValue *value, #ifdef DBUS_HAVE_INT64 value->u64 = DBUS_UINT64_SWAP_LE_BE (value->u64); #else - swap_bytes ((unsigned char *)value, 8); + swap_bytes (&value->bytes, 8); #endif } } @@ -190,7 +197,7 @@ unpack_8_octets (int byte_order, else r.u64 = DBUS_UINT64_FROM_BE (*(dbus_uint64_t*)data); #else - r.u64 = *(DBus8ByteStruct*)data; + r.eight = *(DBus8ByteStruct*)data; swap_8_octets (&r, byte_order); #endif diff --git a/dbus/dbus-marshal-basic.h b/dbus/dbus-marshal-basic.h index 0c27fc9..95547ef 100644 --- a/dbus/dbus-marshal-basic.h +++ b/dbus/dbus-marshal-basic.h @@ -146,40 +146,6 @@ # define DBUS_UINT64_FROM_BE(val) (DBUS_UINT64_TO_BE (val)) #endif /* DBUS_HAVE_INT64 */ -#ifndef DBUS_HAVE_INT64 -/** - * An 8-byte struct you could use to access int64 without having - * int64 support - */ -typedef struct -{ - dbus_uint32_t first32; /**< first 32 bits in the 8 bytes (beware endian issues) */ - dbus_uint32_t second32; /**< second 32 bits in the 8 bytes (beware endian issues) */ -} DBus8ByteStruct; -#endif /* DBUS_HAVE_INT64 */ - -/** - * A simple 8-byte value union that lets you access 8 bytes as if they - * were various types; useful when dealing with basic types via - * void pointers and varargs. - */ -typedef union -{ - dbus_int16_t i16; /**< as int16 */ - dbus_uint16_t u16; /**< as int16 */ - dbus_int32_t i32; /**< as int32 */ - dbus_uint32_t u32; /**< as int32 */ -#ifdef DBUS_HAVE_INT64 - dbus_int64_t i64; /**< as int64 */ - dbus_uint64_t u64; /**< as int64 */ -#else - DBus8ByteStruct u64; /**< as 8-byte-struct */ -#endif - double dbl; /**< as double */ - unsigned char byt; /**< as byte */ - char *str; /**< as char* */ -} DBusBasicValue; - #ifdef DBUS_DISABLE_ASSERT #define _dbus_unpack_uint16(byte_order, data) \ (((byte_order) == DBUS_LITTLE_ENDIAN) ? \ diff --git a/dbus/dbus-marshal-recursive-util.c b/dbus/dbus-marshal-recursive-util.c index 3508bb0..f7d629a 100644 --- a/dbus/dbus-marshal-recursive-util.c +++ b/dbus/dbus-marshal-recursive-util.c @@ -38,8 +38,8 @@ basic_value_zero (DBusBasicValue *value) #ifdef DBUS_HAVE_INT64 value->u64 = 0; #else - value->u64.first32 = 0; - value->u64.second32 = 0; + value->eight.first32 = 0; + value->eight.second32 = 0; #endif } @@ -59,8 +59,8 @@ basic_value_equal (int type, #ifdef DBUS_HAVE_INT64 return lhs->u64 == rhs->u64; #else - return lhs->u64.first32 == rhs->u64.first32 && - lhs->u64.second32 == rhs->u64.second32; + return lhs->eight.first32 == rhs->eight.first32 && + lhs->eight.second32 == rhs->eight.second32; #endif } } diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index a95ad0f..33dab8a 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -2129,22 +2129,22 @@ dbus_message_iter_get_signature (DBusMessageIter *iter) * descriptors), you can get all the array elements at once with * dbus_message_iter_get_fixed_array(). Otherwise, you have to iterate * over the container's contents one value at a time. - * - * All basic-typed values are guaranteed to fit in 8 bytes. So you can - * write code like this: + * + * All basic-typed values are guaranteed to fit in a #DBusBasicValue, + * so in versions of libdbus that have that type, you can write code like this: * * @code - * dbus_uint64_t value; + * DBusBasicValue value; * int type; * dbus_message_iter_get_basic (&read_iter, &value); * type = dbus_message_iter_get_arg_type (&read_iter); * dbus_message_iter_append_basic (&write_iter, type, &value); * @endcode * - * On some really obscure platforms dbus_uint64_t might not exist, if - * you need to worry about this you will know. dbus_uint64_t is just - * one example of a type that's large enough to hold any possible - * value, you could use a struct or char[8] instead if you like. + * (All D-Bus basic types are either numeric and 8 bytes or smaller, or + * behave like a string; so in older versions of libdbus, DBusBasicValue + * can be replaced with union { char *string; unsigned char bytes[8]; }, + * for instance.) * * @param iter the iterator * @param value location to store the value diff --git a/dbus/dbus-types.h b/dbus/dbus-types.h index 54f348f..7251d65 100644 --- a/dbus/dbus-types.h +++ b/dbus/dbus-types.h @@ -134,6 +134,43 @@ typedef dbus_uint32_t dbus_bool_t; * giving a literal such as "325145246765ULL" */ +/** + * An 8-byte struct you could use to access int64 without having + * int64 support + */ +typedef struct +{ + dbus_uint32_t first32; /**< first 32 bits in the 8 bytes (beware endian issues) */ + dbus_uint32_t second32; /**< second 32 bits in the 8 bytes (beware endian issues) */ +} DBus8ByteStruct; + +/** + * A simple value union that lets you access bytes as if they + * were various types; useful when dealing with basic types via + * void pointers and varargs. + * + * This union also contains a pointer member (which can be used + * to retrieve a string from dbus_message_iter_get_basic(), for + * instance), so on future platforms it could conceivably be larger + * than 8 bytes. + */ +typedef union +{ + unsigned char bytes[8]; /**< as 8 individual bytes */ + dbus_int16_t i16; /**< as int16 */ + dbus_uint16_t u16; /**< as int16 */ + dbus_int32_t i32; /**< as int32 */ + dbus_uint32_t u32; /**< as int32 */ +#ifdef DBUS_HAVE_INT64 + dbus_int64_t i64; /**< as int64 */ + dbus_uint64_t u64; /**< as int64 */ +#endif + DBus8ByteStruct eight; /**< as 8-byte struct */ + double dbl; /**< as double */ + unsigned char byt; /**< as byte */ + char *str; /**< as char* */ +} DBusBasicValue; + /** @} */ #endif /* DBUS_TYPES_H */ -- 1.7.7.2