linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC 00/15] Unaligned memory access fixes
@ 2012-08-31  8:36 Szymon Janc
  2012-08-31  8:36 ` [RFC 01/15] lib: Add host order unaligned access functions Szymon Janc
                   ` (14 more replies)
  0 siblings, 15 replies; 23+ messages in thread
From: Szymon Janc @ 2012-08-31  8:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

Hi,

A list of patches that address unaligned memory access issues all over
the code base.

It looks like recent versions of GCC are more picky about that. These
patches allow to build BlueZ in boostrap-configure configuration
(excluding gstreamer code) with GCC 4.7.1 for ARM.

In few places I had to do a small refactor instead of just use bt_put/get
functions. So please have a look at those.

There could be also bt_put_* functions added for putting u16/32/64 in
unaligned memory in host order. That could allow to eliminate some of
	memcpy(ptr, &some_u32, sizeof(some_u32);
with more convenient
	bt_put_32(some_u32, ptr);


Comments are welcome.

-- 
BR
Szymon Janc

Szymon Janc (15):
  lib: Add host order unaligned access functions
  sap-u8500: Fix compile error due to unaligned memory access
  sdp: Use helper functions instead of bt_get_unaligned macro
  Add helper functions for putting integers on unaligned memory address
  sdp: Fix compilation errors due to unaligned memory access
  l2test: Fix compilation errors due to unaligned memory access
  rctest: Fix compilation errors due to unaligned memory access
  monitor: Fix compilation errors due to unaligned memory access
  scotest: Fix compilation errors due to unaligned memory access
  avrcp: Fix compilation errors due to unaligned memory access
  sap: Fix compilation errors due to unaligned memory access
  adaptername: Refactor handle_inotify_cb
  sdpd-request: Fix build errors due to unaligned memory access
  sdpd-service: Fix build errors due to unaligned memory access
  hciemu: Fix build errors due to unaligned memory access

 audio/avrcp.c            |   10 ++---
 lib/bluetooth.h          |   76 +++++++++++++++++++++++++++++++++++++
 lib/sdp.c                |   94 ++++++++++++++++++++++++----------------------
 monitor/control.c        |    7 +++-
 monitor/hcidump.c        |   23 ++++++++----
 plugins/adaptername.c    |   44 ++++++++++++----------
 profiles/sap/sap-u8500.c |   10 +++--
 profiles/sap/server.c    |    9 ++---
 src/sdpd-request.c       |   62 ++++++++++++++++--------------
 src/sdpd-service.c       |   21 +++++------
 test/hciemu.c            |    6 ++-
 test/l2test.c            |    9 +++--
 test/rctest.c            |    5 ++-
 test/scotest.c           |    5 ++-
 14 files changed, 243 insertions(+), 138 deletions(-)

-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [RFC 01/15] lib: Add host order unaligned access functions
  2012-08-31  8:36 [RFC 00/15] Unaligned memory access fixes Szymon Janc
@ 2012-08-31  8:36 ` Szymon Janc
  2012-08-31  8:36 ` [RFC 02/15] sap-u8500: Fix compile error due to unaligned memory access Szymon Janc
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 23+ messages in thread
From: Szymon Janc @ 2012-08-31  8:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

Change-Id: I3c647249a88b8ca05ef215096e6b9a5609e8eab9
---
 lib/bluetooth.h |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/lib/bluetooth.h b/lib/bluetooth.h
index 0fc4508..f0776aa 100644
--- a/lib/bluetooth.h
+++ b/lib/bluetooth.h
@@ -153,6 +153,21 @@ do {						\
 	__p->__v = (val);			\
 } while(0)
 
+static inline uint64_t bt_get_64(const void *ptr)
+{
+	return bt_get_unaligned((const uint64_t *) ptr);
+}
+
+static inline uint32_t bt_get_32(const void *ptr)
+{
+	return bt_get_unaligned((const uint32_t *) ptr);
+}
+
+static inline uint16_t bt_get_16(const void *ptr)
+{
+	return bt_get_unaligned((const uint16_t *) ptr);
+}
+
 #if __BYTE_ORDER == __LITTLE_ENDIAN
 static inline uint64_t bt_get_le64(const void *ptr)
 {
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFC 02/15] sap-u8500: Fix compile error due to unaligned memory access
  2012-08-31  8:36 [RFC 00/15] Unaligned memory access fixes Szymon Janc
  2012-08-31  8:36 ` [RFC 01/15] lib: Add host order unaligned access functions Szymon Janc
@ 2012-08-31  8:36 ` Szymon Janc
  2012-08-31  8:36 ` [RFC 03/15] sdp: Use helper functions instead of bt_get_unaligned macro Szymon Janc
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 23+ messages in thread
From: Szymon Janc @ 2012-08-31  8:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This fix following compilation error on ARM.

  CC     profiles/sap/sap-u8500.o
profiles/sap/sap-u8500.c: In function recv_card_status:
profiles/sap/sap-u8500.c:323:16: error: cast increases required
	alignment of target type [-Werror=cast-align]
profiles/sap/sap-u8500.c: In function recv_response:
profiles/sap/sap-u8500.c:423:12: error: cast increases required
	alignment of target type [-Werror=cast-align]
cc1: all warnings being treated as errors

Change-Id: Id771049b5c18ddd95a28aa50b48775f3ee3e9074
---
 profiles/sap/sap-u8500.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/profiles/sap/sap-u8500.c b/profiles/sap/sap-u8500.c
index f07209d..6e95d7a 100644
--- a/profiles/sap/sap-u8500.c
+++ b/profiles/sap/sap-u8500.c
@@ -32,6 +32,8 @@
 #include <sys/socket.h>
 #include <sys/un.h>
 
+#include <bluetooth/bluetooth.h>
+
 #include "log.h"
 #include "sap.h"
 
@@ -313,16 +315,16 @@ static void recv_status(uint32_t status)
 
 static void recv_card_status(uint32_t status, uint8_t *param)
 {
-	uint32_t *card_status;
+	uint32_t card_status;
 	uint8_t result;
 	uint8_t iccrs;
 
 	if (status != STE_STATUS_OK)
 		return;
 
-	card_status = (uint32_t *)param;
+	card_status = bt_get_32(param);
 
-	if (get_sap_reader_status(*card_status, &iccrs) < 0)
+	if (get_sap_reader_status(card_status, &iccrs) < 0)
 		result = SAP_RESULT_ERROR_NO_REASON;
 	else
 		result = get_sap_result(STE_GET_STATUS_MSG, status);
@@ -420,7 +422,7 @@ static void recv_response(struct ste_message *msg)
 	}
 
 	param = msg->payload;
-	status = *(uint32_t *)param;
+	status = bt_get_32(param);
 	param += sizeof(status);
 
 	SAP_VDBG("status 0x%x", status);
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFC 03/15] sdp: Use helper functions instead of bt_get_unaligned macro
  2012-08-31  8:36 [RFC 00/15] Unaligned memory access fixes Szymon Janc
  2012-08-31  8:36 ` [RFC 01/15] lib: Add host order unaligned access functions Szymon Janc
  2012-08-31  8:36 ` [RFC 02/15] sap-u8500: Fix compile error due to unaligned memory access Szymon Janc
@ 2012-08-31  8:36 ` Szymon Janc
  2012-08-31 10:55   ` Anderson Lizardo
  2012-08-31 16:14   ` Marcel Holtmann
  2012-08-31  8:36 ` [RFC 04/15] Add helper functions for putting integers on unaligned memory address Szymon Janc
                   ` (11 subsequent siblings)
  14 siblings, 2 replies; 23+ messages in thread
From: Szymon Janc @ 2012-08-31  8:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This fix number of compilation errors on ARM similar to one below.

lib/sdp.c: In function 'sdp_uuid_extract':
lib/sdp.c:1019:27: error: cast increases required alignment
	of target type [-Werror=cast-align]
lib/sdp.c:1019:27: error: cast increases required alignment
	of target type [-Werror=cast-align]
lib/sdp.c:1026:27: error: cast increases required alignment
	of target type [-Werror=cast-align]
lib/sdp.c:1026:27: error: cast increases required alignment
	of target type [-Werror=cast-align]

Change-Id: I587fb99328d7e5b9053af81597bd48b3a4e610ad
---
 lib/sdp.c |   56 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/lib/sdp.c b/lib/sdp.c
index 36b4d08..d40500f 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -376,27 +376,27 @@ sdp_data_t *sdp_data_alloc_with_length(uint8_t dtd, const void *value,
 		d->unitSize += sizeof(int8_t);
 		break;
 	case SDP_UINT16:
-		d->val.uint16 = bt_get_unaligned((uint16_t *) value);
+		d->val.uint16 = bt_get_16(value);
 		d->unitSize += sizeof(uint16_t);
 		break;
 	case SDP_INT16:
-		d->val.int16 = bt_get_unaligned((int16_t *) value);
+		d->val.int16 = bt_get_16(value);
 		d->unitSize += sizeof(int16_t);
 		break;
 	case SDP_UINT32:
-		d->val.uint32 = bt_get_unaligned((uint32_t *) value);
+		d->val.uint32 = bt_get_32(value);
 		d->unitSize += sizeof(uint32_t);
 		break;
 	case SDP_INT32:
-		d->val.int32 = bt_get_unaligned((int32_t *) value);
+		d->val.int32 = bt_get_32(value);
 		d->unitSize += sizeof(int32_t);
 		break;
 	case SDP_INT64:
-		d->val.int64 = bt_get_unaligned((int64_t *) value);
+		d->val.int64 = bt_get_64(value);
 		d->unitSize += sizeof(int64_t);
 		break;
 	case SDP_UINT64:
-		d->val.uint64 = bt_get_unaligned((uint64_t *) value);
+		d->val.uint64 = bt_get_64(value);
 		d->unitSize += sizeof(uint64_t);
 		break;
 	case SDP_UINT128:
@@ -408,11 +408,11 @@ sdp_data_t *sdp_data_alloc_with_length(uint8_t dtd, const void *value,
 		d->unitSize += sizeof(uint128_t);
 		break;
 	case SDP_UUID16:
-		sdp_uuid16_create(&d->val.uuid, bt_get_unaligned((uint16_t *) value));
+		sdp_uuid16_create(&d->val.uuid, bt_get_16(value));
 		d->unitSize += sizeof(uint16_t);
 		break;
 	case SDP_UUID32:
-		sdp_uuid32_create(&d->val.uuid, bt_get_unaligned((uint32_t *) value));
+		sdp_uuid32_create(&d->val.uuid, bt_get_32(value));
 		d->unitSize += sizeof(uint32_t);
 		break;
 	case SDP_UUID128:
@@ -1016,14 +1016,14 @@ int sdp_uuid_extract(const uint8_t *p, int bufsize, uuid_t *uuid, int *scanned)
 			SDPERR("Not enough room for 16-bit UUID");
 			return -1;
 		}
-		sdp_uuid16_create(uuid, ntohs(bt_get_unaligned((uint16_t *) p)));
+		sdp_uuid16_create(uuid, bt_get_be16(p));
 		*scanned += sizeof(uint16_t);
 	} else if (type == SDP_UUID32) {
 		if (bufsize < (int) sizeof(uint32_t)) {
 			SDPERR("Not enough room for 32-bit UUID");
 			return -1;
 		}
-		sdp_uuid32_create(uuid, ntohl(bt_get_unaligned((uint32_t *) p)));
+		sdp_uuid32_create(uuid, bt_get_be32(p));
 		*scanned += sizeof(uint32_t);
 	} else {
 		if (bufsize < (int) sizeof(uint128_t)) {
@@ -1078,7 +1078,7 @@ static sdp_data_t *extract_int(const void *p, int bufsize, int *len)
 			return NULL;
 		}
 		*len += sizeof(uint16_t);
-		d->val.uint16 = ntohs(bt_get_unaligned((uint16_t *) p));
+		d->val.uint16 = bt_get_be16(p);
 		break;
 	case SDP_INT32:
 	case SDP_UINT32:
@@ -1088,7 +1088,7 @@ static sdp_data_t *extract_int(const void *p, int bufsize, int *len)
 			return NULL;
 		}
 		*len += sizeof(uint32_t);
-		d->val.uint32 = ntohl(bt_get_unaligned((uint32_t *) p));
+		d->val.uint32 = bt_get_be32(p);
 		break;
 	case SDP_INT64:
 	case SDP_UINT64:
@@ -1098,7 +1098,7 @@ static sdp_data_t *extract_int(const void *p, int bufsize, int *len)
 			return NULL;
 		}
 		*len += sizeof(uint64_t);
