Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH v3 5/9] android/bluetooth: Make property handling function return HAL status
From: Szymon Janc @ 2013-12-02 12:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1385986848-8023-1-git-send-email-szymon.janc@tieto.com>

This makes funtions follow have similar style and makes properties
dispatch function much simpler.
---
 android/bluetooth.c | 85 +++++++++++++++++++----------------------------------
 1 file changed, 30 insertions(+), 55 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index eb8dbc5..a39e7bf 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -1133,7 +1133,7 @@ static void uuid16_to_uint128(uint16_t uuid, uint128_t *u128)
 	ntoh128(&uuid128.value.uuid128, u128);
 }
 
-static bool get_uuids(void)
+static uint8_t get_uuids(void)
 {
 	struct hal_ev_adapter_props_changed *ev;
 	GSList *list = adapter.uuids;
@@ -1169,7 +1169,7 @@ static bool get_uuids(void)
 	ipc_send_notif(HAL_SERVICE_ID_BLUETOOTH, HAL_EV_ADAPTER_PROPS_CHANGED,
 							sizeof(buf), ev);
 
-	return true;
+	return HAL_STATUS_SUCCESS;
 }
 
 static void remove_uuid_complete(uint8_t status, uint16_t length,
@@ -1691,7 +1691,7 @@ static bool set_discoverable(uint8_t mode, uint16_t timeout)
 	return false;
 }
 
-static void get_address(void)
+static uint8_t get_address(void)
 {
 	uint8_t buf[BASELEN_PROP_CHANGED + sizeof(bdaddr_t)];
 	struct hal_ev_adapter_props_changed *ev = (void *) buf;
@@ -1705,65 +1705,67 @@ static void get_address(void)
 
 	ipc_send_notif(HAL_SERVICE_ID_BLUETOOTH, HAL_EV_ADAPTER_PROPS_CHANGED,
 							sizeof(buf), buf);
+
+	return HAL_STATUS_SUCCESS;
 }
 
-static bool get_name(void)
+static uint8_t get_name(void)
 {
 	if (!adapter.name)
-		return false;
+		return HAL_STATUS_FAILED;
 
 	adapter_name_changed((uint8_t *) adapter.name);
 
-	return true;
+	return HAL_STATUS_SUCCESS;
 }
 
 
-static bool get_class(void)
+static uint8_t get_class(void)
 {
 	DBG("");
 
 	adapter_class_changed();
 
-	return true;
+	return HAL_STATUS_SUCCESS;
 }
 
-static bool get_type(void)
+static uint8_t get_type(void)
 {
 	DBG("Not implemented");
 
 	/* TODO: Add implementation */
 
-	return false;
+	return HAL_STATUS_FAILED;
 }
 
-static bool get_service(void)
+static uint8_t get_service(void)
 {
 	DBG("Not implemented");
 
 	/* TODO: Add implementation */
 
-	return false;
+	return HAL_STATUS_FAILED;
 }
 
-static bool get_scan_mode(void)
+static uint8_t get_scan_mode(void)
 {
 	DBG("");
 
 	scan_mode_changed();
 
-	return true;
+	return HAL_STATUS_SUCCESS;
 }
 
-static bool get_devices(void)
+static uint8_t get_devices(void)
 {
 	DBG("Not implemented");
 
 	/* TODO: Add implementation */
 
-	return false;
+	return HAL_STATUS_FAILED;
 }
 
-static bool get_discoverable_timeout(void)
+static uint8_t get_discoverable_timeout(void)
 {
 	struct hal_ev_adapter_props_changed *ev;
 	uint8_t buf[BASELEN_PROP_CHANGED + sizeof(uint32_t)];
@@ -1782,7 +1784,7 @@ static bool get_discoverable_timeout(void)
 	ipc_send_notif(HAL_SERVICE_ID_BLUETOOTH, HAL_EV_ADAPTER_PROPS_CHANGED,
 							sizeof(buf), ev);
 
-	return true;
+	return HAL_STATUS_SUCCESS;
 }
 
 static void handle_get_adapter_prop_cmd(const void *buf, uint16_t len)
@@ -1792,64 +1794,37 @@ static void handle_get_adapter_prop_cmd(const void *buf, uint16_t len)
 
 	switch (cmd->type) {
 	case HAL_PROP_ADAPTER_ADDR:
-		get_address();
+		status = get_address();
 		break;
 	case HAL_PROP_ADAPTER_NAME:
-		if (!get_name()) {
-			status = HAL_STATUS_FAILED;
-			goto failed;
-		}
+		status = get_name();
 		break;
 	case HAL_PROP_ADAPTER_UUIDS:
-		if (!get_uuids()) {
-			status = HAL_STATUS_FAILED;
-			goto failed;
-		}
+		status = get_uuids();
 		break;
 	case HAL_PROP_ADAPTER_CLASS:
-		if (!get_class()) {
-			status = HAL_STATUS_FAILED;
-			goto failed;
-		}
+		status = get_class();
 		break;
 	case HAL_PROP_ADAPTER_TYPE:
-		if (!get_type()) {
-			status = HAL_STATUS_FAILED;
-			goto failed;
-		}
+		status = get_type();
 		break;
 	case HAL_PROP_ADAPTER_SERVICE_REC:
-		if (!get_service()) {
-			status = HAL_STATUS_FAILED;
-			goto failed;
-		}
+		status = get_service();
 		break;
 	case HAL_PROP_ADAPTER_SCAN_MODE:
-		if (!get_scan_mode()) {
-			status = HAL_STATUS_FAILED;
-			goto failed;
-		}
+		status = get_scan_mode();
 		break;
 	case HAL_PROP_ADAPTER_BONDED_DEVICES:
-		if (!get_devices()) {
-			status = HAL_STATUS_FAILED;
-			goto failed;
-		}
+		status = get_devices();
 		break;
 	case HAL_PROP_ADAPTER_DISC_TIMEOUT:
-		if (!get_discoverable_timeout()) {
-			status = HAL_STATUS_FAILED;
-			goto failed;
-		}
+		status = get_discoverable_timeout();
 		break;
 	default:
 		status = HAL_STATUS_FAILED;
-		goto failed;
+		break;
 	}
 
-	status = HAL_STATUS_SUCCESS;
-
-failed:
 	ipc_send_rsp(HAL_SERVICE_ID_BLUETOOTH, HAL_OP_GET_ADAPTER_PROP, status);
 }
 
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH v3 6/9] android/hidhost: Use generic IPC message handling for commands
From: Szymon Janc @ 2013-12-02 12:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1385986848-8023-1-git-send-email-szymon.janc@tieto.com>

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

diff --git a/android/hidhost.c b/android/hidhost.c
index 44310ed..38194d0 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -720,10 +720,11 @@ 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;
 	struct hid_device *dev;
+	uint8_t status;
 	char addr[18];
 	bdaddr_t dst;
 	GSList *l;
@@ -731,14 +732,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 failed;
+	}
 
 	dev = g_new0(struct hid_device, 1);
 	bacpy(&dev->dst, &dst);
@@ -752,32 +752,36 @@ 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 failed;
 	}
 
 	devices = g_slist_append(devices, dev);
 	bt_hid_notify_state(dev, HAL_HIDHOST_STATE_CONNECTING);
 
-	return HAL_STATUS_SUCCESS;
+	status = HAL_STATUS_SUCCESS;
+
+failed:
+	ipc_send_rsp(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;
 	struct hid_device *dev;
+	uint8_t status;
 	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 failed;
+	}
 
 	dev = l->data;
 
@@ -790,33 +794,38 @@ 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;
+	status = HAL_STATUS_SUCCESS;
+
+failed:
+	ipc_send_rsp(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;
 	struct hid_device *dev;
 	GSList *l;
+	uint8_t status;
 	bdaddr_t dst;
 	uint8_t hdr;
 	int fd;
 
 	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 failed;
+	}
 
 	dev = l->data;
 
-	if (!(dev->ctrl_io))
-		return HAL_STATUS_FAILED;
+	if (!(dev->ctrl_io)) {
+		status = HAL_STATUS_FAILED;
+		goto failed;
+	}
 
 	hdr = HID_MSG_CONTROL | HID_VIRTUAL_CABLE_UNPLUG;
 
@@ -825,7 +834,8 @@ 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;
+		status = HAL_STATUS_FAILED;
+		goto failed;
 	}
 
 	/* Wait either channels to HUP */
@@ -837,10 +847,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;
+
+failed:
+	ipc_send_rsp(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
@@ -848,33 +862,36 @@ 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(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;
 	struct hid_device *dev;
 	GSList *l;
 	bdaddr_t dst;
 	int fd;
 	uint8_t hdr;
+	uint8_t status;
 
 	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 failed;
+	}
 
 	dev = l->data;
 
-	if (dev->boot_dev)
-		return HAL_STATUS_UNSUPPORTED;
+	if (dev->boot_dev) {
+		status = HAL_STATUS_UNSUPPORTED;
+		goto failed;
+	}
 
 	hdr = HID_MSG_GET_PROTOCOL | cmd->mode;
 	fd = g_io_channel_unix_get_fd(dev->ctrl_io);
@@ -882,37 +899,45 @@ 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 failed;
 	}
 
 	dev->last_hid_msg = HID_MSG_GET_PROTOCOL;
-	return HAL_STATUS_SUCCESS;
+
+	status = HAL_STATUS_SUCCESS;
+
+failed:
+	ipc_send_rsp(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;
 	struct hid_device *dev;
 	GSList *l;
 	bdaddr_t dst;
 	int fd;
 	uint8_t hdr;
+	uint8_t status;
 
 	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 failed;
+	}
 
 	dev = l->data;
 
-	if (dev->boot_dev)
-		return HAL_STATUS_UNSUPPORTED;
+	if (dev->boot_dev) {
+		status = HAL_STATUS_UNSUPPORTED;
+		goto failed;
+	}
 
 	hdr = HID_MSG_SET_PROTOCOL | cmd->mode;
 	fd = g_io_channel_unix_get_fd(dev->ctrl_io);
@@ -920,39 +945,47 @@ 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 failed;
 	}
 
 	dev->last_hid_msg = HID_MSG_SET_PROTOCOL;
-	return HAL_STATUS_SUCCESS;
+
+	status = HAL_STATUS_SUCCESS;
+
+failed:
+	ipc_send_rsp(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;
 	struct hid_device *dev;
 	GSList *l;
 	bdaddr_t dst;
 	int fd;
 	uint8_t *req;
 	uint8_t req_size;
+	uint8_t status;
 
 	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 failed;
+	}
 
 	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 failed;
