All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/13] android: IPC improvements - daemon part
@ 2013-11-27 11:54 Szymon Janc
  2013-11-27 11:54 ` [PATCH 01/13] android: Add initial code for IPC message handlers Szymon Janc
                   ` (12 more replies)
  0 siblings, 13 replies; 14+ messages in thread
From: Szymon Janc @ 2013-11-27 11:54 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

Hi,

This serie implements IPC message handling iprovments in daemon similar
to what is already done in HAL part.

-- 
BR
Szymon Janc

Szymon Janc (13):
  android: Add initial code for IPC message handlers
  android/main: Use generic IPC message handling for core service
  android/main: Use IPC helper for all replies in core service
  android: Pass command socket to services while registering
  android/bluetooth: Use generic IPC msg handling for commands
  android/bluetooth: Add stubs for missing commands handlers
  android/hidhost: Use generic IPC message handling for commands
  android/hidhost: Use stack buffer in bt_hid_get_report
  android/pan: Use generic IPC message handling for commands
  android/a2dp: Use generic IPC message handling for commands
  android/socket: Use generic IPC message handling for commands
  android/hal-bluetooth: Rename create_enum_prop to enum_prop_to_hal
  android/hal-bluetooth: Fix sending invalid adapter property

 android/a2dp.c          |  80 ++++----
 android/a2dp.h          |   4 +-
 android/bluetooth.c     | 523 +++++++++++++++++++++++++++++++++++-------------
 android/bluetooth.h     |   2 +-
 android/hal-bluetooth.c |  44 +++-
 android/hidhost.c       | 330 ++++++++++++++++++------------
 android/hidhost.h       |   4 +-
 android/ipc.c           |  79 ++++++++
 android/ipc.h           |  12 ++
 android/main.c          | 128 ++++--------
 android/pan.c           |  81 ++++----
 android/pan.h           |   4 +-
 android/socket.c        | 115 ++++++-----
 android/socket.h        |   2 +-
 14 files changed, 898 insertions(+), 510 deletions(-)

-- 
1.8.3.2


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

* [PATCH 01/13] android: Add initial code for IPC message handlers
  2013-11-27 11:54 [PATCH 00/13] android: IPC improvements - daemon part Szymon Janc
@ 2013-11-27 11:54 ` Szymon Janc
  2013-11-27 11:54 ` [PATCH 02/13] android/main: Use generic IPC message handling for core service Szymon Janc
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Szymon Janc @ 2013-11-27 11:54 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

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/ipc.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 android/ipc.h | 12 +++++++++
 2 files changed, 91 insertions(+)

diff --git a/android/ipc.c b/android/ipc.c
index 25c36fd..e55c82b 100644
--- a/android/ipc.c
+++ b/android/ipc.c
@@ -30,12 +30,20 @@
 #include <stdint.h>
 #include <string.h>
 #include <signal.h>
+#include <stdbool.h>
 #include <sys/socket.h>
 
 #include "hal-msg.h"
 #include "ipc.h"
 #include "log.h"
 
+struct service_handler {
+	const struct ipc_handler *handler;
+	uint8_t size;
+};
+
+static struct service_handler services[HAL_SERVICE_ID_MAX + 1];
+
 void ipc_send(int sk, uint8_t service_id, uint8_t opcode, uint16_t len,
 							void *param, int fd)
 {
@@ -94,3 +102,74 @@ void ipc_send_rsp(int sk, uint8_t service_id, uint8_t opcode, uint8_t status)
 
 	ipc_send(sk, service_id, HAL_OP_STATUS, sizeof(s), &s, -1);
 }
