Linux bluetooth development
 help / color / mirror / Atom feed
* [RFC v3 5/5] android/hal-a2dp: Use generic IPC message handling for events
From: Szymon Janc @ 2013-11-15 20:49 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384548578-18710-1-git-send-email-szymon.janc@tieto.com>

Register handlers on service init and unregister on cleanup.
---
 android/hal-a2dp.c | 40 ++++++++++++++++++++--------------------
 android/hal.h      |  1 -
 2 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/android/hal-a2dp.c b/android/hal-a2dp.c
index e9fadb7..baa4b10 100644
--- a/android/hal-a2dp.c
+++ b/android/hal-a2dp.c
@@ -31,7 +31,7 @@ static bool interface_ready(void)
 	return cbs != NULL;
 }
 
-static void handle_conn_state(void *buf)
+static void handle_conn_state(void *buf, uint16_t len, int fd)
 {
 	struct hal_ev_a2dp_conn_state *ev = buf;
 
@@ -40,7 +40,7 @@ static void handle_conn_state(void *buf)
 						(bt_bdaddr_t *) (ev->bdaddr));
 }
 
-static void handle_audio_state(void *buf)
+static void handle_audio_state(void *buf, uint16_t len, int fd)
 {
 	struct hal_ev_a2dp_audio_state *ev = buf;
 
@@ -48,24 +48,19 @@ static void handle_audio_state(void *buf)
 		cbs->audio_state_cb(ev->state, (bt_bdaddr_t *)(ev->bdaddr));
 }
 
-/* will be called from notification thread context */
-void bt_notify_a2dp(uint8_t opcode, void *buf, uint16_t len)
-{
-	if (!interface_ready())
-		return;
-
-	switch (opcode) {
-	case HAL_EV_A2DP_CONN_STATE:
-		handle_conn_state(buf);
-		break;
-	case HAL_EV_A2DP_AUDIO_STATE:
-		handle_audio_state(buf);
-		break;
-	default:
-		DBG("Unhandled callback opcode=0x%x", opcode);
-		break;
-	}
-}
+/* handlers will be called from notification thread context */
+static const struct hal_ipc_handler ev_handlers[] = {
+	{
+		.handler = handle_conn_state,
+		.var_len = false,
+		.data_len = sizeof(struct hal_ev_pan_conn_state),
+	},
+	{
+		.handler = handle_audio_state,
+		.var_len = false,
+		.data_len = sizeof(struct hal_ev_pan_ctrl_state),
+	},
+};
 
 static bt_status_t a2dp_connect(bt_bdaddr_t *bd_addr)
 {
@@ -105,6 +100,9 @@ static bt_status_t init(btav_callbacks_t *callbacks)
 
 	cbs = callbacks;
 
+	hal_ipc_register(HAL_SERVICE_ID_A2DP, ev_handlers,
+				sizeof(ev_handlers)/sizeof(ev_handlers[0]));
+
 	cmd.service_id = HAL_SERVICE_ID_A2DP;
 
 	return hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
@@ -126,6 +124,8 @@ static void cleanup()
 
 	hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_UNREGISTER_MODULE,
 					sizeof(cmd), &cmd, 0, NULL, NULL);
+
+	hal_ipc_unregister(HAL_SERVICE_ID_A2DP);
 }
 
 static btav_interface_t iface = {
diff --git a/android/hal.h b/android/hal.h
index 6bd4c5a..b475411 100644
--- a/android/hal.h
+++ b/android/hal.h
@@ -28,4 +28,3 @@ btav_interface_t *bt_get_a2dp_interface(void);
 
 void bt_thread_associate(void);
 void bt_thread_disassociate(void);
-void bt_notify_a2dp(uint8_t opcode, void *buf, uint16_t len);
-- 
1.8.4.3


^ permalink raw reply related

* Re: [PATCH] Staging: btmtk_usb: Add hdev parameter to hdev->send driver callback
From: Greg Kroah-Hartman @ 2013-11-15 21:29 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Dan Carpenter, Geert Uytterhoeven, devel, Johan Hedberg,
	Yu-Chen Cho, linux-kernel@vger.kernel.org list,
	linux-bluetooth@vger.kernel.org development
In-Reply-To: <F615BA13-BEC1-457B-8002-BC6C149039EA@holtmann.org>

On Fri, Nov 15, 2013 at 10:26:41PM +0900, Marcel Holtmann wrote:
> Hi Dan,
> 
> >> while this is patch is correct, I do not really care about staging drivers that actually bluntly violate my copyright.
> >> 
> > 
> > That's very cryptic.
> > 
> > What is going on here?  I googled it and I wasn't able to find what you
> > are talking about.  Care to give us a hint and what you want us to do
> > here?
> 
> the last time I checked, the majority of drivers/bluetooth/btusb.c has been written by myself. Now go and compare btusb.c to btmtk_usb.[ch].
> 
> > I have also added Johan Hedberg to the CC list because he also helped
> > break the build.  Don't do that.
> 
> Yes, we are doing exactly that. It is a staging driver. I could not care less if a staging drivers breaks the build or not.
> 
> If anybody cares about this driver, then take the time to merge it upstream. It has never been submitted to linux-bluetooth mailing list.
> 
> There are drivers that should have never been merged into staging.
> This is one of them. Look for yourself and explain to me why this
> driver is part of staging in the first place.

Because it was sent to me by a developer?

Seriously, what's the issue here, I didn't notice it was a fork of your
code, sorry, I didn't check.  I figured it would be eventually cleaned
up properly and then it will be sent to linux-bluetooth for proper
merging.

Yu-Chen, what's the satus of getting this cleaned up "properly"?  You
haven't really done anything on this since I took the driver in May.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH] Staging: btmtk_usb: Add hdev parameter to hdev->send driver callback
From: Marcel Holtmann @ 2013-11-15 22:26 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Dan Carpenter, Geert Uytterhoeven, devel, Johan Hedberg,
	Yu-Chen Cho, linux-kernel@vger.kernel.org list,
	linux-bluetooth@vger.kernel.org development, jay.hung
In-Reply-To: <20131115212936.GC14897@kroah.com>

Hi Greg,

>>>> while this is patch is correct, I do not really care about staging drivers that actually bluntly violate my copyright.
>>>> 
>>> 
>>> That's very cryptic.
>>> 
>>> What is going on here?  I googled it and I wasn't able to find what you
>>> are talking about.  Care to give us a hint and what you want us to do
>>> here?
>> 
>> the last time I checked, the majority of drivers/bluetooth/btusb.c has been written by myself. Now go and compare btusb.c to btmtk_usb.[ch].
>> 
>>> I have also added Johan Hedberg to the CC list because he also helped
>>> break the build.  Don't do that.
>> 
>> Yes, we are doing exactly that. It is a staging driver. I could not care less if a staging drivers breaks the build or not.
>> 
>> If anybody cares about this driver, then take the time to merge it upstream. It has never been submitted to linux-bluetooth mailing list.
>> 
>> There are drivers that should have never been merged into staging.
>> This is one of them. Look for yourself and explain to me why this
>> driver is part of staging in the first place.
> 
> Because it was sent to me by a developer?

it is a problem when staging just becomes a dumping ground for drivers that the distributions find somewhere on the Internet or CD-ROMs. And then nobody has any intentions to clean up and integrate properly. This one did not even go through linux-bluetooth mailing list once. It was submitted right to staging. And then the submitter walked away.

> Seriously, what's the issue here, I didn't notice it was a fork of your
> code, sorry, I didn't check.  I figured it would be eventually cleaned
> up properly and then it will be sent to linux-bluetooth for proper
> merging.

Kernel subsystem maintainers should not be responsible for fixing staging drivers. I did not even know that this driver existed in staging. I remember you saying that we can just ignore staging. The group of people looking after staging will take care of drivers that break.

Actually I am taken personal offense when someone takes my code, removes my copyright and slaps their copyright notice on top of it. Yes, I am looking at you Mediatek. I feel completely in my right to say that I am not touching this driver or care if it breaks.

And of course there is the technical problem that this driver as it is should not exist in the first place. We do not need duplicated code. The only difference between btusb.c and btmtk_usb.c is the driver init for loading firmware and poking at vendor specific registers. The Bluetooth subsystem has a vendor specific driver setup stage for exactly this. That should be used instead of forking the driver.

> Yu-Chen, what's the satus of getting this cleaned up "properly"?  You
> haven't really done anything on this since I took the driver in May.

My comment above stands, distributions do not seem to care :(

The Realtek Bluetooth driver is the same mess. I rejected it for the same reasons, but so far nobody made the effort to clean it up and build it as mini-driver of btusb.c.

Only our Intel guys managed to get that one done properly for their hardware. So yes, it can be done. I am not talking about unicorns here ;)

Regards

Marcel


^ permalink raw reply

* Re: [PATCH] Staging: btmtk_usb: Add hdev parameter to hdev->send driver callback
From: Marcel Holtmann @ 2013-11-15 22:36 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Dan Carpenter, Geert Uytterhoeven, devel, Johan Hedberg,
	Yu-Chen Cho, linux-kernel@vger.kernel.org list,
	linux-bluetooth@vger.kernel.org development, jay.hung
In-Reply-To: <6C5AC1E9-D9A0-473A-B47A-345018DFAC3C@holtmann.org>

Hi Greg,

>>>>> while this is patch is correct, I do not really care about staging drivers that actually bluntly violate my copyright.
>>>>> 
>>>> 
>>>> That's very cryptic.
>>>> 
>>>> What is going on here?  I googled it and I wasn't able to find what you
>>>> are talking about.  Care to give us a hint and what you want us to do
>>>> here?
>>> 
>>> the last time I checked, the majority of drivers/bluetooth/btusb.c has been written by myself. Now go and compare btusb.c to btmtk_usb.[ch].
>>> 
>>>> I have also added Johan Hedberg to the CC list because he also helped
>>>> break the build.  Don't do that.
>>> 
>>> Yes, we are doing exactly that. It is a staging driver. I could not care less if a staging drivers breaks the build or not.
>>> 
>>> If anybody cares about this driver, then take the time to merge it upstream. It has never been submitted to linux-bluetooth mailing list.
>>> 
>>> There are drivers that should have never been merged into staging.
>>> This is one of them. Look for yourself and explain to me why this
>>> driver is part of staging in the first place.
>> 
>> Because it was sent to me by a developer?
> 
> it is a problem when staging just becomes a dumping ground for drivers that the distributions find somewhere on the Internet or CD-ROMs. And then nobody has any intentions to clean up and integrate properly. This one did not even go through linux-bluetooth mailing list once. It was submitted right to staging. And then the submitter walked away.

and if I quote the TODO file:

  TODO:
          - checkpatch.pl clean
          - determine if the driver should not be using a duplicate
            version of the usb-bluetooth interface code, but should
            be merged into the drivers/bluetooth/ directory and
            infrastructure instead.
          - review by the bluetooth developer community

  Please send any patches for this driver to Yu-Chen, Cho <acho@suse.com> and
  jay.hung@mediatek.com

So from the submission we can assume that the submitter knew that this was duplicated code. The code also never got submitted for review to linux-bluetooth. And now 6 month later, none of the TODO items have been actually worked on.

I do not know what your timeline is for removing drivers from staging, but this one seems to be a good candidate to get removed next.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH] Staging: btmtk_usb: Add hdev parameter to hdev->send driver callback
From: Greg Kroah-Hartman @ 2013-11-15 23:39 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Dan Carpenter, Geert Uytterhoeven, devel, Johan Hedberg,
	Yu-Chen Cho, linux-kernel@vger.kernel.org list,
	linux-bluetooth@vger.kernel.org development, jay.hung
In-Reply-To: <324E99F5-B9B7-4E56-BE91-42964AFF5BA8@holtmann.org>

On Sat, Nov 16, 2013 at 07:36:31AM +0900, Marcel Holtmann wrote:
> Hi Greg,
> 
> >>>>> while this is patch is correct, I do not really care about staging drivers that actually bluntly violate my copyright.
> >>>>> 
> >>>> 
> >>>> That's very cryptic.
> >>>> 
> >>>> What is going on here?  I googled it and I wasn't able to find what you
> >>>> are talking about.  Care to give us a hint and what you want us to do
> >>>> here?
> >>> 
> >>> the last time I checked, the majority of drivers/bluetooth/btusb.c has been written by myself. Now go and compare btusb.c to btmtk_usb.[ch].
> >>> 
> >>>> I have also added Johan Hedberg to the CC list because he also helped
> >>>> break the build.  Don't do that.
> >>> 
> >>> Yes, we are doing exactly that. It is a staging driver. I could not care less if a staging drivers breaks the build or not.
> >>> 
> >>> If anybody cares about this driver, then take the time to merge it upstream. It has never been submitted to linux-bluetooth mailing list.
> >>> 
> >>> There are drivers that should have never been merged into staging.
> >>> This is one of them. Look for yourself and explain to me why this
> >>> driver is part of staging in the first place.
> >> 
> >> Because it was sent to me by a developer?
> > 
> > it is a problem when staging just becomes a dumping ground for drivers that the distributions find somewhere on the Internet or CD-ROMs. And then nobody has any intentions to clean up and integrate properly. This one did not even go through linux-bluetooth mailing list once. It was submitted right to staging. And then the submitter walked away.
> 
> and if I quote the TODO file:
> 
>   TODO:
>           - checkpatch.pl clean
>           - determine if the driver should not be using a duplicate
>             version of the usb-bluetooth interface code, but should
>             be merged into the drivers/bluetooth/ directory and
>             infrastructure instead.
>           - review by the bluetooth developer community
> 
>   Please send any patches for this driver to Yu-Chen, Cho <acho@suse.com> and
>   jay.hung@mediatek.com
> 
> So from the submission we can assume that the submitter knew that this
> was duplicated code. The code also never got submitted for review to
> linux-bluetooth. And now 6 month later, none of the TODO items have
> been actually worked on.
> 
> I do not know what your timeline is for removing drivers from staging,
> but this one seems to be a good candidate to get removed next.

6 months without any active contribution to getting it cleaned up and
merged is the timeline.  Which this one fits, so yes, I will remove it
for 3.14, unless Jay or Cho is going to start doing work on this.

thanks,

greg k-h

^ permalink raw reply

* [PATCH 1/3] android: Fix sending remote device property if name is not present
From: Szymon Janc @ 2013-11-16 14:09 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This fix missing bdaddr to string convertion if name was NULL. This
was resulting in using undefined dst value.
---
 android/bluetooth.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 83f20e2..4439cc6 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -589,8 +589,10 @@ static void send_remote_device_name_prop(const bdaddr_t *bdaddr)
 
 	/* Use cached name or bdaddr string */
 	name = get_device_name(bdaddr);
-	if (!name)
+	if (!name) {
+		ba2str(bdaddr, dst);
 		name = dst;
+	}
 
 	ev_len = BASELEN_REMOTE_DEV_PROP + strlen(name);
 	ev = g_malloc0(ev_len);
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH 2/3] android: Fix sending invalid remote device property event
From: Szymon Janc @ 2013-11-16 14:09 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384610986-17990-1-git-send-email-szymon.janc@tieto.com>

Remote device property event has variable length, pass whole event
length to ipc_send, not only header.
---
 android/bluetooth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 4439cc6..6a7ba9d 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -605,7 +605,7 @@ static void send_remote_device_name_prop(const bdaddr_t *bdaddr)
 	memcpy(&ev->props[0].val, name, strlen(name));
 
 	ipc_send(notification_sk, HAL_SERVICE_ID_BLUETOOTH,
-			HAL_EV_REMOTE_DEVICE_PROPS, sizeof(ev), ev, -1);
+			HAL_EV_REMOTE_DEVICE_PROPS, ev_len, ev, -1);
 
 	g_free(ev);
 }
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH 3/3] android: Refactor update_found_device function
From: Szymon Janc @ 2013-11-16 14:09 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384610986-17990-1-git-send-email-szymon.janc@tieto.com>

This makes function flow easier to follow and understand. Besides that
it also fix issue with sending to many bytes if some prop were not
present in EIR.
---
 android/bluetooth.c | 141 +++++++++++++++++++++-------------------------------
 1 file changed, 57 insertions(+), 84 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 6a7ba9d..5c5c61e 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -776,54 +776,32 @@ static void confirm_device_name(const bdaddr_t *addr, uint8_t addr_type)
 		error("Failed to send confirm name request");
 }
 
-static int fill_device_props(struct hal_property *prop, bdaddr_t *addr,
-					uint32_t cod, int8_t rssi, char *name)
-{
-	uint8_t num_props = 0;
-
-	/* fill Class of Device */
-	if (cod) {
-		prop->type = HAL_PROP_DEVICE_CLASS;
-		prop->len = sizeof(cod);
-		memcpy(prop->val, &cod, prop->len);
-		prop = ((void *) prop) + sizeof(*prop) + sizeof(cod);
-		num_props++;
-	}
 
-	/* fill RSSI */
-	if (rssi) {
-		prop->type = HAL_PROP_DEVICE_RSSI;
-		prop->len = sizeof(rssi);
-		memcpy(prop->val, &rssi, prop->len);
-		prop = ((void *) prop) + sizeof(*prop) + sizeof(rssi);
-		num_props++;
-	}
+static int fill_hal_prop(void *buf, uint8_t type, uint16_t len,
+							const void *val)
+{
+	struct hal_property *prop = buf;
 
-	/* fill name */
-	if (name) {
-		prop->type = HAL_PROP_DEVICE_NAME;
-		prop->len = strlen(name);
-		memcpy(prop->val, name, prop->len);
-		prop = ((void *) prop) + sizeof(*prop) + prop->len;
-		num_props++;
-	}
+	prop->type = type;
+	prop->len = len;
+	memcpy(prop->val, val, len);
 
-	return num_props;
+	return sizeof(*prop) + len;
 }
 
 static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,
 					int8_t rssi, bool confirm,
 					const uint8_t *data, uint8_t data_len)
 {
-	bool is_new_dev = false;
-	size_t props_size = 0;
-	size_t buff_size = 0;
-	void *buf;
+	uint8_t buf[BLUEZ_HAL_MTU];
+	bool new_dev = false;
 	struct eir_data eir;
-	GSList *l;
-	bdaddr_t *remote = NULL;
+	uint8_t *num_prop;
+	uint8_t opcode;
+	int size = 0;
 	int err;
 
+	memset(buf, 0, sizeof(buf));
 	memset(&eir, 0, sizeof(eir));
 
 	err = eir_parse(&eir, data, data_len);
@@ -832,75 +810,70 @@ static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,
 		return;
 	}
 
-	l = g_slist_find_custom(found_devices, bdaddr, bdaddr_cmp);
-	if (l)
-		remote = l->data;
-
-	if (!remote) {
+	if (!g_slist_find_custom(found_devices, bdaddr, bdaddr_cmp)) {
+		bdaddr_t *new_bdaddr;
 		char addr[18];
 
-		remote = g_new0(bdaddr_t, 1);
-		bacpy(remote, bdaddr);
+		new_bdaddr = g_new0(bdaddr_t, 1);
+		bacpy(new_bdaddr, bdaddr);
 
-		found_devices = g_slist_prepend(found_devices, remote);
-		is_new_dev = true;
+		found_devices = g_slist_prepend(found_devices, new_bdaddr);
 
-		ba2str(remote, addr);
+		ba2str(new_bdaddr, addr);
 		DBG("New device found: %s", addr);
-	}
 
-	props_size += sizeof(struct hal_property) + sizeof(eir.class);
-	props_size += sizeof(struct hal_property) + sizeof(rssi);
-
-	if (eir.name) {
-		props_size += sizeof(struct hal_property) + strlen(eir.name);
-		cache_device_name(remote, eir.name);
+		new_dev = true;
 	}
 
-	if (is_new_dev) {
-		struct hal_ev_device_found *ev = NULL;
-		struct hal_property *prop = NULL;
-
-		/* with new device we also send bdaddr prop */
-		props_size += sizeof(struct hal_property) + sizeof(eir.addr);
+	if (new_dev) {
+		struct hal_ev_device_found *ev = (void *) buf;
+		bdaddr_t android_bdaddr;
 
-		buff_size = sizeof(struct hal_ev_device_found) + props_size;
-		buf = g_new0(char, buff_size);
-		ev = buf;
-		prop = ev->props;
+		size += sizeof(*ev);
 
-		/* fill first prop with bdaddr */
-		prop->type = HAL_PROP_DEVICE_ADDR;
-		prop->len = sizeof(bdaddr_t);
-		bdaddr2android(bdaddr, prop->val);
-		prop = ((void *) prop) + sizeof(*prop) + sizeof(bdaddr_t);
-		ev->num_props += 1;
+		num_prop = &ev->num_props;
+		opcode = HAL_EV_DEVICE_FOUND;
 
-		/* fill eir, name, and cod props */
-		ev->num_props += fill_device_props(prop, remote, eir.class,
-								rssi, eir.name);
+		bdaddr2android(bdaddr, &android_bdaddr);
 
-		ipc_send(notification_sk, HAL_SERVICE_ID_BLUETOOTH,
-				HAL_EV_DEVICE_FOUND, buff_size, ev, -1);
-		g_free(buf);
+		size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_ADDR,
+				sizeof(android_bdaddr), &android_bdaddr);
+		(*num_prop)++;
 	} else {
-		struct hal_ev_remote_device_props *ev = NULL;
+		struct hal_ev_remote_device_props *ev = (void *) buf;
 
-		buff_size = sizeof(*ev) + props_size;
-		buf = g_new0(char, buff_size);
-		ev = buf;
+		size += sizeof(*ev);
 
-		ev->num_props = fill_device_props(ev->props, remote, eir.class,
-								rssi, eir.name);
+		num_prop = &ev->num_props;
+		opcode = HAL_EV_REMOTE_DEVICE_PROPS;
 
 		ev->status = HAL_STATUS_SUCCESS;
 		bdaddr2android(bdaddr, ev->bdaddr);
+	}
+
+	if (eir.class) {
+		size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_CLASS,
+						sizeof(eir.class), &eir.class);
+		(*num_prop)++;
+	}
 
-		ipc_send(notification_sk, HAL_SERVICE_ID_BLUETOOTH,
-				HAL_EV_REMOTE_DEVICE_PROPS, buff_size, ev, -1);
-		g_free(buf);
+	if (rssi) {
+		size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_RSSI,
+							sizeof(rssi), &rssi);
+		(*num_prop)++;
+	}
+
+	if (eir.name) {
+		cache_device_name(bdaddr, eir.name);
+
+		size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_NAME,
+						strlen(eir.name), eir.name);
+		(*num_prop)++;
 	}
 
+	ipc_send(notification_sk, HAL_SERVICE_ID_BLUETOOTH, opcode, size, buf,
+									-1);
+
 	if (confirm) {
 		char addr[18];
 
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH 0/5] android: IPC improvements
From: Szymon Janc @ 2013-11-16 14:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

Hi,

Changes since RFCv3:
 - use correct email for commit author
 - no longer RFC - as per my testing this works fine and already
   start catching up IPC bugs which would not be introduced with those
   patches present

-- 
BR
Szymon Janc

Szymon Janc (5):
  android/hal: Add initial code for IPC message handlers
  android/hal-bluetooth: Register IPC message handlers
  android/hal-hidhost: Use generic IPC message handling for events
  android/hal-pan: Use generic IPC message handling for events
  android/hal-a2dp: Use generic IPC message handling for events

 android/hal-a2dp.c      |  40 ++++-----
 android/hal-bluetooth.c | 223 ++++++++++++++++++++++++++++++------------------
 android/hal-hidhost.c   |  75 +++++++++-------
 android/hal-ipc.c       | 115 ++++++++++++++++---------
 android/hal-ipc.h       |  10 +++
 android/hal-pan.c       |  39 ++++-----
 android/hal.h           |   4 -
 7 files changed, 312 insertions(+), 194 deletions(-)

-- 
1.8.4.2


^ permalink raw reply

* [PATCH 1/5] android/hal: Add initial code for IPC message handlers
From: Szymon Janc @ 2013-11-16 14:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384611208-18536-1-git-send-email-szymon.janc@tieto.com>

This will allow to register and unregister handlers for IPC messages
Basic sanity check will be done in common code. Commands with variable
length will be verified against minimum size only.
---
 android/hal-ipc.c | 115 ++++++++++++++++++++++++++++++++++++------------------
 android/hal-ipc.h |  10 +++++
 2 files changed, 87 insertions(+), 38 deletions(-)

diff --git a/android/hal-ipc.c b/android/hal-ipc.c
index 026e245..53af897 100644
--- a/android/hal-ipc.c
+++ b/android/hal-ipc.c
@@ -43,26 +43,84 @@ static pthread_mutex_t cmd_sk_mutex = PTHREAD_MUTEX_INITIALIZER;
 
 static pthread_t notif_th = 0;
 