+	}
 
 	req[0] = HID_MSG_GET_REPORT | cmd->type;
 	req[1] = cmd->id;
@@ -968,44 +1001,60 @@ 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 failed;
 	}
 
 	dev->last_hid_msg = HID_MSG_GET_REPORT;
 	g_free(req);
-	return HAL_STATUS_SUCCESS;
+
+	status = HAL_STATUS_SUCCESS;
+
+failed:
+	ipc_send_rsp(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;
 	struct hid_device *dev;
 	GSList *l;
 	bdaddr_t dst;
 	int i, fd;
 	uint8_t *req;
 	uint8_t req_size;
+	uint8_t status;
 
 	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 failed;
+	}
 
 	dev = l->data;
 
-	if (!(dev->ctrl_io))
-		return HAL_STATUS_FAILED;
+	if (!(dev->ctrl_io)) {
+		status = HAL_STATUS_FAILED;
+		goto failed;
+	}
 
 	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 failed;
+	}
 
 	req[0] = HID_MSG_SET_REPORT | cmd->type;
 	/* Report data coming to HAL is in ascii format, HAL sends
@@ -1019,44 +1068,60 @@ 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 failed;
 	}
 
 	dev->last_hid_msg = HID_MSG_SET_REPORT;
 	g_free(req);
-	return HAL_STATUS_SUCCESS;
+
+	status = HAL_STATUS_SUCCESS;
+
+failed:
+	ipc_send_rsp(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;
 	struct hid_device *dev;
 	GSList *l;
 	bdaddr_t dst;
 	int i, fd;
 	uint8_t *req;
 	uint8_t req_size;
+	uint8_t status;
 
 	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 failed;
+	}
 
 	dev = l->data;
 
-	if (!(dev->intr_io))
-		return HAL_STATUS_FAILED;
+	if (!(dev->intr_io)) {
+		status = HAL_STATUS_FAILED;
+		goto failed;
+	}
 
 	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 failed;
+	}
 
 	req[0] = HID_MSG_DATA | HID_DATA_TYPE_OUTPUT;
 	/* Report data coming to HAL is in ascii format, HAL sends
@@ -1070,53 +1135,42 @@ 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 failed;
 	}
 
 	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;
+	status = HAL_STATUS_SUCCESS;
 
-	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(HAL_SERVICE_ID_HIDHOST, opcode, status);
+failed:
+	ipc_send_rsp(HAL_SERVICE_ID_HIDHOST, HAL_OP_HIDHOST_SEND_DATA, status);
 }
 
+static const struct ipc_handler cmd_handlers[] = {
+	/* HAL_OP_HIDHOST_CONNECT */
+	{ bt_hid_connect, false, sizeof(struct hal_cmd_hidhost_connect) },
+	/* HAL_OP_HIDHOST_DISCONNECT */
+	{ bt_hid_disconnect, false, sizeof(struct hal_cmd_hidhost_disconnect) },
+	/* HAL_OP_HIDHOST_VIRTUAL_UNPLUG */
+	{ bt_hid_virtual_unplug, false,
+				sizeof(struct hal_cmd_hidhost_virtual_unplug) },
+	/* HAL_OP_HIDHOST_SET_INFO */
+	{ bt_hid_info, true, sizeof(struct hal_cmd_hidhost_set_info) },
+	/* HAL_OP_HIDHOST_GET_PROTOCOL */
+	{ bt_hid_get_protocol, false,
+				sizeof(struct hal_cmd_hidhost_get_protocol) },
+	/* HAL_OP_HIDHOST_SET_PROTOCOL */
+	{ bt_hid_set_protocol, false,
+				sizeof(struct hal_cmd_hidhost_get_protocol) },
+	/* HAL_OP_HIDHOST_GET_REPORT */
+	{ bt_hid_get_report, false, sizeof(struct hal_cmd_hidhost_get_report) },
+	/* HAL_OP_HIDHOST_SET_REPORT */
+	{ bt_hid_set_report, true, sizeof(struct hal_cmd_hidhost_set_report) },
+	/* HAL_OP_HIDHOST_SEND_DATA */
+	{ bt_hid_send_data, true, sizeof(struct hal_cmd_hidhost_send_data)  },
+};
+
 static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
 {
 	struct hid_device *dev;
@@ -1224,6 +1278,9 @@ bool bt_hid_register(const bdaddr_t *addr)
 		return false;
 	}
 
+	ipc_register(HAL_SERVICE_ID_HIDHOST, cmd_handlers,
+				sizeof(cmd_handlers)/sizeof(cmd_handlers[0]));
+
 	return true;
 }
 
@@ -1253,4 +1310,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 b5545fb..ea14446 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(const bdaddr_t *addr);
 void bt_hid_unregister(void);
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH v3 7/9] android/pan: Use generic IPC message handling for commands
From: Szymon Janc @ 2013-12-02 12:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1385986848-8023-1-git-send-email-szymon.janc@tieto.com>

Handlers are registered on service register and unregistered on
unregister.

This also fix sending two IPC responses for get pan role command.
---
 android/pan.c | 87 +++++++++++++++++++++++++++++------------------------------
 android/pan.h |  2 --
 2 files changed, 42 insertions(+), 47 deletions(-)

diff --git a/android/pan.c b/android/pan.c
index 9e388c3..3270aa4 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -188,9 +188,11 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer data)
 	}
 }
 
-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)
 {
+	const struct hal_cmd_pan_connect *cmd = buf;
 	struct pan_device *dev;
+	uint8_t status;
 	bdaddr_t dst;
 	char addr[18];
 	GSList *l;
@@ -198,14 +200,13 @@ static uint8_t bt_pan_connect(struct hal_cmd_pan_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 failed;
+	}
 
 	dev = g_new0(struct pan_device, 1);
 	bacpy(&dev->dst, &dst);
@@ -227,32 +228,36 @@ static uint8_t bt_pan_connect(struct hal_cmd_pan_connect *cmd, uint16_t len)
 		error("%s", gerr->message);
 		g_error_free(gerr);
 		g_free(dev);
-		return HAL_STATUS_FAILED;
+		status = HAL_STATUS_FAILED;
+		goto failed;
 	}
 
 	devices = g_slist_append(devices, dev);
 	bt_pan_notify_conn_state(dev, HAL_PAN_STATE_CONNECTING);
 
-	return HAL_STATUS_SUCCESS;
+	status =  HAL_STATUS_SUCCESS;
+
+failed:
+	ipc_send_rsp(HAL_SERVICE_ID_PAN, HAL_OP_PAN_CONNECT, status);
 }
 
-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)
 {
+	const struct hal_cmd_pan_disconnect *cmd = buf;
 	struct pan_device *dev;
+	uint8_t status;
 	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 failed;
+	}
 
 	dev = l->data;
 
@@ -267,17 +272,20 @@ static uint8_t bt_pan_disconnect(struct hal_cmd_pan_disconnect *cmd,
 	bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
 	pan_device_free(dev);
 
-	return HAL_STATUS_SUCCESS;
+	status = HAL_STATUS_SUCCESS;
+
+failed:
+	ipc_send_rsp(HAL_SERVICE_ID_PAN, HAL_OP_PAN_DISCONNECT, status);
 }
 
-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(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)
 {
 	struct hal_rsp_pan_get_role rsp;
 
@@ -286,34 +294,18 @@ static uint8_t bt_pan_get_role(void *cmd, uint16_t len)
 	rsp.local_role = local_role;
 	ipc_send_rsp_full(HAL_SERVICE_ID_PAN, HAL_OP_PAN_GET_ROLE, sizeof(rsp),
 								&rsp, -1);
-
-	return HAL_STATUS_SUCCESS;
 }
 
-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(HAL_SERVICE_ID_PAN, opcode, status);
-}
+static const struct ipc_handler cmd_handlers[] = {
+	/* HAL_OP_PAN_ENABLE */
+	{ bt_pan_enable, false, sizeof(struct hal_cmd_pan_enable) },
+	/* HAL_OP_PAN_GET_ROLE */
+	{ bt_pan_get_role, false, 0 },
+	/* HAL_OP_PAN_CONNECT */
+	{ bt_pan_connect, false, sizeof(struct hal_cmd_pan_connect) },
+	/* HAL_OP_PAN_DISCONNECT */
+	{ bt_pan_disconnect, false, sizeof(struct hal_cmd_pan_disconnect) },
+};
 
 bool bt_pan_register(const bdaddr_t *addr)
 {
@@ -329,6 +321,9 @@ bool bt_pan_register(const bdaddr_t *addr)
 		return false;
 	}
 
+	ipc_register(HAL_SERVICE_ID_PAN, cmd_handlers,
+				sizeof(cmd_handlers)/sizeof(cmd_handlers[0]));
+
 	return true;
 }
 
@@ -337,4 +332,6 @@ void bt_pan_unregister(void)
 	DBG("");
 
 	bnep_cleanup();
+
+	ipc_unregister(HAL_SERVICE_ID_PAN);
 }
diff --git a/android/pan.h b/android/pan.h
index dd18f68..3178d88 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(const bdaddr_t *addr);
 void bt_pan_unregister(void);
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH v3 8/9] android/a2dp: Use generic IPC message handling for commands
From: Szymon Janc @ 2013-12-02 12:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1385986848-8023-1-git-send-email-szymon.janc@tieto.com>

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

diff --git a/android/a2dp.c b/android/a2dp.c
index 99aa14d..98c138e 100644
--- a/android/a2dp.c
+++ b/android/a2dp.c
@@ -164,9 +164,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;
 	char addr[18];
 	bdaddr_t dst;
 	GSList *l;
@@ -174,14 +176,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 failed;
+	}
 
 	dev = a2dp_device_new(&dst);
 	dev->io = bt_io_connect(signaling_connect_cb, dev, NULL, &err,
@@ -194,7 +195,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 failed;
 	}
 
 	ba2str(&dev->dst, addr);
@@ -202,26 +204,29 @@ 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;
+	status = HAL_STATUS_SUCCESS;
+
+failed:
+	ipc_send_rsp(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;
 	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 failed;
+	}
 
 	dev = l->data;
 
@@ -231,28 +236,19 @@ 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;
-}
-
-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;
-	}
+	status = HAL_STATUS_SUCCESS;
 