+
+void ipc_register(uint8_t service, const struct ipc_handler *handlers,
+								uint8_t size)
+{
+	services[service].handler = handlers;
+	services[service].size = size;
+}
+
+void ipc_unregister(uint8_t service)
+{
+	services[service].handler = NULL;
+	services[service].size = 0;
+}
+
+void ipc_handle_msg(const void *buf, ssize_t len)
+{
+	const struct hal_hdr *msg = buf;
+	const struct ipc_handler *handler;
+
+	if (len < (ssize_t) sizeof(*msg)) {
+		error("IPC: message too small (%zd bytes), terminating", len);
+		raise(SIGTERM);
+		return;
+	}
+
+	if (len != (ssize_t) (sizeof(*msg) + msg->len)) {
+		error("IPC: message malformed (%zd bytes), terminating", len);
+		raise(SIGTERM);
+		return;
+	}
+
+	/* if service is valid */
+	if (msg->service_id > HAL_SERVICE_ID_MAX) {
+		error("IPC: unknown service (0x%x), terminating",
+							msg->service_id);
+		raise(SIGTERM);
+		return;
+	}
+
+	/* if service is registered */
+	if (!services[msg->service_id].handler) {
+		error("IPC: unregistered service (0x%x), terminating",
+							msg->service_id);
+		raise(SIGTERM);
+		return;
+	}
+
+	/* if opcode is valid */
+	if (msg->opcode == HAL_OP_STATUS ||
+			msg->opcode > services[msg->service_id].size) {
+		error("IPC: invalid opcode for service 0x%x (0x%x), "
+				"terminating", msg->service_id, msg->opcode);
+		raise(SIGTERM);
+		return;
+	}
+
+	/* opcode is table offset + 1 */
+	handler = &services[msg->service_id].handler[msg->opcode - 1];
+
+	/* 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), terminating",
+				msg->service_id, msg->opcode, msg->len);
+		raise(SIGTERM);
+		return;
+	}
+
+	handler->handler(msg->payload, msg->len);
+}
diff --git a/android/ipc.h b/android/ipc.h
index ad4a2d2..cb03670 100644
--- a/android/ipc.h
+++ b/android/ipc.h
@@ -21,6 +21,18 @@
  *
  */
 
+struct ipc_handler {
+	void (*handler) (const void *buf, uint16_t len);
+	bool var_len;
+	size_t data_len;
+};
+
 void ipc_send(int sk, uint8_t service_id, uint8_t opcode, uint16_t len,
 							void *param, int fd);
 void ipc_send_rsp(int sk, uint8_t service_id, uint8_t opcode, uint8_t status);
+
+void ipc_register(uint8_t service, const struct ipc_handler *handlers,
+								uint8_t size);
+void ipc_unregister(uint8_t service);
+
+void ipc_handle_msg(const void *buf, ssize_t len);
-- 
1.8.3.2


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

* [PATCH 02/13] android/main: Use generic IPC message handling for core service
  2013-11-27 11:54 [PATCH 00/13] android: IPC improvements - daemon part Szymon Janc
  2013-11-27 11:54 ` [PATCH 01/13] android: Add initial code for IPC message handlers Szymon Janc
@ 2013-11-27 11:54 ` Szymon Janc
  2013-11-27 11:54 ` [PATCH 03/13] android/main: Use IPC helper for all replies in " Szymon Janc
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Szymon Janc @ 2013-11-27 11:54 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

Handlers are registered on daemon start and unregistered on shutdown.
---
 android/main.c | 93 +++++++++++++++-------------------------------------------
 1 file changed, 23 insertions(+), 70 deletions(-)

diff --git a/android/main.c b/android/main.c
index c9733f3..6139ed7 100644
--- a/android/main.c
+++ b/android/main.c
@@ -74,9 +74,9 @@ static GIOChannel *hal_notif_io = NULL;
 
 static bool services[HAL_SERVICE_ID_MAX + 1] = { false };
 
-static void service_register(void *buf, uint16_t len)
+static void service_register(const void *buf, uint16_t len)
 {
-	struct hal_cmd_register_module *m = buf;
+	const struct hal_cmd_register_module *m = buf;
 	int sk = g_io_channel_unix_get_fd(hal_notif_io);
 
 	if (m->service_id > HAL_SERVICE_ID_MAX || services[m->service_id])
@@ -126,9 +126,9 @@ failed:
 				HAL_STATUS_FAILED);
 }
 
-static void service_unregister(void *buf, uint16_t len)
+static void service_unregister(const void *buf, uint16_t len)
 {
-	struct hal_cmd_unregister_module *m = buf;
+	const struct hal_cmd_unregister_module *m = buf;
 
 	if (m->service_id > HAL_SERVICE_ID_MAX || !services[m->service_id])
 		goto failed;
@@ -169,22 +169,19 @@ failed:
 				HAL_STATUS_FAILED);
 }
 
-static void handle_service_core(uint8_t opcode, void *buf, uint16_t len)
-{
-	switch (opcode) {
-	case HAL_OP_REGISTER_MODULE:
-		service_register(buf, len);
-		break;
-	case HAL_OP_UNREGISTER_MODULE:
-		service_unregister(buf, len);
-		break;
-	default:
-		ipc_send_rsp(g_io_channel_unix_get_fd(hal_cmd_io),
-						HAL_SERVICE_ID_CORE, opcode,
-						HAL_STATUS_FAILED);
-		break;
-	}
-}
+static const struct ipc_handler cmd_handlers[] = {
+	{	/* HAL_OP_REGISTER_MODULE */
+		.handler = service_register,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_register_module)
+	},
+	{	/* HAL_OP_UNREGISTER_MODULE */
+		.handler = service_unregister,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_unregister_module)
+	},
+};
+
 
 static void bluetooth_stopped(void)
 {
@@ -218,7 +215,6 @@ static gboolean cmd_watch_cb(GIOChannel *io, GIOCondition cond,
 							gpointer user_data)
 {
 	char buf[BLUEZ_HAL_MTU];
-	struct hal_hdr *msg = (void *) buf;
 	ssize_t ret;
 	int fd;
 
@@ -236,52 +232,7 @@ static gboolean cmd_watch_cb(GIOChannel *io, GIOCondition cond,
 		goto fail;
 	}
 
-	if (ret < (ssize_t) sizeof(*msg)) {
-		error("HAL command too small, terminating (%zd)", ret);
-		goto fail;
-	}
-
-	if (ret != (ssize_t) (sizeof(*msg) + msg->len)) {
-		error("Malformed HAL command (%zd bytes), terminating", ret);
-		goto fail;
-	}
-
-	DBG("service_id %u opcode %u len %u", msg->service_id, msg->opcode,
-								msg->len);
-
-	if (msg->service_id > HAL_SERVICE_ID_MAX ||
-						!services[msg->service_id]) {
-		error("HAL command for unregistered service %u, terminating",
-							msg->service_id);
-		goto fail;
-	}
-
-	switch (msg->service_id) {
-	case HAL_SERVICE_ID_CORE:
-		handle_service_core(msg->opcode, msg->payload, msg->len);
-		break;
-	case HAL_SERVICE_ID_BLUETOOTH:
-		bt_bluetooth_handle_cmd(fd, msg->opcode, msg->payload,
-								msg->len);
-		break;
-	case HAL_SERVICE_ID_HIDHOST:
-		bt_hid_handle_cmd(fd, msg->opcode, msg->payload, msg->len);
-		break;
-	case HAL_SERVICE_ID_SOCK:
-		bt_sock_handle_cmd(fd, msg->opcode, msg->payload, msg->len);
-		break;
-	case HAL_SERVICE_ID_A2DP:
-		bt_a2dp_handle_cmd(fd, msg->opcode, msg->payload, msg->len);
-		break;
-	case HAL_SERVICE_ID_PAN:
-		bt_pan_handle_cmd(fd, msg->opcode, msg->payload, msg->len);
-		break;
-	default:
-		ipc_send_rsp(fd, msg->service_id, msg->opcode,
-							HAL_STATUS_FAILED);
-		break;
-	}
-
+	ipc_handle_msg(buf, ret);
 	return TRUE;
 
 fail:
@@ -535,9 +486,6 @@ int main(int argc, char *argv[])
 	GError *err = NULL;
 	guint signal;
 
-	/* Core Service (ID=0) should always be considered registered */
-	services[0] = true;
-
 	context = g_option_context_new(NULL);
 	g_option_context_add_main_entries(context, options, NULL);
 
@@ -581,6 +529,9 @@ int main(int argc, char *argv[])
 	/* Use params: mtu = 0, flags = 0 */
 	start_sdp_server(0, 0);
 
+	ipc_register(HAL_SERVICE_ID_CORE, cmd_handlers,
+				sizeof(cmd_handlers)/sizeof(cmd_handlers[0]));
+
 	DBG("Entering main loop");
 
 	g_main_loop_run(event_loop);
@@ -592,6 +543,8 @@ int main(int argc, char *argv[])
 	bt_bluetooth_cleanup();
 	g_main_loop_unref(event_loop);
 
+	ipc_unregister(HAL_SERVICE_ID_CORE);
+
 	info("Exit");
 
 	__btd_log_cleanup();
-- 
1.8.3.2


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

* [PATCH 03/13] android/main: Use IPC helper for all replies in core service
  2013-11-27 11:54 [PATCH 00/13] android: IPC improvements - daemon part Szymon Janc
  2013-11-27 11:54 ` [PATCH 01/13] android: Add initial code for IPC message handlers Szymon Janc
  2013-11-27 11:54 ` [PATCH 02/13] android/main: Use generic IPC message handling for core service Szymon Janc
@ 2013-11-27 11:54 ` Szymon Janc
  2013-11-27 11:54 ` [PATCH 04/13] android: Pass command socket to services while registering Szymon Janc
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Szymon Janc @ 2013-11-27 11:54 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This makes functions exit path simpler.
---
 android/main.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/android/main.c b/android/main.c
index 6139ed7..de19127 100644
--- a/android/main.c
+++ b/android/main.c
@@ -78,6 +78,7 @@ static void service_register(const void *buf, uint16_t len)
 {
 	const struct hal_cmd_register_module *m = buf;
 	int sk = g_io_channel_unix_get_fd(hal_notif_io);
+	uint8_t status = HAL_STATUS_FAILED;
 
 	if (m->service_id > HAL_SERVICE_ID_MAX || services[m->service_id])
 		goto failed;
@@ -115,20 +116,19 @@ static void service_register(const void *buf, uint16_t len)
 
 	services[m->service_id] = true;
 
-	ipc_send(g_io_channel_unix_get_fd(hal_cmd_io), HAL_SERVICE_ID_CORE,
-					HAL_OP_REGISTER_MODULE, 0, NULL, -1);
+	status = HAL_STATUS_SUCCESS;
 
 	info("Service ID=%u registered", m->service_id);
-	return;
+
 failed:
-	ipc_send_rsp(g_io_channel_unix_get_fd(hal_cmd_io),
-				HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
-				HAL_STATUS_FAILED);
+	ipc_send_rsp(g_io_channel_unix_get_fd(hal_cmd_io), HAL_SERVICE_ID_CORE,
+					HAL_OP_REGISTER_MODULE, status);
 }
 
 static void service_unregister(const void *buf, uint16_t len)
 {
 	const struct hal_cmd_unregister_module *m = buf;
+	uint8_t status = HAL_STATUS_FAILED;
 
 	if (m->service_id > HAL_SERVICE_ID_MAX || !services[m->service_id])
 		goto failed;
@@ -158,15 +158,13 @@ static void service_unregister(const void *buf, uint16_t len)
 
 	services[m->service_id] = false;
 
-	ipc_send(g_io_channel_unix_get_fd(hal_cmd_io), HAL_SERVICE_ID_CORE,
-					HAL_OP_UNREGISTER_MODULE, 0, NULL, -1);
+	status = HAL_STATUS_SUCCESS;
 
 	info("Service ID=%u unregistered", m->service_id);
-	return;
+
 failed:
-	ipc_send_rsp(g_io_channel_unix_get_fd(hal_cmd_io),
-				HAL_SERVICE_ID_CORE, HAL_OP_UNREGISTER_MODULE,
-				HAL_STATUS_FAILED);
+	ipc_send_rsp(g_io_channel_unix_get_fd(hal_cmd_io), HAL_SERVICE_ID_CORE,
+					HAL_OP_UNREGISTER_MODULE, status);
 }
 
 static const struct ipc_handler cmd_handlers[] = {
-- 
1.8.3.2


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

* [PATCH 04/13] android: Pass command socket to services while registering
  2013-11-27 11:54 [PATCH 00/13] android: IPC improvements - daemon part Szymon Janc
                   ` (2 preceding siblings ...)
  2013-11-27 11:54 ` [PATCH 03/13] android/main: Use IPC helper for all replies in " Szymon Janc
@ 2013-11-27 11:54 ` Szymon Janc
  2013-11-27 11:54 ` [PATCH 05/13] android/bluetooth: Use generic IPC msg handling for commands Szymon Janc
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Szymon Janc @ 2013-11-27 11:54 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

It will be used to reply to commands from IPC handlers.
---
 android/a2dp.c      |  7 +++++--
 android/a2dp.h      |  2 +-
 android/bluetooth.c |  8 ++++++--
 android/bluetooth.h |  2 +-
 android/hidhost.c   |  8 ++++++--
 android/hidhost.h   |  2 +-
 android/main.c      | 13 +++++++------
 android/pan.c       |  8 ++++++--
 android/pan.h       |  2 +-
 android/socket.c    |  2 +-
 android/socket.h    |  2 +-
 11 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/android/a2dp.c b/android/a2dp.c
index 2251001..87b1abb 100644
--- a/android/a2dp.c
+++ b/android/a2dp.c
@@ -49,6 +49,7 @@
 #define SVC_HINT_CAPTURING 0x08
 
 static int notification_sk = -1;
+static int command_sk = -1;
 static GIOChannel *server = NULL;
 static GSList *devices = NULL;
 static bdaddr_t adapter_addr;
@@ -350,7 +351,7 @@ static sdp_record_t *a2dp_record(void)
 	return record;
 }
 
-bool bt_a2dp_register(int sk, const bdaddr_t *addr)
+bool bt_a2dp_register(int cmd_sk, int notif_sk, const bdaddr_t *addr)
 {
 	GError *err = NULL;
 	sdp_record_t *rec;
@@ -384,7 +385,8 @@ bool bt_a2dp_register(int sk, const bdaddr_t *addr)
 	}
 	record_id = rec->handle;
 
-	notification_sk = sk;
+	notification_sk = notif_sk;
+	command_sk = cmd_sk;
 
 	return true;
 }
@@ -407,6 +409,7 @@ void bt_a2dp_unregister(void)
 	devices = NULL;
 
 	notification_sk = -1;
+	command_sk = -1;
 
 	bt_adapter_remove_record(record_id);
 	record_id = 0;
diff --git a/android/a2dp.h b/android/a2dp.h
index 3531618..720b681 100644
--- a/android/a2dp.h
+++ b/android/a2dp.h
@@ -23,5 +23,5 @@
 
 void bt_a2dp_handle_cmd(int sk, uint8_t opcode, void *buf, uint16_t len);
 
-bool bt_a2dp_register(int sk, const bdaddr_t *addr);
+bool bt_a2dp_register(int cmd_sk, int notif_sk, const bdaddr_t *addr);
 void bt_a2dp_unregister(void);
diff --git a/android/bluetooth.c b/android/bluetooth.c
index 0e45131..700e5ce 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -60,6 +60,7 @@
 
 static uint16_t option_index = MGMT_INDEX_NONE;
 
+static int command_sk = -1;
 static int notification_sk = -1;
 
 #define BASELEN_REMOTE_DEV_PROP (sizeof(struct hal_ev_remote_device_props) \
@@ -2272,14 +2273,16 @@ error:
 	ipc_send_rsp(sk, HAL_SERVICE_ID_BLUETOOTH, opcode, status);
 }
 
-bool bt_bluetooth_register(int sk)
+bool bt_bluetooth_register(int cmd_sk, int notif_sk)
 {
 	DBG("");
 
 	if (notification_sk >= 0)
 		return false;
 
-	notification_sk = sk;
+	command_sk = cmd_sk;
+	notification_sk = notif_sk;
+
 
 	return true;
 }
@@ -2291,5 +2294,6 @@ void bt_bluetooth_unregister(void)
 	if (notification_sk < 0)
 		return;
 
+	command_sk = -1;
 	notification_sk = -1;
 }
diff --git a/android/bluetooth.h b/android/bluetooth.h
index 44b8e9e..d104a5b 100644
--- a/android/bluetooth.h
+++ b/android/bluetooth.h
@@ -31,7 +31,7 @@ void bt_bluetooth_cleanup(void);
 
 void bt_bluetooth_handle_cmd(int sk, uint8_t opcode, void *buf, uint16_t len);
 
-bool bt_bluetooth_register(int sk);
+bool bt_bluetooth_register(int cmd_sk, int notif_sk);
 void bt_bluetooth_unregister(void);
 
 int bt_adapter_add_record(sdp_record_t *rec, uint8_t svc_hint);
diff --git a/android/hidhost.c b/android/hidhost.c
index d50c5b8..d97af03 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -79,6 +79,8 @@
 static bdaddr_t adapter_addr;
 
 static int notification_sk = -1;
+static int command_sk = -1;
+
 static GIOChannel *ctrl_io = NULL;
 static GIOChannel *intr_io = NULL;
 static GSList *devices = NULL;
@@ -1190,7 +1192,7 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
 	}
 }
 
-bool bt_hid_register(int sk, const bdaddr_t *addr)
+bool bt_hid_register(int cmd_sk, int notif_sk, const bdaddr_t *addr)
 {
 	GError *err = NULL;
 
@@ -1224,7 +1226,8 @@ bool bt_hid_register(int sk, const bdaddr_t *addr)
 		return false;
 	}
 
-	notification_sk = sk;
+	command_sk = cmd_sk;
+	notification_sk = notif_sk;
 
 	return true;
 }
@@ -1246,6 +1249,7 @@ void bt_hid_unregister(void)
 
 	g_slist_foreach(devices, free_hid_devices, NULL);
 	devices = NULL;
+	command_sk = -1;
 	notification_sk = -1;
 
 	if (ctrl_io) {
diff --git a/android/hidhost.h b/android/hidhost.h
index 688086a..4486ee9 100644
--- a/android/hidhost.h
+++ b/android/hidhost.h
@@ -23,5 +23,5 @@
 
 void bt_hid_handle_cmd(int sk, uint8_t opcode, void *buf, uint16_t len);
 
-bool bt_hid_register(int sk, const bdaddr_t *addr);
+bool bt_hid_register(int cmd_sk, int notif_sk, const bdaddr_t *addr);
 void bt_hid_unregister(void);
diff --git a/android/main.c b/android/main.c
index de19127..d005be1 100644
--- a/android/main.c
+++ b/android/main.c
@@ -77,7 +77,8 @@ static bool services[HAL_SERVICE_ID_MAX + 1] = { false };
 static void service_register(const void *buf, uint16_t len)
 {
 	const struct hal_cmd_register_module *m = buf;
-	int sk = g_io_channel_unix_get_fd(hal_notif_io);
+	int cmd_sk = g_io_channel_unix_get_fd(hal_cmd_io);
+	int notif_sk = g_io_channel_unix_get_fd(hal_notif_io);
 	uint8_t status = HAL_STATUS_FAILED;
 
 	if (m->service_id > HAL_SERVICE_ID_MAX || services[m->service_id])
@@ -85,27 +86,27 @@ static void service_register(const void *buf, uint16_t len)
 
 	switch (m->service_id) {
 	case HAL_SERVICE_ID_BLUETOOTH:
-		if (!bt_bluetooth_register(sk))
+		if (!bt_bluetooth_register(cmd_sk, notif_sk))
 			goto failed;
 
 		break;
 	case HAL_SERVICE_ID_SOCK:
-		if (!bt_socket_register(sk, &adapter_bdaddr))
+		if (!bt_socket_register(cmd_sk, notif_sk, &adapter_bdaddr))
 			goto failed;
 
 		break;
 	case HAL_SERVICE_ID_HIDHOST:
-		if (!bt_hid_register(sk, &adapter_bdaddr))
+		if (!bt_hid_register(cmd_sk, notif_sk, &adapter_bdaddr))
 			goto failed;
 
 		break;
 	case HAL_SERVICE_ID_A2DP:
-		if (!bt_a2dp_register(sk, &adapter_bdaddr))
+		if (!bt_a2dp_register(cmd_sk, notif_sk, &adapter_bdaddr))
 			goto failed;
 
 		break;
 	case HAL_SERVICE_ID_PAN:
-		if (!bt_pan_register(sk, &adapter_bdaddr))
+		if (!bt_pan_register(cmd_sk, notif_sk, &adapter_bdaddr))
 			goto failed;
 
 		break;
diff --git a/android/pan.c b/android/pan.c
index ada458a..35d5608 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -36,6 +36,7 @@
 #include "ipc.h"
 
 static int notification_sk = -1;
+static int command_sk = -1;
 
 static uint8_t bt_pan_enable(struct hal_cmd_pan_enable *cmd, uint16_t len)
 {
@@ -91,14 +92,16 @@ void bt_pan_handle_cmd(int sk, uint8_t opcode, void *buf, uint16_t len)
 	ipc_send_rsp(sk, HAL_SERVICE_ID_PAN, opcode, status);
 }
 
-bool bt_pan_register(int sk, const bdaddr_t *addr)
+bool bt_pan_register(int cmd_sk, int notif_sk, const bdaddr_t *addr)
 {
 	DBG("");
 
 	if (notification_sk >= 0)
 		return false;
 
-	notification_sk = sk;
+	notification_sk = notif_sk;
+	command_sk = cmd_sk;
+
 
 	return true;
 }
@@ -111,4 +114,5 @@ void bt_pan_unregister(void)
 		return;
 
 	notification_sk = -1;
+	command_sk = -1;
 }
diff --git a/android/pan.h b/android/pan.h
index 2430378..9ceebe5 100644
--- a/android/pan.h
+++ b/android/pan.h
@@ -23,5 +23,5 @@
 
 void bt_pan_handle_cmd(int sk, uint8_t opcode, void *buf, uint16_t len);
 
-bool bt_pan_register(int sk, const bdaddr_t *addr);
+bool bt_pan_register(int cmd_sk, int notif_sk, const bdaddr_t *addr);
 void bt_pan_unregister(void);
diff --git a/android/socket.c b/android/socket.c
index 1fb154d..47338e7 100644
--- a/android/socket.c
+++ b/android/socket.c
@@ -934,7 +934,7 @@ void bt_sock_handle_cmd(int sk, uint8_t opcode, void *buf, uint16_t len)
 	ipc_send_rsp(sk, HAL_SERVICE_ID_SOCK, opcode, HAL_STATUS_FAILED);
 }
 
-bool bt_socket_register(int sk, const bdaddr_t *addr)
+bool bt_socket_register(int cmd_sk, int notif_sk, const bdaddr_t *addr)
 {
 	DBG("");
 
diff --git a/android/socket.h b/android/socket.h
index ba56c9b..6caf44d 100644
--- a/android/socket.h
+++ b/android/socket.h
@@ -30,5 +30,5 @@ struct hal_sock_connect_signal {
 
 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);
+bool bt_socket_register(int cmd_sk, int notif_sk, const bdaddr_t *addr);
 void bt_socket_unregister(void);
-- 
1.8.3.2


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

* [PATCH 05/13] android/bluetooth: Use generic IPC msg handling for commands
  2013-11-27 11:54 [PATCH 00/13] android: IPC improvements - daemon part Szymon Janc
                   ` (3 preceding siblings ...)
  2013-11-27 11:54 ` [PATCH 04/13] android: Pass command socket to services while registering Szymon Janc
@ 2013-11-27 11:54 ` Szymon Janc
  2013-11-27 11:54 ` [PATCH 06/13] android/bluetooth: Add stubs for missing commands handlers Szymon Janc
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Szymon Janc @ 2013-11-27 11:54 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

Handlers are registered on service register and unregistered on
unregister.
---
 android/bluetooth.c | 478 ++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 335 insertions(+), 143 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 700e5ce..c7ec19b 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -1369,7 +1369,7 @@ static void set_adapter_name_complete(uint8_t status, uint16_t length,
 	adapter_set_name(rp->name);
 }
 
-static uint8_t set_adapter_name(uint8_t *name, uint16_t len)
+static uint8_t set_adapter_name(const uint8_t *name, uint16_t len)
 {
 	struct mgmt_cp_set_local_name cp;
 
@@ -1386,8 +1386,17 @@ static uint8_t set_adapter_name(uint8_t *name, uint16_t len)
 	return HAL_STATUS_FAILED;
 }
 
-static uint8_t set_discoverable_timeout(uint8_t *timeout)
+static uint8_t set_discoverable_timeout(const void *buf, uint16_t len)
 {
+	const uint32_t *timeout = buf;
+
+	if (len != sizeof(*timeout)) {
+		error("Invalid set discoverable timeout size (%u bytes), "
+							"terminating", len);
+		raise(SIGTERM);
+		return HAL_STATUS_FAILED;
+	}
+
 	/* Android handles discoverable timeout in Settings app.
 	 * There is no need to use kernel feature for that.
 	 * Just need to store this value here */
@@ -1784,33 +1793,54 @@ static bool get_discoverable_timeout(void)
 	return true;
 }
 
-static bool get_property(void *buf, uint16_t len)
+static void handle_get_adapter_prop_cmd(const void *buf, uint16_t len)
 {
-	struct hal_cmd_get_adapter_prop *cmd = buf;
+	const struct hal_cmd_get_adapter_prop *cmd = buf;
+	uint8_t status = HAL_STATUS_SUCCESS;
 
 	switch (cmd->type) {
 	case HAL_PROP_ADAPTER_ADDR:
 		get_address();
-		return true;
+		break;
 	case HAL_PROP_ADAPTER_NAME:
-		return get_name();
+		if (!get_name())
+			status = HAL_STATUS_FAILED;
+		break;
 	case HAL_PROP_ADAPTER_UUIDS:
-		return get_uuids();
+		if (!get_uuids())
+			status = HAL_STATUS_FAILED;
+		break;
 	case HAL_PROP_ADAPTER_CLASS:
-		return get_class();
+		if (!get_class())
+			status = HAL_STATUS_FAILED;
+		break;
 	case HAL_PROP_ADAPTER_TYPE:
-		return get_type();
+		if (!get_type())
+			status = HAL_STATUS_FAILED;
+		break;
 	case HAL_PROP_ADAPTER_SERVICE_REC:
-		return get_service();
+		if (!get_service())
+			status = HAL_STATUS_FAILED;
+		break;
 	case HAL_PROP_ADAPTER_SCAN_MODE:
-		return get_scan_mode();
+		if (!get_scan_mode())
+			status = HAL_STATUS_FAILED;
+		break;
 	case HAL_PROP_ADAPTER_BONDED_DEVICES:
-		return get_devices();
+		if (!get_devices())
+			status = HAL_STATUS_FAILED;
+		break;
 	case HAL_PROP_ADAPTER_DISC_TIMEOUT:
-		return get_discoverable_timeout();
+		if (!get_discoverable_timeout())
+			status = HAL_STATUS_FAILED;
+		break;
 	default:
-		return false;
+		status = HAL_STATUS_FAILED;
+		break;
 	}
+
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_BLUETOOTH,
+					HAL_OP_GET_ADAPTER_PROP, status);
 }
 
 static void get_properties(void)
@@ -1866,11 +1896,18 @@ static bool stop_discovery(void)
 	return false;
 }
 
-static uint8_t set_scan_mode(void *buf, uint16_t len)
+static uint8_t set_scan_mode(const void *buf, uint16_t len)
 {
-	uint8_t *mode = buf;
+	const uint8_t *mode = buf;
 	bool conn, disc, cur_conn, cur_disc;
 
+	if (len != sizeof(*mode)) {
+		error("Invalid set scan mode size (%u bytes), terminating",
+								len);
+		raise(SIGTERM);
+		return HAL_STATUS_FAILED;
+	}
+
 	cur_conn = adapter.current_settings & MGMT_SETTING_CONNECTABLE;
 	cur_disc = adapter.current_settings & MGMT_SETTING_DISCOVERABLE;
 
@@ -1922,21 +1959,36 @@ done:
 	return HAL_STATUS_DONE;
 }
 
-static uint8_t set_property(void *buf, uint16_t len)
+static void handle_set_adapter_prop_cmd(const void *buf, uint16_t len)
 {
-	struct hal_cmd_set_adapter_prop *cmd = buf;
+	const struct hal_cmd_set_adapter_prop *cmd = buf;
+	uint8_t status;
+
+	if (len != sizeof(*cmd) + cmd->len ) {
+		error("Invalid set adapter prop cmd (0x%x), terminating",
+								cmd->type);
+		raise(SIGTERM);
+		return;
+	}
 
 	switch (cmd->type) {
 	case HAL_PROP_ADAPTER_SCAN_MODE:
-		return set_scan_mode(cmd->val, cmd->len);
+		status = set_scan_mode(cmd->val, cmd->len);
+		break;
 	case HAL_PROP_ADAPTER_NAME:
-		return set_adapter_name(cmd->val, cmd->len);
+		status = set_adapter_name(cmd->val, cmd->len);
+		break;
 	case HAL_PROP_ADAPTER_DISC_TIMEOUT:
-		return set_discoverable_timeout(cmd->val);
+		status = set_discoverable_timeout(cmd->val, cmd->len);
+		break;
 	default:
 		DBG("Unhandled property type 0x%x", cmd->type);
-		return HAL_STATUS_FAILED;
+		status = HAL_STATUS_FAILED;
+		break;
 	}
+
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_BLUETOOTH,
+					HAL_OP_SET_ADAPTER_PROP, status);
 }
 
 static void pair_device_complete(uint8_t status, uint16_t length,
@@ -1955,9 +2007,10 @@ static void pair_device_complete(uint8_t status, uint16_t length,
 							HAL_BOND_STATE_NONE);
 }
 
-static bool create_bond(void *buf, uint16_t len)
+static void handle_create_bond_cmd(const void *buf, uint16_t len)
 {
-	struct hal_cmd_create_bond *cmd = buf;
+	const struct hal_cmd_create_bond *cmd = buf;
+	uint8_t status = HAL_STATUS_SUCCESS;
 	struct mgmt_cp_pair_device cp;
 
 	cp.io_cap = DEFAULT_IO_CAPABILITY;
@@ -1965,25 +2018,36 @@ static bool create_bond(void *buf, uint16_t len)
 	android2bdaddr(cmd->bdaddr, &cp.addr.bdaddr);
 
 	if (mgmt_send(mgmt_if, MGMT_OP_PAIR_DEVICE, adapter.index, sizeof(cp),
-				&cp, pair_device_complete, NULL, NULL) == 0)
-		return false;
+				&cp, pair_device_complete, NULL, NULL) == 0) {
+		status = HAL_STATUS_FAILED;
+		goto fail;
+	}
 
 	set_device_bond_state(&cp.addr.bdaddr, HAL_STATUS_SUCCESS,
 						HAL_BOND_STATE_BONDING);
 
-	return true;
+fail:
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_BLUETOOTH, HAL_OP_CREATE_BOND,
+								status);
 }
 
-static bool cancel_bond(void *buf, uint16_t len)
+static void handle_cancel_bond_cmd(const void *buf, uint16_t len)
 {
-	struct hal_cmd_cancel_bond *cmd = buf;
+	const struct hal_cmd_cancel_bond *cmd = buf;
 	struct mgmt_addr_info cp;
+	uint8_t status;
 
 	cp.type = BDADDR_BREDR;
 	android2bdaddr(cmd->bdaddr, &cp.bdaddr);
 
-	return mgmt_reply(mgmt_if, MGMT_OP_CANCEL_PAIR_DEVICE, adapter.index,
-					sizeof(cp), &cp, NULL, NULL, NULL) > 0;
+	if (mgmt_reply(mgmt_if, MGMT_OP_CANCEL_PAIR_DEVICE, adapter.index,
+					sizeof(cp), &cp, NULL, NULL, NULL) > 0)
+		status = HAL_STATUS_SUCCESS;
+	else
+		status = HAL_STATUS_FAILED;
+
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_BLUETOOTH, HAL_OP_CANCEL_BOND,
+								status);
 }
 
 static void unpair_device_complete(uint8_t status, uint16_t length,
@@ -2000,23 +2064,31 @@ static void unpair_device_complete(uint8_t status, uint16_t length,
 							HAL_BOND_STATE_NONE);
 }
 
-static bool remove_bond(void *buf, uint16_t len)
+static void handle_remove_bond_cmd(const void *buf, uint16_t len)
 {
-	struct hal_cmd_remove_bond *cmd = buf;
+	const struct hal_cmd_remove_bond *cmd = buf;
 	struct mgmt_cp_unpair_device cp;
+	uint8_t status;
 
 	cp.disconnect = 1;
 	cp.addr.type = BDADDR_BREDR;
 	android2bdaddr(cmd->bdaddr, &cp.addr.bdaddr);
 
-	return mgmt_send(mgmt_if, MGMT_OP_UNPAIR_DEVICE, adapter.index,
+	if (mgmt_send(mgmt_if, MGMT_OP_UNPAIR_DEVICE, adapter.index,
 				sizeof(cp), &cp, unpair_device_complete,
-				NULL, NULL) > 0;
+				NULL, NULL) > 0)
+		status = HAL_STATUS_SUCCESS;
+	else
+		status = HAL_STATUS_FAILED;
+
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_BLUETOOTH, HAL_OP_REMOVE_BOND,
+								status);
 }
 
-static uint8_t pin_reply(void *buf, uint16_t len)
+static void handle_pin_reply_cmd(const void *buf, uint16_t len)
 {
-	struct hal_cmd_pin_reply *cmd = buf;
+	const struct hal_cmd_pin_reply *cmd = buf;
+	uint8_t status = HAL_STATUS_SUCCESS;
 	bdaddr_t bdaddr;
 	char addr[18];
 
@@ -2025,8 +2097,10 @@ static uint8_t pin_reply(void *buf, uint16_t len)
 
 	DBG("%s accept %u pin_len %u", addr, cmd->accept, cmd->pin_len);
 
-	if (!cmd->accept && cmd->pin_len)
-		return HAL_STATUS_INVALID;
+	if (!cmd->accept && cmd->pin_len) {
+		status = HAL_STATUS_INVALID;
+		goto fail;
+	}
 
 	if (cmd->accept) {
 		struct mgmt_cp_pin_code_reply rp;
@@ -2040,7 +2114,7 @@ static uint8_t pin_reply(void *buf, uint16_t len)
 
 		if (mgmt_reply(mgmt_if, MGMT_OP_PIN_CODE_REPLY, adapter.index,
 				sizeof(rp), &rp, NULL, NULL, NULL) == 0)
-			return HAL_STATUS_FAILED;
+			status = HAL_STATUS_FAILED;
 	} else {
 		struct mgmt_cp_pin_code_neg_reply rp;
 
@@ -2050,10 +2124,12 @@ static uint8_t pin_reply(void *buf, uint16_t len)
 		if (mgmt_reply(mgmt_if, MGMT_OP_PIN_CODE_NEG_REPLY,
 						adapter.index, sizeof(rp), &rp,
 						NULL, NULL, NULL) == 0)
-			return HAL_STATUS_FAILED;
+			status = HAL_STATUS_FAILED;
 	}
 
-	return HAL_STATUS_SUCCESS;
+fail:
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_BLUETOOTH, HAL_OP_PIN_REPLY,
+								status);
 }
 
 static uint8_t user_confirm_reply(const bdaddr_t *bdaddr, bool accept)
@@ -2110,11 +2186,11 @@ static uint8_t user_passkey_reply(const bdaddr_t *bdaddr, bool accept,
 	return HAL_STATUS_SUCCESS;
 }
 
-static uint8_t ssp_reply(void *buf, uint16_t len)
+static void handle_ssp_reply_cmd(const void *buf, uint16_t len)
 {
-	struct hal_cmd_ssp_reply *cmd = buf;
-	uint8_t status;
+	const struct hal_cmd_ssp_reply *cmd = buf;
 	bdaddr_t bdaddr;
+	uint8_t status;
 	char addr[18];
 
 	/* TODO should parameters sanity be verified here? */
@@ -2141,138 +2217,250 @@ static uint8_t ssp_reply(void *buf, uint16_t len)
 		break;
 	}
 
-	return status;
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_BLUETOOTH, HAL_OP_SSP_REPLY,
+								status);
 }
 
-static uint8_t get_remote_services(void *buf, uint16_t len)
+static void handle_get_remote_services_cmd(const void *buf, uint16_t len)
 {
-	struct hal_cmd_get_remote_services *cmd = buf;
+	const struct hal_cmd_get_remote_services *cmd = buf;
+	uint8_t status;
 	bdaddr_t addr;
 
 	android2bdaddr(&cmd->bdaddr, &addr);
 
-	return browse_remote_sdp(&addr);
+	status = browse_remote_sdp(&addr);
+
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_BLUETOOTH,
+					HAL_OP_GET_REMOTE_SERVICES, status);
 }
 
-void bt_bluetooth_handle_cmd(int sk, uint8_t opcode, void *buf, uint16_t len)
+static void handle_enable_cmd(const void *buf, uint16_t len)
 {
-	uint8_t status = HAL_STATUS_FAILED;
-
-	switch (opcode) {
-	case HAL_OP_ENABLE:
-		/* Framework expects all properties to be emitted while
-		 * enabling adapter */
-		get_properties();
-
-		if (adapter.current_settings & MGMT_SETTING_POWERED) {
-			status = HAL_STATUS_DONE;
-			goto error;
-		}
+	uint8_t status = HAL_STATUS_SUCCESS;
 
-		if (!set_mode(MGMT_OP_SET_POWERED, 0x01))
-			goto error;
+	/* Framework expects all properties to be emitted while
+	 * enabling adapter */
+	get_properties();
 
-		break;
-	case HAL_OP_DISABLE:
-		if (!(adapter.current_settings & MGMT_SETTING_POWERED)) {
-			status = HAL_STATUS_DONE;
-			goto error;
-		}
+	if (adapter.current_settings & MGMT_SETTING_POWERED) {
+		status = HAL_STATUS_DONE;
+		goto done;
+	}
 
-		if (!set_mode(MGMT_OP_SET_POWERED, 0x00))
-			goto error;
+	if (!set_mode(MGMT_OP_SET_POWERED, 0x01))
+		status = HAL_STATUS_FAILED;
 
-		break;
-	case HAL_OP_GET_ADAPTER_PROPS:
-		get_properties();
+done:
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_BLUETOOTH, HAL_OP_ENABLE,
+								status);
+}
 
-		break;
-	case HAL_OP_GET_ADAPTER_PROP:
-		if (!get_property(buf, len))
-			goto error;
+static void handle_disable_cmd(const void *buf, uint16_t len)
+{
+	uint8_t status = HAL_STATUS_SUCCESS;
 
-		break;
-	case HAL_OP_SET_ADAPTER_PROP:
-		status = set_property(buf, len);
-		if (status != HAL_STATUS_SUCCESS && status != HAL_STATUS_DONE)
-			goto error;
+	if (!(adapter.current_settings & MGMT_SETTING_POWERED)) {
+		status = HAL_STATUS_DONE;
+		goto done;
+	}
 
-		break;
-	case HAL_OP_CREATE_BOND:
-		if (!create_bond(buf, len))
-			goto error;
+	if (!set_mode(MGMT_OP_SET_POWERED, 0x00))
+		status = HAL_STATUS_FAILED;
+done:
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_BLUETOOTH, HAL_OP_DISABLE,
+									status);
+}
 
-		break;
-	case HAL_OP_CANCEL_BOND:
-		if (!cancel_bond(buf, len))
-			goto error;
+static void handle_get_adapter_props_cmd(const void *buf, uint16_t len)
+{
+	get_properties();
 
-		break;
-	case HAL_OP_REMOVE_BOND:
-		if (!remove_bond(buf, len))
-			goto error;
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_BLUETOOTH,
+				HAL_OP_GET_ADAPTER_PROPS, HAL_STATUS_SUCCESS);
+}
 
-		break;
-	case HAL_OP_PIN_REPLY:
-		status = pin_reply(buf, len);
-		if (status != HAL_STATUS_SUCCESS)
-			goto error;
+static void handle_get_remote_device_props_cmd(const void *buf, uint16_t len)
+{
+	/* TODO */
 
-		break;
-	case HAL_OP_SSP_REPLY:
-		status = ssp_reply(buf, len);
-		if (status != HAL_STATUS_SUCCESS)
-			goto error;
-		break;
-	case HAL_OP_START_DISCOVERY:
-		if (adapter.discovering) {
-			status = HAL_STATUS_DONE;
-			goto error;
-		}
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_BLUETOOTH,
+			HAL_OP_GET_REMOTE_DEVICE_PROPS, HAL_STATUS_FAILED);
+}
 
-		if (!(adapter.current_settings & MGMT_SETTING_POWERED)) {
-			status = HAL_STATUS_NOT_READY;
-			goto error;
-		}
+static void handle_get_remote_device_prop_cmd(const void *buf, uint16_t len)
+{
+	/* TODO */
 
-		if (!start_discovery())
-			goto error;
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_BLUETOOTH,
+			HAL_OP_GET_REMOTE_DEVICE_PROP, HAL_STATUS_FAILED);
+}
 
-		break;
-	case HAL_OP_CANCEL_DISCOVERY:
-		if (!adapter.discovering) {
-			status = HAL_STATUS_DONE;
-			goto error;
-		}
+static void handle_set_remote_device_prop_cmd(const void *buf, uint16_t len)
+{
+	const struct hal_cmd_set_remote_device_prop *cmd = buf;
+	uint8_t status;
 
-		if (!(adapter.current_settings & MGMT_SETTING_POWERED)) {
-			status = HAL_STATUS_NOT_READY;
-			goto error;
-		}
+	if (len != sizeof(*cmd) + cmd->len ) {
+		error("Invalid set remote device prop cmd (0x%x), terminating",
+								cmd->type);
+		raise(SIGTERM);
+		return;
+	}
 
-		if (!stop_discovery())
-			goto error;
+	/* TODO */
 
-		break;
-	case HAL_OP_GET_REMOTE_SERVICES:
-		status = get_remote_services(buf, len);
-		if (status != HAL_STATUS_SUCCESS)
-			goto error;
-		break;
+	switch (cmd->type) {
 	default:
-		DBG("Unhandled command, opcode 0x%x", opcode);
-		goto error;
+		DBG("Unhandled property type 0x%x", cmd->type);
+		status = HAL_STATUS_FAILED;
+		break;
 	}
 
-	ipc_send(sk, HAL_SERVICE_ID_BLUETOOTH, opcode, 0, NULL, -1);
-	return;
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_BLUETOOTH,
+					HAL_OP_SET_REMOTE_DEVICE_PROP, status);
+}
 
-error:
-	error("Error handling command 0x%02x status %u", opcode, status);
+static void handle_get_remote_service_rec_cmd(const void *buf, uint16_t len)
+{
+	/* TODO */
 
-	ipc_send_rsp(sk, HAL_SERVICE_ID_BLUETOOTH, opcode, status);
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_BLUETOOTH,
+			HAL_OP_GET_REMOTE_SERVICE_REC, HAL_STATUS_FAILED);
 }
 
+static void handle_start_discovery_cmd(const void *buf, uint16_t len)
+{
+	uint8_t status = HAL_STATUS_SUCCESS;
+
+	if (adapter.discovering) {
+		status = HAL_STATUS_DONE;
+		goto fail;
+	}
+
+	if (!(adapter.current_settings & MGMT_SETTING_POWERED)) {
+		status = HAL_STATUS_NOT_READY;
+		goto fail;
+	}
+
+	if (!start_discovery())
+		status = HAL_STATUS_FAILED;
+
+fail:
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_BLUETOOTH,
+					HAL_OP_START_DISCOVERY, status);
+}
+
+static void handle_cancel_discovery_cmd(const void *buf, uint16_t len)
+{
+	uint8_t status = HAL_STATUS_SUCCESS;
+
+	if (!adapter.discovering) {
+		status = HAL_STATUS_DONE;
+		goto fail;
+	}
+
+	if (!(adapter.current_settings & MGMT_SETTING_POWERED)) {
+		status = HAL_STATUS_NOT_READY;
+		goto fail;
+	}
+
+	if (!stop_discovery())
+		status = HAL_STATUS_FAILED;
+
+fail:
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_BLUETOOTH,
+					HAL_OP_CANCEL_DISCOVERY, status);
+}
+
+static const struct ipc_handler cmd_handlers[] = {
+	{	/* HAL_OP_ENABLE */
+		.handler = handle_enable_cmd,
+		.var_len = false,
+		.data_len = 0
+	},
+	{	/* HAL_OP_DISABLE */
+		.handler = handle_disable_cmd,
+		.var_len = false,
+		.data_len = 0
+	},
+	{	/* HAL_OP_GET_ADAPTER_PROPS */
+		.handler = handle_get_adapter_props_cmd,
+		.var_len = false,
+		.data_len = 0
+	},
+	{	/* HAL_OP_GET_ADAPTER_PROP */
+		.handler = handle_get_adapter_prop_cmd,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_get_adapter_prop)
+	},
+	{	/* HAL_OP_SET_ADAPTER_PROP */
+		.handler = handle_set_adapter_prop_cmd,
+		.var_len = true,
+		.data_len = sizeof(struct hal_cmd_set_adapter_prop)
+	},
+	{	/* HAL_OP_GET_REMOTE_DEVICE_PROPS */
+		.handler = handle_get_remote_device_props_cmd,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_get_remote_device_props)
+	},
+	{	/* HAL_OP_GET_REMOTE_DEVICE_PROP */
+		.handler = handle_get_remote_device_prop_cmd,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_get_remote_device_prop)
+	},
+	{	/* HAL_OP_SET_REMOTE_DEVICE_PROP */
+		.handler = handle_set_remote_device_prop_cmd,
+		.var_len = true,
+		.data_len = sizeof(struct hal_cmd_set_remote_device_prop)
+	},
+	{	/* HAL_OP_GET_REMOTE_SERVICE_REC */
+		.handler = handle_get_remote_service_rec_cmd,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_get_remote_service_rec)
+	},
+	{	/* HAL_OP_GET_REMOTE_SERVICES */
+		.handler = handle_get_remote_services_cmd,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_get_remote_services)
+	},
+	{	/* HAL_OP_START_DISCOVERY */
+		.handler = handle_start_discovery_cmd,
+		.var_len = false,
+		.data_len = 0
+	},
+	{	/* HAL_OP_CANCEL_DISCOVERY */
+		.handler = handle_cancel_discovery_cmd,
+		.var_len = false,
+		.data_len = 0
+	},
+	{	/* HAL_OP_CREATE_BOND */
+		.handler = handle_create_bond_cmd,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_create_bond)
+	},
+	{	/* HAL_OP_REMOVE_BOND */
+		.handler = handle_remove_bond_cmd,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_remove_bond)
+	},
+	{	/* HAL_OP_CANCEL_BOND */
+		.handler = handle_cancel_bond_cmd,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_cancel_bond)
+	},
+	{	/* HAL_OP_PIN_REPLY */
+		.handler = handle_pin_reply_cmd,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_pin_reply)
+	},
+	{	/* HAL_OP_SSP_REPLY */
+		.handler = handle_ssp_reply_cmd,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_ssp_reply)
+	},
+};
+
 bool bt_bluetooth_register(int cmd_sk, int notif_sk)
 {
 	DBG("");
@@ -2283,6 +2471,8 @@ bool bt_bluetooth_register(int cmd_sk, int notif_sk)
 	command_sk = cmd_sk;
 	notification_sk = notif_sk;
 
+	ipc_register(HAL_SERVICE_ID_BLUETOOTH, cmd_handlers,
+				sizeof(cmd_handlers)/sizeof(cmd_handlers[0]));
 
 	return true;
 }
@@ -2296,4 +2486,6 @@ void bt_bluetooth_unregister(void)
 
 	command_sk = -1;
 	notification_sk = -1;
+
+	ipc_unregister(HAL_SERVICE_ID_CORE);
 }
-- 
1.8.3.2


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

* [PATCH 06/13] android/bluetooth: Add stubs for missing commands handlers
  2013-11-27 11:54 [PATCH 00/13] android: IPC improvements - daemon part Szymon Janc
                   ` (4 preceding siblings ...)
  2013-11-27 11:54 ` [PATCH 05/13] android/bluetooth: Use generic IPC msg handling for commands Szymon Janc
@ 2013-11-27 11:54 ` Szymon Janc
  2013-11-27 11:54 ` [PATCH 07/13] android/hidhost: Use generic IPC message handling for commands Szymon Janc
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Szymon Janc @ 2013-11-27 11:54 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

Unknown IPC command is now considered IPC error so all commands should
at least have stub implementation.
---
 android/bluetooth.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index c7ec19b..81f9d79 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -2373,6 +2373,30 @@ fail:
 					HAL_OP_CANCEL_DISCOVERY, status);
 }
 
+static void handle_dut_mode_conf_cmd(const void *buf, uint16_t len)
+{
+	/* TODO */
+
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_BLUETOOTH,
+				HAL_OP_DUT_MODE_CONF, HAL_STATUS_FAILED);
+}
+
+static void handle_dut_mode_send_cmd(const void *buf, uint16_t len)
+{
+	/* TODO */
+
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_BLUETOOTH,
+				HAL_OP_DUT_MODE_SEND, HAL_STATUS_FAILED);
+}
+
+static void handle_le_test_mode_cmd(const void *buf, uint16_t len)
+{
+	/* TODO */
+
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_BLUETOOTH,
+				HAL_OP_LE_TEST_MODE, HAL_STATUS_FAILED);
+}
+
 static const struct ipc_handler cmd_handlers[] = {
 	{	/* HAL_OP_ENABLE */
 		.handler = handle_enable_cmd,
@@ -2459,6 +2483,21 @@ static const struct ipc_handler cmd_handlers[] = {
 		.var_len = false,
 		.data_len = sizeof(struct hal_cmd_ssp_reply)
 	},
+	{	/* HAL_OP_DUT_MODE_CONF */
+		.handler = handle_dut_mode_conf_cmd,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_dut_mode_conf)
+	},
+	{	/* HAL_OP_DUT_MODE_SEND */
+		.handler = handle_dut_mode_send_cmd,
+		.var_len = true,
+		.data_len = sizeof(struct hal_cmd_dut_mode_send)
+	},
+	{	/* HAL_OP_DUT_MODE_SEND */
+		.handler = handle_le_test_mode_cmd,
+		.var_len = true,
+		.data_len = sizeof(struct hal_cmd_le_test_mode)
+	},
 };
 
 bool bt_bluetooth_register(int cmd_sk, int notif_sk)
-- 
1.8.3.2


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

* [PATCH 07/13] android/hidhost: Use generic IPC message handling for commands
  2013-11-27 11:54 [PATCH 00/13] android: IPC improvements - daemon part Szymon Janc
                   ` (5 preceding siblings ...)
  2013-11-27 11:54 ` [PATCH 06/13] android/bluetooth: Add stubs for missing commands handlers Szymon Janc
@ 2013-11-27 11:54 ` Szymon Janc
  2013-11-27 11:55 ` [PATCH 08/13] android/hidhost: Use stack buffer in bt_hid_get_report Szymon Janc
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Szymon Janc @ 2013-11-27 11:54 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

Handlers are registered on service register and unregistered on
unregister.
---
 android/hidhost.c | 317 +++++++++++++++++++++++++++++++++---------------------
 android/hidhost.h |   2 -
 2 files changed, 193 insertions(+), 126 deletions(-)

diff --git a/android/hidhost.c b/android/hidhost.c
index d97af03..5b98152 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -723,9 +723,10 @@ fail:
 	hid_device_free(dev);
 }
 
-static uint8_t bt_hid_connect(struct hal_cmd_hidhost_connect *cmd,
-								uint16_t len)
+static void bt_hid_connect(const void *buf, uint16_t len)
 {
+	const struct hal_cmd_hidhost_connect *cmd = buf;
+	uint8_t status = HAL_STATUS_SUCCESS;
 	struct hid_device *dev;
 	char addr[18];
 	bdaddr_t dst;
@@ -734,14 +735,13 @@ static uint8_t bt_hid_connect(struct hal_cmd_hidhost_connect *cmd,
 
 	DBG("");
 
-	if (len < sizeof(*cmd))
-		return HAL_STATUS_INVALID;
-
 	android2bdaddr(&cmd->bdaddr, &dst);
 
 	l = g_slist_find_custom(devices, &dst, device_cmp);
-	if (l)
-		return HAL_STATUS_FAILED;
+	if (l) {
+		status = HAL_STATUS_FAILED;
+		goto fail;
+	}
 
 	dev = g_new0(struct hid_device, 1);
 	bacpy(&dev->dst, &dst);
@@ -755,32 +755,35 @@ static uint8_t bt_hid_connect(struct hal_cmd_hidhost_connect *cmd,
 					hid_sdp_search_cb, dev, NULL) < 0) {
 		error("Failed to search sdp details");
 		hid_device_free(dev);
-		return HAL_STATUS_FAILED;
+		status = HAL_STATUS_FAILED;
+		goto fail;
 	}
 
 	devices = g_slist_append(devices, dev);
 	bt_hid_notify_state(dev, HAL_HIDHOST_STATE_CONNECTING);
 
-	return HAL_STATUS_SUCCESS;
+fail:
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_HIDHOST,
+					HAL_OP_HIDHOST_CONNECT, status);
 }
 
-static uint8_t bt_hid_disconnect(struct hal_cmd_hidhost_disconnect *cmd,
-								uint16_t len)
+static void bt_hid_disconnect(const void *buf, uint16_t len)
 {
+	const struct hal_cmd_hidhost_disconnect *cmd = buf;
+	uint8_t status = HAL_STATUS_SUCCESS;
 	struct hid_device *dev;
 	GSList *l;
 	bdaddr_t dst;
 
 	DBG("");
 
-	if (len < sizeof(*cmd))
-		return HAL_STATUS_INVALID;
-
 	android2bdaddr(&cmd->bdaddr, &dst);
 
 	l = g_slist_find_custom(devices, &dst, device_cmp);
-	if (!l)
-		return HAL_STATUS_FAILED;
+	if (!l) {
+		status = HAL_STATUS_FAILED;
+		goto fail;
+	}
 
 	dev = l->data;
 
@@ -793,12 +796,15 @@ static uint8_t bt_hid_disconnect(struct hal_cmd_hidhost_disconnect *cmd,
 
 	bt_hid_notify_state(dev, HAL_HIDHOST_STATE_DISCONNECTING);
 
-	return HAL_STATUS_SUCCESS;
+fail:
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_HIDHOST,
+					HAL_OP_HIDHOST_DISCONNECT, status);
 }
 
-static uint8_t bt_hid_virtual_unplug(struct hal_cmd_hidhost_virtual_unplug *cmd,
-								uint16_t len)
+static void bt_hid_virtual_unplug(const void *buf, uint16_t len)
 {
+	const struct hal_cmd_hidhost_virtual_unplug *cmd = buf;
+	uint8_t status = HAL_STATUS_FAILED;
 	struct hid_device *dev;
 	GSList *l;
 	bdaddr_t dst;
@@ -807,19 +813,16 @@ static uint8_t bt_hid_virtual_unplug(struct hal_cmd_hidhost_virtual_unplug *cmd,
 
 	DBG("");
 
-	if (len < sizeof(*cmd))
-		return HAL_STATUS_INVALID;
-
 	android2bdaddr(&cmd->bdaddr, &dst);
 
 	l = g_slist_find_custom(devices, &dst, device_cmp);
 	if (!l)
-		return HAL_STATUS_FAILED;
+		goto fail;
 
 	dev = l->data;
 
 	if (!(dev->ctrl_io))
-		return HAL_STATUS_FAILED;
+		goto fail;
 
 	hdr = HID_MSG_CONTROL | HID_VIRTUAL_CABLE_UNPLUG;
 
@@ -828,7 +831,7 @@ static uint8_t bt_hid_virtual_unplug(struct hal_cmd_hidhost_virtual_unplug *cmd,
 	if (write(fd, &hdr, sizeof(hdr)) < 0) {
 		error("error writing virtual unplug command: %s (%d)",
 						strerror(errno), errno);
-		return HAL_STATUS_FAILED;
+		goto fail;
 	}
 
 	/* Wait either channels to HUP */
@@ -840,10 +843,14 @@ static uint8_t bt_hid_virtual_unplug(struct hal_cmd_hidhost_virtual_unplug *cmd,
 
 	bt_hid_notify_state(dev, HAL_HIDHOST_STATE_DISCONNECTING);
 
-	return HAL_STATUS_SUCCESS;
+	status = HAL_STATUS_SUCCESS;
+
+fail:
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_HIDHOST,
+					HAL_OP_HIDHOST_VIRTUAL_UNPLUG, status);
 }
 
-static uint8_t bt_hid_info(struct hal_cmd_hidhost_set_info *cmd, uint16_t len)
+static void bt_hid_info(const void *buf, uint16_t len)
 {
 	/* Data from hal_cmd_hidhost_set_info is usefull only when we create
 	 * UHID device. Once device is created all the transactions will be
@@ -851,12 +858,14 @@ static uint8_t bt_hid_info(struct hal_cmd_hidhost_set_info *cmd, uint16_t len)
 	 * once device is created with HID internals. */
 	DBG("Not supported");
 
-	return HAL_STATUS_UNSUPPORTED;
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_HIDHOST,
+			HAL_OP_HIDHOST_SET_INFO, HAL_STATUS_UNSUPPORTED);
 }
 
-static uint8_t bt_hid_get_protocol(struct hal_cmd_hidhost_get_protocol *cmd,
-								uint16_t len)
+static void bt_hid_get_protocol(const void *buf, uint16_t len)
 {
+	const struct hal_cmd_hidhost_get_protocol *cmd = buf;
+	uint8_t status = HAL_STATUS_SUCCESS;
 	struct hid_device *dev;
 	GSList *l;
 	bdaddr_t dst;
@@ -865,19 +874,20 @@ static uint8_t bt_hid_get_protocol(struct hal_cmd_hidhost_get_protocol *cmd,
 
 	DBG("");
 
-	if (len < sizeof(*cmd))
-		return HAL_STATUS_INVALID;
-
 	android2bdaddr(&cmd->bdaddr, &dst);
 
 	l = g_slist_find_custom(devices, &dst, device_cmp);
-	if (!l)
-		return HAL_STATUS_FAILED;
+	if (!l) {
+		status = HAL_STATUS_FAILED;
+		goto fail;
+	}
 
 	dev = l->data;
 
-	if (dev->boot_dev)
-		return HAL_STATUS_UNSUPPORTED;
+	if (dev->boot_dev) {
+		status = HAL_STATUS_UNSUPPORTED;
+		goto fail;
+	}
 
 	hdr = HID_MSG_GET_PROTOCOL | cmd->mode;
 	fd = g_io_channel_unix_get_fd(dev->ctrl_io);
@@ -885,16 +895,21 @@ static uint8_t bt_hid_get_protocol(struct hal_cmd_hidhost_get_protocol *cmd,
 	if (write(fd, &hdr, sizeof(hdr)) < 0) {
 		error("error writing device_get_protocol: %s (%d)",
 						strerror(errno), errno);
-		return HAL_STATUS_FAILED;
+		status = HAL_STATUS_FAILED;
+		goto fail;
 	}
 
 	dev->last_hid_msg = HID_MSG_GET_PROTOCOL;
-	return HAL_STATUS_SUCCESS;
+
+fail:
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_HIDHOST,
+					HAL_OP_HIDHOST_GET_PROTOCOL, status);
 }
 
-static uint8_t bt_hid_set_protocol(struct hal_cmd_hidhost_set_protocol *cmd,
-								uint16_t len)
+static void bt_hid_set_protocol(const void *buf, uint16_t len)
 {
+	const struct hal_cmd_hidhost_set_protocol *cmd = buf;
+	uint8_t status = HAL_STATUS_SUCCESS;
 	struct hid_device *dev;
 	GSList *l;
 	bdaddr_t dst;
@@ -903,19 +918,20 @@ static uint8_t bt_hid_set_protocol(struct hal_cmd_hidhost_set_protocol *cmd,
 
 	DBG("");
 
-	if (len < sizeof(*cmd))
-		return HAL_STATUS_INVALID;
-
 	android2bdaddr(&cmd->bdaddr, &dst);
 
 	l = g_slist_find_custom(devices, &dst, device_cmp);
-	if (!l)
-		return HAL_STATUS_FAILED;
+	if (!l) {
+		status = HAL_STATUS_FAILED;
+		goto fail;
+	}
 
 	dev = l->data;
 
-	if (dev->boot_dev)
-		return HAL_STATUS_UNSUPPORTED;
+	if (dev->boot_dev) {
+		status = HAL_STATUS_UNSUPPORTED;
+		goto fail;
+	}
 
 	hdr = HID_MSG_SET_PROTOCOL | cmd->mode;
 	fd = g_io_channel_unix_get_fd(dev->ctrl_io);
@@ -923,16 +939,21 @@ static uint8_t bt_hid_set_protocol(struct hal_cmd_hidhost_set_protocol *cmd,
 	if (write(fd, &hdr, sizeof(hdr)) < 0) {
 		error("error writing device_set_protocol: %s (%d)",
 						strerror(errno), errno);
-		return HAL_STATUS_FAILED;
+		status = HAL_STATUS_FAILED;
+		goto fail;
 	}
 
 	dev->last_hid_msg = HID_MSG_SET_PROTOCOL;
-	return HAL_STATUS_SUCCESS;
+
+fail:
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_HIDHOST,
+					HAL_OP_HIDHOST_SET_PROTOCOL, status);
 }
 
-static uint8_t bt_hid_get_report(struct hal_cmd_hidhost_get_report *cmd,
-								uint16_t len)
+static void bt_hid_get_report(const void *buf, uint16_t len)
 {
+	const struct hal_cmd_hidhost_get_report *cmd = buf;
+	uint8_t status = HAL_STATUS_SUCCESS;
 	struct hid_device *dev;
 	GSList *l;
 	bdaddr_t dst;
@@ -942,20 +963,21 @@ static uint8_t bt_hid_get_report(struct hal_cmd_hidhost_get_report *cmd,
 
 	DBG("");
 
-	if (len < sizeof(*cmd))
-		return HAL_STATUS_INVALID;
-
 	android2bdaddr(&cmd->bdaddr, &dst);
 
 	l = g_slist_find_custom(devices, &dst, device_cmp);
-	if (!l)
-		return HAL_STATUS_FAILED;
+	if (!l) {
+		status = HAL_STATUS_FAILED;
+		goto fail;
+	}
 
 	dev = l->data;
 	req_size = (cmd->buf_size > 0) ? 4 : 2;
 	req = g_try_malloc0(req_size);
-	if (!req)
-		return HAL_STATUS_NOMEM;
+	if (!req) {
+		status = HAL_STATUS_NOMEM;
+		goto fail;
+	}
 
 	req[0] = HID_MSG_GET_REPORT | cmd->type;
 	req[1] = cmd->id;
@@ -971,17 +993,22 @@ static uint8_t bt_hid_get_report(struct hal_cmd_hidhost_get_report *cmd,
 		error("error writing hid_get_report: %s (%d)",
 						strerror(errno), errno);
 		g_free(req);
-		return HAL_STATUS_FAILED;
+		status = HAL_STATUS_FAILED;
+		goto fail;
 	}
 
 	dev->last_hid_msg = HID_MSG_GET_REPORT;
 	g_free(req);
-	return HAL_STATUS_SUCCESS;
+
+fail:
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_HIDHOST,
+					HAL_OP_HIDHOST_GET_REPORT, status);
 }
 
-static uint8_t bt_hid_set_report(struct hal_cmd_hidhost_set_report *cmd,
-								uint16_t len)
+static void bt_hid_set_report(const void *buf, uint16_t len)
 {
+	const struct hal_cmd_hidhost_set_report *cmd = buf;
+	uint8_t status = HAL_STATUS_SUCCESS;
 	struct hid_device *dev;
 	GSList *l;
 	bdaddr_t dst;
@@ -991,24 +1018,34 @@ static uint8_t bt_hid_set_report(struct hal_cmd_hidhost_set_report *cmd,
 
 	DBG("");
 
-	if (len < sizeof(*cmd))
-		return HAL_STATUS_INVALID;
+	if (len != sizeof(*cmd) + cmd->len) {
+		error("Invalid hid set report size (%u bytes), terminating",
+									len);
+		raise(SIGTERM);
+		return;
+	}
 
 	android2bdaddr(&cmd->bdaddr, &dst);
 
 	l = g_slist_find_custom(devices, &dst, device_cmp);
-	if (!l)
-		return HAL_STATUS_FAILED;
+	if (!l) {
+		status = HAL_STATUS_FAILED;
+		goto fail;
+	}
 
 	dev = l->data;
 
-	if (!(dev->ctrl_io))
-		return HAL_STATUS_FAILED;
+	if (!(dev->ctrl_io)) {
+		status = HAL_STATUS_FAILED;
+		goto fail;
+	}
 
 	req_size = 1 + (cmd->len / 2);
 	req = g_try_malloc0(req_size);
-	if (!req)
-		return HAL_STATUS_NOMEM;
+	if (!req) {
+		status = HAL_STATUS_NOMEM;
+		goto fail;
+	}
 
 	req[0] = HID_MSG_SET_REPORT | cmd->type;
 	/* Report data coming to HAL is in ascii format, HAL sends
@@ -1022,17 +1059,22 @@ static uint8_t bt_hid_set_report(struct hal_cmd_hidhost_set_report *cmd,
 		error("error writing hid_set_report: %s (%d)",
 						strerror(errno), errno);
 		g_free(req);
-		return HAL_STATUS_FAILED;
+		status = HAL_STATUS_FAILED;
+		goto fail;
 	}
 
 	dev->last_hid_msg = HID_MSG_SET_REPORT;
 	g_free(req);
-	return HAL_STATUS_SUCCESS;
+
+fail:
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_HIDHOST,
+					HAL_OP_HIDHOST_SET_REPORT, status);
 }
 
-static uint8_t bt_hid_send_data(struct hal_cmd_hidhost_send_data *cmd,
-								uint16_t len)
+static void bt_hid_send_data(const void *buf, uint16_t len)
 {
+	const struct hal_cmd_hidhost_send_data *cmd = buf;
+	uint8_t status = HAL_STATUS_SUCCESS;
 	struct hid_device *dev;
 	GSList *l;
 	bdaddr_t dst;
@@ -1042,24 +1084,34 @@ static uint8_t bt_hid_send_data(struct hal_cmd_hidhost_send_data *cmd,
 
 	DBG("");
 
-	if (len < sizeof(*cmd))
-		return HAL_STATUS_INVALID;
+	if (len != sizeof(*cmd) + cmd->len) {
+		error("Invalid hid send data size (%u bytes), terminating",
+									len);
+		raise(SIGTERM);
+		return;
+	}
 
 	android2bdaddr(&cmd->bdaddr, &dst);
 
 	l = g_slist_find_custom(devices, &dst, device_cmp);
-	if (!l)
-		return HAL_STATUS_FAILED;
+	if (!l) {
+		status = HAL_STATUS_FAILED;
+		goto fail;
+	}
 
 	dev = l->data;
 
-	if (!(dev->intr_io))
-		return HAL_STATUS_FAILED;
+	if (!(dev->intr_io)) {
+		status = HAL_STATUS_FAILED;
+		goto fail;
+	}
 
 	req_size = 1 + (cmd->len / 2);
 	req = g_try_malloc0(req_size);
-	if (!req)
-		return HAL_STATUS_NOMEM;
+	if (!req) {
+		status = HAL_STATUS_NOMEM;
+		goto fail;
+	}
 
 	req[0] = HID_MSG_DATA | HID_DATA_TYPE_OUTPUT;
 	/* Report data coming to HAL is in ascii format, HAL sends
@@ -1073,53 +1125,65 @@ static uint8_t bt_hid_send_data(struct hal_cmd_hidhost_send_data *cmd,
 		error("error writing data to HID device: %s (%d)",
 						strerror(errno), errno);
 		g_free(req);
-		return HAL_STATUS_FAILED;
+		status = HAL_STATUS_FAILED;
+		goto fail;
 	}
 
 	g_free(req);
-	return HAL_STATUS_SUCCESS;
-}
-
-void bt_hid_handle_cmd(int sk, uint8_t opcode, void *buf, uint16_t len)
-{
-	uint8_t status = HAL_STATUS_FAILED;
 
-	switch (opcode) {
-	case HAL_OP_HIDHOST_CONNECT:
-		status = bt_hid_connect(buf, len);
-		break;
-	case HAL_OP_HIDHOST_DISCONNECT:
-		status = bt_hid_disconnect(buf, len);
-		break;
-	case HAL_OP_HIDHOST_VIRTUAL_UNPLUG:
-		status = bt_hid_virtual_unplug(buf, len);
-		break;
-	case HAL_OP_HIDHOST_SET_INFO:
-		status = bt_hid_info(buf, len);
-		break;
-	case HAL_OP_HIDHOST_GET_PROTOCOL:
-		status = bt_hid_get_protocol(buf, len);
-		break;
-	case HAL_OP_HIDHOST_SET_PROTOCOL:
-		status = bt_hid_set_protocol(buf, len);
-		break;
-	case HAL_OP_HIDHOST_GET_REPORT:
-		status = bt_hid_get_report(buf, len);
-		break;
-	case HAL_OP_HIDHOST_SET_REPORT:
-		status = bt_hid_set_report(buf, len);
-		break;
-	case HAL_OP_HIDHOST_SEND_DATA:
-		status = bt_hid_send_data(buf, len);
-		break;
-	default:
-		DBG("Unhandled command, opcode 0x%x", opcode);
-		break;
-	}
-
-	ipc_send_rsp(sk, HAL_SERVICE_ID_HIDHOST, opcode, status);
+fail:
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_HIDHOST,
+					HAL_OP_HIDHOST_SEND_DATA, status);
 }
 
+static const struct ipc_handler cmd_handlers[] = {
+	{	/* HAL_OP_HIDHOST_CONNECT */
+		.handler = bt_hid_connect,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_hidhost_connect)
+	},
+	{	/* HAL_OP_HIDHOST_DISCONNECT */
+		.handler = bt_hid_disconnect,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_hidhost_disconnect)
+	},
+	{	/* HAL_OP_HIDHOST_VIRTUAL_UNPLUG */
+		.handler = bt_hid_virtual_unplug,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_hidhost_virtual_unplug)
+	},
+	{	/* HAL_OP_HIDHOST_SET_INFO */
+		.handler = bt_hid_info,
+		.var_len = true,
+		.data_len = sizeof(struct hal_cmd_hidhost_set_info)
+	},
+	{	/* HAL_OP_HIDHOST_GET_PROTOCOL */
+		.handler = bt_hid_get_protocol,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_hidhost_get_protocol)
+	},
+	{	/* HAL_OP_HIDHOST_SET_PROTOCOL */
+		.handler = bt_hid_set_protocol,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_hidhost_get_protocol)
+	},
+	{	/* HAL_OP_HIDHOST_GET_REPORT */
+		.handler = bt_hid_get_report,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_hidhost_get_report)
+	},
+	{	/* HAL_OP_HIDHOST_SET_REPORT */
+		.handler = bt_hid_set_report,
+		.var_len = true,
+		.data_len = sizeof(struct hal_cmd_hidhost_set_report)
+	},
+	{	/* HAL_OP_HIDHOST_SEND_DATA */
+		.handler = bt_hid_send_data,
+		.var_len = true,
+		.data_len = sizeof(struct hal_cmd_hidhost_send_data)
+	},
+};
+
 static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
 {
 	struct hid_device *dev;
@@ -1229,6 +1293,9 @@ bool bt_hid_register(int cmd_sk, int notif_sk, const bdaddr_t *addr)
 	command_sk = cmd_sk;
 	notification_sk = notif_sk;
 
+	ipc_register(HAL_SERVICE_ID_HIDHOST, cmd_handlers,
+				sizeof(cmd_handlers)/sizeof(cmd_handlers[0]));
+
 	return true;
 }
 
@@ -1263,4 +1330,6 @@ void bt_hid_unregister(void)
 		g_io_channel_unref(intr_io);
 		intr_io = NULL;
 	}
+
+	ipc_unregister(HAL_SERVICE_ID_HIDHOST);
 }
diff --git a/android/hidhost.h b/android/hidhost.h
index 4486ee9..9738ea5 100644
--- a/android/hidhost.h
+++ b/android/hidhost.h
@@ -21,7 +21,5 @@
  *
  */
 
-void bt_hid_handle_cmd(int sk, uint8_t opcode, void *buf, uint16_t len);
-
 bool bt_hid_register(int cmd_sk, int notif_sk, const bdaddr_t *addr);
 void bt_hid_unregister(void);
-- 
1.8.3.2


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

* [PATCH 08/13] android/hidhost: Use stack buffer in bt_hid_get_report
  2013-11-27 11:54 [PATCH 00/13] android: IPC improvements - daemon part Szymon Janc
                   ` (6 preceding siblings ...)
  2013-11-27 11:54 ` [PATCH 07/13] android/hidhost: Use generic IPC message handling for commands Szymon Janc
@ 2013-11-27 11:55 ` Szymon Janc
  2013-11-27 11:55 ` [PATCH 09/13] android/pan: Use generic IPC message handling for commands Szymon Janc
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Szymon Janc @ 2013-11-27 11:55 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

Request size can be up to 4 bytes and this can be easily put on stack.
---
 android/hidhost.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/android/hidhost.c b/android/hidhost.c
index 5b98152..b94d284 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -958,8 +958,8 @@ static void bt_hid_get_report(const void *buf, uint16_t len)
 	GSList *l;
 	bdaddr_t dst;
 	int fd;
-	uint8_t *req;
-	uint8_t req_size;
+	uint8_t req[4];
+	uint8_t req_size = 2;
 
 	DBG("");
 
@@ -972,12 +972,6 @@ static void bt_hid_get_report(const void *buf, uint16_t len)
 	}
 
 	dev = l->data;
-	req_size = (cmd->buf_size > 0) ? 4 : 2;
-	req = g_try_malloc0(req_size);
-	if (!req) {
-		status = HAL_STATUS_NOMEM;
-		goto fail;
-	}
 
 	req[0] = HID_MSG_GET_REPORT | cmd->type;
 	req[1] = cmd->id;
@@ -985,6 +979,7 @@ static void bt_hid_get_report(const void *buf, uint16_t len)
 	if (cmd->buf_size > 0) {
 		req[0] = req[0] | HID_GET_REPORT_SIZE_FIELD;
 		bt_put_le16(cmd->buf_size, &req[2]);
+		req_size += sizeof(uint16_t);
 	}
 
 	fd = g_io_channel_unix_get_fd(dev->ctrl_io);
@@ -992,13 +987,11 @@ static void bt_hid_get_report(const void *buf, uint16_t len)
 	if (write(fd, req, req_size) < 0) {
 		error("error writing hid_get_report: %s (%d)",
 						strerror(errno), errno);
-		g_free(req);
 		status = HAL_STATUS_FAILED;
 		goto fail;
 	}
 
 	dev->last_hid_msg = HID_MSG_GET_REPORT;
-	g_free(req);
 
 fail:
 	ipc_send_rsp(command_sk, HAL_SERVICE_ID_HIDHOST,
-- 
1.8.3.2


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

* [PATCH 09/13] android/pan: Use generic IPC message handling for commands
  2013-11-27 11:54 [PATCH 00/13] android: IPC improvements - daemon part Szymon Janc
                   ` (7 preceding siblings ...)
  2013-11-27 11:55 ` [PATCH 08/13] android/hidhost: Use stack buffer in bt_hid_get_report Szymon Janc
@ 2013-11-27 11:55 ` Szymon Janc
  2013-11-27 11:55 ` [PATCH 10/13] android/a2dp: " Szymon Janc
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Szymon Janc @ 2013-11-27 11:55 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

Handlers are registered on service register and unregistered on
unregister.
---
 android/pan.c | 71 ++++++++++++++++++++++++++++++++---------------------------
 android/pan.h |  2 --
 2 files changed, 38 insertions(+), 35 deletions(-)

diff --git a/android/pan.c b/android/pan.c
index 35d5608..5a605d2 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -38,59 +38,60 @@
 static int notification_sk = -1;
 static int command_sk = -1;
 
-static uint8_t bt_pan_enable(struct hal_cmd_pan_enable *cmd, uint16_t len)
+static void bt_pan_enable(const void *buf, uint16_t len)
 {
 	DBG("Not Implemented");
 
-	return HAL_STATUS_FAILED;
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_PAN, HAL_OP_PAN_ENABLE,
+							HAL_STATUS_FAILED);
 }
 
-static uint8_t bt_pan_get_role(void *cmd, uint16_t len)
+static void bt_pan_get_role(const void *buf, uint16_t len)
 {
 	DBG("Not Implemented");
 
-	return HAL_STATUS_FAILED;
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_PAN, HAL_OP_PAN_GET_ROLE,
+							HAL_STATUS_FAILED);
 }
 
-static uint8_t bt_pan_connect(struct hal_cmd_pan_connect *cmd, uint16_t len)
+static void bt_pan_connect(const void *buf, uint16_t len)
 {
 	DBG("Not Implemented");
 
-	return HAL_STATUS_FAILED;
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_PAN, HAL_OP_PAN_CONNECT,
+							HAL_STATUS_FAILED);
 }
 
-static uint8_t bt_pan_disconnect(struct hal_cmd_pan_disconnect *cmd,
-								uint16_t len)
+static void bt_pan_disconnect(const void *buf, uint16_t len)
 {
 	DBG("Not Implemented");
 
-	return HAL_STATUS_FAILED;
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_PAN, HAL_OP_PAN_DISCONNECT,
+							HAL_STATUS_FAILED);
 }
 
-void bt_pan_handle_cmd(int sk, uint8_t opcode, void *buf, uint16_t len)
-{
-	uint8_t status = HAL_STATUS_FAILED;
-
-	switch (opcode) {
-	case HAL_OP_PAN_ENABLE:
-		status = bt_pan_enable(buf, len);
-		break;
-	case HAL_OP_PAN_GET_ROLE:
-		status = bt_pan_get_role(buf, len);
-		break;
-	case HAL_OP_PAN_CONNECT:
-		status = bt_pan_connect(buf, len);
-		break;
-	case HAL_OP_PAN_DISCONNECT:
-		status = bt_pan_disconnect(buf, len);
-		break;
-	default:
-		DBG("Unhandled command, opcode 0x%x", opcode);
-		break;
-	}
-
-	ipc_send_rsp(sk, HAL_SERVICE_ID_PAN, opcode, status);
-}
+static const struct ipc_handler cmd_handlers[] = {
+	{	/* HAL_OP_PAN_ENABLE */
+		.handler = bt_pan_enable,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_pan_enable)
+	},
+	{	/* HAL_OP_PAN_GET_ROLE */
+		.handler = bt_pan_get_role,
+		.var_len = false,
+		.data_len = 0
+	},
+	{	/* HAL_OP_PAN_CONNECT */
+		.handler = bt_pan_connect,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_pan_connect)
+	},
+	{	/* HAL_OP_PAN_DISCONNECT */
+		.handler = bt_pan_disconnect,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_pan_disconnect)
+	},
+};
 
 bool bt_pan_register(int cmd_sk, int notif_sk, const bdaddr_t *addr)
 {
@@ -102,6 +103,8 @@ bool bt_pan_register(int cmd_sk, int notif_sk, const bdaddr_t *addr)
 	notification_sk = notif_sk;
 	command_sk = cmd_sk;
 
+	ipc_register(HAL_SERVICE_ID_PAN, cmd_handlers,
+				sizeof(cmd_handlers)/sizeof(cmd_handlers[0]));
 
 	return true;
 }
@@ -115,4 +118,6 @@ void bt_pan_unregister(void)
 
 	notification_sk = -1;
 	command_sk = -1;
+
+	ipc_unregister(HAL_SERVICE_ID_PAN);
 }
diff --git a/android/pan.h b/android/pan.h
index 9ceebe5..91799f2 100644
--- a/android/pan.h
+++ b/android/pan.h
@@ -21,7 +21,5 @@
  *
  */
 
-void bt_pan_handle_cmd(int sk, uint8_t opcode, void *buf, uint16_t len);
-
 bool bt_pan_register(int cmd_sk, int notif_sk, const bdaddr_t *addr);
 void bt_pan_unregister(void);
-- 
1.8.3.2


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

* [PATCH 10/13] android/a2dp: Use generic IPC message handling for commands
  2013-11-27 11:54 [PATCH 00/13] android: IPC improvements - daemon part Szymon Janc
                   ` (8 preceding siblings ...)
  2013-11-27 11:55 ` [PATCH 09/13] android/pan: Use generic IPC message handling for commands Szymon Janc
@ 2013-11-27 11:55 ` Szymon Janc
  2013-11-27 11:55 ` [PATCH 11/13] android/socket: " Szymon Janc
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Szymon Janc @ 2013-11-27 11:55 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

Handlers are registered on service register and unregistered on
unregister.
---
 android/a2dp.c | 73 +++++++++++++++++++++++++++++++---------------------------
 android/a2dp.h |  2 --
 2 files changed, 39 insertions(+), 36 deletions(-)

diff --git a/android/a2dp.c b/android/a2dp.c
index 87b1abb..6f79cc5 100644
--- a/android/a2dp.c
+++ b/android/a2dp.c
@@ -166,9 +166,11 @@ static void signaling_connect_cb(GIOChannel *chan, GError *err,
 	bt_a2dp_notify_state(dev, HAL_A2DP_STATE_CONNECTED);
 }
 
-static uint8_t bt_a2dp_connect(struct hal_cmd_a2dp_connect *cmd, uint16_t len)
+static void bt_a2dp_connect(const void *buf, uint16_t len)
 {
+	const struct hal_cmd_a2dp_connect *cmd = buf;
 	struct a2dp_device *dev;
+	uint8_t status = HAL_STATUS_SUCCESS;
 	char addr[18];
 	bdaddr_t dst;
 	GSList *l;
@@ -176,14 +178,13 @@ static uint8_t bt_a2dp_connect(struct hal_cmd_a2dp_connect *cmd, uint16_t len)
 
 	DBG("");
 
-	if (len < sizeof(*cmd))
-		return HAL_STATUS_INVALID;
-
 	android2bdaddr(&cmd->bdaddr, &dst);
 
 	l = g_slist_find_custom(devices, &dst, device_cmp);
-	if (l)
-		return HAL_STATUS_FAILED;
+	if (l) {
+		status = HAL_STATUS_FAILED;
+		goto fail;
+	}
 
 	dev = a2dp_device_new(&dst);
 	dev->io = bt_io_connect(signaling_connect_cb, dev, NULL, &err,
@@ -196,7 +197,8 @@ static uint8_t bt_a2dp_connect(struct hal_cmd_a2dp_connect *cmd, uint16_t len)
 		error("%s", err->message);
 		g_error_free(err);
 		a2dp_device_free(dev);
-		return HAL_STATUS_FAILED;
+		status = HAL_STATUS_FAILED;
+		goto fail;
 	}
 
 	ba2str(&dev->dst, addr);
@@ -204,26 +206,28 @@ static uint8_t bt_a2dp_connect(struct hal_cmd_a2dp_connect *cmd, uint16_t len)
 
 	bt_a2dp_notify_state(dev, HAL_A2DP_STATE_CONNECTING);
 
-	return HAL_STATUS_SUCCESS;
+fail:
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_A2DP, HAL_OP_A2DP_CONNECT,
+								status);
 }
 
-static uint8_t bt_a2dp_disconnect(struct hal_cmd_a2dp_connect *cmd,
-								uint16_t len)
+static void bt_a2dp_disconnect(const void *buf, uint16_t len)
 {
+	const struct hal_cmd_a2dp_connect *cmd = buf;
+	uint8_t status = HAL_STATUS_SUCCESS;
 	struct a2dp_device *dev;
 	GSList *l;
 	bdaddr_t dst;
 
 	DBG("");
 
-	if (len < sizeof(*cmd))
-		return HAL_STATUS_INVALID;
-
 	android2bdaddr(&cmd->bdaddr, &dst);
 
 	l = g_slist_find_custom(devices, &dst, device_cmp);
-	if (!l)
-		return HAL_STATUS_FAILED;
+	if (!l) {
+		status = HAL_STATUS_FAILED;
+		goto fail;
+	}
 
 	dev = l->data;
 
@@ -233,27 +237,23 @@ static uint8_t bt_a2dp_disconnect(struct hal_cmd_a2dp_connect *cmd,
 
 	bt_a2dp_notify_state(dev, HAL_A2DP_STATE_DISCONNECTING);
 
-	return HAL_STATUS_SUCCESS;
+fail:
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_A2DP, HAL_OP_A2DP_DISCONNECT,
+								status);
 }
 
-void bt_a2dp_handle_cmd(int sk, uint8_t opcode, void *buf, uint16_t len)
-{
-	uint8_t status = HAL_STATUS_FAILED;
-
-	switch (opcode) {
-	case HAL_OP_A2DP_CONNECT:
-		status = bt_a2dp_connect(buf, len);
-		break;
-	case HAL_OP_A2DP_DISCONNECT:
-		status = bt_a2dp_disconnect(buf, len);
-		break;
-	default:
-		DBG("Unhandled command, opcode 0x%x", opcode);
-		break;
-	}
-
-	ipc_send_rsp(sk, HAL_SERVICE_ID_A2DP, opcode, status);
-}
+static const struct ipc_handler cmd_handlers[] = {
+	{	/* HAL_OP_A2DP_CONNECT */
+		.handler = bt_a2dp_connect,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_a2dp_connect)
+	},
+	{	/* HAL_OP_A2DP_DISCONNECT */
+		.handler = bt_a2dp_disconnect,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_a2dp_disconnect)
+	},
+};
 
 static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
 {
@@ -388,6 +388,9 @@ bool bt_a2dp_register(int cmd_sk, int notif_sk, const bdaddr_t *addr)
 	notification_sk = notif_sk;
 	command_sk = cmd_sk;
 
+	ipc_register(HAL_SERVICE_ID_A2DP, cmd_handlers,
+				sizeof(cmd_handlers)/sizeof(cmd_handlers[0]));
+
 	return true;
 }
 
@@ -411,6 +414,8 @@ void bt_a2dp_unregister(void)
 	notification_sk = -1;
 	command_sk = -1;
 
+	ipc_unregister(HAL_SERVICE_ID_A2DP);
+
 	bt_adapter_remove_record(record_id);
 	record_id = 0;
 
diff --git a/android/a2dp.h b/android/a2dp.h
index 720b681..e23fe3a 100644
--- a/android/a2dp.h
+++ b/android/a2dp.h
@@ -21,7 +21,5 @@
  *
  */
 
-void bt_a2dp_handle_cmd(int sk, uint8_t opcode, void *buf, uint16_t len);
-
 bool bt_a2dp_register(int cmd_sk, int notif_sk, const bdaddr_t *addr);
 void bt_a2dp_unregister(void);
-- 
1.8.3.2


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

* [PATCH 11/13] android/socket: Use generic IPC message handling for commands
  2013-11-27 11:54 [PATCH 00/13] android: IPC improvements - daemon part Szymon Janc
                   ` (9 preceding siblings ...)
  2013-11-27 11:55 ` [PATCH 10/13] android/a2dp: " Szymon Janc
@ 2013-11-27 11:55 ` Szymon Janc
  2013-11-27 11:55 ` [PATCH 12/13] android/hal-bluetooth: Rename create_enum_prop to enum_prop_to_hal Szymon Janc
  2013-11-27 11:55 ` [PATCH 13/13] android/hal-bluetooth: Fix sending invalid adapter property Szymon Janc
  12 siblings, 0 replies; 14+ messages in thread
From: Szymon Janc @ 2013-11-27 11:55 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

Handlers are registered on service register and unregistered on
unregister.
---
 android/socket.c | 111 +++++++++++++++++++++++++++++--------------------------
 1 file changed, 59 insertions(+), 52 deletions(-)

diff --git a/android/socket.c b/android/socket.c
index 47338e7..ba97d8a 100644
--- a/android/socket.c
+++ b/android/socket.c
@@ -52,6 +52,7 @@
 
 #define SVC_HINT_OBEX 0x10
 
+static int command_sk = -1;
 static bdaddr_t adapter_addr;
 
 /* Simple list of RFCOMM server sockets */
@@ -655,15 +656,15 @@ static void accept_cb(GIOChannel *io, GError *err, gpointer user_data)
 		rfsock_acc->rfcomm_watch);
 }
 
