Re: [PATCH] android/gatt: Use common code for server and client register

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 04/29/2014 03:31 PM, Szymon Janc wrote:
Hi Jakub,

On Monday 28 of April 2014 16:23:20 Jakub Tyszkowski wrote:
As clients and servers are stored on a single list we can use common
code in both of register handlers.
---
  android/gatt.c | 121 ++++++++++++++++++++++++---------------------------------
  1 file changed, 50 insertions(+), 71 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index e634aa6..1454000 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -154,8 +154,6 @@ static struct queue *gatt_apps = NULL;
  static struct queue *gatt_devices = NULL;
  static struct queue *app_connections = NULL;

-static int32_t app_id = 1;
-
  static struct queue *listen_clients = NULL;

  static void bt_le_discovery_stop_cb(void);
@@ -534,55 +532,64 @@ static void destroy_gatt_app(void *data)
  	free(app);
  }

-static void handle_client_register(const void *buf, uint16_t len)
-{
-	const struct hal_cmd_gatt_client_register *cmd = buf;
-	struct hal_ev_gatt_client_register_client ev;
-	struct gatt_app *client;
-	uint8_t status;
-
-	DBG("");


Double empty line.

-	memset(&ev, 0, sizeof(ev));
+static int handle_app_register(const uint8_t *uuid, gatt_app_type_t app_type,
+								uint8_t *status)

I'd name this simply register_app(). Also *status parameter is not really useful
since Java is not using it. Returning 0 is enough for error here.

ev.client_if = handle_app_register(..);
if (ev.client_if)
   status = HAL_STATUS_SUCCESS;
else ....

+{
+	static int32_t application_id = 1;
+	struct gatt_app *app;

-	if (queue_find(gatt_apps, match_app_by_uuid, &cmd->uuid)) {
-		error("gatt: client uuid is already on list");
-		status = HAL_STATUS_FAILED;
-		goto failed;
+	if (queue_find(gatt_apps, match_app_by_uuid, (void *) uuid)) {
+		error("gatt: app uuid is already on list");
+		*status = HAL_STATUS_FAILED;
+		return 0;
  	}

-	client = new0(struct gatt_app, 1);
-	client->type = APP_CLIENT;
-	if (!client) {
-		error("gatt: cannot allocate memory for registering client");
-		status = HAL_STATUS_NOMEM;
-		goto failed;
+	app = new0(struct gatt_app, 1);
+	app->type = app_type;
+	if (!app) {

app was already dereferenced line above.

app->type = app_type;
should be after null check.

+		error("gatt: Cannot allocate memory for registering app");
+		*status = HAL_STATUS_NOMEM;
+		return 0;
+	}
+
+	if (app_type == APP_CLIENT) {
+		app->notifications = queue_new();
+		if (!app->notifications) {
+			error("gatt: couldn't allocate notifications queue");
+			destroy_gatt_app(app);
+			*status = HAL_STATUS_NOMEM;
+			return 0;
+		}
  	}

-	client->notifications = queue_new();
-	if (!client->notifications) {
-		error("gatt: couldn't allocate notifications queue");
-		destroy_gatt_app(client);
-		status = HAL_STATUS_NOMEM;
-		goto failed;
+	memcpy(app->uuid, uuid, sizeof(app->uuid));
+
+	app->id = application_id++;
+
+	if (!queue_push_head(gatt_apps, app)) {
+		error("gatt: Cannot push app on the list");
+		free(app);

destroy_gatt_app(app) here.

+		*status = HAL_STATUS_FAILED;
+		return 0;
  	}

-	memcpy(client->uuid, cmd->uuid, sizeof(client->uuid));
+	*status = HAL_STATUS_SUCCESS;
+	return app->id;
+}

-	client->id = app_id++;
+static void handle_client_register(const void *buf, uint16_t len)
+{
+	const struct hal_cmd_gatt_client_register *cmd = buf;
+	struct hal_ev_gatt_client_register_client ev;
+	uint8_t status;

-	if (!queue_push_head(gatt_apps, client)) {
-		error("gatt: Cannot push client on the list");
-		destroy_gatt_app(client);
-		status = HAL_STATUS_NOMEM;
-		goto failed;
-	}
+	DBG("");

-	ev.client_if = client->id;
+	memset(&ev, 0, sizeof(ev));

-	status = HAL_STATUS_SUCCESS;
+	ev.client_if = handle_app_register(cmd->uuid, APP_CLIENT, &status);

-failed:
  	if (status == HAL_STATUS_SUCCESS)
  		ev.status = GATT_SUCCESS;
  	else
@@ -3069,45 +3076,17 @@ static void handle_server_register(const void *buf, uint16_t len)
  {
  	const struct hal_cmd_gatt_server_register *cmd = buf;
  	struct hal_ev_gatt_server_register ev;
-	struct gatt_app *server;
-	uint32_t status;
+	uint8_t status;

  	DBG("");

  	memset(&ev, 0, sizeof(ev));

-	if (queue_find(gatt_apps, match_app_by_uuid, &cmd->uuid)) {
-		error("gatt: Server uuid is already on list");
-		status = HAL_STATUS_FAILED;
-		goto failed;
-	}
-
-	server = new0(struct gatt_app, 1);
-	server->type = APP_SERVER;
-	if (!server) {
-		error("gatt: Cannot allocate memory for registering server");
-		status = HAL_STATUS_NOMEM;
-		goto failed;
-	}
+	ev.server_if = handle_app_register(cmd->uuid, APP_SERVER, &status);

-	memcpy(server->uuid, cmd->uuid, sizeof(server->uuid));
-
-	server->id = app_id++;
-
-	if (!queue_push_head(gatt_apps, server)) {
-		error("gatt: Cannot push server on the list");
-		free(server);
-		status = HAL_STATUS_FAILED;
-		goto failed;
-	}
-
-	ev.status = GATT_SUCCESS;
-	ev.server_if = server->id;
-
-	status = HAL_STATUS_SUCCESS;
-
-failed:
-	if (status != HAL_STATUS_SUCCESS)
+	if (status == HAL_STATUS_SUCCESS)
+		ev.status = GATT_SUCCESS;
+	else
  		ev.status = GATT_FAILURE;

  	memcpy(ev.uuid, cmd->uuid, sizeof(ev.uuid));



I'll fix all of these in v2.

Thanks,
Jakub

--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux