Hi Elvis,
I'm trying to understand the problem being addressed by these patches. I
am guessing that there is a difference in behaviour between little
endian and big endian architectures, but I still question some of the
changes.
One example below.
On 11-03-04 08:20 AM, Elvis Pfützenreuter wrote:
[...]
+
+#if __BYTE_ORDER == __BIG_ENDIAN
+static uint128_t bluetooth_base_uuid = {
+ .data = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00,
+ 0x80, 0x00, 0x00, 0x80, 0x5F, 0x9B, 0x34, 0xFB }
+};
+
+#define BASE_UUID16_OFFSET 2
+#define BASE_UUID32_OFFSET 0
+
+#else
+static uint128_t bluetooth_base_uuid = {
+ .data = { 0xFB, 0x34, 0x9B, 0x5F, 0x80, 0x00, 0x00, 0x80,
+ 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }
+};
+
+#define BASE_UUID16_OFFSET 12
+#define BASE_UUID32_OFFSET BASE_UUID16_OFFSET
+
+#endif
+
+static void bt_uuid16_to_uuid128(const bt_uuid_t *uuid16, bt_uuid_t *uuid128)
+{
+ uint16_t data;
+
+ uuid128->value.u128 = bluetooth_base_uuid;
+ uuid128->type = BT_UUID128;
+
+ memcpy(&data,&bluetooth_base_uuid.data[BASE_UUID16_OFFSET], 2);
This line should always be copying Zero, and so more directly should be
data = 0; These appear to be bluetooth specific, so the 4 MSBs of the
base are Zero by definition.
+ data += uuid16->value.u16;
Given the above line, this would become "data = uuid16->value.16", and
the other eliminated.
+
+ memcpy(&uuid128->value.u128.data[BASE_UUID16_OFFSET],&data, 2);
I don't believe this line to be always portable between platforms. This
line makes the assumption that a uint16_t is two native units (not
always true) and can therefore be directly memcopy'd into into an array
of uint8_t array members. This is not always true. memcpy should never
be used in portable code when packing data for network datagram usage
models (anything that treats data as an octet stream).
I am not saying I agree with the need to store UUIDs in anything but
network order, however if I were to write a portable function that will
always work in this instance, I would eliminate all usage of memcpy here
and do something like:
uuid128->value.u128.data[SHORT_UUID_LE_0] = (uint8_t) data;
uuid128->value.u128.data[SHORT_UUID_LE_1] = (uint8_t) (data >> 8);
And likewise for uuid32 to uui128:
uuid128->value.u128.data[SHORT_UUID_LE_2] = (uint8_t) (data >> 16);
uuid128->value.u128.data[SHORT_UUID_LE_3] = (uint8_t) (data >> 24);
Where depending on the endian, SHORT_UUID_LE_0 thru SHORT_UUID_LE_3
would be defined as 3 thru 0 for Big Endian storage, and 12 thru 15 for
Little Endian storage.
This is guaranteed to put the desired octet into it's correct position
within the uuid128's data array, while it could not be guaranteed across
platforms with memcpy.
+}
Regards,
--
Brian Gix
bgix@xxxxxxxxxxxxxx
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