-static int handle_listen(void *buf)
+static void handle_listen(const void *buf, uint16_t len)
 {
-	struct hal_cmd_sock_listen *cmd = buf;
+	const struct hal_cmd_sock_listen *cmd = buf;
 	const struct profile_info *profile;
-	struct rfcomm_sock *rfsock;
+	struct rfcomm_sock *rfsock = NULL;
 	BtIOSecLevel sec_level;
 	GIOChannel *io;
 	GError *err = NULL;
-	int hal_fd;
+	int hal_fd = -1;
 	int chan;
 
 	DBG("");
@@ -671,11 +672,10 @@ static int handle_listen(void *buf)
 	profile = get_profile_by_uuid(cmd->uuid);
 	if (!profile) {
 		if (!cmd->channel)
-			return -1;
-		else {
-			chan = cmd->channel;
-			sec_level = BT_IO_SEC_MEDIUM;
-		}
+			goto fail;
+
+		chan = cmd->channel;
+		sec_level = BT_IO_SEC_MEDIUM;
 	} else {
 		chan = profile->channel;
 		sec_level = profile->sec_level;
@@ -685,7 +685,7 @@ static int handle_listen(void *buf)
 
 	rfsock = create_rfsock(-1, &hal_fd);
 	if (!rfsock)
-		return -1;
+		goto fail;
 
 	io = bt_io_listen(accept_cb, NULL, rfsock, NULL, &err,
 				BT_IO_OPT_SOURCE_BDADDR, &adapter_addr,
@@ -695,8 +695,7 @@ static int handle_listen(void *buf)
 	if (!io) {
 		error("Failed listen: %s", err->message);
 		g_error_free(err);
-		cleanup_rfsock(rfsock);
-		return -1;
+		goto fail;
 	}
 
 	rfsock->real_sock = g_io_channel_unix_get_fd(io);
@@ -711,13 +710,25 @@ static int handle_listen(void *buf)
 
 	if (write(rfsock->fd, &chan, sizeof(chan)) != sizeof(chan)) {
 		error("Error sending RFCOMM channel");
-		cleanup_rfsock(rfsock);
-		return -1;
+		goto fail;
 	}
 
 	rfsock->service_handle = sdp_service_register(profile, cmd->name);
 
-	return hal_fd;
+	ipc_send(command_sk, HAL_SERVICE_ID_SOCK, HAL_OP_SOCK_LISTEN, 0, NULL,
+								hal_fd);
+	close(hal_fd);
+	return;
+
+fail:
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_SOCK, HAL_OP_SOCK_LISTEN,
+							HAL_STATUS_FAILED);
+
+	if (rfsock)
+		cleanup_rfsock(rfsock);
+
+	if (hal_fd >= 0)
+		close(hal_fd);
 }
 
 static bool sock_send_connect(struct rfcomm_sock *rfsock, bdaddr_t *bdaddr)
@@ -868,9 +879,9 @@ fail:
 	cleanup_rfsock(rfsock);
 }
 