-static void notification_dispatch(struct hal_hdr *msg, int fd)
+struct service_handler {
+	const struct hal_ipc_handler *handler;
+	uint8_t size;
+};
+
+static struct service_handler services[HAL_SERVICE_ID_MAX + 1];
+
+void hal_ipc_register(uint8_t service, const struct hal_ipc_handler *handlers,
+								uint8_t size)
+{
+	services[service].handler = handlers;
+	services[service].size = size;
+}
+
+void hal_ipc_unregister(uint8_t service)
 {
-	switch (msg->service_id) {
-	case HAL_SERVICE_ID_BLUETOOTH:
-		bt_notify_adapter(msg->opcode, msg->payload, msg->len);
-		break;
-	case HAL_SERVICE_ID_HIDHOST:
-		bt_notify_hidhost(msg->opcode, msg->payload, msg->len);
-		break;
-	case HAL_SERVICE_ID_A2DP:
-		bt_notify_a2dp(msg->opcode, msg->payload, msg->len);
-		break;
-	case HAL_SERVICE_ID_PAN:
-		bt_notify_pan(msg->opcode, msg->payload, msg->len);
-		break;
-	default:
-		DBG("Unhandled notification service=%d opcode=0x%x",
+	services[service].handler = NULL;
+	services[service].size = 0;
+}
+
+static void handle_msg(void *buf, ssize_t len, int fd)
+{
+	struct hal_hdr *msg = buf;
+	const struct hal_ipc_handler *handler;
+	uint8_t opcode;
+
+	if (len < (ssize_t) sizeof(*msg)) {
+		error("IPC: message too small (%zd bytes), aborting", len);
+		exit(EXIT_FAILURE);
+	}
+
+	if (len != (ssize_t) (sizeof(*msg) + msg->len)) {
+		error("IPC: message malformed (%zd bytes), aborting", len);
+		exit(EXIT_FAILURE);
+	}
+
+	/* if service is valid */
+	if (msg->service_id > HAL_SERVICE_ID_MAX) {
+		error("IPC: unknown service (0x%x), aborting",
+							msg->service_id);
+		exit(EXIT_FAILURE);
+	}
+
+	/* if service is registered */
+	if (!services[msg->service_id].handler) {
+		error("IPC: unregistered service (0x%x), aborting",
+							msg->service_id);
+		exit(EXIT_FAILURE);
+	}
+
+	/* if opcode fit valid range */
+	if (msg->opcode < HAL_MINIMUM_EVENT) {
+		error("IPC: invalid opcode for service 0x%x (0x%x), aborting",
 						msg->service_id, msg->opcode);
-		break;
+		exit(EXIT_FAILURE);
+	}
+
+	opcode = msg->opcode - HAL_MINIMUM_EVENT;
+
+	/* if opcode is valid */
+	if (opcode >= services[msg->service_id].size) {
+		error("IPC: invalid opcode for service 0x%x (0x%x), aborting",
+						msg->service_id, msg->opcode);
+		exit(EXIT_FAILURE);
+	}
+
+	handler = &services[msg->service_id].handler[opcode];
+
+	/* if payload size is valid */
+	if ((handler->var_len && handler->data_len > msg->len) ||
+			(!handler->var_len && handler->data_len != msg->len)) {
+		error("IPC: message size invalid for service 0x%x opcode 0x%x "
+				"(%u bytes), aborting",
+				msg->service_id, msg->opcode, msg->len);
+		exit(EXIT_FAILURE);
 	}
+
+	handler->handler(msg->payload, msg->len, fd);
 }
 
 static void *notification_handler(void *data)
@@ -72,7 +130,6 @@ static void *notification_handler(void *data)
 	struct cmsghdr *cmsg;
 	char cmsgbuf[CMSG_SPACE(sizeof(int))];
 	char buf[BLUEZ_HAL_MTU];
-	struct hal_hdr *ev = (void *) buf;
 	ssize_t ret;
 	int fd;
 
@@ -83,7 +140,7 @@ static void *notification_handler(void *data)
 		memset(buf, 0, sizeof(buf));
 		memset(cmsgbuf, 0, sizeof(cmsgbuf));
 
-		iv.iov_base = ev;
+		iv.iov_base = buf;
 		iv.iov_len = sizeof(buf);
 
 		msg.msg_iov = &iv;
@@ -108,24 +165,6 @@ static void *notification_handler(void *data)
 			exit(EXIT_FAILURE);
 		}
 
-		if (ret < (ssize_t) sizeof(*ev)) {
-			error("Too small notification (%zd bytes), aborting",
-									ret);
-			exit(EXIT_FAILURE);
-		}
-
-		if (ev->opcode < HAL_MINIMUM_EVENT) {
-			error("Invalid notification (0x%x), aborting",
-							ev->opcode);
-			exit(EXIT_FAILURE);
-		}
-
-		if (ret != (ssize_t) (sizeof(*ev) + ev->len)) {
-			error("Malformed notification(%zd bytes), aborting",
-									ret);
-			exit(EXIT_FAILURE);
-		}
-
 		fd = -1;
 
 		/* Receive auxiliary data in msg */
@@ -138,7 +177,7 @@ static void *notification_handler(void *data)
 			}
 		}
 
-		notification_dispatch(ev, fd);
+		handle_msg(buf, ret, fd);
 	}
 
 	close(notif_sk);
diff --git a/android/hal-ipc.h b/android/hal-ipc.h
index ea53e1c..9f867f8 100644
--- a/android/hal-ipc.h
+++ b/android/hal-ipc.h
@@ -15,8 +15,18 @@
  *
  */
 
+struct hal_ipc_handler {
+	void (*handler) (void *buf, uint16_t len, int fd);
+	bool var_len;
+	size_t data_len;
+};
+
 bool hal_ipc_init(void);
 void hal_ipc_cleanup(void);
 
 int hal_ipc_cmd(uint8_t service_id, uint8_t opcode, uint16_t len, void *param,
 					size_t *rsp_len, void *rsp, int *fd);
+
+void hal_ipc_register(uint8_t service, const struct hal_ipc_handler *handlers,
+								uint8_t size);
+void hal_ipc_unregister(uint8_t service);
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH 2/5] android/hal-bluetooth: Register IPC message handlers
From: Szymon Janc @ 2013-11-16 14:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384611208-18536-1-git-send-email-szymon.janc@tieto.com>

Register handlers on service init. Since this requires all handlers to
be registered (unknown opcode is considered IPC error) missing handlers
stubs are provided.
---
 android/hal-bluetooth.c | 223 ++++++++++++++++++++++++++++++------------------
 android/hal.h           |   1 -
 2 files changed, 142 insertions(+), 82 deletions(-)

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 078d537..2ec7c10 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -35,7 +35,7 @@ static const bt_callbacks_t *bt_hal_cbacks = NULL;
 	*pe = *((uint8_t *) (hal_prop->val)); \
 } while (0)
 
-static void handle_adapter_state_changed(void *buf)
+static void handle_adapter_state_changed(void *buf, uint16_t len, int fd)
 {
 	struct hal_ev_adapter_state_changed *ev = buf;
 
@@ -45,38 +45,35 @@ static void handle_adapter_state_changed(void *buf)
 		bt_hal_cbacks->adapter_state_changed_cb(ev->state);
 }
 