-	ipc_send_rsp(HAL_SERVICE_ID_A2DP, opcode, status);
+failed:
+	ipc_send_rsp(HAL_SERVICE_ID_A2DP, HAL_OP_A2DP_DISCONNECT, status);
 }
 
+static const struct ipc_handler cmd_handlers[] = {
+	/* HAL_OP_A2DP_CONNECT */
+	{ bt_a2dp_connect, false, sizeof(struct hal_cmd_a2dp_connect) },
+	/* HAL_OP_A2DP_DISCONNECT */
+	{ bt_a2dp_disconnect, false, sizeof(struct hal_cmd_a2dp_disconnect) },
+};
+
 static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
 {
 	struct a2dp_device *dev;
@@ -380,6 +376,9 @@ bool bt_a2dp_register(const bdaddr_t *addr)
 	}
 	record_id = rec->handle;
 
+	ipc_register(HAL_SERVICE_ID_A2DP, cmd_handlers,
+				sizeof(cmd_handlers)/sizeof(cmd_handlers[0]));
+
 	return true;
 }
 
@@ -397,6 +396,8 @@ void bt_a2dp_unregister(void)
 	g_slist_foreach(devices, a2dp_device_disconnected, NULL);
 	devices = NULL;
 
+
+	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 2a1eb3c..7e9b2f6 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(const bdaddr_t *addr);
 void bt_a2dp_unregister(void);
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH v3 9/9] android/socket: Use generic IPC message handling for commands
From: Szymon Janc @ 2013-12-02 12:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1385986848-8023-1-git-send-email-szymon.janc@tieto.com>

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

diff --git a/android/socket.c b/android/socket.c
index 4f47861..76b40c8 100644
--- a/android/socket.c
+++ b/android/socket.c
@@ -650,15 +650,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("");
@@ -666,11 +666,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 failed;
+
+		chan = cmd->channel;
+		sec_level = BT_IO_SEC_MEDIUM;
 	} else {
 		chan = profile->channel;
 		sec_level = profile->sec_level;
@@ -680,7 +679,7 @@ static int handle_listen(void *buf)
 
 	rfsock = create_rfsock(-1, &hal_fd);
 	if (!rfsock)
-		return -1;
+		goto failed;
 
 	io = bt_io_listen(accept_cb, NULL, rfsock, NULL, &err,
 				BT_IO_OPT_SOURCE_BDADDR, &adapter_addr,
@@ -690,8 +689,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 failed;
 	}
 
 	rfsock->real_sock = g_io_channel_unix_get_fd(io);
@@ -703,15 +701,27 @@ 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 failed;
 	}
 
 	rfsock->service_handle = sdp_service_register(profile, cmd->name);
 
 	servers = g_list_append(servers, rfsock);
 
-	return hal_fd;
+	ipc_send_rsp_full(HAL_SERVICE_ID_SOCK, HAL_OP_SOCK_LISTEN, 0, NULL,
+									hal_fd);
+	close(hal_fd);
+	return;
+
+failed:
+	ipc_send_rsp(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)
@@ -865,9 +875,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;
@@ -876,7 +886,7 @@ static int handle_connect(void *buf)
 
 	rfsock = create_rfsock(-1, &hal_fd);
 	if (!rfsock)
-		return -1;
+		goto failed;
 
 	android2bdaddr(cmd->bdaddr, &rfsock->dst);
 
@@ -890,55 +900,41 @@ static int handle_connect(void *buf)
 					sdp_search_cb, rfsock, NULL) < 0) {
 		error("Failed to search SDP records");
 		cleanup_rfsock(rfsock);
-		return -1;
+		goto failed;
 	}
 
-	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_rsp_full(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_rsp_full(HAL_SERVICE_ID_SOCK, opcode, 0, NULL, fd);
+	ipc_send_rsp_full(HAL_SERVICE_ID_SOCK, HAL_OP_SOCK_CONNECT, 0, NULL,
+									hal_fd);
+	close(hal_fd);
+	return;
 
-		if (close(fd) < 0)
-			error("close() fd %d failed: %s", fd, strerror(errno));
+failed:
+	ipc_send_rsp(HAL_SERVICE_ID_SOCK, HAL_OP_SOCK_CONNECT,
+							HAL_STATUS_FAILED);
 
-		return;
-	default:
-		DBG("Unhandled command, opcode 0x%x", opcode);
-		break;
-	}
-
-	ipc_send_rsp(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 */
+	{ handle_listen, false, sizeof(struct hal_cmd_sock_listen) },
+	/* HAL_OP_SOCK_CONNECT */
+	{ handle_connect, false, sizeof(struct hal_cmd_sock_connect) },
+};
+
 void bt_socket_register(const bdaddr_t *addr)
 {
 	DBG("");
 
 	bacpy(&adapter_addr, addr);
+	ipc_register(HAL_SERVICE_ID_SOCK, cmd_handlers,
+				sizeof(cmd_handlers)/sizeof(cmd_handlers[0]));
 }
 
 void bt_socket_unregister(void)
 {
 	DBG("");
+
+	ipc_unregister(HAL_SERVICE_ID_SOCK);
 }
-- 
1.8.3.2


^ permalink raw reply related

* Re: [PATCH v3 0/9] android: IPC improvements - daemon part
From: Luiz Augusto von Dentz @ 2013-12-02 14:53 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1385986848-8023-1-git-send-email-szymon.janc@tieto.com>

Hi Szymon,

On Mon, Dec 2, 2013 at 2:20 PM, Szymon Janc <szymon.janc@tieto.com> wrote:
> v3:
>  - rebased againt latest pan changes
>
> v2:
>  - rebased against latest IPC helpers improvements
>  - more compact command handlers table format
>  - error handling path in command handlers improved according to Johan comments
>  - randmon small fixes
>  - patches not directly related to refactor removed from serie, will
>    be send after this is merged
>
> v1:
> This serie implements IPC message handling iprovments in daemon similar
> to what is already done in HAL part.
>
> Szymon Janc (9):
>   android: Add initial code for IPC message handlers
>   android/main: Use generic IPC message handling for core service
>   android/main: Use common exit path in core service functions
>   android/bluetooth: Use generic IPC msg handling for commands
>   android/bluetooth: Make property handling function return HAL status
>   android/hidhost: Use generic IPC message handling for commands
>   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/a2dp.c      |  69 ++++----
>  android/a2dp.h      |   2 -
>  android/bluetooth.c | 477 ++++++++++++++++++++++++++++++++++------------------
>  android/hidhost.c   | 309 ++++++++++++++++++++--------------
>  android/hidhost.h   |   2 -
>  android/ipc.c       |  78 +++++++++
>  android/ipc.h       |  10 ++
>  android/main.c      | 123 +++++---------
>  android/pan.c       |  87 +++++-----
>  android/pan.h       |   2 -
>  android/socket.c    | 102 ++++++-----
>  11 files changed, 754 insertions(+), 507 deletions(-)
>
> --
> 1.8.3.2

Pushed, thanks.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* [PATCH] android: Move sockets handling from main to IPC code
From: Szymon Janc @ 2013-12-02 14:55 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This moves IO handling to IPC code making it fully responsible for
creating and veryfing IPC messages exchange.
---

This should be applied on top of IPC daemon improvements.

 android/ipc.c  | 273 ++++++++++++++++++++++++++++++++++++++++++---------------
 android/ipc.h  |   4 +-
 android/main.c | 151 +------------------------------
 3 files changed, 207 insertions(+), 221 deletions(-)

diff --git a/android/ipc.c b/android/ipc.c
index 56f328b..1d369a8 100644
--- a/android/ipc.c
+++ b/android/ipc.c
@@ -32,6 +32,10 @@
 #include <signal.h>
 #include <stdbool.h>
 #include <sys/socket.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <unistd.h>
+#include <glib.h>
 
 #include "hal-msg.h"
 #include "ipc.h"
