* [PATCH] android/gatt: Use common code for server and client register
@ 2014-04-28 14:23 Jakub Tyszkowski
2014-04-29 13:31 ` Szymon Janc
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Tyszkowski @ 2014-04-28 14:23 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jakub Tyszkowski
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("");
- memset(&ev, 0, sizeof(ev));
+static int handle_app_register(const uint8_t *uuid, gatt_app_type_t app_type,
+ uint8_t *status)
+{
+ 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) {
+ 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);
+ *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));
--
1.9.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] android/gatt: Use common code for server and client register
2014-04-28 14:23 [PATCH] android/gatt: Use common code for server and client register Jakub Tyszkowski
@ 2014-04-29 13:31 ` Szymon Janc
2014-04-29 13:52 ` Tyszkowski Jakub
0 siblings, 1 reply; 3+ messages in thread
From: Szymon Janc @ 2014-04-29 13:31 UTC (permalink / raw)
To: Jakub Tyszkowski; +Cc: linux-bluetooth
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));
>
--
Best regards,
Szymon Janc
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] android/gatt: Use common code for server and client register
2014-04-29 13:31 ` Szymon Janc
@ 2014-04-29 13:52 ` Tyszkowski Jakub
0 siblings, 0 replies; 3+ messages in thread
From: Tyszkowski Jakub @ 2014-04-29 13:52 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-04-29 13:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-28 14:23 [PATCH] android/gatt: Use common code for server and client register Jakub Tyszkowski
2014-04-29 13:31 ` Szymon Janc
2014-04-29 13:52 ` Tyszkowski Jakub
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).