-static void adapter_props_to_hal(bt_property_t *send_props,
-					struct hal_property *hal_prop,
-					uint8_t num_props, void *buff_end)
+static void adapter_props_to_hal(bt_property_t *send_props, void *buf,
+							uint8_t num_props)
 {
-	void *p = hal_prop;
+	struct hal_property *prop = buf;
 	uint8_t i;
 
 	for (i = 0; i < num_props; i++) {
-		if (p + sizeof(*hal_prop) + hal_prop->len > buff_end) {
-			error("invalid adapter properties event, aborting");
-			exit(EXIT_FAILURE);
-		}
-
-		send_props[i].type = hal_prop->type;
+		send_props[i].type = prop->type;
 
-		switch (hal_prop->type) {
+		switch (prop->type) {
 		case HAL_PROP_ADAPTER_TYPE:
-			create_enum_prop(send_props[i], hal_prop,
+			create_enum_prop(send_props[i], prop,
 							bt_device_type_t);
 			break;
 		case HAL_PROP_ADAPTER_SCAN_MODE:
-			create_enum_prop(send_props[i], hal_prop,
+			create_enum_prop(send_props[i], prop,
 							bt_scan_mode_t);
 			break;
 		case HAL_PROP_ADAPTER_SERVICE_REC:
 		default:
-			send_props[i].len = hal_prop->len;
-			send_props[i].val = hal_prop->val;
+			send_props[i].len = prop->len;
+			send_props[i].val = prop->val;
 			break;
 		}
 
 		DBG("prop[%d]: %s", i, btproperty2str(&send_props[i]));
+
+		buf += sizeof(*prop) + prop->len;
+		prop = buf;
 	}
 }
 
@@ -96,36 +93,30 @@ static void adapter_hal_props_cleanup(bt_property_t *props, uint8_t num)
 	}
 }
 
-static void device_props_to_hal(bt_property_t *send_props,
-					struct hal_property *hal_prop,
-					uint8_t num_props, void *buff_end)
+static void device_props_to_hal(bt_property_t *send_props, void *buf,
+							uint8_t num_props)
 {
-	void *p = hal_prop;
+	struct hal_property *prop = buf;
 	uint8_t i;
 
 	for (i = 0; i < num_props; i++) {
-		if (p + sizeof(*hal_prop) + hal_prop->len > buff_end) {
-			error("invalid adapter properties event, aborting");
-			exit(EXIT_FAILURE);
-		}
+		send_props[i].type = prop->type;
 
-		send_props[i].type = hal_prop->type;
-
-		switch (hal_prop->type) {
+		switch (prop->type) {
 		case HAL_PROP_DEVICE_TYPE:
-			create_enum_prop(send_props[i], hal_prop,
+			create_enum_prop(send_props[i], prop,
 							bt_device_type_t);
 			break;
 		case HAL_PROP_DEVICE_SERVICE_REC:
 		case HAL_PROP_DEVICE_VERSION_INFO:
 		default:
-			send_props[i].len = hal_prop->len;
-			send_props[i].val = hal_prop->val;
+			send_props[i].len = prop->len;
+			send_props[i].val = prop->val;
 			break;
 		}
 
-		p += sizeof(*hal_prop) + hal_prop->len;
-		hal_prop = p;
+		buf += sizeof(*prop) + prop->len;
+		prop = buf;
 
 		DBG("prop[%d]: %s", i, btproperty2str(&send_props[i]));
 	}
@@ -147,24 +138,48 @@ static void device_hal_props_cleanup(bt_property_t *props, uint8_t num)
 	}
 }
 
-static void handle_adapter_props_changed(void *buf, uint16_t len)
+static void check_props(int num, const struct hal_property *prop, uint16_t len)
+{
+	int i;
+
+	for (i = 0; i < num; i++) {
+		if (sizeof(*prop) + prop->len > len) {
+			error("invalid properties (%zu > %u), aborting",
+					sizeof(*prop) + prop->len, len);
+			exit(EXIT_FAILURE);
+		}
+
+		len -= sizeof(*prop) + prop->len;
+		prop = ((void *) prop) + sizeof(*prop) + prop->len;
+	}
+
+	if (!len)
+		return;
+
+	error("invalid properties length (%u bytes left), aborting", len);
+	exit(EXIT_FAILURE);
+}
+
+static void handle_adapter_props_changed(void *buf, uint16_t len, int fd)
 {
 	struct hal_ev_adapter_props_changed *ev = buf;
 	bt_property_t props[ev->num_props];
 
 	DBG("");
 
+	check_props(ev->num_props, ev->props, len - sizeof(*ev));
+
 	if (!bt_hal_cbacks->adapter_properties_cb)
 		return;
 
-	adapter_props_to_hal(props, ev->props, ev->num_props, buf + len);
+	adapter_props_to_hal(props, ev->props, ev->num_props);
 
 	bt_hal_cbacks->adapter_properties_cb(ev->status, ev->num_props, props);
 
 	adapter_hal_props_cleanup(props, ev->num_props);
 }
 
-static void handle_bond_state_change(void *buf)
+static void handle_bond_state_change(void *buf, uint16_t len, int fd)
 {
 	struct hal_ev_bond_state_changed *ev = buf;
 	bt_bdaddr_t *addr = (bt_bdaddr_t *) ev->bdaddr;
@@ -176,7 +191,7 @@ static void handle_bond_state_change(void *buf)
 								ev->state);
 }
 
-static void handle_pin_request(void *buf)
+static void handle_pin_request(void *buf, uint16_t len, int fd)
 {
 	struct hal_ev_pin_request *ev = buf;
 	/* Those are declared as packed, so it's safe to assign pointers */
@@ -189,7 +204,7 @@ static void handle_pin_request(void *buf)
 		bt_hal_cbacks->pin_request_cb(addr, name, ev->class_of_dev);
 }
 
-static void handle_ssp_request(void *buf)
+static void handle_ssp_request(void *buf, uint16_t len, int fd)
 {
 	struct hal_ev_ssp_request *ev = buf;
 	/* Those are declared as packed, so it's safe to assign pointers */
@@ -221,7 +236,7 @@ static bool interface_ready(void)
 	return bt_hal_cbacks != NULL;
 }
 
-static void handle_discovery_state_changed(void *buf)
+static void handle_discovery_state_changed(void *buf, uint16_t len, int fd)
 {
 	struct hal_ev_discovery_state_changed *ev = buf;
 
@@ -231,34 +246,38 @@ static void handle_discovery_state_changed(void *buf)
 		bt_hal_cbacks->discovery_state_changed_cb(ev->state);
 }
 
-static void handle_device_found(void *buf, uint16_t len)
+static void handle_device_found(void *buf, uint16_t len, int fd)
 {
 	struct hal_ev_device_found *ev = buf;
 	bt_property_t props[ev->num_props];
 
 	DBG("");
 
+	check_props(ev->num_props, ev->props, len - sizeof(*ev));
+
 	if (!bt_hal_cbacks->device_found_cb)
 		return;
 
-	device_props_to_hal(props, ev->props, ev->num_props, buf + len);
+	device_props_to_hal(props, ev->props, ev->num_props);
 
 	bt_hal_cbacks->device_found_cb(ev->num_props, props);
 
 	device_hal_props_cleanup(props, ev->num_props);
 }
 
-static void handle_device_state_changed(void *buf, uint16_t len)
+static void handle_device_state_changed(void *buf, uint16_t len, int fd)
 {
 	struct hal_ev_remote_device_props *ev = buf;
 	bt_property_t props[ev->num_props];
 
 	DBG("");
 
+	check_props(ev->num_props, ev->props, len - sizeof(*ev));
+
 	if (!bt_hal_cbacks->remote_device_properties_cb)
 		return;
 
-	device_props_to_hal(props, ev->props, ev->num_props, buf + len);
+	device_props_to_hal(props, ev->props, ev->num_props);
 
 	bt_hal_cbacks->remote_device_properties_cb(ev->status,
 						(bt_bdaddr_t *)ev->bdaddr,
@@ -267,7 +286,7 @@ static void handle_device_state_changed(void *buf, uint16_t len)
 	device_hal_props_cleanup(props, ev->num_props);
 }
 
-static void handle_acl_state_changed(void *buf)
+static void handle_acl_state_changed(void *buf, uint16_t len, int fd)
 {
 	struct hal_ev_acl_state_changed *ev = buf;
 	bt_bdaddr_t *addr = (bt_bdaddr_t *) ev->bdaddr;
@@ -279,48 +298,82 @@ static void handle_acl_state_changed(void *buf)
 								ev->state);
 }
 
-/* will be called from notification thread context */
-void bt_notify_adapter(uint8_t opcode, void *buf, uint16_t len)
+static void handle_dut_mode_receive(void *buf, uint16_t len, int fd)
 {
-	if (!interface_ready())
-		return;
-
-	DBG("opcode 0x%x", opcode);
+	DBG("");
 
-	switch (opcode) {
-	case HAL_EV_ADAPTER_STATE_CHANGED:
-		handle_adapter_state_changed(buf);
-		break;
-	case HAL_EV_ADAPTER_PROPS_CHANGED:
-		handle_adapter_props_changed(buf, len);
-		break;
-	case HAL_EV_DISCOVERY_STATE_CHANGED:
-		handle_discovery_state_changed(buf);
-		break;
-	case HAL_EV_DEVICE_FOUND:
-		handle_device_found(buf, len);
-		break;
-	case HAL_EV_REMOTE_DEVICE_PROPS:
-		handle_device_state_changed(buf, len);
-		break;
-	case HAL_EV_BOND_STATE_CHANGED:
-		handle_bond_state_change(buf);
-		break;
-	case HAL_EV_PIN_REQUEST:
-		handle_pin_request(buf);
-		break;
-	case HAL_EV_SSP_REQUEST:
-		handle_ssp_request(buf);
-		break;
-	case HAL_EV_ACL_STATE_CHANGED:
-		handle_acl_state_changed(buf);
-		break;
-	default:
-		DBG("Unhandled callback opcode=0x%x", opcode);
-		break;
-	}
+	/* TODO */
 }
 
+static void handle_le_test_mode(void *buf, uint16_t len, int fd)
+{
+	DBG("");
+
+	/* TODO */
+}
+
+/* handlers will be called from notification thread context */
+static const struct hal_ipc_handler ev_handlers[] = {
+	{
+		.handler = handle_adapter_state_changed,
+		.var_len = false,
+		.data_len = sizeof(struct hal_ev_adapter_state_changed)
+	},
+	{
+		.handler = handle_adapter_props_changed,
+		.var_len = true,
+		.data_len = sizeof(struct hal_ev_adapter_props_changed) +
+						sizeof(struct hal_property),
+	},
+	{
+		.handler = handle_device_state_changed,
+		.var_len = true,
+		.data_len = sizeof(struct hal_ev_remote_device_props) +
+						sizeof(struct hal_property),
+	},
+	{
+		.handler = handle_device_found,
+		.var_len = true,
+		.data_len = sizeof(struct hal_ev_device_found) +
+						sizeof(struct hal_property),
+	},
+	{
+		.handler = handle_discovery_state_changed,
+		.var_len = false,
+		.data_len = sizeof(struct hal_ev_discovery_state_changed),
+	},
+	{
+		.handler = handle_pin_request,
+		.var_len = false,
+		.data_len = sizeof(struct hal_ev_pin_request),
+	},
+	{
+		.handler = handle_ssp_request,
+		.var_len = false,
+		.data_len = sizeof(handle_ssp_request),
+	},
+	{
+		.handler = handle_bond_state_change,
+		.var_len = false,
+		.data_len = sizeof(struct hal_ev_bond_state_changed),
+	},
+	{
+		.handler = handle_acl_state_changed,
+		.var_len = false,
+		.data_len = sizeof(struct hal_ev_acl_state_changed),
+	},
+	{
+		.handler = handle_dut_mode_receive,
+		.var_len = true,
+		.data_len = sizeof(struct hal_ev_dut_mode_receive),
+	},
+	{
+		.handler = handle_le_test_mode,
+		.var_len = false,
+		.data_len = sizeof(struct hal_ev_le_test_mode),
+	},
+};
+
 static int init(bt_callbacks_t *callbacks)
 {
 	struct hal_cmd_register_module cmd;
@@ -333,6 +386,9 @@ static int init(bt_callbacks_t *callbacks)
 
 	bt_hal_cbacks = callbacks;
 
+	hal_ipc_register(HAL_SERVICE_ID_BLUETOOTH, ev_handlers,
+				sizeof(ev_handlers)/sizeof(ev_handlers[0]));
+
 	if (!hal_ipc_init()) {
 		bt_hal_cbacks = NULL;
 		return BT_STATUS_FAIL;
@@ -361,6 +417,9 @@ static int init(bt_callbacks_t *callbacks)
 fail:
 	hal_ipc_cleanup();
 	bt_hal_cbacks = NULL;
+
+	hal_ipc_unregister(HAL_SERVICE_ID_BLUETOOTH);
+
 	return status;
 }
 
@@ -396,6 +455,8 @@ static void cleanup(void)
 	hal_ipc_cleanup();
 
 	bt_hal_cbacks = NULL;
+
+	hal_ipc_unregister(HAL_SERVICE_ID_BLUETOOTH);
 }
 
 static int get_adapter_properties(void)
diff --git a/android/hal.h b/android/hal.h
index 72090fe..67dad5d 100644
--- a/android/hal.h
+++ b/android/hal.h
@@ -26,7 +26,6 @@ bthh_interface_t *bt_get_hidhost_interface(void);
 btpan_interface_t *bt_get_pan_interface(void);
 btav_interface_t *bt_get_a2dp_interface(void);
 
-void bt_notify_adapter(uint8_t opcode, void *buf, uint16_t len);
 void bt_thread_associate(void);
 void bt_thread_disassociate(void);
 void bt_notify_hidhost(uint8_t opcode, void *buf, uint16_t len);
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH 3/5] android/hal-hidhost: Use generic IPC message handling for events
From: Szymon Janc @ 2013-11-16 14:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384611208-18536-1-git-send-email-szymon.janc@tieto.com>

Register handlers on service init and unregister on cleanup.
---
 android/hal-hidhost.c | 75 +++++++++++++++++++++++++++++----------------------
 android/hal.h         |  1 -
 2 files changed, 43 insertions(+), 33 deletions(-)

diff --git a/android/hal-hidhost.c b/android/hal-hidhost.c
index 2ce17a3..27ce492 100644
--- a/android/hal-hidhost.c
+++ b/android/hal-hidhost.c
@@ -32,7 +32,7 @@ static bool interface_ready(void)
 	return cbacks != NULL;
 }
 
-static void handle_conn_state(void *buf)
+static void handle_conn_state(void *buf, uint16_t len, int fd)
 {
 	struct hal_ev_hidhost_conn_state *ev = buf;
 
@@ -41,7 +41,7 @@ static void handle_conn_state(void *buf)
 								ev->state);
 }
 
-static void handle_info(void *buf)
+static void handle_info(void *buf, uint16_t len, int fd)
 {
 	struct hal_ev_hidhost_info *ev = buf;
 	bthh_hid_info_t info;
@@ -60,7 +60,7 @@ static void handle_info(void *buf)
 		cbacks->hid_info_cb((bt_bdaddr_t *) ev->bdaddr, info);
 }
 
-static void handle_proto_mode(void *buf)
+static void handle_proto_mode(void *buf, uint16_t len, int fd)
 {
 	struct hal_ev_hidhost_proto_mode *ev = buf;
 
@@ -69,16 +69,21 @@ static void handle_proto_mode(void *buf)
 							ev->status, ev->mode);
 }
 
-static void handle_get_report(void *buf)
+static void handle_get_report(void *buf, uint16_t len, int fd)
 {
 	struct hal_ev_hidhost_get_report *ev = buf;
 
+	if (len != sizeof(*ev) + ev->len) {
+		error("invalid get report event, aborting");
+		exit(EXIT_FAILURE);
+	}
+
 	if (cbacks->get_report_cb)
 		cbacks->get_report_cb((bt_bdaddr_t *) ev->bdaddr, ev->status,
 							ev->data, ev->len);
 }
 
-static void handle_virtual_unplug(void *buf)
+static void handle_virtual_unplug(void *buf, uint16_t len, int fd)
 {
 	struct hal_ev_hidhost_virtual_unplug *ev = buf;
 
@@ -87,33 +92,34 @@ static void handle_virtual_unplug(void *buf)
 								ev->status);
 }
 