@@ -44,19 +48,202 @@ struct service_handler {
 
 static struct service_handler services[HAL_SERVICE_ID_MAX + 1];
 
-static int cmd_sk = -1;
-static int notif_sk = -1;
+static GIOChannel *cmd_io = NULL;
+static GIOChannel *notif_io = NULL;
 
-void ipc_init(int command_sk, int notification_sk)
+static void ipc_handle_msg(const void *buf, ssize_t len)
 {
-	cmd_sk = command_sk;
-	notif_sk = notification_sk;
+	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 0x%x for service 0x%x, terminating",
+						msg->opcode, msg->service_id);
+		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: size invalid opcode 0x%x service 0x%x, terminating",
+						msg->service_id, msg->opcode);
+		raise(SIGTERM);
+		return;
+	}
+
+	handler->handler(msg->payload, msg->len);
+}
+
+static gboolean cmd_watch_cb(GIOChannel *io, GIOCondition cond,
+							gpointer user_data)
+{
+	char buf[BLUEZ_HAL_MTU];
+	ssize_t ret;
+	int fd;
+
+	if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
+		info("IPC: command socket closed, terminating");
+		goto fail;
+	}
+
+	fd = g_io_channel_unix_get_fd(io);
+
+	ret = read(fd, buf, sizeof(buf));
+	if (ret < 0) {
+		error("IPC: command read failed, terminating (%s)",
+							strerror(errno));
+		goto fail;
+	}
+
+	ipc_handle_msg(buf, ret);
+	return TRUE;
+
+fail:
+	raise(SIGTERM);
+	return FALSE;
+}
+
+static gboolean notif_watch_cb(GIOChannel *io, GIOCondition cond,
+							gpointer user_data)
+{
+	info("IPC: notification socket closed, terminating");
+	raise(SIGTERM);
+
+	return FALSE;
+}
+
+static GIOChannel *connect_hal(GIOFunc connect_cb)
+{
+	struct sockaddr_un addr;
+	GIOCondition cond;
+	GIOChannel *io;
+	int sk;
+
+	sk = socket(PF_LOCAL, SOCK_SEQPACKET, 0);
+	if (sk < 0) {
+		error("IPC: failed to create socket: %d (%s)", errno,
+							strerror(errno));
+		return NULL;
+	}
+
+	io = g_io_channel_unix_new(sk);
+
+	g_io_channel_set_close_on_unref(io, TRUE);
+	g_io_channel_set_flags(io, G_IO_FLAG_NONBLOCK, NULL);
+
+	memset(&addr, 0, sizeof(addr));
+	addr.sun_family = AF_UNIX;
+
+	memcpy(addr.sun_path, BLUEZ_HAL_SK_PATH, sizeof(BLUEZ_HAL_SK_PATH));
+
+	if (connect(sk, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
+		error("IPC: failed to connect HAL socket: %d (%s)", errno,
+							strerror(errno));
+		g_io_channel_unref(io);
+		return NULL;
+	}
+
+	cond = G_IO_OUT | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
+
+	g_io_add_watch(io, cond, connect_cb, NULL);
+
+	return io;
+}
+
+static gboolean notif_connect_cb(GIOChannel *io, GIOCondition cond,
+							gpointer user_data)
+{
+	DBG("");
+
+	if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
+		error("IPC: notification socket connect failed, terminating");
+		raise(SIGTERM);
+		return FALSE;
+	}
+
+	cond = G_IO_ERR | G_IO_HUP | G_IO_NVAL;
+
+	g_io_add_watch(io, cond, notif_watch_cb, NULL);
+
+	cond = G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
+
+	g_io_add_watch(cmd_io, cond, cmd_watch_cb, NULL);
+
+	info("IPC: successfully connected");
+
+	return FALSE;
+}
+
+static gboolean cmd_connect_cb(GIOChannel *io, GIOCondition cond,
+							gpointer user_data)
+{
+	DBG("");
+
+	if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
+		error("IPC: command socket connect failed, terminating");
+		raise(SIGTERM);
+		return FALSE;
+	}
+
+	notif_io = connect_hal(notif_connect_cb);
+	if (!notif_io)
+		raise(SIGTERM);
+
+	return FALSE;
+}
+
+void ipc_init(void)
+{
+	cmd_io = connect_hal(cmd_connect_cb);
 }
 
 void ipc_cleanup(void)
 {
-	cmd_sk = -1;
-	notif_sk = -1;
+	if (cmd_io) {
+		g_io_channel_shutdown(cmd_io, TRUE, NULL);
+		g_io_channel_unref(cmd_io);
+		cmd_io = NULL;
+	}
+
+	if (notif_io) {
+		g_io_channel_shutdown(notif_io, TRUE, NULL);
+		g_io_channel_unref(notif_io);
+		notif_io = NULL;
+	}
 }
 
 static void ipc_send(int sk, uint8_t service_id, uint8_t opcode, uint16_t len,
@@ -107,30 +294,35 @@ static void ipc_send(int sk, uint8_t service_id, uint8_t opcode, uint16_t len,
 void ipc_send_rsp(uint8_t service_id, uint8_t opcode, uint8_t status)
 {
 	struct hal_status s;
+	int sk;
+
+	sk = g_io_channel_unix_get_fd(cmd_io);
 
 	if (status == HAL_STATUS_SUCCESS) {
-		ipc_send(cmd_sk, service_id, opcode, 0, NULL, -1);
+		ipc_send(sk, service_id, opcode, 0, NULL, -1);
 		return;
 	}
 
 	s.code = status;
 
-	ipc_send(cmd_sk, service_id, HAL_OP_STATUS, sizeof(s), &s, -1);
+	ipc_send(sk, service_id, HAL_OP_STATUS, sizeof(s), &s, -1);
 }
 
 void ipc_send_rsp_full(uint8_t service_id, uint8_t opcode, uint16_t len,
 							void *param, int fd)
 {
-	ipc_send(cmd_sk, service_id, opcode, len, param, fd);
+	ipc_send(g_io_channel_unix_get_fd(cmd_io), service_id, opcode, len,
+								param, fd);
 }
 
 void ipc_send_notif(uint8_t service_id, uint8_t opcode,  uint16_t len,
 								void *param)
 {
-	if (notif_sk < 0)
+	if (!notif_io)
 		return;
 
-	ipc_send(notif_sk, service_id, opcode, len, param, -1);
+	ipc_send(g_io_channel_unix_get_fd(notif_io), service_id, opcode, len,
+								param, -1);
 }
 
 void ipc_register(uint8_t service, const struct ipc_handler *handlers,
@@ -145,60 +337,3 @@ 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 0x%x for service 0x%x, terminating",
-						msg->opcode, msg->service_id);
-		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: size invalid opcode 0x%x service 0x%x, terminating",
-						msg->service_id, msg->opcode);
-		raise(SIGTERM);
-		return;
-	}
-
-	handler->handler(msg->payload, msg->len);
-}
diff --git a/android/ipc.h b/android/ipc.h
index 9d0c5e1..6cd102b 100644
--- a/android/ipc.h
+++ b/android/ipc.h
@@ -26,7 +26,7 @@ struct ipc_handler {
 	bool var_len;
 	size_t data_len;
 };
-void ipc_init(int command_sk, int notification_sk);
+void ipc_init(void);
 void ipc_cleanup(void);
 
 void ipc_send_rsp(uint8_t service_id, uint8_t opcode, uint8_t status);
@@ -37,5 +37,3 @@ void ipc_send_notif(uint8_t service_id, uint8_t opcode,  uint16_t len,
 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);
diff --git a/android/main.c b/android/main.c
index c0f8901..b9655c5 100644
--- a/android/main.c
+++ b/android/main.c
@@ -36,8 +36,6 @@
 #include <unistd.h>
 
 #include <sys/signalfd.h>
-#include <sys/socket.h>
-#include <sys/un.h>
 
 #include <glib.h>
 
@@ -69,9 +67,6 @@ static bdaddr_t adapter_bdaddr;
 
 static GMainLoop *event_loop;
 
-static GIOChannel *hal_cmd_io = NULL;
-static GIOChannel *hal_notif_io = NULL;
-
 static bool services[HAL_SERVICE_ID_MAX + 1] = { false };
 
 static void service_register(const void *buf, uint16_t len)
@@ -209,127 +204,6 @@ static void stop_bluetooth(void)
 	g_timeout_add_seconds(SHUTDOWN_GRACE_SECONDS, quit_eventloop, NULL);
 }
 
-static gboolean cmd_watch_cb(GIOChannel *io, GIOCondition cond,
-							gpointer user_data)
-{
-	char buf[BLUEZ_HAL_MTU];
-	ssize_t ret;
-	int fd;
-
-	if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
-		info("HAL command socket closed, terminating");
-		goto fail;
-	}
-
-	fd = g_io_channel_unix_get_fd(io);
-
-	ret = read(fd, buf, sizeof(buf));
-	if (ret < 0) {
-		error("HAL command read failed, terminating (%s)",
-							strerror(errno));
-		goto fail;
-	}
-
-	ipc_handle_msg(buf, ret);
-	return TRUE;
-
-fail:
-	stop_bluetooth();
-	return FALSE;
-}
-
-static gboolean notif_watch_cb(GIOChannel *io, GIOCondition cond,
-							gpointer user_data)
-{
-	info("HAL notification socket closed, terminating");
-	stop_bluetooth();
-
-	return FALSE;
-}
-
-static GIOChannel *connect_hal(GIOFunc connect_cb)
-{
-	struct sockaddr_un addr;
-	GIOCondition cond;
-	GIOChannel *io;
-	int sk;
-
-	sk = socket(PF_LOCAL, SOCK_SEQPACKET, 0);
-	if (sk < 0) {
-		error("Failed to create socket: %d (%s)", errno,
-							strerror(errno));
-		return NULL;
-	}
-
-	io = g_io_channel_unix_new(sk);
-
-	g_io_channel_set_close_on_unref(io, TRUE);
-	g_io_channel_set_flags(io, G_IO_FLAG_NONBLOCK, NULL);
-
-	memset(&addr, 0, sizeof(addr));
-	addr.sun_family = AF_UNIX;
-
-	memcpy(addr.sun_path, BLUEZ_HAL_SK_PATH, sizeof(BLUEZ_HAL_SK_PATH));
-
-	if (connect(sk, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
-		error("Failed to connect HAL socket: %d (%s)", errno,
-							strerror(errno));
-		g_io_channel_unref(io);
-		return NULL;
-	}
-
-	cond = G_IO_OUT | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
-
-	g_io_add_watch(io, cond, connect_cb, NULL);
-
-	return io;
-}
-
-static gboolean notif_connect_cb(GIOChannel *io, GIOCondition cond,
-							gpointer user_data)
-{
-	DBG("");
-
-	if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
-		stop_bluetooth();
-		return FALSE;
-	}
-
-	cond = G_IO_ERR | G_IO_HUP | G_IO_NVAL;
-
-	g_io_add_watch(io, cond, notif_watch_cb, NULL);
-
-	cond = G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
-
-	g_io_add_watch(hal_cmd_io, cond, cmd_watch_cb, NULL);
-
-	ipc_init(g_io_channel_unix_get_fd(hal_cmd_io),
-				g_io_channel_unix_get_fd(hal_notif_io));
-
-	info("Successfully connected to HAL");
-
-	return FALSE;
-}
-
-static gboolean cmd_connect_cb(GIOChannel *io, GIOCondition cond,
-							gpointer user_data)
-{
-	DBG("");
-
-	if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
-		stop_bluetooth();
-		return FALSE;
-	}
-
-	hal_notif_io = connect_hal(notif_connect_cb);
-	if (!hal_notif_io) {
-		error("Cannot connect to HAL, terminating");
-		stop_bluetooth();
-	}
-
-	return FALSE;
-}
-
 static void adapter_ready(int err, const bdaddr_t *addr)
 {
 	if (err < 0) {
@@ -346,11 +220,7 @@ static void adapter_ready(int err, const bdaddr_t *addr)
 
 	info("Adapter initialized");
 
-	hal_cmd_io = connect_hal(cmd_connect_cb);
-	if (!hal_cmd_io) {
-		error("Cannot connect to HAL, terminating");
-		stop_bluetooth();
-	}
+	ipc_init();
 }
 
 static gboolean signal_handler(GIOChannel *channel, GIOCondition cond,
@@ -433,23 +303,6 @@ static GOptionEntry options[] = {
 	{ NULL }
 };
 
-static void cleanup_hal_connection(void)
-{
-	if (hal_cmd_io) {
-		g_io_channel_shutdown(hal_cmd_io, TRUE, NULL);
-		g_io_channel_unref(hal_cmd_io);
-		hal_cmd_io = NULL;
-	}
-
-	if (hal_notif_io) {
-		g_io_channel_shutdown(hal_notif_io, TRUE, NULL);
-		g_io_channel_unref(hal_notif_io);
-		hal_notif_io = NULL;
-	}
-
-	ipc_cleanup();
-}
-
 static void cleanup_services(void)
 {
 	int i;
@@ -592,7 +445,7 @@ int main(int argc, char *argv[])
 
 	cleanup_services();
 
-	cleanup_hal_connection();
+	ipc_cleanup();
 	stop_sdp_server();
 	bt_bluetooth_cleanup();
 	g_main_loop_unref(event_loop);
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH] android: Use G_N_ELEMENTS macro for table elements calculation
From: Szymon Janc @ 2013-12-02 15:17 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

It is more common in codebase to use this macro instead of opencoded
(sizeof(foo)/sizeof(foo[0])).
---
 android/a2dp.c      | 2 +-
 android/bluetooth.c | 2 +-
 android/hidhost.c   | 2 +-
 android/main.c      | 2 +-
 android/pan.c       | 2 +-
 android/socket.c    | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/android/a2dp.c b/android/a2dp.c
index 98c138e..cee4bfa 100644
--- a/android/a2dp.c
+++ b/android/a2dp.c
@@ -377,7 +377,7 @@ bool bt_a2dp_register(const bdaddr_t *addr)
 	record_id = rec->handle;
 
 	ipc_register(HAL_SERVICE_ID_A2DP, cmd_handlers,
-				sizeof(cmd_handlers)/sizeof(cmd_handlers[0]));
+						G_N_ELEMENTS(cmd_handlers));
 
 	return true;
 }