-static int handle_connect(void *buf)
+static void handle_connect(const void *buf, uint16_t len)
 {
-	struct hal_cmd_sock_connect *cmd = buf;
+	const struct hal_cmd_sock_connect *cmd = buf;
 	struct rfcomm_sock *rfsock;
 	uuid_t uuid;
 	int hal_fd = -1;
@@ -893,57 +904,53 @@ static int handle_connect(void *buf)
 					sdp_search_cb, rfsock, NULL) < 0) {
 		error("Failed to search SDP records");
 		cleanup_rfsock(rfsock);
-		return -1;
+		goto fail;
 	}
 
-	return hal_fd;
-}
-
-void bt_sock_handle_cmd(int sk, uint8_t opcode, void *buf, uint16_t len)
-{
-	int fd;
-
-	switch (opcode) {
-	case HAL_OP_SOCK_LISTEN:
-		fd = handle_listen(buf);
-		if (fd < 0)
-			break;
-
-		ipc_send(sk, HAL_SERVICE_ID_SOCK, opcode, 0, NULL, fd);
-
-		if (close(fd) < 0)
-			error("close() fd %d failed: %s", fd, strerror(errno));
-
-		return;
-	case HAL_OP_SOCK_CONNECT:
-		fd = handle_connect(buf);
-		if (fd < 0)
-			break;
-
-		ipc_send(sk, HAL_SERVICE_ID_SOCK, opcode, 0, NULL, fd);
-
-		if (close(fd) < 0)
-			error("close() fd %d failed: %s", fd, strerror(errno));
+	ipc_send(command_sk, HAL_SERVICE_ID_SOCK, HAL_OP_SOCK_CONNECT, 0, NULL,
+								hal_fd);
+	close(hal_fd);
+	return;
 
-		return;
-	default:
-		DBG("Unhandled command, opcode 0x%x", opcode);
-		break;
-	}
+fail:
+	ipc_send_rsp(command_sk, HAL_SERVICE_ID_SOCK, HAL_OP_SOCK_CONNECT,
+							HAL_STATUS_FAILED);
 
-	ipc_send_rsp(sk, HAL_SERVICE_ID_SOCK, opcode, HAL_STATUS_FAILED);
+	if (hal_fd >= 0)
+		close(hal_fd);
 }
 