-/* will be called from notification thread context */
-void bt_notify_hidhost(uint8_t opcode, void *buf, uint16_t len)
-{
-	if (!interface_ready())
-		return;
-
-	switch (opcode) {
-	case HAL_EV_HIDHOST_CONN_STATE:
-		handle_conn_state(buf);
-		break;
-	case HAL_EV_HIDHOST_INFO:
-		handle_info(buf);
-		break;
-	case HAL_EV_HIDHOST_PROTO_MODE:
-		handle_proto_mode(buf);
-		break;
-	case HAL_EV_HIDHOST_GET_REPORT:
-		handle_get_report(buf);
-		break;
-	case HAL_EV_HIDHOST_VIRTUAL_UNPLUG:
-		handle_virtual_unplug(buf);
-		break;
-	default:
-		DBG("Unhandled callback opcode=0x%x", opcode);
-		break;
-	}
-}
+/* handlers will be called from notification thread context */
+static const struct hal_ipc_handler ev_handlers[] = {
+	{
+		.handler = handle_conn_state,
+		.var_len = false,
+		.data_len = sizeof(struct hal_ev_hidhost_conn_state)
+	},
+	{
+		.handler = handle_info,
+		.var_len = false,
+		.data_len = sizeof(struct hal_ev_hidhost_info),
+	},
+	{
+		.handler = handle_proto_mode,
+		.var_len = false,
+		.data_len = sizeof(struct hal_ev_hidhost_proto_mode),
+	},
+	{
+		.handler = handle_get_report,
+		.var_len = true,
+		.data_len = sizeof(struct hal_ev_hidhost_get_report),
+	},
+	{
+		.handler = handle_virtual_unplug,
+		.var_len = false,
+		.data_len = sizeof(struct hal_ev_hidhost_virtual_unplug),
+	},
+};
 
 static bt_status_t hidhost_connect(bt_bdaddr_t *bd_addr)
 {
@@ -362,6 +368,9 @@ static bt_status_t init(bthh_callbacks_t *callbacks)
 	/* store reference to user callbacks */
 	cbacks = callbacks;
 
+	hal_ipc_register(HAL_SERVICE_ID_HIDHOST, ev_handlers,
+				sizeof(ev_handlers)/sizeof(ev_handlers[0]));
+
 	cmd.service_id = HAL_SERVICE_ID_HIDHOST;
 
 	return hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
@@ -383,6 +392,8 @@ static void cleanup(void)
 
 	hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_UNREGISTER_MODULE,
 					sizeof(cmd), &cmd, 0, NULL, NULL);
+
+	hal_ipc_unregister(HAL_SERVICE_ID_HIDHOST);
 }
 
 static bthh_interface_t hidhost_if = {
diff --git a/android/hal.h b/android/hal.h
index 67dad5d..58426a9 100644
--- a/android/hal.h
+++ b/android/hal.h
@@ -28,6 +28,5 @@ btav_interface_t *bt_get_a2dp_interface(void);
 
 void bt_thread_associate(void);
 void bt_thread_disassociate(void);
-void bt_notify_hidhost(uint8_t opcode, void *buf, uint16_t len);
 void bt_notify_a2dp(uint8_t opcode, void *buf, uint16_t len);
 void bt_notify_pan(uint8_t opcode, void *buf, uint16_t len);
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH 4/5] android/hal-pan: Use generic IPC message handling for events
From: Szymon Janc @ 2013-11-16 14:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384611208-18536-1-git-send-email-szymon.janc@tieto.com>

Register handlers on service init and unregister on cleanup.
---
 android/hal-pan.c | 39 ++++++++++++++++++++-------------------
 android/hal.h     |  1 -
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/android/hal-pan.c b/android/hal-pan.c
index 2bc560e..dff2a44 100644
--- a/android/hal-pan.c
+++ b/android/hal-pan.c
@@ -31,7 +31,7 @@ static bool interface_ready(void)
 	return cbs != NULL;
 }
 
-static void handle_conn_state(void *buf)
+static void handle_conn_state(void *buf, uint16_t len, int fd)
 {
 	struct hal_ev_pan_conn_state *ev = buf;
 
@@ -41,7 +41,7 @@ static void handle_conn_state(void *buf)
 					ev->local_role, ev->remote_role);
 }
 
-static void handle_ctrl_state(void *buf)
+static void handle_ctrl_state(void *buf, uint16_t len, int fd)
 {
 	struct hal_ev_pan_ctrl_state *ev = buf;
 
@@ -50,23 +50,19 @@ static void handle_ctrl_state(void *buf)
 					ev->local_role, (char *)ev->name);
 }
 
-void bt_notify_pan(uint8_t opcode, void *buf, uint16_t len)
-{
-	if (!interface_ready())
-		return;
-
-	switch (opcode) {
-	case HAL_EV_PAN_CONN_STATE:
-		handle_conn_state(buf);
-		break;
-	case HAL_EV_PAN_CTRL_STATE:
-		handle_ctrl_state(buf);
-		break;
-	default:
-		DBG("Unhandled callback opcode=0x%x", opcode);
-		break;
-	}
-}
+/* handlers will be called from notification thread context */
+static const struct hal_ipc_handler ev_handlers[] = {
+	{
+		.handler = handle_conn_state,
+		.var_len = false,
+		.data_len = sizeof(struct hal_ev_pan_conn_state),
+	},
+	{
+		.handler = handle_ctrl_state,
+		.var_len = false,
+		.data_len = sizeof(struct hal_ev_pan_ctrl_state),
+	},
+};
 
 static bt_status_t pan_enable(int local_role)
 {
@@ -143,6 +139,9 @@ static bt_status_t pan_init(const btpan_callbacks_t *callbacks)
 
 	cbs = callbacks;
 
+	hal_ipc_register(HAL_SERVICE_ID_PAN, ev_handlers,
+				sizeof(ev_handlers)/sizeof(ev_handlers[0]));
+
 	cmd.service_id = HAL_SERVICE_ID_PAN;
 
 	return hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
@@ -164,6 +163,8 @@ static void pan_cleanup()
 
 	hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_UNREGISTER_MODULE,
 					sizeof(cmd), &cmd, 0, NULL, NULL);
+
+	hal_ipc_unregister(HAL_SERVICE_ID_PAN);
 }
 
 static btpan_interface_t pan_if = {
diff --git a/android/hal.h b/android/hal.h
index 58426a9..6bd4c5a 100644
--- a/android/hal.h
+++ b/android/hal.h
@@ -29,4 +29,3 @@ btav_interface_t *bt_get_a2dp_interface(void);
 void bt_thread_associate(void);
 void bt_thread_disassociate(void);
 void bt_notify_a2dp(uint8_t opcode, void *buf, uint16_t len);
-void bt_notify_pan(uint8_t opcode, void *buf, uint16_t len);
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH 5/5] android/hal-a2dp: Use generic IPC message handling for events
From: Szymon Janc @ 2013-11-16 14:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1384611208-18536-1-git-send-email-szymon.janc@tieto.com>

Register handlers on service init and unregister on cleanup.
---
 android/hal-a2dp.c | 40 ++++++++++++++++++++--------------------
 android/hal.h      |  1 -
 2 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/android/hal-a2dp.c b/android/hal-a2dp.c
index e9fadb7..baa4b10 100644
--- a/android/hal-a2dp.c
+++ b/android/hal-a2dp.c
@@ -31,7 +31,7 @@ static bool interface_ready(void)
 	return cbs != NULL;
 }
 
-static void handle_conn_state(void *buf)
+static void handle_conn_state(void *buf, uint16_t len, int fd)
 {
 	struct hal_ev_a2dp_conn_state *ev = buf;
 
@@ -40,7 +40,7 @@ static void handle_conn_state(void *buf)
 						(bt_bdaddr_t *) (ev->bdaddr));
 }
 
-static void handle_audio_state(void *buf)
+static void handle_audio_state(void *buf, uint16_t len, int fd)
 {
 	struct hal_ev_a2dp_audio_state *ev = buf;
 
@@ -48,24 +48,19 @@ static void handle_audio_state(void *buf)
 		cbs->audio_state_cb(ev->state, (bt_bdaddr_t *)(ev->bdaddr));
 }
 
-/* will be called from notification thread context */
-void bt_notify_a2dp(uint8_t opcode, void *buf, uint16_t len)
-{
-	if (!interface_ready())
-		return;
-
-	switch (opcode) {
-	case HAL_EV_A2DP_CONN_STATE:
-		handle_conn_state(buf);
-		break;
-	case HAL_EV_A2DP_AUDIO_STATE:
-		handle_audio_state(buf);
-		break;
-	default:
-		DBG("Unhandled callback opcode=0x%x", opcode);
-		break;
-	}
-}
+/* handlers will be called from notification thread context */
+static const struct hal_ipc_handler ev_handlers[] = {
+	{
+		.handler = handle_conn_state,
+		.var_len = false,
+		.data_len = sizeof(struct hal_ev_pan_conn_state),
+	},
+	{
+		.handler = handle_audio_state,
+		.var_len = false,
+		.data_len = sizeof(struct hal_ev_pan_ctrl_state),
+	},
+};
 
 static bt_status_t a2dp_connect(bt_bdaddr_t *bd_addr)
 {
@@ -105,6 +100,9 @@ static bt_status_t init(btav_callbacks_t *callbacks)
 
 	cbs = callbacks;
 
+	hal_ipc_register(HAL_SERVICE_ID_A2DP, ev_handlers,
+				sizeof(ev_handlers)/sizeof(ev_handlers[0]));
+
 	cmd.service_id = HAL_SERVICE_ID_A2DP;
 
 	return hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
@@ -126,6 +124,8 @@ static void cleanup()
 
 	hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_UNREGISTER_MODULE,
 					sizeof(cmd), &cmd, 0, NULL, NULL);
+
+	hal_ipc_unregister(HAL_SERVICE_ID_A2DP);
 }
 
 static btav_interface_t iface = {
diff --git a/android/hal.h b/android/hal.h
index 6bd4c5a..b475411 100644
--- a/android/hal.h
+++ b/android/hal.h
@@ -28,4 +28,3 @@ btav_interface_t *bt_get_a2dp_interface(void);
 
 void bt_thread_associate(void);
 void bt_thread_disassociate(void);
-void bt_notify_a2dp(uint8_t opcode, void *buf, uint16_t len);
-- 
1.8.4.2


^ permalink raw reply related

* Re: [PATCH 1/3] android: Fix sending remote device property if name is not present
From: Johan Hedberg @ 2013-11-16 18:23 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1384610986-17990-1-git-send-email-szymon.janc@tieto.com>

Hi Szymon,

On Sat, Nov 16, 2013, Szymon Janc wrote:
> This fix missing bdaddr to string convertion if name was NULL. This
> was resulting in using undefined dst value.
> ---
>  android/bluetooth.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

All three patches have been applied. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH 2/5] android/hal-bluetooth: Register IPC message handlers
From: Andrei Emeltchenko @ 2013-11-17 19:39 UTC (permalink / raw)
  To: Szymon Janc; +Cc: Bluetooth Linux
In-Reply-To: <1384611208-18536-3-git-send-email-szymon.janc@tieto.com>

Hi Szymon,

On Sat, Nov 16, 2013 at 4:13 PM, Szymon Janc <szymon.janc@tieto.com> wrote:
>
> Register handlers on service init. Since this requires all handlers to
> be registered (unknown opcode is considered IPC error) missing handlers
> stubs are provided.
> ---
>  android/hal-bluetooth.c | 223 ++++++++++++++++++++++++++++++------------------
>  android/hal.h           |   1 -
>  2 files changed, 142 insertions(+), 82 deletions(-)
>
> diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
> index 078d537..2ec7c10 100644
> --- a/android/hal-bluetooth.c
> +++ b/android/hal-bluetooth.c
> @@ -35,7 +35,7 @@ static const bt_callbacks_t *bt_hal_cbacks = NULL;
>         *pe = *((uint8_t *) (hal_prop->val)); \
>  } while (0)
>
> -static void handle_adapter_state_changed(void *buf)
> +static void handle_adapter_state_changed(void *buf, uint16_t len, int fd)

Why do we need fd here and in all other places? Who uses it?

Regards,
Andrei