diff --git a/android/bluetooth.c b/android/bluetooth.c
index a39e7bf..6174b1f 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -2418,7 +2418,7 @@ void bt_bluetooth_register(void)
 	DBG("");
 
 	ipc_register(HAL_SERVICE_ID_BLUETOOTH, cmd_handlers,
-				sizeof(cmd_handlers)/sizeof(cmd_handlers[0]));
+						G_N_ELEMENTS(cmd_handlers));
 }
 
 void bt_bluetooth_unregister(void)
diff --git a/android/hidhost.c b/android/hidhost.c
index 38194d0..8bfdfed 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -1279,7 +1279,7 @@ bool bt_hid_register(const bdaddr_t *addr)
 	}
 
 	ipc_register(HAL_SERVICE_ID_HIDHOST, cmd_handlers,
-				sizeof(cmd_handlers)/sizeof(cmd_handlers[0]));
+						G_N_ELEMENTS(cmd_handlers));
 
 	return true;
 }
diff --git a/android/main.c b/android/main.c
index b9655c5..5210b4b 100644
--- a/android/main.c
+++ b/android/main.c
@@ -430,7 +430,7 @@ int main(int argc, char *argv[])
 	start_sdp_server(0, 0);
 
 	ipc_register(HAL_SERVICE_ID_CORE, cmd_handlers,
-				sizeof(cmd_handlers)/sizeof(cmd_handlers[0]));
+						G_N_ELEMENTS(cmd_handlers));
 
 	DBG("Entering main loop");
 
diff --git a/android/pan.c b/android/pan.c
index 3270aa4..fe6ee26 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -322,7 +322,7 @@ bool bt_pan_register(const bdaddr_t *addr)
 	}
 
 	ipc_register(HAL_SERVICE_ID_PAN, cmd_handlers,
-				sizeof(cmd_handlers)/sizeof(cmd_handlers[0]));
+						G_N_ELEMENTS(cmd_handlers));
 
 	return true;
 }
diff --git a/android/socket.c b/android/socket.c
index 76b40c8..c9eca44 100644
--- a/android/socket.c
+++ b/android/socket.c
@@ -929,7 +929,7 @@ void bt_socket_register(const bdaddr_t *addr)
 
 	bacpy(&adapter_addr, addr);
 	ipc_register(HAL_SERVICE_ID_SOCK, cmd_handlers,
-				sizeof(cmd_handlers)/sizeof(cmd_handlers[0]));
+						G_N_ELEMENTS(cmd_handlers));
 }
 
 void bt_socket_unregister(void)
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 1/4] android/socket: Cleanup sockets on unregister
From: Andrei Emeltchenko @ 2013-12-02 15:46 UTC (permalink / raw)
  To: linux-bluetooth

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

This cleans up rfsock structures closing all sockets and making general cleanup
for servers and for connections. This will be called form socket unregister.
---
 android/socket.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/android/socket.c b/android/socket.c
index 76b40c8..4502e90 100644
--- a/android/socket.c
+++ b/android/socket.c
@@ -93,8 +93,10 @@ static struct rfcomm_sock *create_rfsock(int sock, int *hal_fd)
 	return rfsock;
 }
 
-static void cleanup_rfsock(struct rfcomm_sock *rfsock)
+static void cleanup_rfsock(gpointer data)
 {
+	struct rfcomm_sock *rfsock = data;
+
 	DBG("rfsock: %p fd %d real_sock %d chan %u",
 		rfsock, rfsock->fd, rfsock->real_sock, rfsock->channel);
 
@@ -936,5 +938,8 @@ void bt_socket_unregister(void)
 {
 	DBG("");
 
+	g_list_free_full(connections, cleanup_rfsock);
+	g_list_free_full(servers, cleanup_rfsock);
+
 	ipc_unregister(HAL_SERVICE_ID_SOCK);
 }
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 2/4] android/a2dp: Fix possible NULL dereference
From: Andrei Emeltchenko @ 2013-12-02 15:46 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1385999188-1546-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

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

Since a2dp_record may return NULL, check return value. This
silences static analysers tools.
---
 android/a2dp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/android/a2dp.c b/android/a2dp.c
index 98c138e..324a211 100644
--- a/android/a2dp.c
+++ b/android/a2dp.c
@@ -366,9 +366,10 @@ bool bt_a2dp_register(const bdaddr_t *addr)
 	}
 
 	rec = a2dp_record();
-	if (bt_adapter_add_record(rec, SVC_HINT_CAPTURING) < 0) {
+	if (!rec || bt_adapter_add_record(rec, SVC_HINT_CAPTURING) < 0) {
 		error("Failed to register on A2DP record");
-		sdp_record_free(rec);
+		if (rec)
+			sdp_record_free(rec);
 		g_io_channel_shutdown(server, TRUE, NULL);
 		g_io_channel_unref(server);
 		server = NULL;
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 3/4] android/pan: Remove unneeded NULL assignment
From: Andrei Emeltchenko @ 2013-12-02 15:46 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1385999188-1546-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

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

---
 android/pan.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/android/pan.c b/android/pan.c
index 3270aa4..8e719b2 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -81,7 +81,6 @@ static void pan_device_free(struct pan_device *dev)
 
 	devices = g_slist_remove(devices, dev);
 	g_free(dev);
-	dev = NULL;
 }
 
 static void bt_pan_notify_conn_state(struct pan_device *dev, uint8_t state)
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 4/4] android/pan: Fix no return on error path
From: Andrei Emeltchenko @ 2013-12-02 15:46 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1385999188-1546-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

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

This fixes possible crash in case connect fails.
---
 android/pan.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/android/pan.c b/android/pan.c
index 8e719b2..1c1015a 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -172,6 +172,7 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer data)
 		error("%s", err->message);
 		bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
 		pan_device_free(dev);
+		return;
 	}
 
 	src = (local_role == HAL_PAN_ROLE_NAP) ? BNEP_SVC_NAP : BNEP_SVC_PANU;
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH] android/socket Use 4K buffer for socket handling
From: Andrei Emeltchenko @ 2013-12-02 16:06 UTC (permalink / raw)
  To: linux-bluetooth

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

Make SOCKET_BUFFER define and use 4K instead of 1K.
---
 android/socket.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/android/socket.c b/android/socket.c
index 4502e90..98ec3ec 100644
--- a/android/socket.c
+++ b/android/socket.c
@@ -52,6 +52,8 @@
 
 #define SVC_HINT_OBEX 0x10
 
+#define SOCKET_BUFFER 4096
+
 static bdaddr_t adapter_addr;
 
 /* Simple list of RFCOMM server sockets */