-		d->val.uint64 = ntoh64(bt_get_unaligned((uint64_t *) p));
+		d->val.uint64 = bt_get_be64(p);
 		break;
 	case SDP_INT128:
 	case SDP_UINT128:
@@ -1181,7 +1181,7 @@ static sdp_data_t *extract_str(const void *p, int bufsize, int *len)
 			free(d);
 			return NULL;
 		}
-		n = ntohs(bt_get_unaligned((uint16_t *) p));
+		n = bt_get_be16(p);
 		p += sizeof(uint16_t);
 		*len += sizeof(uint16_t) + n;
 		bufsize -= sizeof(uint16_t);
@@ -1251,7 +1251,7 @@ int sdp_extract_seqtype(const uint8_t *buf, int bufsize, uint8_t *dtdp, int *siz
 			SDPERR("Unexpected end of packet");
 			return 0;
 		}
-		*size = ntohs(bt_get_unaligned((uint16_t *) buf));
+		*size = bt_get_be16(buf);
 		scanned += sizeof(uint16_t);
 		break;
 	case SDP_SEQ32:
@@ -1260,7 +1260,7 @@ int sdp_extract_seqtype(const uint8_t *buf, int bufsize, uint8_t *dtdp, int *siz
 			SDPERR("Unexpected end of packet");
 			return 0;
 		}
-		*size = ntohl(bt_get_unaligned((uint32_t *) buf));
+		*size = bt_get_be32(buf);
 		scanned += sizeof(uint32_t);
 		break;
 	default:
@@ -1427,7 +1427,7 @@ sdp_record_t *sdp_extract_pdu(const uint8_t *buf, int bufsize, int *scanned)
 		}
 
 		dtd = *(uint8_t *) p;
-		attr = ntohs(bt_get_unaligned((uint16_t *) (p + n)));
+		attr = bt_get_be16((p + n));
 		n += sizeof(uint16_t);
 
 		SDPDBG("DTD of attrId : %d Attr id : 0x%x \n", dtd, attr);
@@ -2891,7 +2891,7 @@ int sdp_device_record_register_binary(sdp_session_t *session, bdaddr_t *device,
 			goto end;
 		}
 		if (handle)
-			*handle  = ntohl(bt_get_unaligned((uint32_t *) p));
+			*handle  = bt_get_be32(p);
 	}
 
 end:
