From: Andrei Emeltchenko <[email protected]>
sdp_set_access_protos() always returns 0, there is no sense to check for
error code. Fixes compile warnings.
---
profiles/health/hdp_util.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/profiles/health/hdp_util.c b/profiles/health/hdp_util.c
index 58770b5..ff427a6 100644
--- a/profiles/health/hdp_util.c
+++ b/profiles/health/hdp_util.c
@@ -362,7 +362,7 @@ static gboolean set_sdp_services_uuid(sdp_record_t *record, HdpRole role)
static gboolean register_service_protocols(struct hdp_adapter *adapter,
sdp_record_t *sdp_record)
{
- gboolean ret;
+ gboolean ret = TRUE;
uuid_t l2cap_uuid, mcap_c_uuid;
sdp_list_t *l2cap_list, *proto_list = NULL, *mcap_list = NULL;
sdp_list_t *access_proto_list = NULL;
@@ -425,11 +425,7 @@ static gboolean register_service_protocols(struct hdp_adapter *adapter,
goto end;
}
- if (sdp_set_access_protos(sdp_record, access_proto_list) < 0) {
- ret = FALSE;
- goto end;
- }
- ret = TRUE;
+ sdp_set_access_protos(sdp_record, access_proto_list);
end:
if (l2cap_list != NULL)
--
1.8.3.2
From: Andrei Emeltchenko <[email protected]>
Simplify PTS testing by automatically auto accept Just Works pairing.
---
android/client/if-bt.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/android/client/if-bt.c b/android/client/if-bt.c
index 4d6ff83..764979e 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,14 @@ 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:
+ EXEC(if_bluetooth->ssp_reply, remote_bd_addr, pairing_variant,
+ 1, 0);
+ break;
+ default:
+ haltest_info("Not automatically handled\n");
+ break;
}
}
--
1.8.3.2
From: Andrei Emeltchenko <[email protected]>
In case of error err is always set so the check is not needed.
---
android/health.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/android/health.c b/android/health.c
index 0462e99..f893d0f 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1063,9 +1063,7 @@ static void search_cb(sdp_list_t *recs, int err, gpointer data)
if (!mcap_create_mcl(mcap, &channel->dev->dst, channel->dev->ccpsm,
create_mcl_cb, channel, NULL, &gerr)) {
error("error creating mcl %s", gerr->message);
-
- if (gerr)
- g_error_free(gerr);
+ g_error_free(gerr);
goto fail;
}
--
1.8.3.2
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 b256077..cb27f34 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -4280,7 +4280,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.8.3.2
From: Andrei Emeltchenko <[email protected]>
---
android/mcap-lib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/android/mcap-lib.c b/android/mcap-lib.c
index be9ce79..346688e 100644
--- a/android/mcap-lib.c
+++ b/android/mcap-lib.c
@@ -2357,7 +2357,7 @@ void mcap_sync_stop(struct mcap_mcl *mcl)
static uint64_t time_us(struct timespec *tv)
{
- return tv->tv_sec * 1000000 + tv->tv_nsec / 1000;
+ return tv->tv_sec * 1000000ll + tv->tv_nsec / 1000ll;
}
static int64_t bt2us(int bt)
--
1.8.3.2
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 5a13afa..626a65d 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -2888,7 +2888,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.8.3.2
From: Andrei Emeltchenko <[email protected]>
Initialize value_len to -1 since otherwise check value_len >= 0 does not
make sense.
---
android/gatt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/android/gatt.c b/android/gatt.c
index cb27f34..5a13afa 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -4293,7 +4293,7 @@ static void read_requested_attributes(void *data, void *user_data)
struct request_processing_data *process_data = user_data;
uint32_t permissions;
uint8_t *value, error;
- int value_len = 0;
+ int value_len = -1;
if (!gatt_db_get_attribute_permissions(gatt_db, resp_data->handle,
&permissions)) {
--
1.8.3.2
From: Andrei Emeltchenko <[email protected]>
---
android/mcap-lib.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/android/mcap-lib.c b/android/mcap-lib.c
index 346688e..2a86416 100644
--- a/android/mcap-lib.c
+++ b/android/mcap-lib.c
@@ -2654,12 +2654,14 @@ static gboolean get_all_clocks(struct mcap_mcl *mcl, uint32_t *btclock,
while (latency > caps(mcl)->preempt_thresh && --retry >= 0) {
- clock_gettime(CLK, &t0);
+ if (clock_gettime(CLK, &t0) < 0)
+ return FALSE;
if (!read_btclock(mcl, btclock, &btres))
continue;
- clock_gettime(CLK, base_time);
+ if (clock_gettime(CLK, base_time) < 0)
+ return FALSE;
/*
* Tries to detect preemption between clock_gettime
@@ -2668,6 +2670,9 @@ static gboolean get_all_clocks(struct mcap_mcl *mcl, uint32_t *btclock,
latency = time_us(base_time) - time_us(&t0);
}
+ if (retry < 0)
+ return FALSE;
+
*timestamp = mcap_get_timestamp(mcl, base_time);
return TRUE;
--
1.8.3.2
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 2c21a85..d93f557 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.8.3.2
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 626a65d..2bef5ab 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -5969,7 +5969,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.8.3.2
From: Andrei Emeltchenko <[email protected]>
---
android/health.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/android/health.c b/android/health.c
index f893d0f..8625779 100644
--- a/android/health.c
+++ b/android/health.c
@@ -1255,7 +1255,6 @@ bool bt_health_register(struct ipc *ipc, const bdaddr_t *addr, uint8_t mode)
mcl_disconnected, mcl_uncached,
NULL, /* CSP is not used right now */
NULL, &err);
-
if (!mcap) {
error("Error creating MCAP instance : %s", err->message);
g_error_free(err);
--
1.8.3.2
From: Andrei Emeltchenko <[email protected]>
---
android/hal-health.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/android/hal-health.c b/android/hal-health.c
index ac6e87f..0d9136d 100644
--- a/android/hal-health.c
+++ b/android/hal-health.c
@@ -116,8 +116,7 @@ static bt_status_t register_application(bthl_reg_param_t *reg, int *app_id)
cmd->len = off;
status = hal_ipc_cmd(HAL_SERVICE_ID_HEALTH, HAL_OP_HEALTH_REG_APP,
sizeof(*cmd) + cmd->len, buf,
- &rsp_len, &rsp, NULL);
-
+ &rsp_len, &rsp, NULL);
if (status != BT_STATUS_SUCCESS)
return status;
--
1.8.3.2
Hi Szymon,
On Tue, Jun 17, 2014 at 11:27:13PM +0200, Szymon Janc wrote:
> Hi Andrei,
>
> On Monday 16 June 2014 10:57:36 Andrei Emeltchenko wrote:
> > From: Andrei Emeltchenko <[email protected]>
> >
> > sdp_set_access_protos() always returns 0, there is no sense to check for
> > error code. Fixes compile warnings.
> > ---
> > profiles/health/hdp_util.c | 8 ++------
> > 1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/profiles/health/hdp_util.c b/profiles/health/hdp_util.c
> > index 58770b5..ff427a6 100644
> > --- a/profiles/health/hdp_util.c
> > +++ b/profiles/health/hdp_util.c
> > @@ -362,7 +362,7 @@ static gboolean set_sdp_services_uuid(sdp_record_t
> > *record, HdpRole role) static gboolean register_service_protocols(struct
> > hdp_adapter *adapter, sdp_record_t *sdp_record)
> > {
> > - gboolean ret;
> > + gboolean ret = TRUE;
> > uuid_t l2cap_uuid, mcap_c_uuid;
> > sdp_list_t *l2cap_list, *proto_list = NULL, *mcap_list = NULL;
> > sdp_list_t *access_proto_list = NULL;
> > @@ -425,11 +425,7 @@ static gboolean register_service_protocols(struct
> > hdp_adapter *adapter, goto end;
> > }
> >
> > - if (sdp_set_access_protos(sdp_record, access_proto_list) < 0) {
> > - ret = FALSE;
> > - goto end;
> > - }
> > - ret = TRUE;
> > + sdp_set_access_protos(sdp_record, access_proto_list);
>
> I'd leave setting ret here instead of initializing it to TRUE.
>
OK
Best regards
Andrei Emeltchenko
Hi Szymon,
On Tue, Jun 17, 2014 at 10:51:39PM +0200, Szymon Janc wrote:
> Hi Andrei,
>
> On Monday 16 June 2014 10:57:50 Andrei Emeltchenko wrote:
> > From: Andrei Emeltchenko <[email protected]>
> >
> > ---
> > profiles/health/hdp_util.c | 16 +++++++++-------
> > 1 file changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/profiles/health/hdp_util.c b/profiles/health/hdp_util.c
> > index 7185805..876a105 100644
> > --- a/profiles/health/hdp_util.c
> > +++ b/profiles/health/hdp_util.c
> > @@ -597,6 +597,13 @@ fail:
> > return NULL;
> > }
> >
> > +static void free_hdp_list(void *list)
> > +{
> > + sdp_list_t *hdp_list = list;
> > +
> > + sdp_list_free(hdp_list, (sdp_free_func_t)sdp_data_free);
> > +}
> > +
> > static gboolean register_features(struct hdp_application *app,
> > sdp_list_t **sup_features)
> > {
> > @@ -619,16 +626,11 @@ static gboolean register_features(struct
> > hdp_application *app, fail:
> > if (hdp_feature != NULL)
> > sdp_list_free(hdp_feature, (sdp_free_func_t)sdp_data_free);
> > + if (sup_features != NULL)
> > + sdp_list_free(*sup_features, free_hdp_list);
>
> I have a feeling that this should be fixed in caller code ie
> register_service_sup_features.
The problem is that it gets allocated here and we return FALSE if
allocation fails for example, so it does make sense to free structure when
FALSE is returned.
>
> Also sup_features was already dereferences few lines above so this check is
> not needed (or should it be *sup_features != NULL ?).
Right, will fix this.
Best regards
Andrei Emeltchenko
Hi Szymon,
On Tue, Jun 17, 2014 at 11:11:01PM +0200, Szymon Janc wrote:
> Hi Andrei,
>
> On Monday 16 June 2014 10:57:44 Andrei Emeltchenko wrote:
> > From: Andrei Emeltchenko <[email protected]>
> >
> > ---
> > profiles/health/mcap_sync.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/profiles/health/mcap_sync.c b/profiles/health/mcap_sync.c
> > index cc89d47..a0cc02a 100644
> > --- a/profiles/health/mcap_sync.c
> > +++ b/profiles/health/mcap_sync.c
> > @@ -187,7 +187,7 @@ void mcap_sync_stop(struct mcap_mcl *mcl)
> >
> > static uint64_t time_us(struct timespec *tv)
> > {
> > - return tv->tv_sec * 1000000 + tv->tv_nsec / 1000;
> > + return tv->tv_sec * 1000000ll + tv->tv_nsec / 1000ll;
> > }
> >
> > static int64_t bt2us(int bt)
>
> Commit message explaining why there is possible overflow would be nice.
>
We have this 'll' in newer code dealing with similar conversions, I suppose
we might use 32 bit arithmetic and then widen to 64 bit.
Best regards
Andrei Emeltchenko
Hi Szymon,
On Tue, Jun 17, 2014 at 11:21:16PM +0200, Szymon Janc wrote:
> Hi Andrei,
>
> On Monday 16 June 2014 10:57:40 Andrei Emeltchenko wrote:
> > From: Andrei Emeltchenko <[email protected]>
> >
> > Initialize value_len to -1 since otherwise check value_len >= 0 does not
> > make sense.
> > ---
> > android/gatt.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/android/gatt.c b/android/gatt.c
> > index eb977d5..c5cb58f 100644
> > --- a/android/gatt.c
> > +++ b/android/gatt.c
> > @@ -4288,7 +4288,7 @@ static void read_requested_attributes(void *data, void
> > *user_data) struct request_processing_data *process_data = user_data;
> > uint32_t permissions;
> > uint8_t *value, error;
> > - int value_len = 0;
> > + int value_len = -1;
> >
> > if (!gatt_db_get_attribute_permissions(gatt_db, resp_data->handle,
> > &permissions)) {
>
> This is not correct. value_len == -1 means that callback was called from
> gatt_db_read() and response will be filled later.
Do you mean that in gatt_db_read() we should check for !*length not
!length like it is checked now? Otherwise it does not have effect.
Best regards
Andrei Emeltchenko
Hi Andrei,
On Monday 16 June 2014 10:57:45 Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> ---
> android/hal-sco.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/android/hal-sco.c b/android/hal-sco.c
> index ef04dc5..faed406 100644
> --- a/android/hal-sco.c
> +++ b/android/hal-sco.c
> @@ -712,7 +712,7 @@ static int sco_open_output_stream(struct audio_hw_device
> *dev, RESAMPLER_QUALITY_DEFAULT, NULL,
> &out->resampler);
> if (ret) {
> - error("Failed to create resampler (%s)", strerror(ret));
> + error("Failed to create resampler (%s)", strerror(-ret));
> goto failed;
> }
>
> @@ -1143,7 +1143,7 @@ static int sco_open_input_stream(struct
> audio_hw_device *dev, RESAMPLER_QUALITY_DEFAULT, NULL,
> &in->resampler);
> if (ret) {
> - error("Failed to create resampler (%s)", strerror(ret));
> + error("Failed to create resampler (%s)", strerror(-ret));
> goto failed;
> }
That one doesn't apply. Please rebase, thanks.
--
Szymon K. Janc
[email protected]
Hi Andrei,
On Monday 16 June 2014 10:57:38 Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> Make code cleaner lowering scope for mdep to the place it is actually
> used.
> ---
> android/hal-health.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/android/hal-health.c b/android/hal-health.c
> index 012b6b7..b8df5a7 100644
> --- a/android/hal-health.c
> +++ b/android/hal-health.c
> @@ -68,7 +68,6 @@ static bt_status_t register_application(bthl_reg_param_t
> *reg, int *app_id) {
> uint8_t buf[IPC_MTU];
> struct hal_cmd_health_reg_app *cmd = (void *) buf;
> - struct hal_cmd_health_mdep *mdep = (void *) buf;
> struct hal_rsp_health_reg_app rsp;
> size_t rsp_len = sizeof(rsp);
> bt_status_t status;
> @@ -123,6 +122,8 @@ static bt_status_t register_application(bthl_reg_param_t
> *reg, int *app_id) return status;
>
> for (i = 0; i < reg->number_of_mdeps; i++) {
> + struct hal_cmd_health_mdep *mdep = (void *) buf;
> +
> memset(buf, 0, IPC_MTU);
> mdep->app_id = rsp.app_id;
> mdep->role = reg->mdep_cfg[i].mdep_role;
Patches 3,6,7,8,11,12,14 and 17 are now applied, thanks.
--
Szymon K. Janc
[email protected]
Hi Andrei,
On Monday 16 June 2014 10:57:44 Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> ---
> profiles/health/mcap_sync.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/profiles/health/mcap_sync.c b/profiles/health/mcap_sync.c
> index cc89d47..a0cc02a 100644
> --- a/profiles/health/mcap_sync.c
> +++ b/profiles/health/mcap_sync.c
> @@ -187,7 +187,7 @@ void mcap_sync_stop(struct mcap_mcl *mcl)
>
> static uint64_t time_us(struct timespec *tv)
> {
> - return tv->tv_sec * 1000000 + tv->tv_nsec / 1000;
> + return tv->tv_sec * 1000000ll + tv->tv_nsec / 1000ll;
> }
>
> static int64_t bt2us(int bt)
Commit message explaining why there is possible overflow would be nice.
--
Szymon K. Janc
[email protected]
Hi Andrei,
On Monday 16 June 2014 10:57:36 Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> sdp_set_access_protos() always returns 0, there is no sense to check for
> error code. Fixes compile warnings.
> ---
> profiles/health/hdp_util.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/profiles/health/hdp_util.c b/profiles/health/hdp_util.c
> index 58770b5..ff427a6 100644
> --- a/profiles/health/hdp_util.c
> +++ b/profiles/health/hdp_util.c
> @@ -362,7 +362,7 @@ static gboolean set_sdp_services_uuid(sdp_record_t
> *record, HdpRole role) static gboolean register_service_protocols(struct
> hdp_adapter *adapter, sdp_record_t *sdp_record)
> {
> - gboolean ret;
> + gboolean ret = TRUE;
> uuid_t l2cap_uuid, mcap_c_uuid;
> sdp_list_t *l2cap_list, *proto_list = NULL, *mcap_list = NULL;
> sdp_list_t *access_proto_list = NULL;
> @@ -425,11 +425,7 @@ static gboolean register_service_protocols(struct
> hdp_adapter *adapter, goto end;
> }
>
> - if (sdp_set_access_protos(sdp_record, access_proto_list) < 0) {
> - ret = FALSE;
> - goto end;
> - }
> - ret = TRUE;
> + sdp_set_access_protos(sdp_record, access_proto_list);
I'd leave setting ret here instead of initializing it to TRUE.
--
Szymon K. Janc
[email protected]
Hi Andrei,
On Monday 16 June 2014 10:57:40 Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> Initialize value_len to -1 since otherwise check value_len >= 0 does not
> make sense.
> ---
> android/gatt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/android/gatt.c b/android/gatt.c
> index eb977d5..c5cb58f 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -4288,7 +4288,7 @@ static void read_requested_attributes(void *data, void
> *user_data) struct request_processing_data *process_data = user_data;
> uint32_t permissions;
> uint8_t *value, error;
> - int value_len = 0;
> + int value_len = -1;
>
> if (!gatt_db_get_attribute_permissions(gatt_db, resp_data->handle,
> &permissions)) {
This is not correct. value_len == -1 means that callback was called from
gatt_db_read() and response will be filled later.
--
Szymon K. Janc
[email protected]
Hi Andrei,
On Monday 16 June 2014 10:57:39 Andrei Emeltchenko wrote:
> 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 a19fe5c..eb977d5 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -4275,7 +4275,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("No pending response found! Bogus android response?");
Please prefix error message with "gatt:"
> return;
> }
--
Szymon K. Janc
[email protected]
Hi Andrei,
On Monday 16 June 2014 10:57:50 Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> ---
> profiles/health/hdp_util.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/profiles/health/hdp_util.c b/profiles/health/hdp_util.c
> index 7185805..876a105 100644
> --- a/profiles/health/hdp_util.c
> +++ b/profiles/health/hdp_util.c
> @@ -597,6 +597,13 @@ fail:
> return NULL;
> }
>
> +static void free_hdp_list(void *list)
> +{
> + sdp_list_t *hdp_list = list;
> +
> + sdp_list_free(hdp_list, (sdp_free_func_t)sdp_data_free);
> +}
> +
> static gboolean register_features(struct hdp_application *app,
> sdp_list_t **sup_features)
> {
> @@ -619,16 +626,11 @@ static gboolean register_features(struct
> hdp_application *app, fail:
> if (hdp_feature != NULL)
> sdp_list_free(hdp_feature, (sdp_free_func_t)sdp_data_free);
> + if (sup_features != NULL)
> + sdp_list_free(*sup_features, free_hdp_list);
I have a feeling that this should be fixed in caller code ie
register_service_sup_features.
Also sup_features was already dereferences few lines above so this check is
not needed (or should it be *sup_features != NULL ?).
> return FALSE;
> }
>
> -static void free_hdp_list(void *list)
> -{
> - sdp_list_t *hdp_list = list;
> -
> - sdp_list_free(hdp_list, (sdp_free_func_t)sdp_data_free);
> -}
> -
> static gboolean register_service_sup_features(GSList *app_list,
> sdp_record_t *sdp_record)
> {
--
Szymon K. Janc
[email protected]
Hi Andrei,
On Monday 16 June 2014 10:57:51 Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> ---
> android/hal-sco.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/android/hal-sco.c b/android/hal-sco.c
> index faed406..edff13e 100644
> --- a/android/hal-sco.c
> +++ b/android/hal-sco.c
> @@ -265,9 +265,6 @@ static int sco_ipc_cmd(uint8_t service_id, uint8_t
> opcode, uint16_t len, goto failed;
> }
>
> - if (rsp_len)
> - *rsp_len = cmd.len;
> -
> return SCO_STATUS_SUCCESS;
>
> failed:
This is same as comment on similar patch for hal-audio.
--
Szymon K. Janc
[email protected]
Hi Andrei,
On Monday 16 June 2014 10:57:48 Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> The code does not have effect.
> ---
> android/hal-audio.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/android/hal-audio.c b/android/hal-audio.c
> index 6d2ef3a..36f1f3f 100644
> --- a/android/hal-audio.c
> +++ b/android/hal-audio.c
> @@ -286,9 +286,6 @@ static int audio_ipc_cmd(uint8_t service_id, uint8_t
> opcode, uint16_t len, goto failed;
> }
>
> - if (rsp_len)
> - *rsp_len = cmd.len;
> -
> return AUDIO_STATUS_SUCCESS;
>
> failed:
This is not correct. rsp_len is also output parameter for this function. If
caller expects response it should verify it before accessing response.
(although since rsp_len was already dereferenced before null check is not
really needed here)
--
Szymon K. Janc
[email protected]
On Mon, Jun 16, 2014 at 10:57:36AM +0300, Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <[email protected]>
ping
>
> sdp_set_access_protos() always returns 0, there is no sense to check for
> error code. Fixes compile warnings.
> ---
> profiles/health/hdp_util.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/profiles/health/hdp_util.c b/profiles/health/hdp_util.c
> index 58770b5..ff427a6 100644
> --- a/profiles/health/hdp_util.c
> +++ b/profiles/health/hdp_util.c
> @@ -362,7 +362,7 @@ static gboolean set_sdp_services_uuid(sdp_record_t *record, HdpRole role)
> static gboolean register_service_protocols(struct hdp_adapter *adapter,
> sdp_record_t *sdp_record)
> {
> - gboolean ret;
> + gboolean ret = TRUE;
> uuid_t l2cap_uuid, mcap_c_uuid;
> sdp_list_t *l2cap_list, *proto_list = NULL, *mcap_list = NULL;
> sdp_list_t *access_proto_list = NULL;
> @@ -425,11 +425,7 @@ static gboolean register_service_protocols(struct hdp_adapter *adapter,
> goto end;
> }
>
> - if (sdp_set_access_protos(sdp_record, access_proto_list) < 0) {
> - ret = FALSE;
> - goto end;
> - }
> - ret = TRUE;
> + sdp_set_access_protos(sdp_record, access_proto_list);
>
> end:
> if (l2cap_list != NULL)
> --
> 1.8.3.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Andrei Emeltchenko <[email protected]>
---
android/hal-health.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/android/hal-health.c b/android/hal-health.c
index b8df5a7..af31634 100644
--- a/android/hal-health.c
+++ b/android/hal-health.c
@@ -143,7 +143,6 @@ static bt_status_t register_application(bthl_reg_param_t *reg, int *app_id)
if (status != BT_STATUS_SUCCESS)
return status;
-
}
*app_id = rsp.app_id;
--
1.8.3.2
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 c5cb58f..76336a6 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -2888,7 +2888,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.8.3.2
From: Andrei Emeltchenko <[email protected]>
The code does not have effect.
---
android/hal-audio.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/android/hal-audio.c b/android/hal-audio.c
index 6d2ef3a..36f1f3f 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -286,9 +286,6 @@ static int audio_ipc_cmd(uint8_t service_id, uint8_t opcode, uint16_t len,
goto failed;
}
- if (rsp_len)
- *rsp_len = cmd.len;
-
return AUDIO_STATUS_SUCCESS;
failed:
--
1.8.3.2
From: Andrei Emeltchenko <[email protected]>
---
android/hal-sco.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/android/hal-sco.c b/android/hal-sco.c
index ef04dc5..faed406 100644
--- a/android/hal-sco.c
+++ b/android/hal-sco.c
@@ -712,7 +712,7 @@ static int sco_open_output_stream(struct audio_hw_device *dev,
RESAMPLER_QUALITY_DEFAULT, NULL,
&out->resampler);
if (ret) {
- error("Failed to create resampler (%s)", strerror(ret));
+ error("Failed to create resampler (%s)", strerror(-ret));
goto failed;
}
@@ -1143,7 +1143,7 @@ static int sco_open_input_stream(struct audio_hw_device *dev,
RESAMPLER_QUALITY_DEFAULT, NULL,
&in->resampler);
if (ret) {
- error("Failed to create resampler (%s)", strerror(ret));
+ error("Failed to create resampler (%s)", strerror(-ret));
goto failed;
}
--
1.8.3.2
From: Andrei Emeltchenko <[email protected]>
---
profiles/health/mcap_sync.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/profiles/health/mcap_sync.c b/profiles/health/mcap_sync.c
index cc89d47..a0cc02a 100644
--- a/profiles/health/mcap_sync.c
+++ b/profiles/health/mcap_sync.c
@@ -187,7 +187,7 @@ void mcap_sync_stop(struct mcap_mcl *mcl)
static uint64_t time_us(struct timespec *tv)
{
- return tv->tv_sec * 1000000 + tv->tv_nsec / 1000;
+ return tv->tv_sec * 1000000ll + tv->tv_nsec / 1000ll;
}
static int64_t bt2us(int bt)
--
1.8.3.2
From: Andrei Emeltchenko <[email protected]>
Fixes static analyzer warnings related to casting possible error code to
unsigned.
---
profiles/health/hdp.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/profiles/health/hdp.c b/profiles/health/hdp.c
index e1549cf..65800fa 100644
--- a/profiles/health/hdp.c
+++ b/profiles/health/hdp.c
@@ -863,7 +863,10 @@ static gboolean serve_echo(GIOChannel *io_chan, GIOCondition cond,
chan->edata->echo_done = TRUE;
fd = g_io_channel_unix_get_fd(io_chan);
+
len = read(fd, buf, sizeof(buf));
+ if (len < 0)
+ goto fail;
if (send_echo_data(fd, buf, len) >= 0)
return TRUE;
--
1.8.3.2
From: Andrei Emeltchenko <[email protected]>
Make code cleaner lowering scope for mdep to the place it is actually
used.
---
android/hal-health.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/android/hal-health.c b/android/hal-health.c
index 012b6b7..b8df5a7 100644
--- a/android/hal-health.c
+++ b/android/hal-health.c
@@ -68,7 +68,6 @@ static bt_status_t register_application(bthl_reg_param_t *reg, int *app_id)
{
uint8_t buf[IPC_MTU];
struct hal_cmd_health_reg_app *cmd = (void *) buf;
- struct hal_cmd_health_mdep *mdep = (void *) buf;
struct hal_rsp_health_reg_app rsp;
size_t rsp_len = sizeof(rsp);
bt_status_t status;
@@ -123,6 +122,8 @@ static bt_status_t register_application(bthl_reg_param_t *reg, int *app_id)
return status;
for (i = 0; i < reg->number_of_mdeps; i++) {
+ struct hal_cmd_health_mdep *mdep = (void *) buf;
+
memset(buf, 0, IPC_MTU);
mdep->app_id = rsp.app_id;
mdep->role = reg->mdep_cfg[i].mdep_role;
--
1.8.3.2
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 76336a6..6ecef55 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -5269,7 +5269,7 @@ static uint8_t find_by_type_request(const uint8_t *cmd, uint16_t cmd_len,
}
data->filter_value = malloc0(search_vlen);
- if (!data) {
+ if (!data->filter_value) {
destroy_pending_request(data);
queue_destroy(q, NULL);
return ATT_ECODE_INSUFF_RESOURCES;
--
1.8.3.2
From: Andrei Emeltchenko <[email protected]>
---
profiles/health/hdp_util.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/profiles/health/hdp_util.c b/profiles/health/hdp_util.c
index 7185805..876a105 100644
--- a/profiles/health/hdp_util.c
+++ b/profiles/health/hdp_util.c
@@ -597,6 +597,13 @@ fail:
return NULL;
}
+static void free_hdp_list(void *list)
+{
+ sdp_list_t *hdp_list = list;
+
+ sdp_list_free(hdp_list, (sdp_free_func_t)sdp_data_free);
+}
+
static gboolean register_features(struct hdp_application *app,
sdp_list_t **sup_features)
{
@@ -619,16 +626,11 @@ static gboolean register_features(struct hdp_application *app,
fail:
if (hdp_feature != NULL)
sdp_list_free(hdp_feature, (sdp_free_func_t)sdp_data_free);
+ if (sup_features != NULL)
+ sdp_list_free(*sup_features, free_hdp_list);
return FALSE;
}
-static void free_hdp_list(void *list)
-{
- sdp_list_t *hdp_list = list;
-
- sdp_list_free(hdp_list, (sdp_free_func_t)sdp_data_free);
-}
-
static gboolean register_service_sup_features(GSList *app_list,
sdp_record_t *sdp_record)
{
--
1.8.3.2
From: Andrei Emeltchenko <[email protected]>
Is socket() fails we jumps to failed label.
---
android/ipc-tester.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/android/ipc-tester.c b/android/ipc-tester.c
index 63fd105..f1f93f2 100644
--- a/android/ipc-tester.c
+++ b/android/ipc-tester.c
@@ -279,7 +279,8 @@ static void emulator(int pipe, int hci_index)
failed:
close(pipe);
- close(fd);
+ if (fd >= 0)
+ close(fd);
}
static int accept_connection(int sk)
--
1.8.3.2
From: Andrei Emeltchenko <[email protected]>
---
profiles/health/hdp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/profiles/health/hdp.c b/profiles/health/hdp.c
index fff3f18..e1549cf 100644
--- a/profiles/health/hdp.c
+++ b/profiles/health/hdp.c
@@ -1507,8 +1507,8 @@ static gboolean check_echo(GIOChannel *io_chan, GIOCondition cond,
}
fd = g_io_channel_unix_get_fd(io_chan);
- len = read(fd, buf, sizeof(buf));
+ len = read(fd, buf, sizeof(buf));
if (len != HDP_ECHO_LEN) {
value = FALSE;
goto end;
--
1.8.3.2
From: Andrei Emeltchenko <[email protected]>
Function sdp_set_add_access_protos() always returns 0, so there is no
sense to check for error code.
---
profiles/health/hdp_util.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/profiles/health/hdp_util.c b/profiles/health/hdp_util.c
index ff427a6..7185805 100644
--- a/profiles/health/hdp_util.c
+++ b/profiles/health/hdp_util.c
@@ -472,7 +472,7 @@ static gboolean register_service_additional_protocols(
struct hdp_adapter *adapter,
sdp_record_t *sdp_record)
{
- gboolean ret;
+ gboolean ret = TRUE;
uuid_t l2cap_uuid, mcap_d_uuid;
sdp_list_t *l2cap_list, *proto_list = NULL, *mcap_list = NULL;
sdp_list_t *access_proto_list = NULL;
@@ -523,10 +523,7 @@ static gboolean register_service_additional_protocols(
goto end;
}
- if (sdp_set_add_access_protos(sdp_record, access_proto_list) < 0)
- ret = FALSE;
- else
- ret = TRUE;
+ sdp_set_add_access_protos(sdp_record, access_proto_list);
end:
if (l2cap_list != NULL)
--
1.8.3.2
From: Andrei Emeltchenko <[email protected]>
---
android/hal-sco.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/android/hal-sco.c b/android/hal-sco.c
index faed406..edff13e 100644
--- a/android/hal-sco.c
+++ b/android/hal-sco.c
@@ -265,9 +265,6 @@ static int sco_ipc_cmd(uint8_t service_id, uint8_t opcode, uint16_t len,
goto failed;
}
- if (rsp_len)
- *rsp_len = cmd.len;
-
return SCO_STATUS_SUCCESS;
failed:
--
1.8.3.2
From: Andrei Emeltchenko <[email protected]>
---
android/avrcp-lib.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
index 1de313b..32f6ff6 100644
--- a/android/avrcp-lib.c
+++ b/android/avrcp-lib.c
@@ -571,18 +571,22 @@ static bool check_value(uint8_t attr, uint8_t number, const uint8_t *values)
if (values[i] < AVRCP_EQUALIZER_OFF ||
values[i] > AVRCP_EQUALIZER_ON)
return false;
+ break;
case AVRCP_ATTRIBUTE_REPEAT_MODE:
if (values[i] < AVRCP_REPEAT_MODE_OFF ||
values[i] > AVRCP_REPEAT_MODE_GROUP)
return false;
+ break;
case AVRCP_ATTRIBUTE_SHUFFLE:
if (values[i] < AVRCP_SHUFFLE_OFF ||
values[i] > AVRCP_SHUFFLE_GROUP)
return false;
+ break;
case AVRCP_ATTRIBUTE_SCAN:
if (values[i] < AVRCP_SCAN_OFF ||
values[i] > AVRCP_SCAN_GROUP)
return false;
+ break;
}
}
--
1.8.3.2
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 a19fe5c..eb977d5 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -4275,7 +4275,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("No pending response found! Bogus android response?");
return;
}
--
1.8.3.2
From: Andrei Emeltchenko <[email protected]>
Initialize value_len to -1 since otherwise check value_len >= 0 does not
make sense.
---
android/gatt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/android/gatt.c b/android/gatt.c
index eb977d5..c5cb58f 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -4288,7 +4288,7 @@ static void read_requested_attributes(void *data, void *user_data)
struct request_processing_data *process_data = user_data;
uint32_t permissions;
uint8_t *value, error;
- int value_len = 0;
+ int value_len = -1;
if (!gatt_db_get_attribute_permissions(gatt_db, resp_data->handle,
&permissions)) {
--
1.8.3.2