@@ -487,7 +489,7 @@ static gboolean sock_stack_event_cb(GIOChannel *io, GIOCondition cond,
 								gpointer data)
 {
 	struct rfcomm_sock *rfsock = data;
-	unsigned char buf[1024];
+	unsigned char buf[SOCKET_BUFFER];
 	int len, sent;
 
 	if (cond & G_IO_HUP) {
@@ -526,7 +528,7 @@ static gboolean sock_rfcomm_event_cb(GIOChannel *io, GIOCondition cond,
 								gpointer data)
 {
 	struct rfcomm_sock *rfsock = data;
-	unsigned char buf[1024];
+	unsigned char buf[SOCKET_BUFFER];
 	int len, sent;
 
 	if (cond & G_IO_HUP) {
-- 
1.8.3.2


^ permalink raw reply related

* Help for USB Dongle Asus USB-BT400 with Raspberry Pi and Bluez
From: Alessandro Nicolosi @ 2013-12-02 18:03 UTC (permalink / raw)
  To: linux-bluetooth

Hi guys,

I hope this can be the right place where to find help for my problems.

I need to use the dongle Asus USB-BT400 to provide Bluetooth support
for my Raspberry Pi in which I installed the Raspbian distribution.
I have to work with Raspberry Pi and a BLE device called SensorTag
from Texas Instruments. So I have to use the gatttool command from
Bluez.
I followed the instructions from this site to set up Bluez in my
Raspberry Pi with Raspbian.

 http://mike.saunby.net/2013/04/raspberry-pi-and-ti-cc2541-sensortag.html

I downloaded and installed the last version of Bluez 5.11.
I plugged in my BLE USB dongle in my Raspberry Pi but I think there
are some problems.
When I type in the command "hciconfig" I have no output results.
And also when I type in "hcitool dev" no devices is listed.
But when I type in "lsusb" the output is:

Bus 001 Device 002: ID 0424:9512 Standard Microsystems Corp.
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 001 Device 003: ID 0424:ec00 Standard Microsystems Corp.
Bus 001 Device 004: ID 0b05:17cb ASUSTek Computer, Inc.

The last one is my USB dongle, so I think the hardware is correctly recognized.
Which could be my problem?

I also tried the dongle Asus USB-BT400 in my notebook with Xubuntu
13.10 and it worked very well (hciconfig sees the bluetooth
interface).

I also read something about my dongle by googling and I found the
message linked below from this mailing list, so I thought you can help
me or give me a way forward.

http://marc.info/?l=linux-bluetooth&m=137812308709357&w=2

Maybe I can edit some source files to add the device ID? If yes, which
changes I have to do?

Thanks in advance for your help
Regards

-- 
Alessandro Nicolosi

^ permalink raw reply

* Missing BT_POWER option for l2cap sockets in bluetooth.h?
From: Mathieu Laurendeau @ 2013-12-02 20:22 UTC (permalink / raw)
  To: linux-bluetooth

Hello,

I believe the following defines are missing in the bluetooth.h file:

#define BT_POWER        9
struct bt_power {
         uint8_t force_active;
};
#define BT_POWER_FORCE_ACTIVE_OFF 0
#define BT_POWER_FORCE_ACTIVE_ON 1

Regards,
Mathieu

^ permalink raw reply

* Re: have to re-pair mouse every few hours
From: Brian J. Murrell @ 2013-12-02 22:35 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera
In-Reply-To: <1385936884.5405.3.camel@nuvo>

[-- Attachment #1: Type: text/plain, Size: 1172 bytes --]

On Sun, 2013-12-01 at 23:28 +0100, Bastien Nocera wrote: 
> 
> Upstream developers don't work on BlueZ 4.x anymore. Any chance for you
> to test using Fedora 20? It uses BlueZ 5.x which has a lot of fixes and
> architectural changes compared to 4.x.

So, assuming I'm supposed to use something like bluetooth-wizard to
start the pairing, that doesn't even see my mouse when I have the mouse
in pairing mode.  It does see a headset if I put it into pairing mode
though, so basic bluetooth functionality is at least working.  It's just
not seeing my mouse.

Unfortunately though F20 is a bit of a nightmare at the moment for me to
keep it up and running on the workstation where this mouse is having
problems so being able to test will be difficult as it will mean having
to switch back and forth between F19 to get work done and F20 where too
much stuff crashes for it to be usable.  It doesn't help that abrt
doesn't seem to be even able to install debuginfos so I can't even
report these crashes.  But I digress.

I will try to leave this F20 system up for the next little while in case
there is any debugging you want me to try.

Cheers,
b.



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* Re: Add empty udev rule to disable hid2hci by default
From: Alexander Holler @ 2013-12-02 23:15 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <529B7153.2060204@ahsoftware.de>

Am 01.12.2013 18:26, schrieb Alexander Holler:
> Hello.
>
> Almost every distribution gets it wrong and enables hid2hci by default
> (besides Fedora where I already intervened twice).
>
> This is a real problem, because it disables Bluetooth keyboards and/or
> mice which aren't paired with bluez, thus many Live-CDs and default
> installs aren't usable when only a Bluetooth keyboard is connect.
>
> An easy solution to disable that behaviour would be to install an empty
> rule in /etc/udev/rules.d named the same as the one in
> /lib/udev/rules.d. It could just contain a comment like

(...)

Unfortunately that isn't a very practical idea, as the rule(-file) would 
likely become installed again with every upgrade or reinstall. :(

> I think otherwise that problem will never go away. It's really
> unbelievable how many distributions got this wrong and thus how many
> Live-CDs and default installations are unusable when only a Bluetooth
> keyboard is used with a hid-aware Bluetooth dongle.

So another idea seems to be necessary to either educate distributions 
(or their package maintainers) that they should build a distinct package 
for hid2hci and don't install that package by default, or some mechanism 
which enables the user to enable/disable hid2hci (as it was the case 
before the udev-rule went to /lib),

Regards,

Alexander Holler

^ permalink raw reply

* Re: [PATCH 0/4] more sixaxis goodies
From: Johan Hedberg @ 2013-12-03  7:48 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1385905316-21800-1-git-send-email-szymon.janc@gmail.com>

Hi Szymon,

On Sun, Dec 01, 2013, Szymon Janc wrote:
> This adds support for setting PS3 controller LEDs both over Bluetooth
> and USB. More details in commits messages.
> 
> One thing I noticed is that setting LEDs over Bluetooth fails on kernel
> 3.10+ (write always returns 0). Could this be related to HIDP related
> work that was merged in 3.10?
> 
> Comments and testing are welcome.
>
> Szymon Janc (4):
>   sixaxis: Add support for setting PS3 controller LEDs
>   Rename device_is_connected to btd_device_is_connected
>   sixaxis: Skip controller setup if already connected over Bluetooth
>   sixaxis: Add support for setting LEDs when connected over USB
> 
>  plugins/sixaxis.c       | 164 +++++++++++++++++++++++++++++++++++++++++++++---
>  profiles/input/device.c |   4 +-
>  src/adapter.c           |   6 +-
>  src/device.c            |   6 +-
>  src/device.h            |   2 +-
>  5 files changed, 165 insertions(+), 17 deletions(-)

All four patches have been applied. Thanks.

Johan

^ permalink raw reply

* Re: Missing BT_POWER option for l2cap sockets in bluetooth.h?
From: Johan Hedberg @ 2013-12-03  9:22 UTC (permalink / raw)
  To: Mathieu Laurendeau; +Cc: linux-bluetooth
In-Reply-To: <529CEC22.60107@laurendeau.net>

Hi Mathieu,

On Mon, Dec 02, 2013, Mathieu Laurendeau wrote:
> I believe the following defines are missing in the bluetooth.h file:
> 
> #define BT_POWER        9
> struct bt_power {
>         uint8_t force_active;
> };
> #define BT_POWER_FORCE_ACTIVE_OFF 0
> #define BT_POWER_FORCE_ACTIVE_ON 1

I think the only user of this has been pre-4.2 Android versions
(considering that the feature came from Android developers), which is
probably why this never made it to bluez.git. Anyway, I just pushed a
patch to add it there.

What are you planning to use this for? Do you have a patch to contribute
to our A2DP implementation?

Johan

^ permalink raw reply

* [PATCH BlueZ] audio/AVCTP: Fix crash
From: Luiz Augusto von Dentz @ 2013-12-03  9:36 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

The following crash happens because the list l is modified within the
loop so it is no longer safe to call l->next directly:

Invalid read of size 8
   at 0x41F276: pending_create (avctp.c:1491)
   by 0x41F7C0: avctp_send_req.isra.6 (avctp.c:1539)
   by 0x41F887: avctp_passthrough_release (avctp.c:1643)
   by 0x41F9DF: avctp_passthrough_rsp (avctp.c:1698)
   by 0x41E9AC: session_cb (avctp.c:782)
   by 0x31D1647DF5: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3600.3)
   by 0x31D1648147: ??? (in /usr/lib64/libglib-2.0.so.0.3600.3)
   by 0x31D1648549: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3600.3)
   by 0x40A49F: main (main.c:587)
 Address 0x8 is not stack'd, malloc'd or (recently) free'd
---
 profiles/audio/avctp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index 4de981c..6669ddc 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -1488,7 +1488,7 @@ static struct avctp_pending_req *pending_create(struct avctp_channel *chan,
 	tmp = g_slist_copy(chan->processed);
 
 	/* Find first unused transaction id */
-	for (l = tmp; l; l = l->next) {
+	for (l = tmp; l; l = g_slist_next(l)) {
 		struct avctp_pending_req *req = l->data;
 
 		if (req->transaction == chan->transaction) {
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH] android/socket Use 64K buffer for socket handling
From: Andrei Emeltchenko @ 2013-12-03 10:17 UTC (permalink / raw)
  To: linux-bluetooth

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

Make SOCKET_BUFFER define and use 64K instead of 1K.
---
 android/socket.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/android/socket.c b/android/socket.c
index 4502e90..598d248 100644
--- a/android/socket.c
+++ b/android/socket.c
@@ -52,6 +52,8 @@
 
 #define SVC_HINT_OBEX 0x10
 
+#define SOCKET_BUFFER 65536
+
 static bdaddr_t adapter_addr;
 
 /* Simple list of RFCOMM server sockets */
@@ -487,7 +489,7 @@ static gboolean sock_stack_event_cb(GIOChannel *io, GIOCondition cond,
 								gpointer data)
 {
 	struct rfcomm_sock *rfsock = data;
-	unsigned char buf[1024];
+	unsigned char buf[SOCKET_BUFFER];
 	int len, sent;
 
 	if (cond & G_IO_HUP) {
@@ -526,7 +528,7 @@ static gboolean sock_rfcomm_event_cb(GIOChannel *io, GIOCondition cond,
 								gpointer data)
 {
 	struct rfcomm_sock *rfsock = data;
-	unsigned char buf[1024];
+	unsigned char buf[SOCKET_BUFFER];
 	int len, sent;
 
 	if (cond & G_IO_HUP) {
-- 
1.8.3.2


^ permalink raw reply related

* Re: [PATCH BlueZ] audio/AVCTP: Fix crash
From: Anderson Lizardo @ 2013-12-03 11:24 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: BlueZ development
In-Reply-To: <1386063385-31263-1-git-send-email-luiz.dentz@gmail.com>

Hi Luiz,

On Tue, Dec 3, 2013 at 5:36 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> --- a/profiles/audio/avctp.c
> +++ b/profiles/audio/avctp.c
> @@ -1488,7 +1488,7 @@ static struct avctp_pending_req *pending_create(struct avctp_channel *chan,
>         tmp = g_slist_copy(chan->processed);
>
>         /* Find first unused transaction id */
> -       for (l = tmp; l; l = l->next) {
> +       for (l = tmp; l; l = g_slist_next(l)) {

Are you sure this fixes the problem? AFAIK g_list_next() will still
access invalid memory unless the "next" pointer is saved *before* the
current entry is freed. See e.g. remove_temp_devices() in
src/adapter.c.

>                 struct avctp_pending_req *req = l->data;
>
>                 if (req->transaction == chan->transaction) {

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

^ permalink raw reply

* Re: [PATCH BlueZ] audio/AVCTP: Fix crash
From: Luiz Augusto von Dentz @ 2013-12-03 11:40 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: BlueZ development
In-Reply-To: <CAJdJm_M5nf5L-efYVtY-4UYT2S4JRypGKYgu7PoVOz9DFxBS6g@mail.gmail.com>

Hi Anderson,

On Tue, Dec 3, 2013 at 1:24 PM, Anderson Lizardo
<anderson.lizardo@openbossa.org> wrote:
> Hi Luiz,
>
> On Tue, Dec 3, 2013 at 5:36 AM, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>> --- a/profiles/audio/avctp.c
>> +++ b/profiles/audio/avctp.c
>> @@ -1488,7 +1488,7 @@ static struct avctp_pending_req *pending_create(struct avctp_channel *chan,
>>         tmp = g_slist_copy(chan->processed);
>>
>>         /* Find first unused transaction id */
>> -       for (l = tmp; l; l = l->next) {
>> +       for (l = tmp; l; l = g_slist_next(l)) {
>
> Are you sure this fixes the problem? AFAIK g_list_next() will still
> access invalid memory unless the "next" pointer is saved *before* the
> current entry is freed. See e.g. remove_temp_devices() in
> src/adapter.c.

Yep, I tested and it fixes the problem. The problem is not actually
removing the item, but reassigning tmp to the head of the list which
can be NULL so doing l = l->next before deleting the node doesn't help
in this case. Anyway this is now applied but we will be changing the
logic how to check the available transaction to use bitmask operations
so we find the next transaction without looping in a list.

^ permalink raw reply

* Re: [PATCH] android: Move sockets handling from main to IPC code
From: Luiz Augusto von Dentz @ 2013-12-03 12:09 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1385996138-17313-1-git-send-email-szymon.janc@tieto.com>

Hi Syzmon,

On Mon, Dec 2, 2013 at 4:55 PM, Szymon Janc <szymon.janc@tieto.com> wrote:
> This moves IO handling to IPC code making it fully responsible for
> creating and veryfing IPC messages exchange.
> ---
>
> This should be applied on top of IPC daemon improvements.
>
>  android/ipc.c  | 273 ++++++++++++++++++++++++++++++++++++++++++---------------
>  android/ipc.h  |   4 +-
>  android/main.c | 151 +------------------------------
>  3 files changed, 207 insertions(+), 221 deletions(-)
>
> diff --git a/android/ipc.c b/android/ipc.c
> index 56f328b..1d369a8 100644
> --- a/android/ipc.c
> +++ b/android/ipc.c
> @@ -32,6 +32,10 @@
>  #include <signal.h>
>  #include <stdbool.h>
>  #include <sys/socket.h>
> +#include <sys/socket.h>
> +#include <sys/un.h>
> +#include <unistd.h>
> +#include <glib.h>
>
>  #include "hal-msg.h"
>  #include "ipc.h"
> @@ -44,19 +48,202 @@ struct service_handler {
>
>  static struct service_handler services[HAL_SERVICE_ID_MAX + 1];
>
> -static int cmd_sk = -1;
> -static int notif_sk = -1;
> +static GIOChannel *cmd_io = NULL;
> +static GIOChannel *notif_io = NULL;
>
> -void ipc_init(int command_sk, int notification_sk)
> +static void ipc_handle_msg(const void *buf, ssize_t len)
>  {
> -       cmd_sk = command_sk;
> -       notif_sk = notification_sk;
> +       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 0x%x for service 0x%x, terminating",
> +                                               msg->opcode, msg->service_id);
> +               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: size invalid opcode 0x%x service 0x%x, terminating",
> +                                               msg->service_id, msg->opcode);
> +               raise(SIGTERM);
> +               return;
> +       }
> +
> +       handler->handler(msg->payload, msg->len);
> +}
> +
> +static gboolean cmd_watch_cb(GIOChannel *io, GIOCondition cond,
> +                                                       gpointer user_data)
> +{
> +       char buf[BLUEZ_HAL_MTU];
> +       ssize_t ret;
> +       int fd;
> +
> +       if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
> +               info("IPC: command socket closed, terminating");
> +               goto fail;
> +       }
> +
> +       fd = g_io_channel_unix_get_fd(io);
> +
> +       ret = read(fd, buf, sizeof(buf));
> +       if (ret < 0) {
> +               error("IPC: command read failed, terminating (%s)",
> +                                                       strerror(errno));
> +               goto fail;
> +       }
> +
> +       ipc_handle_msg(buf, ret);
> +       return TRUE;
> +
> +fail:
> +       raise(SIGTERM);
> +       return FALSE;
> +}
> +
> +static gboolean notif_watch_cb(GIOChannel *io, GIOCondition cond,
> +                                                       gpointer user_data)
> +{
> +       info("IPC: notification socket closed, terminating");
> +       raise(SIGTERM);
> +
> +       return FALSE;
> +}
> +
> +static GIOChannel *connect_hal(GIOFunc connect_cb)
> +{
> +       struct sockaddr_un addr;
> +       GIOCondition cond;
> +       GIOChannel *io;
> +       int sk;
> +
> +       sk = socket(PF_LOCAL, SOCK_SEQPACKET, 0);
> +       if (sk < 0) {
> +               error("IPC: failed to create socket: %d (%s)", errno,
> +                                                       strerror(errno));
> +               return NULL;
> +       }
> +
> +       io = g_io_channel_unix_new(sk);
> +
> +       g_io_channel_set_close_on_unref(io, TRUE);
> +       g_io_channel_set_flags(io, G_IO_FLAG_NONBLOCK, NULL);
> +
> +       memset(&addr, 0, sizeof(addr));
> +       addr.sun_family = AF_UNIX;
> +
> +       memcpy(addr.sun_path, BLUEZ_HAL_SK_PATH, sizeof(BLUEZ_HAL_SK_PATH));
> +
> +       if (connect(sk, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
> +               error("IPC: failed to connect HAL socket: %d (%s)", errno,
> +                                                       strerror(errno));
> +               g_io_channel_unref(io);
> +               return NULL;
> +       }
> +
> +       cond = G_IO_OUT | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
> +
> +       g_io_add_watch(io, cond, connect_cb, NULL);
> +
> +       return io;
> +}
> +
> +static gboolean notif_connect_cb(GIOChannel *io, GIOCondition cond,
> +                                                       gpointer user_data)
> +{
> +       DBG("");
> +
> +       if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
> +               error("IPC: notification socket connect failed, terminating");
> +               raise(SIGTERM);
> +               return FALSE;
> +       }
> +
> +       cond = G_IO_ERR | G_IO_HUP | G_IO_NVAL;
> +
> +       g_io_add_watch(io, cond, notif_watch_cb, NULL);
> +
> +       cond = G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
> +
> +       g_io_add_watch(cmd_io, cond, cmd_watch_cb, NULL);
> +
> +       info("IPC: successfully connected");
> +
> +       return FALSE;
> +}
> +
> +static gboolean cmd_connect_cb(GIOChannel *io, GIOCondition cond,
> +                                                       gpointer user_data)
> +{
> +       DBG("");
> +
> +       if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
> +               error("IPC: command socket connect failed, terminating");
> +               raise(SIGTERM);
> +               return FALSE;
> +       }
> +
> +       notif_io = connect_hal(notif_connect_cb);
> +       if (!notif_io)
> +               raise(SIGTERM);
> +
> +       return FALSE;
> +}
> +
> +void ipc_init(void)
> +{
> +       cmd_io = connect_hal(cmd_connect_cb);
>  }
>
>  void ipc_cleanup(void)
>  {
> -       cmd_sk = -1;
> -       notif_sk = -1;
> +       if (cmd_io) {
> +               g_io_channel_shutdown(cmd_io, TRUE, NULL);
> +               g_io_channel_unref(cmd_io);
> +               cmd_io = NULL;
> +       }
> +
> +       if (notif_io) {
> +               g_io_channel_shutdown(notif_io, TRUE, NULL);
> +               g_io_channel_unref(notif_io);
> +               notif_io = NULL;
> +       }
>  }
>
>  static void ipc_send(int sk, uint8_t service_id, uint8_t opcode, uint16_t len,
> @@ -107,30 +294,35 @@ static void ipc_send(int sk, uint8_t service_id, uint8_t opcode, uint16_t len,
>  void ipc_send_rsp(uint8_t service_id, uint8_t opcode, uint8_t status)
>  {
>         struct hal_status s;
> +       int sk;
> +
> +       sk = g_io_channel_unix_get_fd(cmd_io);
>
>         if (status == HAL_STATUS_SUCCESS) {
> -               ipc_send(cmd_sk, service_id, opcode, 0, NULL, -1);
> +               ipc_send(sk, service_id, opcode, 0, NULL, -1);
>                 return;
>         }
>
>         s.code = status;
>
> -       ipc_send(cmd_sk, service_id, HAL_OP_STATUS, sizeof(s), &s, -1);
> +       ipc_send(sk, service_id, HAL_OP_STATUS, sizeof(s), &s, -1);
>  }
>
>  void ipc_send_rsp_full(uint8_t service_id, uint8_t opcode, uint16_t len,
>                                                         void *param, int fd)
>  {
> -       ipc_send(cmd_sk, service_id, opcode, len, param, fd);
> +       ipc_send(g_io_channel_unix_get_fd(cmd_io), service_id, opcode, len,
> +                                                               param, fd);
>  }
>
>  void ipc_send_notif(uint8_t service_id, uint8_t opcode,  uint16_t len,
>                                                                 void *param)
>  {
> -       if (notif_sk < 0)
> +       if (!notif_io)
>                 return;
>
> -       ipc_send(notif_sk, service_id, opcode, len, param, -1);
> +       ipc_send(g_io_channel_unix_get_fd(notif_io), service_id, opcode, len,
> +                                                               param, -1);
>  }
>
>  void ipc_register(uint8_t service, const struct ipc_handler *handlers,
> @@ -145,60 +337,3 @@ 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 0x%x for service 0x%x, terminating",
> -                                               msg->opcode, msg->service_id);
> -               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: size invalid opcode 0x%x service 0x%x, terminating",
> -                                               msg->service_id, msg->opcode);
> -               raise(SIGTERM);
> -               return;
> -       }
> -
> -       handler->handler(msg->payload, msg->len);
> -}
> diff --git a/android/ipc.h b/android/ipc.h
> index 9d0c5e1..6cd102b 100644
> --- a/android/ipc.h
> +++ b/android/ipc.h
> @@ -26,7 +26,7 @@ struct ipc_handler {
>         bool var_len;
>         size_t data_len;
>  };
> -void ipc_init(int command_sk, int notification_sk);
> +void ipc_init(void);
>  void ipc_cleanup(void);
>
>  void ipc_send_rsp(uint8_t service_id, uint8_t opcode, uint8_t status);
> @@ -37,5 +37,3 @@ void ipc_send_notif(uint8_t service_id, uint8_t opcode,  uint16_t len,
>  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);
> diff --git a/android/main.c b/android/main.c
> index c0f8901..b9655c5 100644
> --- a/android/main.c
> +++ b/android/main.c
> @@ -36,8 +36,6 @@
>  #include <unistd.h>
>
>  #include <sys/signalfd.h>
> -#include <sys/socket.h>
> -#include <sys/un.h>
>
>  #include <glib.h>
>
> @@ -69,9 +67,6 @@ static bdaddr_t adapter_bdaddr;
>
>  static GMainLoop *event_loop;
>
> -static GIOChannel *hal_cmd_io = NULL;
> -static GIOChannel *hal_notif_io = NULL;
> -
>  static bool services[HAL_SERVICE_ID_MAX + 1] = { false };
>
>  static void service_register(const void *buf, uint16_t len)
> @@ -209,127 +204,6 @@ static void stop_bluetooth(void)
>         g_timeout_add_seconds(SHUTDOWN_GRACE_SECONDS, quit_eventloop, NULL);
>  }
>
> -static gboolean cmd_watch_cb(GIOChannel *io, GIOCondition cond,
> -                                                       gpointer user_data)
> -{
> -       char buf[BLUEZ_HAL_MTU];
> -       ssize_t ret;
> -       int fd;
> -
> -       if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
> -               info("HAL command socket closed, terminating");
> -               goto fail;
> -       }
> -
> -       fd = g_io_channel_unix_get_fd(io);
> -
> -       ret = read(fd, buf, sizeof(buf));
> -       if (ret < 0) {
> -               error("HAL command read failed, terminating (%s)",
> -                                                       strerror(errno));
> -               goto fail;
> -       }
> -
> -       ipc_handle_msg(buf, ret);
> -       return TRUE;
> -
> -fail:
> -       stop_bluetooth();
> -       return FALSE;
> -}
> -
> -static gboolean notif_watch_cb(GIOChannel *io, GIOCondition cond,
> -                                                       gpointer user_data)
> -{
> -       info("HAL notification socket closed, terminating");
> -       stop_bluetooth();
> -
> -       return FALSE;
> -}
> -
> -static GIOChannel *connect_hal(GIOFunc connect_cb)
> -{
> -       struct sockaddr_un addr;
> -       GIOCondition cond;
> -       GIOChannel *io;
> -       int sk;
> -
> -       sk = socket(PF_LOCAL, SOCK_SEQPACKET, 0);
> -       if (sk < 0) {
> -               error("Failed to create socket: %d (%s)", errno,
> -                                                       strerror(errno));
> -               return NULL;
> -       }
> -
> -       io = g_io_channel_unix_new(sk);
> -
> -       g_io_channel_set_close_on_unref(io, TRUE);
> -       g_io_channel_set_flags(io, G_IO_FLAG_NONBLOCK, NULL);
> -
> -       memset(&addr, 0, sizeof(addr));
> -       addr.sun_family = AF_UNIX;
> -
> -       memcpy(addr.sun_path, BLUEZ_HAL_SK_PATH, sizeof(BLUEZ_HAL_SK_PATH));
> -
> -       if (connect(sk, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
> -               error("Failed to connect HAL socket: %d (%s)", errno,
> -                                                       strerror(errno));
> -               g_io_channel_unref(io);
> -               return NULL;
> -       }
> -
> -       cond = G_IO_OUT | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
> -
> -       g_io_add_watch(io, cond, connect_cb, NULL);
> -
> -       return io;
> -}
> -
> -static gboolean notif_connect_cb(GIOChannel *io, GIOCondition cond,
> -                                                       gpointer user_data)
> -{
> -       DBG("");
> -
> -       if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
> -               stop_bluetooth();
> -               return FALSE;
> -       }
> -
> -       cond = G_IO_ERR | G_IO_HUP | G_IO_NVAL;
> -
> -       g_io_add_watch(io, cond, notif_watch_cb, NULL);
> -
> -       cond = G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
> -
> -       g_io_add_watch(hal_cmd_io, cond, cmd_watch_cb, NULL);
> -
> -       ipc_init(g_io_channel_unix_get_fd(hal_cmd_io),
> -                               g_io_channel_unix_get_fd(hal_notif_io));
> -
> -       info("Successfully connected to HAL");
> -
> -       return FALSE;
> -}
> -
> -static gboolean cmd_connect_cb(GIOChannel *io, GIOCondition cond,
> -                                                       gpointer user_data)
> -{
> -       DBG("");
> -
> -       if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
> -               stop_bluetooth();
> -               return FALSE;
> -       }
> -
> -       hal_notif_io = connect_hal(notif_connect_cb);
> -       if (!hal_notif_io) {
> -               error("Cannot connect to HAL, terminating");
> -               stop_bluetooth();
> -       }
> -
> -       return FALSE;
> -}
> -
>  static void adapter_ready(int err, const bdaddr_t *addr)
>  {
>         if (err < 0) {
> @@ -346,11 +220,7 @@ static void adapter_ready(int err, const bdaddr_t *addr)
>
>         info("Adapter initialized");
>
> -       hal_cmd_io = connect_hal(cmd_connect_cb);
> -       if (!hal_cmd_io) {
> -               error("Cannot connect to HAL, terminating");
> -               stop_bluetooth();
> -       }
> +       ipc_init();
>  }
>
>  static gboolean signal_handler(GIOChannel *channel, GIOCondition cond,
> @@ -433,23 +303,6 @@ static GOptionEntry options[] = {
>         { NULL }
>  };
>
> -static void cleanup_hal_connection(void)
> -{
> -       if (hal_cmd_io) {
> -               g_io_channel_shutdown(hal_cmd_io, TRUE, NULL);
> -               g_io_channel_unref(hal_cmd_io);
> -               hal_cmd_io = NULL;
> -       }
> -
> -       if (hal_notif_io) {
> -               g_io_channel_shutdown(hal_notif_io, TRUE, NULL);
> -               g_io_channel_unref(hal_notif_io);
> -               hal_notif_io = NULL;
> -       }
> -
> -       ipc_cleanup();
> -}
> -
>  static void cleanup_services(void)
>  {
>         int i;
> @@ -592,7 +445,7 @@ int main(int argc, char *argv[])
>
>         cleanup_services();
>
> -       cleanup_hal_connection();
> +       ipc_cleanup();
>         stop_sdp_server();
>         bt_bluetooth_cleanup();
>         g_main_loop_unref(event_loop);
> --
> 1.8.3.2
>
> --

Applied, thanks


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH] android: Use G_N_ELEMENTS macro for table elements calculation
From: Luiz Augusto von Dentz @ 2013-12-03 12:10 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1385997478-18883-1-git-send-email-szymon.janc@tieto.com>

Hi Szymon,

On Mon, Dec 2, 2013 at 5:17 PM, Szymon Janc <szymon.janc@tieto.com> wrote:
> It is more common in codebase to use this macro instead of opencoded
> (sizeof(foo)/sizeof(foo[0])).
> ---
>  android/a2dp.c      | 2 +-
>  android/bluetooth.c | 2 +-
>  android/hidhost.c   | 2 +-
>  android/main.c      | 2 +-
>  android/pan.c       | 2 +-
>  android/socket.c    | 2 +-
>  6 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/android/a2dp.c b/android/a2dp.c
> index 98c138e..cee4bfa 100644
> --- a/android/a2dp.c
> +++ b/android/a2dp.c
> @@ -377,7 +377,7 @@ bool bt_a2dp_register(const bdaddr_t *addr)
>         record_id = rec->handle;
>
>         ipc_register(HAL_SERVICE_ID_A2DP, cmd_handlers,
> -                               sizeof(cmd_handlers)/sizeof(cmd_handlers[0]));
> +                                               G_N_ELEMENTS(cmd_handlers));
>
>         return true;
>  }
> diff --git a/android/bluetooth.c b/android/bluetooth.c
> index a39e7bf..6174b1f 100644
> --- a/android/bluetooth.c
> +++ b/android/bluetooth.c
> @@ -2418,7 +2418,7 @@ void bt_bluetooth_register(void)
>         DBG("");
>
>         ipc_register(HAL_SERVICE_ID_BLUETOOTH, cmd_handlers,
> -                               sizeof(cmd_handlers)/sizeof(cmd_handlers[0]));
> +                                               G_N_ELEMENTS(cmd_handlers));
>  }
>
>  void bt_bluetooth_unregister(void)
> diff --git a/android/hidhost.c b/android/hidhost.c
> index 38194d0..8bfdfed 100644
> --- a/android/hidhost.c
> +++ b/android/hidhost.c
> @@ -1279,7 +1279,7 @@ bool bt_hid_register(const bdaddr_t *addr)
>         }
>
>         ipc_register(HAL_SERVICE_ID_HIDHOST, cmd_handlers,
> -                               sizeof(cmd_handlers)/sizeof(cmd_handlers[0]));
> +                                               G_N_ELEMENTS(cmd_handlers));
>
>         return true;
>  }
> diff --git a/android/main.c b/android/main.c
> index b9655c5..5210b4b 100644
> --- a/android/main.c
> +++ b/android/main.c
> @@ -430,7 +430,7 @@ int main(int argc, char *argv[])
>         start_sdp_server(0, 0);
>
>         ipc_register(HAL_SERVICE_ID_CORE, cmd_handlers,
> -                               sizeof(cmd_handlers)/sizeof(cmd_handlers[0]));
> +                                               G_N_ELEMENTS(cmd_handlers));
>
>         DBG("Entering main loop");
>
> diff --git a/android/pan.c b/android/pan.c
> index 3270aa4..fe6ee26 100644
> --- a/android/pan.c
> +++ b/android/pan.c
> @@ -322,7 +322,7 @@ bool bt_pan_register(const bdaddr_t *addr)
>         }
>
>         ipc_register(HAL_SERVICE_ID_PAN, cmd_handlers,
> -                               sizeof(cmd_handlers)/sizeof(cmd_handlers[0]));
> +                                               G_N_ELEMENTS(cmd_handlers));
>
>         return true;
>  }
> diff --git a/android/socket.c b/android/socket.c
> index 76b40c8..c9eca44 100644
> --- a/android/socket.c
> +++ b/android/socket.c
> @@ -929,7 +929,7 @@ void bt_socket_register(const bdaddr_t *addr)
>
>         bacpy(&adapter_addr, addr);
>         ipc_register(HAL_SERVICE_ID_SOCK, cmd_handlers,
> -                               sizeof(cmd_handlers)/sizeof(cmd_handlers[0]));
> +                                               G_N_ELEMENTS(cmd_handlers));
>  }
>
>  void bt_socket_unregister(void)
> --
> 1.8.3.2

Applied, thanks.

-- 
Luiz Augusto von Dentz

^ permalink raw reply


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