2014-07-04 13:43:49

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 00/12] Varios fixes

From: Andrei Emeltchenko <[email protected]>

Fixes hanging in my tree for some time

Andrei Emeltchenko (12):
android/gatt: Change DBG() to error()
android/gatt: Fix memory leak
android/hal-msg: trivial: Correct coding style
android/gatt: Fix warning by rearranging allocation
android/haltest: Auto-accept pairing
android/hal-gatt: Remove unneeded never read initialization
android/gatt: Add missing error check
android/haltest: Add check for valid channel
android/tester: Remove unused assignment
android/gatt: Fix length check
cups: Remove dead code
core: Remove dead code

android/android-tester.c | 2 +-
android/client/if-bt.c | 16 +++++++++++++++-
android/client/if-hl.c | 4 ++++
android/gatt.c | 17 +++++++++++------
android/hal-gatt.c | 4 +---
android/hal-msg.h | 8 ++++----
profiles/cups/main.c | 7 +------
src/profile.c | 4 ----
8 files changed, 37 insertions(+), 25 deletions(-)

--
1.9.1



2014-07-04 14:55:59

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 00/12] Varios fixes

Hi,

On Fri, Jul 04, 2014, Szymon Janc wrote:
> On Friday 04 of July 2014 16:43:49 Andrei Emeltchenko wrote:
> > From: Andrei Emeltchenko <[email protected]>
> >
> > Fixes hanging in my tree for some time
> >
> > Andrei Emeltchenko (12):
> > android/gatt: Change DBG() to error()
> > android/gatt: Fix memory leak
> > android/hal-msg: trivial: Correct coding style
> > android/gatt: Fix warning by rearranging allocation
> > android/haltest: Auto-accept pairing
> > android/hal-gatt: Remove unneeded never read initialization
> > android/gatt: Add missing error check
> > android/haltest: Add check for valid channel
> > android/tester: Remove unused assignment
> > android/gatt: Fix length check
> > cups: Remove dead code
> > core: Remove dead code
> >
> > android/android-tester.c | 2 +-
> > android/client/if-bt.c | 16 +++++++++++++++-
> > android/client/if-hl.c | 4 ++++
> > android/gatt.c | 17 +++++++++++------
> > android/hal-gatt.c | 4 +---
> > android/hal-msg.h | 8 ++++----
> > profiles/cups/main.c | 7 +------
> > src/profile.c | 4 ----
> > 8 files changed, 37 insertions(+), 25 deletions(-)
>
> Patches 1-10 are now applied, thanks.

And I applied the remaining two.

Johan

2014-07-04 14:48:21

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH 00/12] Varios fixes

Hi Andrei,

On Friday 04 of July 2014 16:43:49 Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> Fixes hanging in my tree for some time
>
> Andrei Emeltchenko (12):
> android/gatt: Change DBG() to error()
> android/gatt: Fix memory leak
> android/hal-msg: trivial: Correct coding style
> android/gatt: Fix warning by rearranging allocation
> android/haltest: Auto-accept pairing
> android/hal-gatt: Remove unneeded never read initialization
> android/gatt: Add missing error check
> android/haltest: Add check for valid channel
> android/tester: Remove unused assignment
> android/gatt: Fix length check
> cups: Remove dead code
> core: Remove dead code
>
> android/android-tester.c | 2 +-
> android/client/if-bt.c | 16 +++++++++++++++-
> android/client/if-hl.c | 4 ++++
> android/gatt.c | 17 +++++++++++------
> android/hal-gatt.c | 4 +---
> android/hal-msg.h | 8 ++++----
> profiles/cups/main.c | 7 +------
> src/profile.c | 4 ----
> 8 files changed, 37 insertions(+), 25 deletions(-)

Patches 1-10 are now applied, thanks.


--
Best regards,
Szymon Janc

2014-07-04 13:44:01

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 12/12] core: Remove dead code

From: Andrei Emeltchenko <[email protected]>

rfcomm can only be NULL in this path
---
src/profile.c | 4 ----
1 file changed, 4 deletions(-)