+static const struct ipc_handler cmd_handlers[] = {
+	{	/* HAL_OP_SOCK_LISTEN */
+		.handler = handle_listen,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_sock_listen)
+	},
+	{	/* HAL_OP_SOCK_CONNECT */
+		.handler = handle_connect,
+		.var_len = false,
+		.data_len = sizeof(struct hal_cmd_sock_connect)
+	},
+};
+
 bool bt_socket_register(int cmd_sk, int notif_sk, const bdaddr_t *addr)
 {
 	DBG("");
 
+	command_sk = cmd_sk;
 	bacpy(&adapter_addr, addr);
 
+	ipc_register(HAL_SERVICE_ID_SOCK, cmd_handlers,
+				sizeof(cmd_handlers)/sizeof(cmd_handlers[0]));
+
 	return true;
 }
 
 void bt_socket_unregister(void)
 {
 	DBG("");
+
+	command_sk = -1;
+
+	ipc_unregister(HAL_SERVICE_ID_SOCK);
 }
-- 
1.8.3.2


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

* [PATCH 12/13] android/hal-bluetooth: Rename create_enum_prop to enum_prop_to_hal
  2013-11-27 11:54 [PATCH 00/13] android: IPC improvements - daemon part Szymon Janc
                   ` (10 preceding siblings ...)
  2013-11-27 11:55 ` [PATCH 11/13] android/socket: " Szymon Janc