>  {
>         struct hal_ev_adapter_state_changed *ev = buf;
>
> @@ -45,38 +45,35 @@ static void handle_adapter_state_changed(void *buf)
>                 bt_hal_cbacks->adapter_state_changed_cb(ev->state);
>  }
>
> -static void adapter_props_to_hal(bt_property_t *send_props,
> -                                       struct hal_property *hal_prop,
> -                                       uint8_t num_props, void *buff_end)
> +static void adapter_props_to_hal(bt_property_t *send_props, void *buf,
> +                                                       uint8_t num_props)
>  {
> -       void *p = hal_prop;
> +       struct hal_property *prop = buf;
>         uint8_t i;
>
>         for (i = 0; i < num_props; i++) {
> -               if (p + sizeof(*hal_prop) + hal_prop->len > buff_end) {
> -                       error("invalid adapter properties event, aborting");
> -                       exit(EXIT_FAILURE);
> -               }
> -
> -               send_props[i].type = hal_prop->type;
> +               send_props[i].type = prop->type;
>
> -               switch (hal_prop->type) {
> +               switch (prop->type) {
>                 case HAL_PROP_ADAPTER_TYPE:
> -                       create_enum_prop(send_props[i], hal_prop,
> +                       create_enum_prop(send_props[i], prop,
>                                                         bt_device_type_t);
>                         break;
>                 case HAL_PROP_ADAPTER_SCAN_MODE:
> -                       create_enum_prop(send_props[i], hal_prop,
> +                       create_enum_prop(send_props[i], prop,
>                                                         bt_scan_mode_t);
>                         break;
>                 case HAL_PROP_ADAPTER_SERVICE_REC:
>                 default:
> -                       send_props[i].len = hal_prop->len;
> -                       send_props[i].val = hal_prop->val;
> +                       send_props[i].len = prop->len;
> +                       send_props[i].val = prop->val;
>                         break;
>                 }
>
>                 DBG("prop[%d]: %s", i, btproperty2str(&send_props[i]));
> +
> +               buf += sizeof(*prop) + prop->len;
> +               prop = buf;
>         }
>  }
>
> @@ -96,36 +93,30 @@ static void adapter_hal_props_cleanup(bt_property_t *props, uint8_t num)
>         }
>  }
>
> -static void device_props_to_hal(bt_property_t *send_props,
> -                                       struct hal_property *hal_prop,
> -                                       uint8_t num_props, void *buff_end)
> +static void device_props_to_hal(bt_property_t *send_props, void *buf,
> +                                                       uint8_t num_props)
>  {
> -       void *p = hal_prop;
> +       struct hal_property *prop = buf;
>         uint8_t i;
>
>         for (i = 0; i < num_props; i++) {
> -               if (p + sizeof(*hal_prop) + hal_prop->len > buff_end) {
> -                       error("invalid adapter properties event, aborting");
> -                       exit(EXIT_FAILURE);
> -               }
> +               send_props[i].type = prop->type;
>
> -               send_props[i].type = hal_prop->type;
> -
> -               switch (hal_prop->type) {
> +               switch (prop->type) {
>                 case HAL_PROP_DEVICE_TYPE:
> -                       create_enum_prop(send_props[i], hal_prop,
> +                       create_enum_prop(send_props[i], prop,
>                                                         bt_device_type_t);
>                         break;
>                 case HAL_PROP_DEVICE_SERVICE_REC:
>                 case HAL_PROP_DEVICE_VERSION_INFO:
>                 default:
> -                       send_props[i].len = hal_prop->len;
> -                       send_props[i].val = hal_prop->val;
> +                       send_props[i].len = prop->len;
> +                       send_props[i].val = prop->val;
>                         break;
>                 }
>
> -               p += sizeof(*hal_prop) + hal_prop->len;
> -               hal_prop = p;
> +               buf += sizeof(*prop) + prop->len;
> +               prop = buf;
>
>                 DBG("prop[%d]: %s", i, btproperty2str(&send_props[i]));
>         }
> @@ -147,24 +138,48 @@ static void device_hal_props_cleanup(bt_property_t *props, uint8_t num)
>         }
>  }
>
> -static void handle_adapter_props_changed(void *buf, uint16_t len)
> +static void check_props(int num, const struct hal_property *prop, uint16_t len)
> +{
> +       int i;
> +
> +       for (i = 0; i < num; i++) {
> +               if (sizeof(*prop) + prop->len > len) {
> +                       error("invalid properties (%zu > %u), aborting",
> +                                       sizeof(*prop) + prop->len, len);
> +                       exit(EXIT_FAILURE);
> +               }
> +
> +               len -= sizeof(*prop) + prop->len;
> +               prop = ((void *) prop) + sizeof(*prop) + prop->len;
> +       }
> +
> +       if (!len)
> +               return;
> +
> +       error("invalid properties length (%u bytes left), aborting", len);
> +       exit(EXIT_FAILURE);
> +}
> +
> +static void handle_adapter_props_changed(void *buf, uint16_t len, int fd)
>  {
>         struct hal_ev_adapter_props_changed *ev = buf;
>         bt_property_t props[ev->num_props];
>
>         DBG("");
>
> +       check_props(ev->num_props, ev->props, len - sizeof(*ev));
> +
>         if (!bt_hal_cbacks->adapter_properties_cb)
>                 return;
>
> -       adapter_props_to_hal(props, ev->props, ev->num_props, buf + len);
> +       adapter_props_to_hal(props, ev->props, ev->num_props);
>
>         bt_hal_cbacks->adapter_properties_cb(ev->status, ev->num_props, props);
>
>         adapter_hal_props_cleanup(props, ev->num_props);
>  }
>
> -static void handle_bond_state_change(void *buf)
> +static void handle_bond_state_change(void *buf, uint16_t len, int fd)
>  {
>         struct hal_ev_bond_state_changed *ev = buf;
>         bt_bdaddr_t *addr = (bt_bdaddr_t *) ev->bdaddr;
> @@ -176,7 +191,7 @@ static void handle_bond_state_change(void *buf)
>                                                                 ev->state);
>  }
>
> -static void handle_pin_request(void *buf)
> +static void handle_pin_request(void *buf, uint16_t len, int fd)
>  {
>         struct hal_ev_pin_request *ev = buf;
>         /* Those are declared as packed, so it's safe to assign pointers */
> @@ -189,7 +204,7 @@ static void handle_pin_request(void *buf)
>                 bt_hal_cbacks->pin_request_cb(addr, name, ev->class_of_dev);
>  }
>
> -static void handle_ssp_request(void *buf)
> +static void handle_ssp_request(void *buf, uint16_t len, int fd)
>  {
>         struct hal_ev_ssp_request *ev = buf;
>         /* Those are declared as packed, so it's safe to assign pointers */
> @@ -221,7 +236,7 @@ static bool interface_ready(void)
>         return bt_hal_cbacks != NULL;
>  }
>
> -static void handle_discovery_state_changed(void *buf)
> +static void handle_discovery_state_changed(void *buf, uint16_t len, int fd)
>  {
>         struct hal_ev_discovery_state_changed *ev = buf;
>
> @@ -231,34 +246,38 @@ static void handle_discovery_state_changed(void *buf)
>                 bt_hal_cbacks->discovery_state_changed_cb(ev->state);
>  }
>
> -static void handle_device_found(void *buf, uint16_t len)
> +static void handle_device_found(void *buf, uint16_t len, int fd)
>  {
>         struct hal_ev_device_found *ev = buf;
>         bt_property_t props[ev->num_props];
>
>         DBG("");
>
> +       check_props(ev->num_props, ev->props, len - sizeof(*ev));
> +
>         if (!bt_hal_cbacks->device_found_cb)
>                 return;
>
> -       device_props_to_hal(props, ev->props, ev->num_props, buf + len);
> +       device_props_to_hal(props, ev->props, ev->num_props);
>
>         bt_hal_cbacks->device_found_cb(ev->num_props, props);
>
>         device_hal_props_cleanup(props, ev->num_props);
>  }
>
> -static void handle_device_state_changed(void *buf, uint16_t len)
> +static void handle_device_state_changed(void *buf, uint16_t len, int fd)
>  {
>         struct hal_ev_remote_device_props *ev = buf;
>         bt_property_t props[ev->num_props];
>
>         DBG("");
>
> +       check_props(ev->num_props, ev->props, len - sizeof(*ev));
> +
>         if (!bt_hal_cbacks->remote_device_properties_cb)
>                 return;
>
> -       device_props_to_hal(props, ev->props, ev->num_props, buf + len);
> +       device_props_to_hal(props, ev->props, ev->num_props);
>
>         bt_hal_cbacks->remote_device_properties_cb(ev->status,
>                                                 (bt_bdaddr_t *)ev->bdaddr,
> @@ -267,7 +286,7 @@ static void handle_device_state_changed(void *buf, uint16_t len)
>         device_hal_props_cleanup(props, ev->num_props);
>  }
>
> -static void handle_acl_state_changed(void *buf)
> +static void handle_acl_state_changed(void *buf, uint16_t len, int fd)
>  {
>         struct hal_ev_acl_state_changed *ev = buf;
>         bt_bdaddr_t *addr = (bt_bdaddr_t *) ev->bdaddr;
> @@ -279,48 +298,82 @@ static void handle_acl_state_changed(void *buf)
>                                                                 ev->state);
>  }
>
> -/* will be called from notification thread context */
> -void bt_notify_adapter(uint8_t opcode, void *buf, uint16_t len)
> +static void handle_dut_mode_receive(void *buf, uint16_t len, int fd)
>  {
> -       if (!interface_ready())
> -               return;
> -
> -       DBG("opcode 0x%x", opcode);
> +       DBG("");
>
> -       switch (opcode) {
> -       case HAL_EV_ADAPTER_STATE_CHANGED:
> -               handle_adapter_state_changed(buf);
> -               break;
> -       case HAL_EV_ADAPTER_PROPS_CHANGED:
> -               handle_adapter_props_changed(buf, len);
> -               break;
> -       case HAL_EV_DISCOVERY_STATE_CHANGED:
> -               handle_discovery_state_changed(buf);
> -               break;
> -       case HAL_EV_DEVICE_FOUND:
> -               handle_device_found(buf, len);
> -               break;
> -       case HAL_EV_REMOTE_DEVICE_PROPS:
> -               handle_device_state_changed(buf, len);
> -               break;
> -       case HAL_EV_BOND_STATE_CHANGED:
> -               handle_bond_state_change(buf);
> -               break;
> -       case HAL_EV_PIN_REQUEST:
> -               handle_pin_request(buf);
> -               break;
> -       case HAL_EV_SSP_REQUEST:
> -               handle_ssp_request(buf);
> -               break;
> -       case HAL_EV_ACL_STATE_CHANGED:
> -               handle_acl_state_changed(buf);
> -               break;
> -       default:
> -               DBG("Unhandled callback opcode=0x%x", opcode);
> -               break;
> -       }
> +       /* TODO */
>  }
>
> +static void handle_le_test_mode(void *buf, uint16_t len, int fd)
> +{
> +       DBG("");
> +
> +       /* TODO */
> +}
> +
> +/* handlers will be called from notification thread context */
> +static const struct hal_ipc_handler ev_handlers[] = {
> +       {
> +               .handler = handle_adapter_state_changed,
> +               .var_len = false,
> +               .data_len = sizeof(struct hal_ev_adapter_state_changed)
> +       },
> +       {
> +               .handler = handle_adapter_props_changed,
> +               .var_len = true,
> +               .data_len = sizeof(struct hal_ev_adapter_props_changed) +
> +                                               sizeof(struct hal_property),
> +       },
> +       {
> +               .handler = handle_device_state_changed,
> +               .var_len = true,
> +               .data_len = sizeof(struct hal_ev_remote_device_props) +
> +                                               sizeof(struct hal_property),
> +       },
> +       {
> +               .handler = handle_device_found,
> +               .var_len = true,
> +               .data_len = sizeof(struct hal_ev_device_found) +
> +                                               sizeof(struct hal_property),
> +       },
> +       {
> +               .handler = handle_discovery_state_changed,
> +               .var_len = false,
> +               .data_len = sizeof(struct hal_ev_discovery_state_changed),
> +       },
> +       {
> +               .handler = handle_pin_request,
> +               .var_len = false,
> +               .data_len = sizeof(struct hal_ev_pin_request),
> +       },
> +       {
> +               .handler = handle_ssp_request,
> +               .var_len = false,
> +               .data_len = sizeof(handle_ssp_request),
> +       },
> +       {
> +               .handler = handle_bond_state_change,
> +               .var_len = false,
> +               .data_len = sizeof(struct hal_ev_bond_state_changed),
> +       },
> +       {
> +               .handler = handle_acl_state_changed,
> +               .var_len = false,
> +               .data_len = sizeof(struct hal_ev_acl_state_changed),
> +       },
> +       {
> +               .handler = handle_dut_mode_receive,
> +               .var_len = true,
> +               .data_len = sizeof(struct hal_ev_dut_mode_receive),
> +       },
> +       {
> +               .handler = handle_le_test_mode,
> +               .var_len = false,
> +               .data_len = sizeof(struct hal_ev_le_test_mode),
> +       },
> +};
> +
>  static int init(bt_callbacks_t *callbacks)
>  {
>         struct hal_cmd_register_module cmd;
> @@ -333,6 +386,9 @@ static int init(bt_callbacks_t *callbacks)
>
>         bt_hal_cbacks = callbacks;
>
> +       hal_ipc_register(HAL_SERVICE_ID_BLUETOOTH, ev_handlers,
> +                               sizeof(ev_handlers)/sizeof(ev_handlers[0]));
> +
>         if (!hal_ipc_init()) {
>                 bt_hal_cbacks = NULL;
>                 return BT_STATUS_FAIL;
> @@ -361,6 +417,9 @@ static int init(bt_callbacks_t *callbacks)
>  fail:
>         hal_ipc_cleanup();
>         bt_hal_cbacks = NULL;
> +
> +       hal_ipc_unregister(HAL_SERVICE_ID_BLUETOOTH);
> +
>         return status;
>  }
>
> @@ -396,6 +455,8 @@ static void cleanup(void)
>         hal_ipc_cleanup();
>
>         bt_hal_cbacks = NULL;
> +
> +       hal_ipc_unregister(HAL_SERVICE_ID_BLUETOOTH);
>  }
>
>  static int get_adapter_properties(void)
> diff --git a/android/hal.h b/android/hal.h
> index 72090fe..67dad5d 100644
> --- a/android/hal.h
> +++ b/android/hal.h
> @@ -26,7 +26,6 @@ bthh_interface_t *bt_get_hidhost_interface(void);
>  btpan_interface_t *bt_get_pan_interface(void);
>  btav_interface_t *bt_get_a2dp_interface(void);
>
> -void bt_notify_adapter(uint8_t opcode, void *buf, uint16_t len);
>  void bt_thread_associate(void);
>  void bt_thread_disassociate(void);
>  void bt_notify_hidhost(uint8_t opcode, void *buf, uint16_t len);
> --
> 1.8.4.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* BUG: unable to handle kernel paging request at 00000015832a8e23 RIP  [<ffffffff81a57c30>] rfcomm_sock_getsockopt+0x140/0x240
From: Sander Eikelenboom @ 2013-11-17 19:47 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Gustavo Padovan, linux-bluetooth

Hi,

This 3.13 merge window seems to have introduced the following bluetooth regression for me:

[   62.040810] BUG: unable to handle kernel paging request at 00000015832a8e23
[   62.049196] IP: [<ffffffff81a57c30>] rfcomm_sock_getsockopt+0x140/0x240
[   62.057800] PGD 0 
[   62.064574] Oops: 0000 [#1] PREEMPT SMP 
[   62.070653] Modules linked in:
[   62.076596] CPU: 4 PID: 5575 Comm: bluetoothd Not tainted 3.12.0-mw-20131117+ #1
[   62.082708] Hardware name: MSI MS-7640/890FXA-GD70 (MS-7640)  , BIOS V1.8B1 09/13/2010
[   62.088827] task: ffff88005863a180 ti: ffff880057688000 task.ti: ffff880057688000
[   62.095020] RIP: e030:[<ffffffff81a57c30>]  [<ffffffff81a57c30>] rfcomm_sock_getsockopt+0x140/0x240
[   62.101372] RSP: e02b:ffff880057689ec8  EFLAGS: 00010246
[   62.107668] RAX: 00000015832a8e23 RBX: 0000000000000003 RCX: 00007fff610ca0e8
[   62.113957] RDX: 0000000000000003 RSI: 0000000000000012 RDI: ffff8800572a9e00
[   62.120174] RBP: ffff880057689f18 R08: 00007fff610ca0ec R09: 00007fff610ca360
[   62.126481] R10: 00007fff610ca0e8 R11: 0000000000000202 R12: ffff8800569c3000
[   62.132872] R13: 00007fff610ca0e8 R14: 0000000000000000 R15: 00007fff610ca0ec
[   62.139187] FS:  00007f0d0b348720(0000) GS:ffff88005f700000(0000) knlGS:0000000000000000
[   62.145551] CS:  e033 DS: 0000 ES: 0000 CR0: 000000008005003b
[   62.151919] CR2: 00000015832a8e23 CR3: 0000000056f53000 CR4: 0000000000000660
[   62.158338] Stack:
[   62.164798]  ffff880057689f18 ffffffff81a5799f 0015832a8e23001f ffff880057689f40
[   62.171410]  ffff880057689f44 ffff8800572a9e00 0000000000000012 0000000000000003
[   62.178096]  0000000000000000 00007fff610ca0e8 ffff880057689f78 ffffffff81905bc2
[   62.184781] Call Trace:
[   62.191344]  [<ffffffff81a5799f>] ? rfcomm_sock_setsockopt+0x5f/0x1b0
[   62.197978]  [<ffffffff81905bc2>] SyS_getsockopt+0x62/0xb0
[   62.204645]  [<ffffffff81a8aa79>] system_call_fastpath+0x16/0x1b
[   62.211366] Code: 84 24 40 04 00 00 4c 89 e9 83 e0 01 e8 ea 0a 9d ff 85 c0 75 b5 45 31 f6 e9 42 ff ff ff 66 0f 1f 44 00 00 49 8b 84 24 50 04 00 00 <48> 8b 00 48 89 45 b8 e8 04 e7 6f ff 4c 89 f8 e8 2c fa 9c ff 85 
[   62.225815] RIP  [<ffffffff81a57c30>] rfcomm_sock_getsockopt+0x140/0x240
[   62.232986]  RSP <ffff880057689ec8>
[   62.240128] CR2: 00000015832a8e23
[   62.247221] ---[ end trace 88f75f0c791ac25b ]---

My bluetooth device is USB stick:
Bus 004 Device 002: ID 0a12:0001 Cambridge Silicon Radio, Ltd Bluetooth Dongle (HCI mode)

--

Sander

^ permalink raw reply

* Re: [PATCH 2/5] android/hal-bluetooth: Register IPC message handlers
From: Szymon Janc @ 2013-11-17 22:32 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: Bluetooth Linux
In-Reply-To: <CAMXE1-oyrNbBKrRpXVwo1p29Q-RASucDZOBPvMOr-Nita-zcEQ@mail.gmail.com>

Hi Andrei,

On Sun, Nov 17, 2013 at 8:39 PM, Andrei Emeltchenko
<andrei.emeltchenko.news@gmail.com> wrote:
> Hi Szymon,
>
> On Sat, Nov 16, 2013 at 4:13 PM, Szymon Janc <szymon.janc@tieto.com> wrote:
>>
>> Register handlers on service init. Since this requires all handlers to
>> be registered (unknown opcode is considered IPC error) missing handlers
>> stubs are provided.
>> ---
>>  android/hal-bluetooth.c | 223 ++++++++++++++++++++++++++++++------------------
>>  android/hal.h           |   1 -
>>  2 files changed, 142 insertions(+), 82 deletions(-)
>>
>> diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
>> index 078d537..2ec7c10 100644
>> --- a/android/hal-bluetooth.c
>> +++ b/android/hal-bluetooth.c
>> @@ -35,7 +35,7 @@ static const bt_callbacks_t *bt_hal_cbacks = NULL;
>>         *pe = *((uint8_t *) (hal_prop->val)); \
>>  } while (0)
>>
>> -static void handle_adapter_state_changed(void *buf)
>> +static void handle_adapter_state_changed(void *buf, uint16_t len, int fd)
>
> Why do we need fd here and in all other places? Who uses it?

This is needed for health HAL for bthl_channel_state_callback callback.
But since this seems to be the only user for that, this can be removed
from callback prototype and a getter function could be added to obtain
fd in notification handler (we do process one notification at time
after all).

Opinions?

-- 
BR
Szymon Janc

^ permalink raw reply

* Re: BUG: unable to handle kernel paging request at 00000015832a8e23 RIP  [<ffffffff81a57c30>] rfcomm_sock_getsockopt+0x140/0x240
From: Marcel Holtmann @ 2013-11-17 22:57 UTC (permalink / raw)
  To: Sander Eikelenboom
  Cc: Gustavo F. Padovan, linux-bluetooth@vger.kernel.org development
In-Reply-To: <581763626.20131117204744@eikelenboom.it>

Hi Sander,

> This 3.13 merge window seems to have introduced the following bluetooth regression for me:
> 
> [   62.040810] BUG: unable to handle kernel paging request at 00000015832a8e23
> [   62.049196] IP: [<ffffffff81a57c30>] rfcomm_sock_getsockopt+0x140/0x240
> [   62.057800] PGD 0 
> [   62.064574] Oops: 0000 [#1] PREEMPT SMP 
> [   62.070653] Modules linked in:
> [   62.076596] CPU: 4 PID: 5575 Comm: bluetoothd Not tainted 3.12.0-mw-20131117+ #1
> [   62.082708] Hardware name: MSI MS-7640/890FXA-GD70 (MS-7640)  , BIOS V1.8B1 09/13/2010
> [   62.088827] task: ffff88005863a180 ti: ffff880057688000 task.ti: ffff880057688000
> [   62.095020] RIP: e030:[<ffffffff81a57c30>]  [<ffffffff81a57c30>] rfcomm_sock_getsockopt+0x140/0x240
> [   62.101372] RSP: e02b:ffff880057689ec8  EFLAGS: 00010246
> [   62.107668] RAX: 00000015832a8e23 RBX: 0000000000000003 RCX: 00007fff610ca0e8
> [   62.113957] RDX: 0000000000000003 RSI: 0000000000000012 RDI: ffff8800572a9e00
> [   62.120174] RBP: ffff880057689f18 R08: 00007fff610ca0ec R09: 00007fff610ca360
> [   62.126481] R10: 00007fff610ca0e8 R11: 0000000000000202 R12: ffff8800569c3000
> [   62.132872] R13: 00007fff610ca0e8 R14: 0000000000000000 R15: 00007fff610ca0ec
> [   62.139187] FS:  00007f0d0b348720(0000) GS:ffff88005f700000(0000) knlGS:0000000000000000
> [   62.145551] CS:  e033 DS: 0000 ES: 0000 CR0: 000000008005003b
> [   62.151919] CR2: 00000015832a8e23 CR3: 0000000056f53000 CR4: 0000000000000660
> [   62.158338] Stack:
> [   62.164798]  ffff880057689f18 ffffffff81a5799f 0015832a8e23001f ffff880057689f40
> [   62.171410]  ffff880057689f44 ffff8800572a9e00 0000000000000012 0000000000000003
> [   62.178096]  0000000000000000 00007fff610ca0e8 ffff880057689f78 ffffffff81905bc2
> [   62.184781] Call Trace:
> [   62.191344]  [<ffffffff81a5799f>] ? rfcomm_sock_setsockopt+0x5f/0x1b0
> [   62.197978]  [<ffffffff81905bc2>] SyS_getsockopt+0x62/0xb0
> [   62.204645]  [<ffffffff81a8aa79>] system_call_fastpath+0x16/0x1b
> [   62.211366] Code: 84 24 40 04 00 00 4c 89 e9 83 e0 01 e8 ea 0a 9d ff 85 c0 75 b5 45 31 f6 e9 42 ff ff ff 66 0f 1f 44 00 00 49 8b 84 24 50 04 00 00 <48> 8b 00 48 89 45 b8 e8 04 e7 6f ff 4c 89 f8 e8 2c fa 9c ff 85 
> [   62.225815] RIP  [<ffffffff81a57c30>] rfcomm_sock_getsockopt+0x140/0x240
> [   62.232986]  RSP <ffff880057689ec8>
> [   62.240128] CR2: 00000015832a8e23
> [   62.247221] ---[ end trace 88f75f0c791ac25b ]---
> 
> My bluetooth device is USB stick:
> Bus 004 Device 002: ID 0a12:0001 Cambridge Silicon Radio, Ltd Bluetooth Dongle (HCI mode)

I assume this is the same one we already fixed. Patch is on its way into Linus’ tree. You can check bluetooth.git tree since that has this patch included.

Regards

Marcel


^ permalink raw reply

* [PATCHv3 00/16] Socket HAL
From: Andrei Emeltchenko @ 2013-11-18  8:26 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

This is initial code implementing socket HAL. OPP currently works with send/receive files. Probaly
other profiles works as well, not tested yet.

Changes:
	* v3: Fixed coding style with write/send between file descriptors.
	* v2: Following Marcel comments changed way copying between file descriptors works, added SDP record
	for OPP and now it is possible to send files through GUI. Merged one patch with structures with actual user.
	* v1: Rebased and use static src address, hal_fd removed from structure and closed after sent to framework,
	added connect calls and SDP parsing, added cleanup_rfcomm function, minor fixes.
	* RFC Initial

TODO:
	* Add SDP record for PBAP and other profiles
	* Use splice() (requires bionic change first)

For tracking rfcomm sockets I use structure rfslot which has following
fields:
 - real_sock - real RFCOMM socket
 - fd - fd to communicate with Android framework

create_rfslot sets hal_fd which is fd passed to Android framework with CMSG

Andrei Emeltchenko (16):
  android/hal-sock: Add debug flag printing
  android/socket: Use static local adapter address
  android/socket: Add connect signal to socket
  android/socket: Define structs and implement listen
  android/socket: Implement socket accepted event
  android/socket: Implement Android RFCOMM stack events
  android/socket: Implement RFCOMM events
  android/socket: Implement accept signal over Android fd
  android/socket: Write channel to Android fd
  android/socket: Implement socket connect HAL method
  android/socket: Parse SDP response and connect
  android/socket: Implement HAL connect call
  android/socket: Send RFCOMM channel to framework
  android/socket: Send connect signal on connect
  android/socket: Close file descriptor after sending
  android/socket: Add SDP record for OPP profile

 android/hal-msg.h  |    2 +
 android/hal-sock.c |    8 +-
 android/socket.c   |  605 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 android/socket.h   |    7 +
 4 files changed, 614 insertions(+), 8 deletions(-)

-- 
1.7.10.4


^ permalink raw reply

* [PATCHv3 01/16] android/hal-sock: Add debug flag printing
From: Andrei Emeltchenko @ 2013-11-18  8:26 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1384763179-2218-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 android/hal-sock.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/android/hal-sock.c b/android/hal-sock.c
index b7bc88e..eafa451 100644
--- a/android/hal-sock.c
+++ b/android/hal-sock.c
@@ -55,8 +55,8 @@ static bt_status_t sock_listen(btsock_type_t type, const char *service_name,
 		return BT_STATUS_PARM_INVALID;
 	}
 
-	DBG("uuid %s chan %d sock %p type %d service_name %s",
-			btuuid2str(uuid), chan, sock, type, service_name);
+	DBG("uuid %s chan %d sock %p type %d service_name %s flags 0x%02x",
+		btuuid2str(uuid), chan, sock, type, service_name, flags);
 
 	switch (type) {
 	case BTSOCK_RFCOMM:
@@ -82,8 +82,8 @@ static bt_status_t sock_connect(const bt_bdaddr_t *bdaddr, btsock_type_t type,
 		return BT_STATUS_PARM_INVALID;
 	}
 
-	DBG("uuid %s chan %d sock %p type %d", btuuid2str(uuid), chan, sock,
-									type);
+	DBG("uuid %s chan %d sock %p type %d flags 0x%02x",
+		btuuid2str(uuid), chan, sock, type, flags);
 
 	if (type != BTSOCK_RFCOMM) {
 		error("Socket type %u not supported", type);
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv3 02/16] android/socket: Use static local adapter address
From: Andrei Emeltchenko @ 2013-11-18  8:26 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1384763179-2218-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 android/socket.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/android/socket.c b/android/socket.c
index c283c5f..e580036 100644
--- a/android/socket.c
+++ b/android/socket.c
@@ -35,6 +35,7 @@
 #include "ipc.h"
 #include "socket.h"
 
+static bdaddr_t adapter_addr;
 
 static int handle_listen(void *buf)
 {
@@ -81,6 +82,8 @@ bool bt_socket_register(int sk, const bdaddr_t *addr)
 {
 	DBG("");
 
+	bacpy(&adapter_addr, addr);
+
 	return true;
 }
 
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv3 03/16] android/socket: Add connect signal to socket
From: Andrei Emeltchenko @ 2013-11-18  8:26 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1384763179-2218-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Connect signal is used to pass information to framework that socket
is accepted.
---
 android/hal-msg.h |    2 ++
 android/socket.h  |    7 +++++++
 2 files changed, 9 insertions(+)

diff --git a/android/hal-msg.h b/android/hal-msg.h
index 44fd5c8..9e3a81f 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -232,6 +232,8 @@ struct hal_cmd_sock_connect {
 	uint8_t  flags;
 } __attribute__((packed));
 
+/* Bluetooth Hidhost HAL api */
+
 #define HAL_OP_HIDHOST_CONNECT		0x01
 struct hal_cmd_hidhost_connect {
 	uint8_t bdaddr[6];
diff --git a/android/socket.h b/android/socket.h
index 7aa5574..ba56c9b 100644
--- a/android/socket.h
+++ b/android/socket.h
@@ -21,6 +21,13 @@
  *
  */
 
+struct hal_sock_connect_signal {
+	short   size;
+	uint8_t bdaddr[6];
+	int     channel;
+	int     status;
+} __attribute__((packed));
+
 void bt_sock_handle_cmd(int sk, uint8_t opcode, void *buf, uint16_t len);
 
 bool bt_socket_register(int sk, const bdaddr_t *addr);
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv3 04/16] android/socket: Define structs and implement listen
From: Andrei Emeltchenko @ 2013-11-18  8:26 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1384763179-2218-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

This defines structures for socket HAL. We need to emulate Android
sockets by sending connect/accept signals over file descriptor.
Handle HAL socket listen call. Create RFCOMM socket and wait for events.
---
 android/socket.c |  118 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 116 insertions(+), 2 deletions(-)

diff --git a/android/socket.c b/android/socket.c
index e580036..276c78c 100644
--- a/android/socket.c
+++ b/android/socket.c
@@ -27,8 +27,12 @@
 
 #include <glib.h>
 #include <stdbool.h>
+#include <unistd.h>
+#include <errno.h>
 
 #include "lib/bluetooth.h"
+#include "btio/btio.h"
+#include "lib/sdp.h"
 #include "log.h"
 #include "hal-msg.h"
 #include "hal-ipc.h"
@@ -37,13 +41,123 @@
 
 static bdaddr_t adapter_addr;
 
-static int handle_listen(void *buf)
+/* Simple list of RFCOMM server sockets */
+GList *rfcomm_srv_list = NULL;
+
+/* Simple list of RFCOMM connected sockets */
+GList *rfcomm_connected_list = NULL;
+
+struct rfcomm_slot {
+	int fd;		/* descriptor for communication with Java framework */
+	int real_sock;	/* real RFCOMM socket */
+	int channel;	/* RFCOMM channel */
+};
+
+static struct rfcomm_slot *create_rfslot(int sock, int *hal_fd)
 {
-	DBG("Not implemented");
+	int fds[2] = {-1, -1};
+	struct rfcomm_slot *rfslot;
+
+	if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) < 0) {
+		error("socketpair(): %s", strerror(errno));
+		return NULL;
+	}
+
+	rfslot = g_malloc0(sizeof(*rfslot));
+	rfslot->fd = fds[0];
+	*hal_fd = fds[1];
+	rfslot->real_sock = sock;
+
+	return rfslot;
+}
+
+static void cleanup_rfslot(struct rfcomm_slot *rfslot)
+{
+	DBG("Cleanup: rfslot: %p fd %d real_sock %d chan %u",
+		rfslot, rfslot->fd, rfslot->real_sock, rfslot->channel);
+
+	if (rfslot->fd > 0)
+		close(rfslot->fd);
+	if (rfslot->real_sock > 0)
+		close(rfslot->real_sock);
+
+	g_free(rfslot);
+}
+
+static struct {
+	uint8_t uuid[16];
+	uint8_t channel;
+} uuid_to_chan[] = {
+	{ .uuid = {
+	0x00, 0x00, 0x11, 0x2F, 0x00, 0x00, 0x10, 0x00,
+	0x80, 0x00, 0x00, 0x80, 0x5F, 0x9B, 0x34, 0xFB
+	},
+#define PBAP_DEFAULT_CHANNEL	15
+	.channel = PBAP_DEFAULT_CHANNEL },
+	{ .uuid = {
+	0x00, 0x00, 0x11, 0x05, 0x00, 0x00, 0x10, 0x00,
+	0x80, 0x00, 0x00, 0x80, 0x5F, 0x9B, 0x34, 0xFB
+	},
+#define OPP_DEFAULT_CHANNEL	9
+	.channel = OPP_DEFAULT_CHANNEL },
+	{ {0} }
+};
+
+static int get_rfcomm_default_chan(const uint8_t *uuid)
+{
+	int i;
+
+	for (i = 0; uuid_to_chan[i].channel; i++) {
+		if (!memcmp(uuid_to_chan[i].uuid, uuid, 16))
+			return uuid_to_chan[i].channel;
+	}
 
 	return -1;
 }
 
+static void accept_cb(GIOChannel *io, GError *err, gpointer user_data)
+{
+}
+
+static int handle_listen(void *buf)
+{
+	struct hal_cmd_sock_listen *cmd = buf;
+	struct rfcomm_slot *rfslot;
+	GIOChannel *io;
+	GError *err = NULL;
+	int hal_fd = -1;
+	int chan;
+
+	DBG("");
+
+	chan = get_rfcomm_default_chan(cmd->uuid);
+	if (chan < 0)
+		return -1;
+
+	DBG("rfcomm channel %d", chan);
+
+	rfslot = create_rfslot(-1, &hal_fd);
+
+	io = bt_io_listen(accept_cb, NULL, rfslot, NULL, &err,
+				BT_IO_OPT_SOURCE_BDADDR, &adapter_addr,
+				BT_IO_OPT_CHANNEL, chan,
+				BT_IO_OPT_INVALID);
+	if (!io) {
+		error("Failed listen: %s", err->message);
+		g_error_free(err);
+		cleanup_rfslot(rfslot);
+		return -1;
+	}
+
+	rfslot->real_sock = g_io_channel_unix_get_fd(io);
+	rfcomm_srv_list = g_list_append(rfcomm_srv_list, rfslot);
+
+	DBG("real_sock %d fd %d hal_fd %d",
+				rfslot->real_sock, rfslot->fd, hal_fd);
+
+	return hal_fd;
+}
+
 static int handle_connect(void *buf)
 {
 	DBG("Not implemented");
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv3 05/16] android/socket: Implement socket accepted event
From: Andrei Emeltchenko @ 2013-11-18  8:26 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1384763179-2218-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

When we get accepted event we create rfcomm slot and start listening
for events from Android framework and from RFCOMM real socket.
---
 android/socket.c |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/android/socket.c b/android/socket.c
index 276c78c..c9ee32f 100644
--- a/android/socket.c
+++ b/android/socket.c
@@ -115,8 +115,60 @@ static int get_rfcomm_default_chan(const uint8_t *uuid)
 	return -1;
 }
 