diff --git a/src/profile.c b/src/profile.c
index f30f4f6..528525a 100644
--- a/src/profile.c
+++ b/src/profile.c
@@ -1295,10 +1295,6 @@ failed:
ext->servers = g_slist_remove(ext->servers, l2cap);
ext_io_destroy(l2cap);
}
- if (rfcomm) {
- ext->servers = g_slist_remove(ext->servers, rfcomm);
- ext_io_destroy(rfcomm);
- }

return 0;
}
--
1.9.1


2014-07-04 13:43:53

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 04/12] android/gatt: Fix warning by rearranging allocation

From: Andrei Emeltchenko <[email protected]>

We need to allocate array of uint8_t so fix allocation parameter, this
also removes unneeded type conversion.

This fixes clang warnings:
...
android/gatt.c:5967:29: warning: Result of 'calloc' is converted to a
pointer of type 'uint8_t', which is incompatible with sizeof operand
type 'uint16_t'
entry->value = (uint8_t *) new0(uint16_t, 1);
~~~~~~~~~ ^~~~~~~~~~~~~~~~~
./src/shared/util.h:81:26: note: expanded from macro 'new0'
^~~~~~ ~~~~~~~~~
...
---
android/gatt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/android/gatt.c b/android/gatt.c
index ca4bd1d..b47ba83 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -6062,7 +6062,7 @@ static void gatt_srvc_change_read_cb(uint16_t handle, uint16_t offset,
ccc = bt_get_gatt_ccc(&dev->bdaddr);
entry->state = REQUEST_DONE;

- entry->value = (uint8_t *) new0(uint16_t, 1);
+ entry->value = new0(uint8_t, 2);
if (!entry->value) {
entry->error = ATT_ECODE_INSUFF_RESOURCES;

--
1.9.1


2014-07-04 13:43:50

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 01/12] android/gatt: Change DBG() to error()

From: Andrei Emeltchenko <[email protected]>

---
android/gatt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/android/gatt.c b/android/gatt.c
index 95de221..5cb08ec 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -4386,7 +4386,7 @@ static void fill_gatt_response_by_handle(uint16_t handle, uint16_t offset,
entry = queue_find(dev->pending_requests, match_dev_request_by_handle,
UINT_TO_PTR(handle));
if (!entry) {
- DBG("No pending response found! Bogus android response?");
+ error("gatt: No pending response! Bogus android response?");
return;
}

--
1.9.1


2014-07-04 13:43:54

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 05/12] android/haltest: Auto-accept pairing

From: Andrei Emeltchenko <[email protected]>

Simplify PTS testing by asking for pairing consent.
---
android/client/if-bt.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/android/client/if-bt.c b/android/client/if-bt.c
index 4d6ff83..218e254 100644
--- a/android/client/if-bt.c
+++ b/android/client/if-bt.c
@@ -266,7 +266,8 @@ static void ssp_request_cb(bt_bdaddr_t *remote_bd_addr, bt_bdname_t *bd_name,
__func__, last_remote_addr, bd_name->name, cod,
bt_ssp_variant_t2str(pairing_variant), pass_key);

- if (pairing_variant == BT_SSP_VARIANT_PASSKEY_CONFIRMATION) {
+ switch (pairing_variant) {
+ case BT_SSP_VARIANT_PASSKEY_CONFIRMATION:
sprintf(prompt, "Does other device show %d [Y/n] ?", pass_key);

ssp_request_addr = *remote_bd_addr;
@@ -274,6 +275,19 @@ static void ssp_request_cb(bt_bdaddr_t *remote_bd_addr, bt_bdname_t *bd_name,
ssp_request_pask_key = pass_key;

terminal_prompt_for(prompt, ssp_request_yes_no_answer);
+ break;
+ case BT_SSP_VARIANT_CONSENT:
+ sprintf(prompt, "Consent pairing [Y/n] ?");
+
+ ssp_request_addr = *remote_bd_addr;
+ ssp_request_variant = pairing_variant;
+ ssp_request_pask_key = 0;
+
+ terminal_prompt_for(prompt, ssp_request_yes_no_answer);
+ break;
+ default:
+ haltest_info("Not automatically handled\n");
+ break;
}
}

--
1.9.1


2014-07-04 13:43:59

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 10/12] android/gatt: Fix length check

From: Andrei Emeltchenko <[email protected]>

Function fill_gatt_response() has check shown below:
if (!len)
return;

This eliminates clang warning:
...
android/gatt.c:4443:3: warning: Function call argument is an
uninitialized value
fill_gatt_response(resp_data, resp_data->handle,
...
---
android/gatt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/android/gatt.c b/android/gatt.c
index fa7e1c3..d37a581 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -4439,7 +4439,7 @@ static void read_requested_attributes(void *data, void *user_data)

done:
/* We have value here already if no callback will be called */
- if (value_len >= 0)
+ if (value_len > 0)
fill_gatt_response(resp_data, resp_data->handle,
resp_data->offset, error, value_len,
value);
--
1.9.1


2014-07-04 13:43:58

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 09/12] android/tester: Remove unused assignment

From: Andrei Emeltchenko <[email protected]>

Fixes clang warning:
...
android/android-tester.c:1014:2: warning: Value stored to 'status' is
never read
status =
data->if_bluetooth->get_remote_device_property(&remote_addr, prop.type);
1 warning generated.
...
---
android/android-tester.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index bf39517..ff29675 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -1011,7 +1011,7 @@ static void remote_setprop_device_found_cb(int num_properties,
&prop);
check_expected_status(status);

- status = data->if_bluetooth->get_remote_device_property(&remote_addr, prop.type);
+ data->if_bluetooth->get_remote_device_property(&remote_addr, prop.type);
}

static void remote_setprop_fail_device_found_cb(int num_properties,
--
1.9.1


2014-07-04 13:44:00

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 11/12] cups: Remove dead code

From: Andrei Emeltchenko <[email protected]>

---
profiles/cups/main.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/profiles/cups/main.c b/profiles/cups/main.c
index 2079812..11ce72f 100644
--- a/profiles/cups/main.c
+++ b/profiles/cups/main.c
@@ -332,13 +332,11 @@ static gboolean device_is_printer(const char *adapter, const char *device_path,
static void remote_device_found(const char *adapter, const char *bdaddr,
const char *name)
{
- DBusMessage *message, *reply, *adapter_reply;
+ DBusMessage *message, *reply;
DBusMessageIter iter;
char *object_path = NULL;
char *id;

- adapter_reply = NULL;
-
assert(adapter != NULL);

message = dbus_message_new_method_call("org.bluez", adapter,
@@ -347,9 +345,6 @@ static void remote_device_found(const char *adapter, const char *bdaddr,
dbus_message_iter_init_append(message, &iter);
dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING, &bdaddr);

- if (adapter_reply != NULL)
- dbus_message_unref(adapter_reply);
-
reply = dbus_connection_send_with_reply_and_block(conn,
message, -1, NULL);

--
1.9.1


2014-07-04 13:43:57

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 08/12] android/haltest: Add check for valid channel

From: Andrei Emeltchenko <[email protected]>

Channel is got from user and needs to be validated before use as index
in the array.
---
android/client/if-hl.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/android/client/if-hl.c b/android/client/if-hl.c
index 17ba16c..a3c351c 100644
--- a/android/client/if-hl.c
+++ b/android/client/if-hl.c
@@ -329,6 +329,10 @@ static void close_channel_p(int argc, const char **argv)
}

channel_id = atoi(argv[4]);
+ if (channel_id >= CHANNEL_ID_SIZE) {
+ haltest_error("Wrong channel id: %u\n", channel_id);
+ return;
+ }

if (app[app_id].mdep[index].channel[channel_id].fd >= 0) {
shutdown(app[app_id].mdep[index].channel[channel_id].fd,
--
1.9.1


2014-07-04 13:43:55

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 06/12] android/hal-gatt: Remove unneeded never read initialization

From: Andrei Emeltchenko <[email protected]>

Fixes clang warning:
...
android/hal-gatt.c:1021:3: warning: Value stored to 'data' is never read
data += service_uuid_len;
^ ~~~~~~~~~~~~~~~~
1 warning generated.
...
---
android/hal-gatt.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/android/hal-gatt.c b/android/hal-gatt.c
index 1cc4897..c563fe9 100644
--- a/android/hal-gatt.c
+++ b/android/hal-gatt.c
@@ -1016,10 +1016,8 @@ static bt_status_t set_adv_data_real(int server_if, bool set_scan_rsp,
data += service_data_len;
}

- if (service_uuid && service_uuid_len) {
+ if (service_uuid && service_uuid_len)
memcpy(data, service_uuid, service_uuid_len);
- data += service_uuid_len;
- }

return hal_ipc_cmd(HAL_SERVICE_ID_GATT, HAL_OP_GATT_CLIENT_SET_ADV_DATA,
cmd_len, cmd, NULL, NULL, NULL);
--
1.9.1


2014-07-04 13:43:51

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 02/12] android/gatt: Fix memory leak

From: Andrei Emeltchenko <[email protected]>

Fixes clang warnings:
...
android/gatt.c:2823:1: warning: Potential leak of memory pointed to by
'cb_data'
...
---
android/gatt.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/android/gatt.c b/android/gatt.c
index 5cb08ec..ca4bd1d 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -2957,7 +2957,8 @@ static void handle_client_write_characteristic(const void *buf, uint16_t len)
goto failed;
}

- if (cmd->write_type != GATT_WRITE_TYPE_NO_RESPONSE) {
+ if (cmd->write_type != GATT_WRITE_TYPE_NO_RESPONSE &&
+ cmd->write_type != GATT_WRITE_TYPE_SIGNED) {
cb_data = create_char_op_data(cmd->conn_id, &srvc->id, &ch->id,
cmd->srvc_id.is_primary);
if (!cb_data) {
--
1.9.1


2014-07-04 13:43:56

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 07/12] android/gatt: Add missing error check

From: Andrei Emeltchenko <[email protected]>

Fixes clang warning:
...
android/gatt.c:2097:4: warning: Value stored to 'srvc_search_success' is
never read
srvc_search_success = search_dev_for_srvc(conn, NULL);
...
---
android/gatt.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index b47ba83..fa7e1c3 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -2093,9 +2093,13 @@ static void handle_client_search_service(const void *buf, uint16_t len)
}
} else {
/* Refresh service cache if only partial search was performed */
- if (conn->device->partial_srvc_search)
+ if (conn->device->partial_srvc_search) {
srvc_search_success = search_dev_for_srvc(conn, NULL);
- else
+ if (!srvc_search_success) {
+ status = HAL_STATUS_FAILED;
+ goto reply;
+ }
+ } else
queue_foreach(conn->device->services,
send_client_primary_notify,
INT_TO_PTR(cmd->conn_id));
--
1.9.1


2014-07-04 13:43:52

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 03/12] android/hal-msg: trivial: Correct coding style

From: Andrei Emeltchenko <[email protected]>

Move rsp close to cmd improving readability of the code.
---
android/hal-msg.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/android/hal-msg.h b/android/hal-msg.h
index af859c6..7f13e84 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -403,6 +403,10 @@ struct hal_cmd_health_reg_app {
uint8_t data[0];
} __attribute__((packed));

+struct hal_rsp_health_reg_app {
+ uint16_t app_id;
+} __attribute__((packed));
+
#define HAL_OP_HEALTH_MDEP 0x02
struct hal_cmd_health_mdep {
uint16_t app_id;
@@ -413,10 +417,6 @@ struct hal_cmd_health_mdep {
uint8_t descr[0];
} __attribute__((packed));

-struct hal_rsp_health_reg_app {
- uint16_t app_id;
-} __attribute__((packed));
-
#define HAL_OP_HEALTH_UNREG_APP 0x03
struct hal_cmd_health_unreg_app {
uint16_t app_id;
--
1.9.1