@ 2013-11-27 11:55 ` Szymon Janc
  2013-11-27 11:55 ` [PATCH 13/13] android/hal-bluetooth: Fix sending invalid adapter property Szymon Janc
  12 siblings, 0 replies; 14+ messages in thread
From: Szymon Janc @ 2013-11-27 11:55 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This better describes purpose of this macro.
---
 android/hal-bluetooth.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index a879583..f232afd 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -28,7 +28,7 @@
 
 static const bt_callbacks_t *bt_hal_cbacks = NULL;
 
-#define create_enum_prop(prop, hal_prop, type) do { \
+#define enum_prop_to_hal(prop, hal_prop, type) do { \
 	static type e; \
 	prop.val = &e; \
 	prop.len = sizeof(e); \
@@ -63,11 +63,11 @@ static void adapter_props_to_hal(bt_property_t *send_props,
 
 		switch (prop->type) {
 		case HAL_PROP_ADAPTER_TYPE:
-			create_enum_prop(send_props[i], prop,
+			enum_prop_to_hal(send_props[i], prop,
 							bt_device_type_t);
 			break;
 		case HAL_PROP_ADAPTER_SCAN_MODE:
-			create_enum_prop(send_props[i], prop,
+			enum_prop_to_hal(send_props[i], prop,
 							bt_scan_mode_t);
 			break;
 		case HAL_PROP_ADAPTER_SERVICE_REC:
@@ -109,7 +109,7 @@ static void device_props_to_hal(bt_property_t *send_props,
 
 		switch (prop->type) {
 		case HAL_PROP_DEVICE_TYPE:
-			create_enum_prop(send_props[i], prop,
+			enum_prop_to_hal(send_props[i], prop,
 							bt_device_type_t);
 			break;
 		case HAL_PROP_DEVICE_SERVICE_REC:
-- 
1.8.3.2


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

* [PATCH 13/13] android/hal-bluetooth: Fix sending invalid adapter property
  2013-11-27 11:54 [PATCH 00/13] android: IPC improvements - daemon part Szymon Janc
                   ` (11 preceding siblings ...)
  2013-11-27 11:55 ` [PATCH 12/13] android/hal-bluetooth: Rename create_enum_prop to enum_prop_to_hal Szymon Janc
@ 2013-11-27 11:55 ` Szymon Janc
  12 siblings, 0 replies; 14+ messages in thread
From: Szymon Janc @ 2013-11-27 11:55 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

If property to be set is of enum type it should be first converted to
byte value as size of enum might varry depending on architecture.

To keep code simple command buffer uses len received from framework
as this is more or equal to HAL property size.
---
 android/hal-bluetooth.c | 36 +++++++++++++++++++++++++++++++-----
 android/socket.c        |  2 +-
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index f232afd..87d6fc7 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -35,6 +35,18 @@ static const bt_callbacks_t *bt_hal_cbacks = NULL;
 	e = *((uint8_t *) (hal_prop->val)); \
 } while (0)
 
+#define enum_prop_from_hal(prop, hal_len, hal_val, enum_type) do { \
+	enum_type e; \
+	if (prop->len != sizeof(e)) { \
+		error("invalid HAL property %u (%u vs %zu), aborting ", \
+					prop->type, prop->len, sizeof(e)); \
+		exit(EXIT_FAILURE); \
+	} \
+	memcpy(&e, prop->val, sizeof(e)); \
+	*((uint8_t *) hal_val) = e; /* enums are mapped to 1 byte */ \
+	*hal_len = 1; \
+} while (0)
+
 static void handle_adapter_state_changed(void *buf, uint16_t len)
 {
 	struct hal_ev_adapter_state_changed *ev = buf;
@@ -91,6 +103,23 @@ static void adapter_props_to_hal(bt_property_t *send_props,
 	exit(EXIT_FAILURE);
 }
 
+static void adapter_prop_from_hal(const bt_property_t *property, uint8_t *type,
+						uint16_t *len, void *val)
+{
+	/* type match IPC type */
+	*type = property->type;
+
+	switch(property->type) {
+	case HAL_PROP_ADAPTER_SCAN_MODE:
+		enum_prop_from_hal(property, len, val, bt_scan_mode_t);
+		break;
+	default:
+		*len = property->len;
+		memcpy(val, property->val, property->len);
+		break;
+	}
+}
+
 static void device_props_to_hal(bt_property_t *send_props,
 				struct hal_property *prop, uint8_t num_props,
 				uint16_t len)
@@ -458,13 +487,10 @@ static int set_adapter_property(const bt_property_t *property)
 	if (!interface_ready())
 		return BT_STATUS_NOT_READY;
 
-	/* type match IPC type */
-	cmd->type = property->type;
-	cmd->len = property->len;
-	memcpy(cmd->val, property->val, property->len);
+	adapter_prop_from_hal(property, &cmd->type, &cmd->len, cmd->val);
 
 	return hal_ipc_cmd(HAL_SERVICE_ID_BLUETOOTH, HAL_OP_SET_ADAPTER_PROP,
-					sizeof(buf), cmd, 0, NULL, NULL);
+				sizeof(*cmd) + cmd->len, cmd, 0, NULL, NULL);
 }
 
 static int get_remote_device_properties(bt_bdaddr_t *remote_addr)
diff --git a/android/socket.c b/android/socket.c
index ba97d8a..7fcb091 100644
--- a/android/socket.c
+++ b/android/socket.c
@@ -890,7 +890,7 @@ static void handle_connect(const void *buf, uint16_t len)
 
 	rfsock = create_rfsock(-1, &hal_fd);
 	if (!rfsock)
-		return -1;
+		goto fail;
 
 	android2bdaddr(cmd->bdaddr, &rfsock->dst);
 
-- 
1.8.3.2


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

end of thread, other threads:[~2013-11-27 11:55 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-27 11:54 [PATCH 00/13] android: IPC improvements - daemon part Szymon Janc
2013-11-27 11:54 ` [PATCH 01/13] android: Add initial code for IPC message handlers Szymon Janc
2013-11-27 11:54 ` [PATCH 02/13] android/main: Use generic IPC message handling for core service Szymon Janc
2013-11-27 11:54 ` [PATCH 03/13] android/main: Use IPC helper for all replies in " Szymon Janc
2013-11-27 11:54 ` [PATCH 04/13] android: Pass command socket to services while registering Szymon Janc
2013-11-27 11:54 ` [PATCH 05/13] android/bluetooth: Use generic IPC msg handling for commands Szymon Janc
2013-11-27 11:54 ` [PATCH 06/13] android/bluetooth: Add stubs for missing commands handlers Szymon Janc
2013-11-27 11:54 ` [PATCH 07/13] android/hidhost: Use generic IPC message handling for commands Szymon Janc
2013-11-27 11:55 ` [PATCH 08/13] android/hidhost: Use stack buffer in bt_hid_get_report Szymon Janc
2013-11-27 11:55 ` [PATCH 09/13] android/pan: Use generic IPC message handling for commands Szymon Janc
2013-11-27 11:55 ` [PATCH 10/13] android/a2dp: " Szymon Janc
2013-11-27 11:55 ` [PATCH 11/13] android/socket: " Szymon Janc
2013-11-27 11:55 ` [PATCH 12/13] android/hal-bluetooth: Rename create_enum_prop to enum_prop_to_hal Szymon Janc
2013-11-27 11:55 ` [PATCH 13/13] android/hal-bluetooth: Fix sending invalid adapter property Szymon Janc

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.