@@ -2991,7 +2991,7 @@ int sdp_device_record_unregister_binary(sdp_session_t *session, bdaddr_t *device
 
 	rsphdr = (sdp_pdu_hdr_t *) rspbuf;
 	p = rspbuf + sizeof(sdp_pdu_hdr_t);
-	status = bt_get_unaligned((uint16_t *) p);
+	status = bt_get_16(p);
 
 	if (rsphdr->pdu_id == SDP_ERROR_RSP) {
 		/* For this case the status always is invalid record handle */
@@ -3096,7 +3096,7 @@ int sdp_device_record_update(sdp_session_t *session, bdaddr_t *device, const sdp
 
 	rsphdr = (sdp_pdu_hdr_t *) rspbuf;
 	p = rspbuf + sizeof(sdp_pdu_hdr_t);
-	status = bt_get_unaligned((uint16_t *) p);
+	status = bt_get_16(p);
 
 	if (rsphdr->pdu_id == SDP_ERROR_RSP) {
 		/* The status can be invalid sintax or invalid record handle */
@@ -3183,7 +3183,7 @@ static void extract_record_handle_seq(uint8_t *pdu, int bufsize, sdp_list_t **se
 		pSvcRec = malloc(sizeof(uint32_t));
 		if (!pSvcRec)
 			break;
-		*pSvcRec = ntohl(bt_get_unaligned((uint32_t *) pdata));
+		*pSvcRec = bt_get_be32(pdata);
 		pSeq = sdp_list_append(pSeq, pSvcRec);
 		pdata += sizeof(uint32_t);
 		*scanned += sizeof(uint32_t);
@@ -3407,7 +3407,7 @@ int sdp_service_search_req(sdp_session_t *session, const sdp_list_t *search,
 		pdata += sizeof(uint16_t);
 		scanned += sizeof(uint16_t);
 		pdata_len -= sizeof(uint16_t);
-		rec_count = ntohs(bt_get_unaligned((uint16_t *) pdata));
+		rec_count = bt_get_be16(pdata);
 		pdata += sizeof(uint16_t);
 		scanned += sizeof(uint16_t);
 		pdata_len -= sizeof(uint16_t);
@@ -3573,7 +3573,7 @@ sdp_record_t *sdp_service_attr_req(sdp_session_t *session, uint32_t handle,
 			goto end;
 		}
 
-		rsp_count = ntohs(bt_get_unaligned((uint16_t *) pdata));
+		rsp_count = bt_get_be16(pdata);
 		attr_list_len += rsp_count;
 		pdata += sizeof(uint16_t);
 		pdata_len -= sizeof(uint16_t);
@@ -4124,9 +4124,9 @@ int sdp_process(sdp_session_t *session)
 		 * CSRC: Current Service Record Count (2 bytes)
 		 */
 		ssr_pdata = pdata;
-		tsrc = ntohs(bt_get_unaligned((uint16_t *) ssr_pdata));
+		tsrc = bt_get_be16(ssr_pdata);
 		ssr_pdata += sizeof(uint16_t);
-		csrc = ntohs(bt_get_unaligned((uint16_t *) ssr_pdata));
+		csrc = bt_get_be16(ssr_pdata);
 
 		/* csrc should never be larger than tsrc */
 		if (csrc > tsrc) {
@@ -4162,7 +4162,7 @@ int sdp_process(sdp_session_t *session)
 		break;
 	case SDP_SVC_ATTR_RSP:
 	case SDP_SVC_SEARCH_ATTR_RSP:
-		rsp_count = ntohs(bt_get_unaligned((uint16_t *) pdata));
+		rsp_count = bt_get_be16(pdata);
 		SDPDBG("Attrlist byte count : %d\n", rsp_count);
 
 		/*
@@ -4175,7 +4175,7 @@ int sdp_process(sdp_session_t *session)
 		status = 0x0000;
 		break;
 	case SDP_ERROR_RSP:
-		status = ntohs(bt_get_unaligned((uint16_t *) pdata));
+		status = bt_get_be16(pdata);
 		size = ntohs(rsphdr->plen);
 
 		goto end;
@@ -4395,7 +4395,7 @@ int sdp_service_search_attr_req(sdp_session_t *session, const sdp_list_t *search
 			goto end;
 		}
 
-		rsp_count = ntohs(bt_get_unaligned((uint16_t *) pdata));
+		rsp_count = bt_get_be16(pdata);
 		attr_list_len += rsp_count;
 		pdata += sizeof(uint16_t); /* pdata points to attribute list */
 		pdata_len -= sizeof(uint16_t);
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFC 04/15] Add helper functions for putting integers on unaligned memory address
  2012-08-31  8:36 [RFC 00/15] Unaligned memory access fixes Szymon Janc
                   ` (2 preceding siblings ...)
  2012-08-31  8:36 ` [RFC 03/15] sdp: Use helper functions instead of bt_get_unaligned macro Szymon Janc
@ 2012-08-31  8:36 ` Szymon Janc
  2012-08-31  8:36 ` [RFC 05/15] sdp: Fix compilation errors due to unaligned memory access Szymon Janc
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 23+ messages in thread
From: Szymon Janc @ 2012-08-31  8:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

Those functions are similar to bt_get_* functions.

Change-Id: I122fd9fcd7cdb3950fbce0ac4a0ea49b2f9ce0f9
---
 lib/bluetooth.h |   61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/lib/bluetooth.h b/lib/bluetooth.h
index f0776aa..7e6ced9 100644
--- a/lib/bluetooth.h
+++ b/lib/bluetooth.h
@@ -198,6 +198,37 @@ static inline uint16_t bt_get_be16(const void *ptr)
 {
 	return bswap_16(bt_get_unaligned((const uint16_t *) ptr));
 }
+
+static inline void bt_put_le64(uint64_t val, const void *ptr)
+{
+	bt_put_unaligned(val, (uint64_t *) ptr);
+}
+
+static inline void bt_put_be64(uint64_t val, const void *ptr)
+{
+	bt_put_unaligned(bswap_64(val), (uint64_t *) ptr);
+}
+
+static inline void bt_put_le32(uint32_t val, const void *ptr)
+{
+	bt_put_unaligned(val, (uint32_t *) ptr);
+}
+
+static inline void bt_put_be32(uint32_t val, const void *ptr)
+{
+	bt_put_unaligned(bswap_32(val), (uint32_t *) ptr);
+}
+
+static inline void bt_put_le16(uint16_t val, const void *ptr)
+{
+	bt_put_unaligned(val, (uint16_t *) ptr);
+}
+
+static inline void bt_put_be16(uint16_t val, const void *ptr)
+{
+	bt_put_unaligned(bswap_16(val), (uint16_t *) ptr);
+}
+
 #elif __BYTE_ORDER == __BIG_ENDIAN
 static inline uint64_t bt_get_le64(const void *ptr)
 {
@@ -228,6 +259,36 @@ static inline uint16_t bt_get_be16(const void *ptr)
 {
 	return bt_get_unaligned((const uint16_t *) ptr);
 }
+
+static inline void bt_put_le64(uint64_t val, const void *ptr)
+{
+	bt_put_unaligned(bswap_64(val), (uint64_t *) ptr);
+}
+
+static inline void bt_put_be64(uint64_t val, const void *ptr)
+{
+	bt_put_unaligned(val, (uint64_t *) ptr);
+}
+
+static inline void bt_put_le32(uint32_t val, const void *ptr)
+{
+	bt_put_unaligned(bswap_32(val), (uint32_t *) ptr);
+}
+
+static inline void bt_put_be32(uint32_t val, const void *ptr)
+{
+	bt_put_unaligned(val, (uint32_t *) ptr);
+}
+
+static inline void bt_put_le16(uint16_t val, const void *ptr)
+{
+	bt_put_unaligned(bswap_16(val), (uint16_t *) ptr);
+}
+
+static inline void bt_put_be16(uint16_t val, const void *ptr)
+{
+	bt_put_unaligned(val, (uint16_t *) ptr);
+}
 #else
 #error "Unknown byte order"
 #endif
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFC 05/15] sdp: Fix compilation errors due to unaligned memory access
  2012-08-31  8:36 [RFC 00/15] Unaligned memory access fixes Szymon Janc
                   ` (3 preceding siblings ...)
  2012-08-31  8:36 ` [RFC 04/15] Add helper functions for putting integers on unaligned memory address Szymon Janc
@ 2012-08-31  8:36 ` Szymon Janc
  2012-08-31  8:36 ` [RFC 06/15] l2test: " Szymon Janc
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 23+ messages in thread
From: Szymon Janc @ 2012-08-31  8:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This fix number of build errors on ARM similar to one below.

lib/sdp.c: In function 'sdp_set_seq_len':
lib/sdp.c:625:3: error: cast increases required alignment of target
	 type [-Werror=cast-align]
lib/sdp.c:625:3: error: cast increases required alignment of target
	type [-Werror=cast-align]
lib/sdp.c:631:3: error: cast increases required alignment of target
	type [-Werror=cast-align]
lib/sdp.c:631:3: error: cast increases required alignment of target
	type [-Werror=cast-align]

Change-Id: Ic539c930f05f905d78fdbc2a77c1f0ff59a04bae
---
 lib/sdp.c |   38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/lib/sdp.c b/lib/sdp.c
index d40500f..59a958f 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -622,13 +622,13 @@ void sdp_set_seq_len(uint8_t *ptr, uint32_t length)
 	case SDP_ALT16:
 	case SDP_TEXT_STR16:
 	case SDP_URL_STR16:
-		bt_put_unaligned(htons(length), (uint16_t *) ptr);
+		bt_put_be16(length, ptr);
 		break;
 	case SDP_SEQ32:
 	case SDP_ALT32:
 	case SDP_TEXT_STR32:
 	case SDP_URL_STR32:
-		bt_put_unaligned(htonl(length), (uint32_t *) ptr);
+		bt_put_be32(length, ptr);
 		break;
 	}
 }
@@ -685,7 +685,7 @@ void sdp_set_attrid(sdp_buf_t *buf, uint16_t attr)
 	/* data type for attr */
 	*p++ = SDP_UINT16;
 	buf->data_size = sizeof(uint8_t);
-	bt_put_unaligned(htons(attr), (uint16_t *) p);
+	bt_put_be16(attr, p);
 	buf->data_size += sizeof(uint16_t);
 }
 
@@ -2791,10 +2791,10 @@ void sdp_append_to_buf(sdp_buf_t *dst, uint8_t *data, uint32_t len)
 		*(uint8_t *) p = dst->data_size - sizeof(uint8_t) - sizeof(uint8_t);
 		break;
 	case SDP_SEQ16:
-		bt_put_unaligned(htons(dst->data_size - sizeof(uint8_t) - sizeof(uint16_t)), (uint16_t *) p);
+		bt_put_be16(dst->data_size - sizeof(uint8_t) - sizeof(uint16_t), p);
 		break;
 	case SDP_SEQ32:
-		bt_put_unaligned(htonl(dst->data_size - sizeof(uint8_t) - sizeof(uint32_t)), (uint32_t *) p);
+		bt_put_be32(dst->data_size - sizeof(uint8_t) - sizeof(uint32_t), p);
 		break;
 	}
 }
@@ -2974,7 +2974,7 @@ int sdp_device_record_unregister_binary(sdp_session_t *session, bdaddr_t *device
 
 	p = reqbuf + sizeof(sdp_pdu_hdr_t);
 	reqsize = sizeof(sdp_pdu_hdr_t);
-	bt_put_unaligned(htonl(handle), (uint32_t *) p);
+	bt_put_be32(handle, p);
 	reqsize += sizeof(uint32_t);
 
 	reqhdr->plen = htons(reqsize - sizeof(sdp_pdu_hdr_t));
@@ -3067,7 +3067,7 @@ int sdp_device_record_update(sdp_session_t *session, bdaddr_t *device, const sdp
 	p = reqbuf + sizeof(sdp_pdu_hdr_t);
 	reqsize = sizeof(sdp_pdu_hdr_t);
 
-	bt_put_unaligned(htonl(handle), (uint32_t *) p);
+	bt_put_be32(handle, p);
 	reqsize += sizeof(uint32_t);
 	p += sizeof(uint32_t);
 
@@ -3354,7 +3354,7 @@ int sdp_service_search_req(sdp_session_t *session, const sdp_list_t *search,
 	pdata += seqlen;
 
 	/* specify the maximum svc rec count that client expects */
-	bt_put_unaligned(htons(max_rec_num), (uint16_t *) pdata);
+	bt_put_be16(max_rec_num, pdata);
 	reqsize += sizeof(uint16_t);
 	pdata += sizeof(uint16_t);
 
@@ -3516,12 +3516,12 @@ sdp_record_t *sdp_service_attr_req(sdp_session_t *session, uint32_t handle,
 	reqsize = sizeof(sdp_pdu_hdr_t);
 
 	/* add the service record handle */
-	bt_put_unaligned(htonl(handle), (uint32_t *) pdata);
+	bt_put_be32(handle, pdata);
 	reqsize += sizeof(uint32_t);
 	pdata += sizeof(uint32_t);
 
 	/* specify the response limit */
-	bt_put_unaligned(htons(65535), (uint16_t *) pdata);
+	bt_put_be16(65535, pdata);
 	reqsize += sizeof(uint16_t);
 	pdata += sizeof(uint16_t);
 
@@ -3775,7 +3775,7 @@ int sdp_service_search_async(sdp_session_t *session, const sdp_list_t *search, u
 	t->reqsize += seqlen;
 	pdata += seqlen;
 
-	bt_put_unaligned(htons(max_rec_num), (uint16_t *) pdata);
+	bt_put_be16(max_rec_num, pdata);
 	t->reqsize += sizeof(uint16_t);
 	pdata += sizeof(uint16_t);
 
@@ -3868,12 +3868,12 @@ int sdp_service_attr_async(sdp_session_t *session, uint32_t handle, sdp_attrreq_
 	t->reqsize = sizeof(sdp_pdu_hdr_t);
 
 	/* add the service record handle */
-	bt_put_unaligned(htonl(handle), (uint32_t *) pdata);
+	bt_put_be32(handle, pdata);
 	t->reqsize += sizeof(uint32_t);
 	pdata += sizeof(uint32_t);
 
 	/* specify the response limit */
-	bt_put_unaligned(htons(65535), (uint16_t *) pdata);
+	bt_put_be16(65535, pdata);
 	t->reqsize += sizeof(uint16_t);
 	pdata += sizeof(uint16_t);
 
@@ -3988,7 +3988,7 @@ int sdp_service_search_attr_async(sdp_session_t *session, const sdp_list_t *sear
 	t->reqsize += seqlen;
 	pdata += seqlen;
 
-	bt_put_unaligned(htons(SDP_MAX_ATTR_LEN), (uint16_t *) pdata);
+	bt_put_be16(SDP_MAX_ATTR_LEN, pdata);
 	t->reqsize += sizeof(uint16_t);
 	pdata += sizeof(uint16_t);
 
@@ -4146,14 +4146,18 @@ int sdp_process(sdp_session_t *session)
 			rsp_count = sizeof(tsrc) + sizeof(csrc) + csrc * 4;
 		} else {
 			/* point to the first csrc */
-			uint16_t *pcsrc = (uint16_t *) (t->rsp_concat_buf.data + 2);
+			uint8_t *pcsrc = t->rsp_concat_buf.data + 2;
+			uint16_t tcsrc;
+
+			tcsrc = bt_get_16(pcsrc);
 
 			/* FIXME: update the interface later. csrc doesn't need be passed to clients */
 
 			pdata += sizeof(uint16_t); /* point to csrc */
 
 			/* the first csrc contains the sum of partial csrc responses */
-			*pcsrc += bt_get_unaligned((uint16_t *) pdata);
+			tcsrc += bt_get_16(pdata);
+			memcpy(pcsrc, &tcsrc, sizeof(tcsrc));
 
 			pdata += sizeof(uint16_t); /* point to the first handle */
 			rsp_count = csrc * 4;
@@ -4336,7 +4340,7 @@ int sdp_service_search_attr_req(sdp_session_t *session, const sdp_list_t *search
 	reqsize += seqlen;
 	pdata += seqlen;
 
-	bt_put_unaligned(htons(SDP_MAX_ATTR_LEN), (uint16_t *) pdata);
+	bt_put_be16(SDP_MAX_ATTR_LEN, pdata);
 	reqsize += sizeof(uint16_t);
 	pdata += sizeof(uint16_t);
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFC 06/15] l2test: Fix compilation errors due to unaligned memory access
  2012-08-31  8:36 [RFC 00/15] Unaligned memory access fixes Szymon Janc
                   ` (4 preceding siblings ...)
  2012-08-31  8:36 ` [RFC 05/15] sdp: Fix compilation errors due to unaligned memory access Szymon Janc
@ 2012-08-31  8:36 ` Szymon Janc
  2012-08-31  8:36 ` [RFC 07/15] rctest: " Szymon Janc
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 23+ messages in thread
From: Szymon Janc @ 2012-08-31  8:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This fix following build errors on ARM.

  CC     test/l2test.o
test/l2test.c: In function recv_mode:
test/l2test.c:826:9: error: cast increases required alignment of target
	type [-Werror=cast-align]
test/l2test.c:834:8: error: cast increases required alignment of target
	type [-Werror=cast-align]
test/l2test.c: In function do_send:
test/l2test.c:893:4: error: cast increases required alignment of target
	type [-Werror=cast-align]
test/l2test.c:894:4: error: cast increases required alignment of target
	type [-Werror=cast-align]
cc1: all warnings being treated as errors
make[1]: *** [test/l2test.o] Error 1

Change-Id: If23f312fcb10d66f74891f6a8bffcac737ae5eda
---
 test/l2test.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/test/l2test.c b/test/l2test.c
index d31be10..7645681 100644
--- a/test/l2test.c
+++ b/test/l2test.c
@@ -823,7 +823,7 @@ static void recv_mode(int sk)
 			}
 
 			/* Check sequence */
-			sq = btohl(*(uint32_t *) buf);
+			sq = bt_get_le32(buf);
 			if (seq != sq) {
 				syslog(LOG_INFO, "seq missmatch: %d -> %d", seq, sq);
 				seq = sq;
@@ -831,7 +831,7 @@ static void recv_mode(int sk)
 			seq++;
 
 			/* Check length */
-			l = btohs(*(uint16_t *) (buf + 4));
+			l = bt_get_le16(buf + 4);
 			if (len != l) {
 				syslog(LOG_INFO, "size missmatch: %d -> %d", len, l);
 				continue;
@@ -890,8 +890,9 @@ static void do_send(int sk)
 
 	seq = 0;
 	while ((num_frames == -1) || (num_frames-- > 0)) {
-		*(uint32_t *) buf = htobl(seq);
-		*(uint16_t *) (buf + 4) = htobs(data_size);
+		bt_put_le32(seq, buf);
+		bt_put_le16(data_size, buf + 4);
+
 		seq++;
 
 		sent = 0;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFC 07/15] rctest: Fix compilation errors due to unaligned memory access
  2012-08-31  8:36 [RFC 00/15] Unaligned memory access fixes Szymon Janc
                   ` (5 preceding siblings ...)
  2012-08-31  8:36 ` [RFC 06/15] l2test: " Szymon Janc
@ 2012-08-31  8:36 ` Szymon Janc
  2012-08-31  8:36 ` [RFC 08/15] monitor: " Szymon Janc
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 23+ messages in thread
From: Szymon Janc @ 2012-08-31  8:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This fix following build errors on ARM.

  CC     test/rctest.o
test/rctest.c: In function do_send:
test/rctest.c:539:4: error: cast increases required alignment of target
	type [-Werror=cast-align]
test/rctest.c:540:4: error: cast increases required alignment of target
	type [-Werror=cast-align]
cc1: all warnings being treated as errors
make[1]: *** [test/rctest.o] Error 1

Change-Id: Ied636b0926fba2fece1ae3fbbdeff73bed764de9
---
 test/rctest.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/test/rctest.c b/test/rctest.c
index 4d7c90a..43ae09d 100644
--- a/test/rctest.c
+++ b/test/rctest.c
@@ -536,8 +536,9 @@ static void do_send(int sk)
 
 	seq = 0;
 	while ((num_frames == -1) || (num_frames-- > 0)) {
-		*(uint32_t *) buf = htobl(seq);
-		*(uint16_t *) (buf + 4) = htobs(data_size);
+		bt_put_le32(seq, buf);
+		bt_put_le16(data_size, buf + 4);
+
 		seq++;
 
 		if (send(sk, buf, data_size, 0) <= 0) {
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFC 08/15] monitor: Fix compilation errors due to unaligned memory access
  2012-08-31  8:36 [RFC 00/15] Unaligned memory access fixes Szymon Janc
                   ` (6 preceding siblings ...)
  2012-08-31  8:36 ` [RFC 07/15] rctest: " Szymon Janc
@ 2012-08-31  8:36 ` Szymon Janc
  2012-08-31  8:36 ` [RFC 09/15] scotest: " Szymon Janc
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 23+ messages in thread
From: Szymon Janc @ 2012-08-31  8:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This fix following compilation errors on ARM.

  CC     monitor/hcidump.o
monitor/hcidump.c: In function device_callback:
monitor/hcidump.c:147:11: error: cast increases required alignment of
	target type [-Werror=cast-align]
monitor/hcidump.c:150:10: error: cast increases required alignment of
	target type [-Werror=cast-align]
monitor/hcidump.c: In function stack_internal_callback:
monitor/hcidump.c:348:9: error: cast increases required alignment of
	target type [-Werror=cast-align]
cc1: all warnings being treated as errors
make[1]: *** [monitor/hcidump.o] Error 1
make: *** [all] Error 2

  CC     monitor/hcidump.o
monitor/hcidump.c: In function stack_internal_callback:
monitor/hcidump.c:357:9: error: cast increases required alignment of
	target type [-Werror=cast-align]
cc1: all warnings being treated as errors
make[1]: *** [monitor/hcidump.o] Error 1
make: *** [all] Error 2

  CC     monitor/control.o
monitor/control.c: In function data_callback:
monitor/control.c:574:10: error: cast increases required alignment of
	target type [-Werror=cast-align]
cc1: all warnings being treated as errors
make[1]: *** [monitor/control.o] Error 1
make: *** [all] Error 2

Change-Id: I94bff30d7c531bd6a7cdbb791df6995cc099a810
---
 monitor/control.c |    7 +++++--
 monitor/hcidump.c |   23 ++++++++++++++++-------
 2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/monitor/control.c b/monitor/control.c
index c300ae9..c5ff26b 100644
--- a/monitor/control.c
+++ b/monitor/control.c
@@ -555,6 +555,7 @@ static void data_callback(int fd, uint32_t events, void *user_data)
 	while (1) {
 		struct cmsghdr *cmsg;
 		struct timeval *tv = NULL;
+		struct timeval ctv;
 		uint16_t opcode, index, pktlen;
 		ssize_t len;
 
@@ -570,8 +571,10 @@ static void data_callback(int fd, uint32_t events, void *user_data)
 			if (cmsg->cmsg_level != SOL_SOCKET)
 				continue;
 
-			if (cmsg->cmsg_type == SCM_TIMESTAMP)
-				tv = (struct timeval *) CMSG_DATA(cmsg);
+			if (cmsg->cmsg_type == SCM_TIMESTAMP) {
+				memcpy(&ctv, CMSG_DATA(cmsg), sizeof(ctv));
+				tv = &ctv;
+			}
 		}
 
 		opcode = btohs(hdr.opcode);
diff --git a/monitor/hcidump.c b/monitor/hcidump.c
index 373d2f5..8653446 100644
--- a/monitor/hcidump.c
+++ b/monitor/hcidump.c
@@ -130,8 +130,10 @@ static void device_callback(int fd, uint32_t events, void *user_data)
 	while (1) {
 		struct cmsghdr *cmsg;
 		struct timeval *tv = NULL;
-		int *dir = NULL;
+		struct timeval ctv;
+		bool dir = false;
 		ssize_t len;
+		bool dir_present = false;
 
 		len = recvmsg(fd, &msg, MSG_DONTWAIT);
 		if (len < 0)
@@ -144,15 +146,19 @@ static void device_callback(int fd, uint32_t events, void *user_data)
 
 			switch (cmsg->cmsg_type) {
 			case HCI_DATA_DIR:
-				dir = (int *) CMSG_DATA(cmsg);
+				dir = !!bt_get_32(CMSG_DATA(cmsg));
+				dir_present = true;
+
 				break;
 			case HCI_CMSG_TSTAMP:
-				tv = (struct timeval *) CMSG_DATA(cmsg);
+				memcpy(&ctv, CMSG_DATA(cmsg), sizeof(ctv));
+				tv = &ctv;
+
 				break;
 			}
 		}
 
-		if (!dir || len < 1)
+		if (!dir_present || len < 1)
 			continue;
 
 		switch (buf[0]) {
@@ -163,11 +169,11 @@ static void device_callback(int fd, uint32_t events, void *user_data)
 			packet_hci_event(tv, data->index, buf + 1, len - 1);
 			break;
 		case HCI_ACLDATA_PKT:
-			packet_hci_acldata(tv, data->index, !!(*dir),
+			packet_hci_acldata(tv, data->index, dir,
 							buf + 1, len - 1);
 			break;
 		case HCI_SCODATA_PKT:
-			packet_hci_scodata(tv, data->index, !!(*dir),
+			packet_hci_scodata(tv, data->index, dir,
 							buf + 1, len - 1);
 			break;
 		}
@@ -314,6 +320,7 @@ static void stack_internal_callback(int fd, uint32_t events, void *user_data)
 	evt_stack_internal *si;
 	evt_si_device *sd;
 	struct timeval *tv = NULL;
+	struct timeval ctv;
 	uint8_t type = 0xff, bus = 0xff;
 	char str[18], name[8] = "";
 	bdaddr_t bdaddr;
@@ -345,7 +352,9 @@ static void stack_internal_callback(int fd, uint32_t events, void *user_data)
 
 		switch (cmsg->cmsg_type) {
 		case HCI_CMSG_TSTAMP:
-			tv = (struct timeval *) CMSG_DATA(cmsg);
+			memcpy(&ctv, CMSG_DATA(cmsg), sizeof(ctv));
+			tv = &ctv;
+
 			break;
 		}
 	}
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFC 09/15] scotest: Fix compilation errors due to unaligned memory access
  2012-08-31  8:36 [RFC 00/15] Unaligned memory access fixes Szymon Janc
                   ` (7 preceding siblings ...)
  2012-08-31  8:36 ` [RFC 08/15] monitor: " Szymon Janc
@ 2012-08-31  8:36 ` Szymon Janc
  2012-08-31  8:36 ` [RFC 10/15] avrcp: " Szymon Janc
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 23+ messages in thread
From: Szymon Janc @ 2012-08-31  8:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This fix following build errors on ARM.

  CC     test/scotest.o
test/scotest.c: In function send_mode:
test/scotest.c:272:4: error: cast increases required alignment of
	target type [-Werror=cast-align]
test/scotest.c:273:4: error: cast increases required alignment of
	target type [-Werror=cast-align]
cc1: all warnings being treated as errors
make[1]: *** [test/scotest.o] Error 1
make: *** [all] Error 2

Change-Id: Ib0fb15e000f333bce4228e96762ee2c849c580d9
---
 test/scotest.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/test/scotest.c b/test/scotest.c
index 17bd8a6..de65edf 100644
--- a/test/scotest.c
+++ b/test/scotest.c
@@ -269,8 +269,9 @@ static void send_mode(char *svr)
 
 	seq = 0;
 	while (1) {
-		*(uint32_t *) buf = htobl(seq);
-		*(uint16_t *) (buf + 4) = htobs(data_size);
+		bt_put_le32(seq, buf);
+		bt_put_le16(data_size, buf + 4);
+
 		seq++;
 
 		if (send(sk, buf, so.mtu, 0) <= 0) {
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFC 10/15] avrcp: Fix compilation errors due to unaligned memory access
  2012-08-31  8:36 [RFC 00/15] Unaligned memory access fixes Szymon Janc
                   ` (8 preceding siblings ...)
  2012-08-31  8:36 ` [RFC 09/15] scotest: " Szymon Janc
@ 2012-08-31  8:36 ` Szymon Janc
  2012-08-31  8:36 ` [RFC 11/15] sap: " Szymon Janc
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 23+ messages in thread
From: Szymon Janc @ 2012-08-31  8:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This fix following build errors on ARM.

  CC     audio/bluetoothd-avrcp.o
audio/avrcp.c: In function avrcp_handle_get_element_attributes:
audio/avrcp.c:667:25: error: cast increases required alignment of
	target type [-Werror=cast-align]
audio/avrcp.c:690:20: error: cast increases required alignment of
	target type [-Werror=cast-align]
cc1: all warnings being treated as errors
make[1]: *** [audio/bluetoothd-avrcp.o] Error 1
make: *** [all] Error 2

Change-Id: I0c4b15063672b602170809ee1aecc6b9305c3a67
---
 audio/avrcp.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/audio/avrcp.c b/audio/avrcp.c
index d925365..9c474f3 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -664,13 +664,13 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp_player *player,
 						uint8_t transaction)
 {
 	uint16_t len = ntohs(pdu->params_len);
-	uint64_t *identifier = (uint64_t *) &pdu->params[0];
+	uint64_t identifier = bt_get_64(&pdu->params[0]);
 	uint16_t pos;
 	uint8_t nattr;
 	GList *attr_ids;
 	uint16_t offset;
 
-	if (len < 9 || *identifier != 0)
+	if (len < 9 || identifier != 0)
 		goto err;
 
 	nattr = pdu->params[8];
@@ -687,10 +687,10 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp_player *player,
 		len = g_list_length(attr_ids);
 	} else {
 		unsigned int i;
-		uint32_t *attr = (uint32_t *) &pdu->params[9];
+		for (i = 0, len = 0, attr_ids = NULL; i < nattr; i++) {
+			uint32_t id;
 
-		for (i = 0, len = 0, attr_ids = NULL; i < nattr; i++, attr++) {
-			uint32_t id = ntohl(bt_get_unaligned(attr));
+			id = bt_get_be32(&pdu->params[9] + (i * sizeof(id)));
 
 			/* Don't add invalid attributes */
 			if (id == AVRCP_MEDIA_ATTRIBUTE_ILLEGAL ||
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFC 11/15] sap: Fix compilation errors due to unaligned memory access
  2012-08-31  8:36 [RFC 00/15] Unaligned memory access fixes Szymon Janc
                   ` (9 preceding siblings ...)
  2012-08-31  8:36 ` [RFC 10/15] avrcp: " Szymon Janc
@ 2012-08-31  8:36 ` Szymon Janc
  2012-08-31  8:36 ` [RFC 12/15] adaptername: Refactor handle_inotify_cb Szymon Janc
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 23+ messages in thread
From: Szymon Janc @ 2012-08-31  8:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This fix following build errors on ARM.

  CC     profiles/sap/bluetoothd-server.o
profiles/sap/server.c: In function connect_req:
profiles/sap/server.c:317:8: error: cast increases required alignment
	of targettype [-Werror=cast-align]
profiles/sap/server.c: In function sap_connect_rsp:
profiles/sap/server.c:676:16: error: cast increases required alignment
	of target type [-Werror=cast-align]
cc1: all warnings being treated as errors
make[1]: *** [profiles/sap/bluetoothd-server.o] Error 1
make: *** [all] Error 2

Change-Id: I7015a0e99df7b49e734f3bcac0f2298129722359
---
 profiles/sap/server.c |    9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/profiles/sap/server.c b/profiles/sap/server.c
index 3adbb1a..78a63bf 100644
--- a/profiles/sap/server.c
+++ b/profiles/sap/server.c
@@ -302,7 +302,7 @@ static void connect_req(struct sap_server *server,
 				struct sap_parameter *param)
 {
 	struct sap_connection *conn = server->conn;
-	uint16_t maxmsgsize, *val;
+	uint16_t maxmsgsize;
 
 	DBG("conn %p state %d", conn, conn->state);
 
@@ -314,8 +314,7 @@ static void connect_req(struct sap_server *server,
 
 	stop_guard_timer(server);
 
-	val = (uint16_t *) &param->val;
-	maxmsgsize = ntohs(*val);
+	maxmsgsize = bt_get_be16(&param->val);
 
 	DBG("Connect MaxMsgSize: 0x%04x", maxmsgsize);
 
@@ -638,7 +637,6 @@ int sap_connect_rsp(void *sap_device, uint8_t status)
 	struct sap_message *msg = (struct sap_message *) buf;
 	struct sap_parameter *param = (struct sap_parameter *) msg->param;
 	size_t size = sizeof(struct sap_message);
-	uint16_t *maxmsgsize;
 
 	if (!conn)
 		return -EINVAL;
@@ -673,8 +671,7 @@ int sap_connect_rsp(void *sap_device, uint8_t status)
 		param = (struct sap_parameter *) &buf[size];
 		param->id = SAP_PARAM_ID_MAX_MSG_SIZE;
 		param->len = htons(SAP_PARAM_ID_MAX_MSG_SIZE_LEN);
-		maxmsgsize = (uint16_t *) &param->val;
-		*maxmsgsize = htons(SAP_BUF_SIZE);
+		bt_put_be16(SAP_BUF_SIZE, &param->val);
 		size += PARAMETER_SIZE(SAP_PARAM_ID_MAX_MSG_SIZE_LEN);
 
 		/* fall */
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFC 12/15] adaptername: Refactor handle_inotify_cb
  2012-08-31  8:36 [RFC 00/15] Unaligned memory access fixes Szymon Janc
                   ` (10 preceding siblings ...)
  2012-08-31  8:36 ` [RFC 11/15] sap: " Szymon Janc
@ 2012-08-31  8:36 ` Szymon Janc
  2012-08-31  8:36 ` [RFC 13/15] sdpd-request: Fix build errors due to unaligned memory access Szymon Janc
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 23+ messages in thread
From: Szymon Janc @ 2012-08-31  8:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

Refactor handle_inotify_cb to avoid unaligned memory access.
Instead of reading whole events buffer and cast to event struct when
iterating over it, read data directly to struct inotify_event one
event at time.

This fix following build error on ARM.

  CC     plugins/bluetoothd-adaptername.o
plugins/adaptername.c: In function handle_inotify_cb:
plugins/adaptername.c:244:34: error: cast increases required alignment
	of target type [-Werror=cast-align]
cc1: all warnings being treated as errors
make[1]: *** [plugins/bluetoothd-adaptername.o] Error 1
make: *** [all] Error 2

Change-Id: Ibcf2daa06fcb79853c71c00502b5ebe13773e6ff
---
 plugins/adaptername.c |   44 +++++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 19 deletions(-)

diff --git a/plugins/adaptername.c b/plugins/adaptername.c
index d3341b5..f5010ea 100644
--- a/plugins/adaptername.c
+++ b/plugins/adaptername.c
@@ -226,35 +226,41 @@ static int adaptername_probe(struct btd_adapter *adapter)
 static gboolean handle_inotify_cb(GIOChannel *channel, GIOCondition cond,
 								gpointer data)
 {
-	char buf[EVENT_BUF_LEN];
+	struct inotify_event event;
+	gsize len;
 	GIOStatus err;
-	gsize len, i;
-	gboolean changed;
+	char name[FILENAME_MAX + 1];
 
-	changed = FALSE;
+	while ((err = g_io_channel_read_chars(channel, (gchar *)&event,
+			sizeof(event), &len, NULL)) != G_IO_STATUS_AGAIN) {
+		if (err != G_IO_STATUS_NORMAL) {
+			error("Error reading inotify event: %d", err);
+			return FALSE;
+		}
 
-	err = g_io_channel_read_chars(channel, buf, EVENT_BUF_LEN, &len, NULL);
-	if (err != G_IO_STATUS_NORMAL) {
-		error("Error reading inotify event: %d\n", err);
-		return FALSE;
-	}
+		if (len != sizeof(event)) {
+			error("Not enough bytes of inotify event read");
+			return FALSE;
+		}
 
-	i = 0;
-	while (i < len) {
-		struct inotify_event *pevent = (struct inotify_event *) &buf[i];
+		if (event.len == 0)
+			continue;
 
-		/* check that it's ours */
-		if (pevent->len && pevent->name != NULL &&
-				strcmp(pevent->name, MACHINE_INFO_FILE) == 0)
-			changed = TRUE;
+		err = g_io_channel_read_chars(channel, name, event.len, &len,
+									NULL);
+		if (err != G_IO_STATUS_NORMAL) {
+			error("Error reading inotify event: %d", err);
+			return FALSE;
+		}
 
-		i += EVENT_SIZE + pevent->len;
-	}
+		if (strcmp(name, MACHINE_INFO_FILE) != 0)
+			continue;
 
-	if (changed != FALSE) {
 		DBG(MACHINE_INFO_DIR MACHINE_INFO_FILE
 				" changed, changing names for adapters");
+
 		manager_foreach_adapter((adapter_cb) adaptername_probe, NULL);
+		break;
 	}
 
 	return TRUE;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFC 13/15] sdpd-request: Fix build errors due to unaligned memory access
  2012-08-31  8:36 [RFC 00/15] Unaligned memory access fixes Szymon Janc
                   ` (11 preceding siblings ...)
  2012-08-31  8:36 ` [RFC 12/15] adaptername: Refactor handle_inotify_cb Szymon Janc
@ 2012-08-31  8:36 ` Szymon Janc
  2012-08-31  8:36 ` [RFC 14/15] sdpd-service: " Szymon Janc
  2012-08-31  8:36 ` [RFC 15/15] hciemu: " Szymon Janc
  14 siblings, 0 replies; 23+ messages in thread
From: Szymon Janc @ 2012-08-31  8:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This fix number of build errors on ARM similar to one below.

  CC     src/bluetooth-sdpd-request.o
src/sdpd-request.c: In function extra_des:
src/sdpd-request.c:181:5: error: cast increases required alignment
    of targettype [-Werror=cast-align]

Change-Id: I7323634e7d51d56936b5253660a5f6eb02ac0645
---
 src/sdpd-request.c |   62 ++++++++++++++++++++++++++++------------------------
 1 file changed, 33 insertions(+), 29 deletions(-)

diff --git a/src/sdpd-request.c b/src/sdpd-request.c
index 6a903c6..6f24a89 100644
--- a/src/sdpd-request.c
+++ b/src/sdpd-request.c
@@ -139,6 +139,7 @@ static int extract_des(uint8_t *buf, int len, sdp_list_t **svcReqSeq, uint8_t *p
 	for (;;) {
 		char *pElem = NULL;
 		int localSeqLength = 0;
+		uuid_t *puuid;
 
 		if (bufsize < sizeof(uint8_t)) {
 			SDPDBG("->Unexpected end of buffer");
@@ -178,11 +179,11 @@ static int extract_des(uint8_t *buf, int len, sdp_list_t **svcReqSeq, uint8_t *p
 				struct attrid *aid;
 				aid = malloc(sizeof(struct attrid));
 				aid->dtd = dataType;
-				bt_put_unaligned(ntohs(bt_get_unaligned((uint16_t *)p)), (uint16_t *)&aid->uint16);
+				aid->uint16 = bt_get_be16(p);
 				pElem = (char *) aid;
 			} else {
 				pElem = malloc(sizeof(uint16_t));
-				bt_put_unaligned(ntohs(bt_get_unaligned((uint16_t *)p)), (uint16_t *)pElem);
+				bt_put_be16(bt_get_16(p), pElem);
 			}
 			p += sizeof(uint16_t);
 			seqlen += sizeof(uint16_t);
@@ -201,11 +202,12 @@ static int extract_des(uint8_t *buf, int len, sdp_list_t **svcReqSeq, uint8_t *p
 				struct attrid *aid;
 				aid = malloc(sizeof(struct attrid));
 				aid->dtd = dataType;
-				bt_put_unaligned(ntohl(bt_get_unaligned((uint32_t *)p)), (uint32_t *)&aid->uint32);
+				aid->uint32 = bt_get_be32(p);
+
 				pElem = (char *) aid;
 			} else {
 				pElem = malloc(sizeof(uint32_t));
-				bt_put_unaligned(ntohl(bt_get_unaligned((uint32_t *)p)), (uint32_t *)pElem);
+				bt_put_be32(bt_get_32(p), pElem);
 			}
 			p += sizeof(uint32_t);
 			seqlen += sizeof(uint32_t);
@@ -214,12 +216,14 @@ static int extract_des(uint8_t *buf, int len, sdp_list_t **svcReqSeq, uint8_t *p
 		case SDP_UUID16:
 		case SDP_UUID32:
 		case SDP_UUID128:
-			pElem = malloc(sizeof(uuid_t));
-			status = sdp_uuid_extract(p, bufsize, (uuid_t *) pElem, &localSeqLength);
+			puuid = malloc(sizeof(uuid_t));
+			status = sdp_uuid_extract(p, bufsize, puuid, &localSeqLength);
 			if (status < 0) {
-				free(pElem);
+				free(puuid);
 				goto failed;
 			}
+
+			pElem = (char *) puuid;
 			seqlen += localSeqLength;
 			p += localSeqLength;
 			bufsize -= localSeqLength;
@@ -359,7 +363,7 @@ static int service_search_req(sdp_req_t *req, sdp_buf_t *buf)
 	uint8_t *pCacheBuffer = NULL;
 	int handleSize = 0;
 	uint32_t cStateId = 0;
-	short *pTotalRecordCount, *pCurrentRecordCount;
+	uint8_t *pTotalRecordCount, *pCurrentRecordCount;
 	uint8_t *pdata = req->buf + sizeof(sdp_pdu_hdr_t);
 	size_t data_left = req->len - sizeof(sdp_pdu_hdr_t);
 
@@ -385,7 +389,7 @@ static int service_search_req(sdp_req_t *req, sdp_buf_t *buf)
 		goto done;
 	}
 
-	expected = ntohs(bt_get_unaligned((uint16_t *)pdata));
+	expected = bt_get_be16(pdata);
 
 	SDPDBG("Expected count: %d", expected);
 	SDPDBG("Bytes scanned : %d", scanned);
@@ -409,14 +413,14 @@ static int service_search_req(sdp_req_t *req, sdp_buf_t *buf)
 	pdata = buf->data;
 
 	/* total service record count = 0 */
-	pTotalRecordCount = (short *)pdata;
-	bt_put_unaligned(0, (uint16_t *)pdata);
+	pTotalRecordCount = pdata;
+	bt_put_be16(0, pdata);
 	pdata += sizeof(uint16_t);
 	buf->data_size += sizeof(uint16_t);
 
 	/* current service record count = 0 */
-	pCurrentRecordCount = (short *)pdata;
-	bt_put_unaligned(0, (uint16_t *)pdata);
+	pCurrentRecordCount = pdata;
+	bt_put_be16(0, pdata);
 	pdata += sizeof(uint16_t);
 	buf->data_size += sizeof(uint16_t);
 
@@ -433,7 +437,7 @@ static int service_search_req(sdp_req_t *req, sdp_buf_t *buf)
 			if (sdp_match_uuid(pattern, rec->pattern) > 0 &&
 					sdp_check_access(rec->handle, &req->device)) {
 				rsp_count++;
-				bt_put_unaligned(htonl(rec->handle), (uint32_t *)pdata);
+				bt_put_be32(rec->handle, pdata);
 				pdata += sizeof(uint32_t);
 				handleSize += sizeof(uint32_t);
 			}
@@ -442,8 +446,8 @@ static int service_search_req(sdp_req_t *req, sdp_buf_t *buf)
 		SDPDBG("Match count: %d", rsp_count);
 
 		buf->data_size += handleSize;
-		bt_put_unaligned(htons(rsp_count), (uint16_t *)pTotalRecordCount);
-		bt_put_unaligned(htons(rsp_count), (uint16_t *)pCurrentRecordCount);
+		bt_put_be16(rsp_count, pTotalRecordCount);
+		bt_put_be16(rsp_count, pCurrentRecordCount);
 
 		if (rsp_count > actual) {
 			/* cache the rsp and generate a continuation state */
@@ -472,7 +476,7 @@ static int service_search_req(sdp_req_t *req, sdp_buf_t *buf)
 			if (pCache) {
 				pCacheBuffer = pCache->data;
 				/* get the rsp_count from the cached buffer */
-				rsp_count = ntohs(bt_get_unaligned((uint16_t *)pCacheBuffer));
+				rsp_count = bt_get_be16(pCacheBuffer);
 
 				/* get index of the last sdp_record_t sent */
 				lastIndex = cstate->cStateValue.lastIndexSent;
@@ -490,7 +494,7 @@ static int service_search_req(sdp_req_t *req, sdp_buf_t *buf)
 		 * current record count and increment the cached
 		 * buffer pointer to beyond the counters
 		 */
-		pdata = (uint8_t *) pCurrentRecordCount + sizeof(uint16_t);
+		pdata = pCurrentRecordCount + sizeof(uint16_t);
 
 		/* increment beyond the totalCount and the currentCount */
 		pCacheBuffer += 2 * sizeof(uint16_t);
@@ -498,7 +502,7 @@ static int service_search_req(sdp_req_t *req, sdp_buf_t *buf)
 		if (cstate) {
 			handleSize = 0;
 			for (i = lastIndex; (i - lastIndex) < actual && i < rsp_count; i++) {
-				bt_put_unaligned(bt_get_unaligned((uint32_t *)(pCacheBuffer + i * sizeof(uint32_t))), (uint32_t *)pdata);
+				memcpy(pdata, pCacheBuffer + i * sizeof(uint32_t), sizeof(uint32_t));
 				pdata += sizeof(uint32_t);
 				handleSize += sizeof(uint32_t);
 			}
@@ -508,8 +512,8 @@ static int service_search_req(sdp_req_t *req, sdp_buf_t *buf)
 		}
 
 		buf->data_size += handleSize;
-		bt_put_unaligned(htons(rsp_count), (uint16_t *)pTotalRecordCount);
-		bt_put_unaligned(htons(i - lastIndex), (uint16_t *)pCurrentRecordCount);
+		bt_put_be16(rsp_count, pTotalRecordCount);
+		bt_put_be16(i - lastIndex, pCurrentRecordCount);
 
 		if (i == rsp_count) {
 			/* set "null" continuationState */
@@ -571,12 +575,12 @@ static int extract_attrs(sdp_record_t *rec, sdp_list_t *seq, sdp_buf_t *buf)
 		SDPDBG("AttrDataType : %d", aid->dtd);
 
 		if (aid->dtd == SDP_UINT16) {
-			uint16_t attr = bt_get_unaligned((uint16_t *)&aid->uint16);
+			uint16_t attr = aid->uint16;
 			sdp_data_t *a = sdp_data_get(rec, attr);
 			if (a)
 				sdp_append_to_pdu(buf, a);
 		} else if (aid->dtd == SDP_UINT32) {
-			uint32_t range = bt_get_unaligned((uint32_t *)&aid->uint32);
+			uint32_t range = aid->uint32;
 			uint16_t attr;
 			uint16_t low = (0xffff0000 & range) >> 16;
 			uint16_t high = 0x0000ffff & range;
@@ -639,7 +643,7 @@ static int service_attr_req(sdp_req_t *req, sdp_buf_t *buf)
 		goto done;
 	}
 
-	handle = ntohl(bt_get_unaligned((uint32_t *)pdata));
+	handle = bt_get_be32(pdata);
 
 	pdata += sizeof(uint32_t);
 	data_left -= sizeof(uint32_t);
@@ -649,7 +653,7 @@ static int service_attr_req(sdp_req_t *req, sdp_buf_t *buf)
 		goto done;
 	}
 
-	max_rsp_size = ntohs(bt_get_unaligned((uint16_t *)pdata));
+	max_rsp_size = bt_get_be16(pdata);
 
 	pdata += sizeof(uint16_t);
 	data_left -= sizeof(uint16_t);
@@ -765,7 +769,7 @@ done:
 		return status;
 
 	/* set attribute list byte count */
-	bt_put_unaligned(htons(buf->data_size - cstate_size), (uint16_t *)buf->data);
+	bt_put_be16(buf->data_size - cstate_size, buf->data);
 	buf->data_size += sizeof(uint16_t);
 	return 0;
 }
@@ -806,7 +810,7 @@ static int service_search_attr_req(sdp_req_t *req, sdp_buf_t *buf)
 		goto done;
 	}
 
-	max = ntohs(bt_get_unaligned((uint16_t *)pdata));
+	max = bt_get_be16(pdata);
 
 	pdata += sizeof(uint16_t);
 	data_left -= sizeof(uint16_t);
@@ -936,7 +940,7 @@ static int service_search_attr_req(sdp_req_t *req, sdp_buf_t *buf)
 
 	if (!status) {
 		/* set attribute list byte count */
-		bt_put_unaligned(htons(buf->data_size - cstate_size), (uint16_t *)buf->data);
+		bt_put_be16(buf->data_size - cstate_size, buf->data);
 		buf->data_size += sizeof(uint16_t);
 	}
 
@@ -1020,7 +1024,7 @@ static void process_request(sdp_req_t *req)
 send_rsp:
 	if (status) {
 		rsphdr->pdu_id = SDP_ERROR_RSP;
-		bt_put_unaligned(htons(status), (uint16_t *)rsp.data);
+		bt_put_be16(status, rsp.data);
 		rsp.data_size = sizeof(uint16_t);
 	}
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFC 14/15] sdpd-service: Fix build errors due to unaligned memory access
  2012-08-31  8:36 [RFC 00/15] Unaligned memory access fixes Szymon Janc
                   ` (12 preceding siblings ...)
  2012-08-31  8:36 ` [RFC 13/15] sdpd-request: Fix build errors due to unaligned memory access Szymon Janc
@ 2012-08-31  8:36 ` Szymon Janc
  2012-08-31  8:36 ` [RFC 15/15] hciemu: " Szymon Janc
  14 siblings, 0 replies; 23+ messages in thread
From: Szymon Janc @ 2012-08-31  8:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This fix number of build errors on ARM similar to one below.

  CC     src/bluetooth-sdpd-service.o
src/sdpd-service.c: In function service_remove_req:
src/sdpd-service.c:517:20: error: cast increases required alignment of
	target type [-Werror=cast-align]
src/sdpd-service.c:517:20: error: cast increases required alignment of
	target type [-Werror=cast-align]
src/sdpd-service.c:536:2: error: cast increases required alignment of
	target type [-Werror=cast-align]
src/sdpd-service.c:536:2: error: cast increases required alignment of
	target type [-Werror=cast-align]
cc1: all warnings being treated as errors

Change-Id: Ic63bc1cdcf803895d9feed5dc36fcdcdd83e1783
---
 src/sdpd-service.c |   21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/src/sdpd-service.c b/src/sdpd-service.c
index 39e05ab..b43ddc2 100644
--- a/src/sdpd-service.c
+++ b/src/sdpd-service.c
@@ -317,7 +317,7 @@ static sdp_record_t *extract_pdu_server(bdaddr_t *device, uint8_t *p,
 		return NULL;
 	}
 
-	lookAheadAttrId = ntohs(bt_get_unaligned((uint16_t *) (p + sizeof(uint8_t))));
+	lookAheadAttrId = bt_get_be16(p + sizeof(uint8_t));
 
 	SDPDBG("Look ahead attr id : %d", lookAheadAttrId);
 
@@ -327,9 +327,8 @@ static sdp_record_t *extract_pdu_server(bdaddr_t *device, uint8_t *p,
 			SDPDBG("Unexpected end of packet");
 			return NULL;
 		}
-		handle = ntohl(bt_get_unaligned((uint32_t *) (p +
-				sizeof(uint8_t) + sizeof(uint16_t) +
-				sizeof(uint8_t))));
+		handle = bt_get_be32(p + sizeof(uint8_t) + sizeof(uint16_t) +
+							sizeof(uint8_t));
 		SDPDBG("SvcRecHandle : 0x%x", handle);
 		rec = sdp_record_find(handle);
 	} else if (handleExpected != 0xffffffff)
@@ -363,7 +362,7 @@ static sdp_record_t *extract_pdu_server(bdaddr_t *device, uint8_t *p,
 							seqlen, localExtractedLength);
 		dtd = *(uint8_t *) p;
 
-		attrId = ntohs(bt_get_unaligned((uint16_t *) (p + attrSize)));
+		attrId = bt_get_be16(p + attrSize);
 		attrSize += sizeof(uint16_t);
 
 		SDPDBG("DTD of attrId : %d Attr id : 0x%x", dtd, attrId);
@@ -454,13 +453,13 @@ success:
 	update_db_timestamp();
 
 	/* Build a rsp buffer */
-	bt_put_unaligned(htonl(rec->handle), (uint32_t *) rsp->data);
+	bt_put_be32(rec->handle, rsp->data);
 	rsp->data_size = sizeof(uint32_t);
 
 	return 0;
 
 invalid:
-	bt_put_unaligned(htons(SDP_INVALID_SYNTAX), (uint16_t *) rsp->data);
+	bt_put_be16(SDP_INVALID_SYNTAX, rsp->data);
 	rsp->data_size = sizeof(uint16_t);
 
 	return -1;
@@ -475,7 +474,7 @@ int service_update_req(sdp_req_t *req, sdp_buf_t *rsp)
 	int status = 0, scanned = 0;
 	uint8_t *p = req->buf + sizeof(sdp_pdu_hdr_t);
 	int bufsize = req->len - sizeof(sdp_pdu_hdr_t);
-	uint32_t handle = ntohl(bt_get_unaligned((uint32_t *) p));
+	uint32_t handle = bt_get_be32(p);
 
 	SDPDBG("Svc Rec Handle: 0x%x", handle);
 
@@ -503,7 +502,7 @@ int service_update_req(sdp_req_t *req, sdp_buf_t *rsp)
 
 done:
 	p = rsp->data;
-	bt_put_unaligned(htons(status), (uint16_t *) p);
+	bt_put_be16(status, p);
 	rsp->data_size = sizeof(uint16_t);
 	return status;
 }
@@ -514,7 +513,7 @@ done:
 int service_remove_req(sdp_req_t *req, sdp_buf_t *rsp)
 {
 	uint8_t *p = req->buf + sizeof(sdp_pdu_hdr_t);
-	uint32_t handle = ntohl(bt_get_unaligned((uint32_t *) p));
+	uint32_t handle = bt_get_be32(p);
 	sdp_record_t *rec;
 	int status = 0;
 
@@ -533,7 +532,7 @@ int service_remove_req(sdp_req_t *req, sdp_buf_t *rsp)
 	}
 
 	p = rsp->data;
-	bt_put_unaligned(htons(status), (uint16_t *) p);
+	bt_put_be16(status, p);
 	rsp->data_size = sizeof(uint16_t);
 
 	return status;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFC 15/15] hciemu: Fix build errors due to unaligned memory access
  2012-08-31  8:36 [RFC 00/15] Unaligned memory access fixes Szymon Janc
                   ` (13 preceding siblings ...)
  2012-08-31  8:36 ` [RFC 14/15] sdpd-service: " Szymon Janc
@ 2012-08-31  8:36 ` Szymon Janc
  14 siblings, 0 replies; 23+ messages in thread
From: Szymon Janc @ 2012-08-31  8:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This fix following build errors on ARM.

  CC     test/hciemu.o
test/hciemu.c: In function num_completed_pkts:
test/hciemu.c:429:4: error: cast increases required alignment of target
	 type [-Werror=cast-align]
test/hciemu.c:430:4: error: cast increases required alignment of target
	type [-Werror=cast-align]
cc1: all warnings being treated as errors
make[1]: *** [test/hciemu.o] Error 1
make: *** [all] Error 2

Change-Id: Ica73d897065bf1116ee8a7052c169a1a3ba25ff7
---
 test/hciemu.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/test/hciemu.c b/test/hciemu.c
index 4c62223..8e31dbb 100644
--- a/test/hciemu.c
+++ b/test/hciemu.c
@@ -426,8 +426,10 @@ static void num_completed_pkts(struct vhci_conn *conn)
 	np = (void *) ptr; ptr += EVT_NUM_COMP_PKTS_SIZE;
 	np->num_hndl = 1;
 
-	*((uint16_t *) ptr) = htobs(conn->handle); ptr += 2;
-	*((uint16_t *) ptr) = htobs(vdev.acl_cnt); ptr += 2;
+	bt_put_be16(conn->handle, ptr);
+	ptr += 2;
+	bt_put_be16(vdev.acl_cnt, ptr);
+	ptr += 2;
 
 	write_snoop(vdev.dd, HCI_EVENT_PKT, 1, buf, ptr - buf);
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [RFC 03/15] sdp: Use helper functions instead of bt_get_unaligned macro
  2012-08-31  8:36 ` [RFC 03/15] sdp: Use helper functions instead of bt_get_unaligned macro Szymon Janc
@ 2012-08-31 10:55   ` Anderson Lizardo
  2012-08-31 11:49     ` Szymon Janc
  2012-08-31 16:14   ` Marcel Holtmann
  1 sibling, 1 reply; 23+ messages in thread
From: Anderson Lizardo @ 2012-08-31 10:55 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth

Hi Szymon,

On Fri, Aug 31, 2012 at 4:36 AM, Szymon Janc <szymon.janc@tieto.com> wrote:
> This fix number of compilation errors on ARM similar to one below.
>
> lib/sdp.c: In function 'sdp_uuid_extract':
> lib/sdp.c:1019:27: error: cast increases required alignment
>         of target type [-Werror=cast-align]
> lib/sdp.c:1019:27: error: cast increases required alignment
>         of target type [-Werror=cast-align]
> lib/sdp.c:1026:27: error: cast increases required alignment
>         of target type [-Werror=cast-align]
> lib/sdp.c:1026:27: error: cast increases required alignment
>         of target type [-Werror=cast-align]
>
> Change-Id: I587fb99328d7e5b9053af81597bd48b3a4e610ad

You should remove the "Change-Id" tags from your patches.

>         case SDP_INT16:
> -               d->val.int16 = bt_get_unaligned((int16_t *) value);
> +               d->val.int16 = bt_get_16(value);
>                 d->unitSize += sizeof(int16_t);
>                 break;

Is this safe? You are assigning a unsigned type (return value from
bt_get_16()) to a signed one. Same applies for SDP_INT32 and SDP_INT64

> @@ -1427,7 +1427,7 @@ sdp_record_t *sdp_extract_pdu(const uint8_t *buf, int bufsize, int *scanned)
>                 }
>
>                 dtd = *(uint8_t *) p;
> -               attr = ntohs(bt_get_unaligned((uint16_t *) (p + n)));
> +               attr = bt_get_be16((p + n));

Extra parenthesis can be dropped.

Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 03/15] sdp: Use helper functions instead of bt_get_unaligned macro
  2012-08-31 10:55   ` Anderson Lizardo
@ 2012-08-31 11:49     ` Szymon Janc
  2012-08-31 12:12       ` Anderson Lizardo
  0 siblings, 1 reply; 23+ messages in thread
From: Szymon Janc @ 2012-08-31 11:49 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: linux-bluetooth@vger.kernel.org

On Friday 31 of August 2012 13:55:08 Anderson Lizardo wrote:
> Hi Szymon,

Hi Anderson,

> 
> On Fri, Aug 31, 2012 at 4:36 AM, Szymon Janc <szymon.janc@tieto.com> wrote:
> > This fix number of compilation errors on ARM similar to one below.
> >
> > lib/sdp.c: In function 'sdp_uuid_extract':
> > lib/sdp.c:1019:27: error: cast increases required alignment
> >         of target type [-Werror=cast-align]
> > lib/sdp.c:1019:27: error: cast increases required alignment
> >         of target type [-Werror=cast-align]
> > lib/sdp.c:1026:27: error: cast increases required alignment
> >         of target type [-Werror=cast-align]
> > lib/sdp.c:1026:27: error: cast increases required alignment
> >         of target type [-Werror=cast-align]
> >
> > Change-Id: I587fb99328d7e5b9053af81597bd48b3a4e610ad
> 
> You should remove the "Change-Id" tags from your patches.

Crap. Will remove it in V2 :)

> 
> >         case SDP_INT16:
> > -               d->val.int16 = bt_get_unaligned((int16_t *) value);
> > +               d->val.int16 = bt_get_16(value);
> >                 d->unitSize += sizeof(int16_t);
> >                 break;
> 
> Is this safe? You are assigning a unsigned type (return value from
> bt_get_16()) to a signed one. Same applies for SDP_INT32 and SDP_INT64

Mixing signed and unsigned could introduce some problem when you interpret data (e.g. in
comparison), but here value will be simply assigned.

I could add an explicit cast here (as this might bother some lint-like tools) if you prefer.

> 
> > @@ -1427,7 +1427,7 @@ sdp_record_t *sdp_extract_pdu(const uint8_t *buf, int bufsize, int *scanned)
> >                 }
> >
> >                 dtd = *(uint8_t *) p;
> > -               attr = ntohs(bt_get_unaligned((uint16_t *) (p + n)));
> > +               attr = bt_get_be16((p + n));
> 
> Extra parenthesis can be dropped.

Will fix.

> 
> Regards,
> 

-- 
BR
Szymon Janc

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 03/15] sdp: Use helper functions instead of bt_get_unaligned macro
  2012-08-31 11:49     ` Szymon Janc
@ 2012-08-31 12:12       ` Anderson Lizardo
  0 siblings, 0 replies; 23+ messages in thread
From: Anderson Lizardo @ 2012-08-31 12:12 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org

Hi Szymon,

On Fri, Aug 31, 2012 at 7:49 AM, Szymon Janc <szymon.janc@tieto.com> wrote:
>> >         case SDP_INT16:
>> > -               d->val.int16 = bt_get_unaligned((int16_t *) value);
>> > +               d->val.int16 = bt_get_16(value);
>> >                 d->unitSize += sizeof(int16_t);
>> >                 break;
>>
>> Is this safe? You are assigning a unsigned type (return value from
>> bt_get_16()) to a signed one. Same applies for SDP_INT32 and SDP_INT64
>
> Mixing signed and unsigned could introduce some problem when you interpret data (e.g. in
> comparison), but here value will be simply assigned.
>
> I could add an explicit cast here (as this might bother some lint-like tools) if you prefer.

I don't think it is necessary (I was just confirming if it was safe
and intentional), so I'll leave to those who will apply your patches
:)

Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 03/15] sdp: Use helper functions instead of bt_get_unaligned macro
  2012-08-31  8:36 ` [RFC 03/15] sdp: Use helper functions instead of bt_get_unaligned macro Szymon Janc
  2012-08-31 10:55   ` Anderson Lizardo
@ 2012-08-31 16:14   ` Marcel Holtmann
  2012-09-03  7:06     ` Szymon Janc
  1 sibling, 1 reply; 23+ messages in thread
From: Marcel Holtmann @ 2012-08-31 16:14 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth

Hi Szymon,

> This fix number of compilation errors on ARM similar to one below.
> 
> lib/sdp.c: In function 'sdp_uuid_extract':
> lib/sdp.c:1019:27: error: cast increases required alignment
> 	of target type [-Werror=cast-align]
> lib/sdp.c:1019:27: error: cast increases required alignment
> 	of target type [-Werror=cast-align]
> lib/sdp.c:1026:27: error: cast increases required alignment
> 	of target type [-Werror=cast-align]
> lib/sdp.c:1026:27: error: cast increases required alignment
> 	of target type [-Werror=cast-align]
> 
> Change-Id: I587fb99328d7e5b9053af81597bd48b3a4e610ad
> ---
>  lib/sdp.c |   56 ++++++++++++++++++++++++++++----------------------------
>  1 file changed, 28 insertions(+), 28 deletions(-)
> 
> diff --git a/lib/sdp.c b/lib/sdp.c
> index 36b4d08..d40500f 100644
> --- a/lib/sdp.c
> +++ b/lib/sdp.c
> @@ -376,27 +376,27 @@ sdp_data_t *sdp_data_alloc_with_length(uint8_t dtd, const void *value,
>  		d->unitSize += sizeof(int8_t);
>  		break;
>  	case SDP_UINT16:
> -		d->val.uint16 = bt_get_unaligned((uint16_t *) value);
> +		d->val.uint16 = bt_get_16(value);
>  		d->unitSize += sizeof(uint16_t);
>  		break;

I do not like this. Either we use be16 here and store it in the native
endian converted from the protocol endian or we leave it as it is.

I want clear get_le16 and get_be16 and not another get_16. That just
makes the code hard to read.

Regards

Marcel



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 03/15] sdp: Use helper functions instead of bt_get_unaligned macro
  2012-08-31 16:14   ` Marcel Holtmann
@ 2012-09-03  7:06     ` Szymon Janc
  2012-09-03  9:57       ` Johan Hedberg
  0 siblings, 1 reply; 23+ messages in thread
From: Szymon Janc @ 2012-09-03  7:06 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org

On Friday 31 of August 2012 19:14:36 Marcel Holtmann wrote:
> Hi Szymon,

Hi Marcel,

> 
> > This fix number of compilation errors on ARM similar to one below.
> > 
> > lib/sdp.c: In function 'sdp_uuid_extract':
> > lib/sdp.c:1019:27: error: cast increases required alignment
> > 	of target type [-Werror=cast-align]
> > lib/sdp.c:1019:27: error: cast increases required alignment
> > 	of target type [-Werror=cast-align]
> > lib/sdp.c:1026:27: error: cast increases required alignment
> > 	of target type [-Werror=cast-align]
> > lib/sdp.c:1026:27: error: cast increases required alignment
> > 	of target type [-Werror=cast-align]
> > 
> > Change-Id: I587fb99328d7e5b9053af81597bd48b3a4e610ad
> > ---
> >  lib/sdp.c |   56 ++++++++++++++++++++++++++++----------------------------
> >  1 file changed, 28 insertions(+), 28 deletions(-)
> > 
> > diff --git a/lib/sdp.c b/lib/sdp.c
> > index 36b4d08..d40500f 100644
> > --- a/lib/sdp.c
> > +++ b/lib/sdp.c
> > @@ -376,27 +376,27 @@ sdp_data_t *sdp_data_alloc_with_length(uint8_t dtd, const void *value,
> >  		d->unitSize += sizeof(int8_t);
> >  		break;
> >  	case SDP_UINT16:
> > -		d->val.uint16 = bt_get_unaligned((uint16_t *) value);
> > +		d->val.uint16 = bt_get_16(value);
> >  		d->unitSize += sizeof(uint16_t);
> >  		break;
> 
> I do not like this. Either we use be16 here and store it in the native
> endian converted from the protocol endian or we leave it as it is.
> 
> I want clear get_le16 and get_be16 and not another get_16. That just
> makes the code hard to read.

This data is already in protocol order, so bt_get_le/be can't be used here.
If you don't like those non-converting helpers functions other option is to
use memcpy.

> Regards
> 
> Marcel
> 

-- 
BR
Szymon Janc

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 03/15] sdp: Use helper functions instead of bt_get_unaligned macro
  2012-09-03  7:06     ` Szymon Janc
@ 2012-09-03  9:57       ` Johan Hedberg
  2012-09-03 10:28         ` Szymon Janc
  0 siblings, 1 reply; 23+ messages in thread
From: Johan Hedberg @ 2012-09-03  9:57 UTC (permalink / raw)
  To: Szymon Janc; +Cc: Marcel Holtmann, linux-bluetooth@vger.kernel.org

Hi Szymon,

On Mon, Sep 03, 2012, Szymon Janc wrote:
> > > This fix number of compilation errors on ARM similar to one below.
> > > 
> > > lib/sdp.c: In function 'sdp_uuid_extract':
> > > lib/sdp.c:1019:27: error: cast increases required alignment
> > > 	of target type [-Werror=cast-align]
> > > lib/sdp.c:1019:27: error: cast increases required alignment
> > > 	of target type [-Werror=cast-align]
> > > lib/sdp.c:1026:27: error: cast increases required alignment
> > > 	of target type [-Werror=cast-align]
> > > lib/sdp.c:1026:27: error: cast increases required alignment
> > > 	of target type [-Werror=cast-align]
> > > 
> > > Change-Id: I587fb99328d7e5b9053af81597bd48b3a4e610ad
> > > ---
> > >  lib/sdp.c |   56 ++++++++++++++++++++++++++++----------------------------
> > >  1 file changed, 28 insertions(+), 28 deletions(-)
> > > 
> > > diff --git a/lib/sdp.c b/lib/sdp.c
> > > index 36b4d08..d40500f 100644
> > > --- a/lib/sdp.c
> > > +++ b/lib/sdp.c
> > > @@ -376,27 +376,27 @@ sdp_data_t *sdp_data_alloc_with_length(uint8_t dtd, const void *value,
> > >  		d->unitSize += sizeof(int8_t);
> > >  		break;
> > >  	case SDP_UINT16:
> > > -		d->val.uint16 = bt_get_unaligned((uint16_t *) value);
> > > +		d->val.uint16 = bt_get_16(value);
> > >  		d->unitSize += sizeof(uint16_t);
> > >  		break;
> > 
> > I do not like this. Either we use be16 here and store it in the native
> > endian converted from the protocol endian or we leave it as it is.
> > 
> > I want clear get_le16 and get_be16 and not another get_16. That just
> > makes the code hard to read.
> 
> This data is already in protocol order, so bt_get_le/be can't be used here.
> If you don't like those non-converting helpers functions other option is to
> use memcpy.

I think the main point is that only PDU buffers used for writing/reading
to/from the connection should contain protocol data order, and anything
else (internal data structures and variables) should be in the host
order. However, since this is a library interface (one which hopefully
will be removed/replaced in the future though) such fixes might not
always be possible, making e.g. your memcpy proposal the best we can do.

Johan

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 03/15] sdp: Use helper functions instead of bt_get_unaligned macro
  2012-09-03  9:57       ` Johan Hedberg
@ 2012-09-03 10:28         ` Szymon Janc
  0 siblings, 0 replies; 23+ messages in thread
From: Szymon Janc @ 2012-09-03 10:28 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: Marcel Holtmann, linux-bluetooth@vger.kernel.org

On Monday 03 of September 2012 12:57:39 Johan Hedberg wrote:
> Hi Szymon,

Hi Johan,

> 
> On Mon, Sep 03, 2012, Szymon Janc wrote:
> > > > This fix number of compilation errors on ARM similar to one below.
> > > > 
> > > > lib/sdp.c: In function 'sdp_uuid_extract':
> > > > lib/sdp.c:1019:27: error: cast increases required alignment
> > > > 	of target type [-Werror=cast-align]
> > > > lib/sdp.c:1019:27: error: cast increases required alignment
> > > > 	of target type [-Werror=cast-align]
> > > > lib/sdp.c:1026:27: error: cast increases required alignment
> > > > 	of target type [-Werror=cast-align]
> > > > lib/sdp.c:1026:27: error: cast increases required alignment
> > > > 	of target type [-Werror=cast-align]
> > > > 
> > > > Change-Id: I587fb99328d7e5b9053af81597bd48b3a4e610ad
> > > > ---
> > > >  lib/sdp.c |   56 ++++++++++++++++++++++++++++----------------------------
> > > >  1 file changed, 28 insertions(+), 28 deletions(-)
> > > > 
> > > > diff --git a/lib/sdp.c b/lib/sdp.c
> > > > index 36b4d08..d40500f 100644
> > > > --- a/lib/sdp.c
> > > > +++ b/lib/sdp.c
> > > > @@ -376,27 +376,27 @@ sdp_data_t *sdp_data_alloc_with_length(uint8_t dtd, const void *value,
> > > >  		d->unitSize += sizeof(int8_t);
> > > >  		break;
> > > >  	case SDP_UINT16:
> > > > -		d->val.uint16 = bt_get_unaligned((uint16_t *) value);
> > > > +		d->val.uint16 = bt_get_16(value);
> > > >  		d->unitSize += sizeof(uint16_t);
> > > >  		break;
> > > 
> > > I do not like this. Either we use be16 here and store it in the native
> > > endian converted from the protocol endian or we leave it as it is.
> > > 
> > > I want clear get_le16 and get_be16 and not another get_16. That just
> > > makes the code hard to read.
> > 
> > This data is already in protocol order, so bt_get_le/be can't be used here.
> > If you don't like those non-converting helpers functions other option is to
> > use memcpy.
> 
> I think the main point is that only PDU buffers used for writing/reading
> to/from the connection should contain protocol data order, and anything
> else (internal data structures and variables) should be in the host
> order. However, since this is a library interface (one which hopefully
> will be removed/replaced in the future though) such fixes might not
> always be possible, making e.g. your memcpy proposal the best we can do.

OK, will prepare v4 series with memcpy.

> Johan

-- 
BR
Szymon Janc

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2012-09-03 10:28 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-31  8:36 [RFC 00/15] Unaligned memory access fixes Szymon Janc
2012-08-31  8:36 ` [RFC 01/15] lib: Add host order unaligned access functions Szymon Janc
2012-08-31  8:36 ` [RFC 02/15] sap-u8500: Fix compile error due to unaligned memory access Szymon Janc
2012-08-31  8:36 ` [RFC 03/15] sdp: Use helper functions instead of bt_get_unaligned macro Szymon Janc
2012-08-31 10:55   ` Anderson Lizardo
2012-08-31 11:49     ` Szymon Janc
2012-08-31 12:12       ` Anderson Lizardo
2012-08-31 16:14   ` Marcel Holtmann
2012-09-03  7:06     ` Szymon Janc
2012-09-03  9:57       ` Johan Hedberg
2012-09-03 10:28         ` Szymon Janc
2012-08-31  8:36 ` [RFC 04/15] Add helper functions for putting integers on unaligned memory address Szymon Janc
2012-08-31  8:36 ` [RFC 05/15] sdp: Fix compilation errors due to unaligned memory access Szymon Janc
2012-08-31  8:36 ` [RFC 06/15] l2test: " Szymon Janc
2012-08-31  8:36 ` [RFC 07/15] rctest: " Szymon Janc
2012-08-31  8:36 ` [RFC 08/15] monitor: " Szymon Janc
2012-08-31  8:36 ` [RFC 09/15] scotest: " Szymon Janc
2012-08-31  8:36 ` [RFC 10/15] avrcp: " Szymon Janc
2012-08-31  8:36 ` [RFC 11/15] sap: " Szymon Janc
2012-08-31  8:36 ` [RFC 12/15] adaptername: Refactor handle_inotify_cb Szymon Janc
2012-08-31  8:36 ` [RFC 13/15] sdpd-request: Fix build errors due to unaligned memory access Szymon Janc
2012-08-31  8:36 ` [RFC 14/15] sdpd-service: " Szymon Janc
2012-08-31  8:36 ` [RFC 15/15] hciemu: " Szymon Janc

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).