+static gboolean sock_stack_event_cb(GIOChannel *io, GIOCondition cond,
+								gpointer data)
+{
+	return TRUE;
+}
+
+static gboolean sock_rfcomm_event_cb(GIOChannel *io, GIOCondition cond,
+								gpointer data)
+{
+	return TRUE;
+}
+
 static void accept_cb(GIOChannel *io, GError *err, gpointer user_data)
 {
+	struct rfcomm_slot *rfslot = user_data;
+	struct rfcomm_slot *rfslot_acc;
+	GIOChannel *io_stack;
+	bdaddr_t dst;
+	char address[18];
+	int sock_acc;
+	int hal_fd = -1;
+
+	bt_io_get(io, &err,
+			BT_IO_OPT_DEST_BDADDR, &dst,
+			BT_IO_OPT_INVALID);
+	if (err) {
+		error("%s", err->message);
+		g_io_channel_shutdown(io, TRUE, NULL);
+		return;
+	}
+
+	ba2str(&dst, address);
+	DBG("Incoming connection from %s rfslot %p", address, rfslot);
+
+	DBG("rfslot: fd %d real_sock %d chan %u sock %d",
+		rfslot->fd, rfslot->real_sock, rfslot->channel,
+		g_io_channel_unix_get_fd(io));
+
+	sock_acc = g_io_channel_unix_get_fd(io);
+	rfslot_acc = create_rfslot(sock_acc, &hal_fd);
+	rfcomm_connected_list =
+			g_list_append(rfcomm_connected_list, rfslot_acc);
+
+	/* Handle events from Android */
+	io_stack = g_io_channel_unix_new(rfslot_acc->fd);
+	g_io_add_watch(io_stack, G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+					sock_stack_event_cb, rfslot_acc);
+	g_io_channel_unref(io_stack);
+
+	/* Handle rfcomm events */
+	g_io_add_watch(io, G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
+				sock_rfcomm_event_cb, rfslot_acc);
+
+	DBG("rfslot %p rfslot_acc %p", rfslot, rfslot_acc);
 }
 
 static int handle_listen(void *buf)
-- 
1.7.10.4


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox