All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] android/avrcp-lib: Use void pointer in register_notification
@ 2015-04-14 13:48 Szymon Janc
  2015-04-16 11:24 ` Szymon Janc
  0 siblings, 1 reply; 4+ messages in thread
From: Szymon Janc @ 2015-04-14 13:48 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

In this callback params vary depending on code. Passing those as void*
allows to avoid extra memcpy that would be otherwise needed to
avoid warnings due to increased alignment when casting.
---
 android/avrcp-lib.h | 2 +-
 android/avrcp.c     | 5 +++--
 unit/test-avrcp.c   | 9 +++++----
 3 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
index add7739..6554b12 100644
--- a/android/avrcp-lib.h
+++ b/android/avrcp-lib.h
@@ -224,7 +224,7 @@ struct avrcp_control_cfm {
 					char **text, void *user_data);
 	bool (*register_notification) (struct avrcp *session, int err,
 					uint8_t code, uint8_t event,
-					uint8_t *params, void *user_data);
+					void *params, void *user_data);
 	void (*set_volume) (struct avrcp *session, int err, uint8_t volume,
 					void *user_data);
 	void (*set_addressed) (struct avrcp *session, int err,
diff --git a/android/avrcp.c b/android/avrcp.c
index bcb4228..8c3e25a 100644
--- a/android/avrcp.c
+++ b/android/avrcp.c
@@ -748,11 +748,12 @@ static const struct avrcp_control_ind control_ind = {
 
 static bool handle_register_notification_rsp(struct avrcp *session, int err,
 						uint8_t code, uint8_t event,
-						uint8_t *params,
+						void *params,
 						void *user_data)
 {
 	struct avrcp_device *dev = user_data;
 	struct hal_ev_avrcp_volume_changed ev;
+	uint8_t *volume = params;
 
 	if (err < 0) {
 		error("AVRCP: %s", strerror(-err));
@@ -766,7 +767,7 @@ static bool handle_register_notification_rsp(struct avrcp *session, int err,
 		return false;
 
 	ev.type = code;
-	ev.volume = params[0] & 0x7f;
+	ev.volume = volume[0];
 
 	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_AVRCP,
 					HAL_EV_AVRCP_VOLUME_CHANGED,
diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
index a610ad5..01307e6 100644
--- a/unit/test-avrcp.c
+++ b/unit/test-avrcp.c
@@ -826,22 +826,23 @@ static void set_volume_rsp(struct avrcp *session, int err, uint8_t volume,
 
 static bool register_notification_rsp(struct avrcp *session, int err,
 					uint8_t code, uint8_t event,
-					uint8_t *params, void *user_data)
+					void *params, void *user_data)
 {
 	struct context *context = user_data;
+	uint8_t *p = params;
 
 	g_assert_cmpint(err, ==, 0);
 
 	switch (event) {
 	case AVRCP_EVENT_VOLUME_CHANGED:
 		if (g_str_equal(context->data->test_name, "/TP/VLH/BV-03-C")) {
-			g_assert_cmpint(params[0], ==, 0);
+			g_assert_cmpint(p[0], ==, 0);
 			break;
 		} else if (code == AVC_CTYPE_INTERIM) {
-			g_assert_cmpint(params[0], ==, 0);
+			g_assert_cmpint(p[0], ==, 0);
 			return true;
 		}
-		g_assert_cmpint(params[0], ==, 1);
+		g_assert_cmpint(p[0], ==, 1);
 		break;
 	}
 
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] android/avrcp-lib: Use void pointer in register_notification
  2015-04-14 13:48 [PATCH] android/avrcp-lib: Use void pointer in register_notification Szymon Janc
@ 2015-04-16 11:24 ` Szymon Janc
  2015-04-16 14:42   ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 4+ messages in thread
From: Szymon Janc @ 2015-04-16 11:24 UTC (permalink / raw)
  To: linux-bluetooth

On Tuesday 14 of April 2015 15:48:35 Szymon Janc wrote:
> In this callback params vary depending on code. Passing those as void*
> allows to avoid extra memcpy that would be otherwise needed to
> avoid warnings due to increased alignment when casting.
> ---
>  android/avrcp-lib.h | 2 +-
>  android/avrcp.c     | 5 +++--
>  unit/test-avrcp.c   | 9 +++++----
>  3 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
> index add7739..6554b12 100644
> --- a/android/avrcp-lib.h
> +++ b/android/avrcp-lib.h
> @@ -224,7 +224,7 @@ struct avrcp_control_cfm {
>  					char **text, void *user_data);
>  	bool (*register_notification) (struct avrcp *session, int err,
>  					uint8_t code, uint8_t event,
> -					uint8_t *params, void *user_data);
> +					void *params, void *user_data);
>  	void (*set_volume) (struct avrcp *session, int err, uint8_t volume,
>  					void *user_data);
>  	void (*set_addressed) (struct avrcp *session, int err,
> diff --git a/android/avrcp.c b/android/avrcp.c
> index bcb4228..8c3e25a 100644
> --- a/android/avrcp.c
> +++ b/android/avrcp.c
> @@ -748,11 +748,12 @@ static const struct avrcp_control_ind control_ind = {
> 
>  static bool handle_register_notification_rsp(struct avrcp *session, int
> err, uint8_t code, uint8_t event,
> -						uint8_t *params,
> +						void *params,
>  						void *user_data)
>  {
>  	struct avrcp_device *dev = user_data;
>  	struct hal_ev_avrcp_volume_changed ev;
> +	uint8_t *volume = params;
> 
>  	if (err < 0) {
>  		error("AVRCP: %s", strerror(-err));
> @@ -766,7 +767,7 @@ static bool handle_register_notification_rsp(struct
> avrcp *session, int err, return false;
> 
>  	ev.type = code;
> -	ev.volume = params[0] & 0x7f;
> +	ev.volume = volume[0];
> 
>  	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_AVRCP,
>  					HAL_EV_AVRCP_VOLUME_CHANGED,
> diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
> index a610ad5..01307e6 100644
> --- a/unit/test-avrcp.c
> +++ b/unit/test-avrcp.c
> @@ -826,22 +826,23 @@ static void set_volume_rsp(struct avrcp *session, int
> err, uint8_t volume,
> 
>  static bool register_notification_rsp(struct avrcp *session, int err,
>  					uint8_t code, uint8_t event,
> -					uint8_t *params, void *user_data)
> +					void *params, void *user_data)
>  {
>  	struct context *context = user_data;
> +	uint8_t *p = params;
> 
>  	g_assert_cmpint(err, ==, 0);
> 
>  	switch (event) {
>  	case AVRCP_EVENT_VOLUME_CHANGED:
>  		if (g_str_equal(context->data->test_name, "/TP/VLH/BV-03-C")) {
> -			g_assert_cmpint(params[0], ==, 0);
> +			g_assert_cmpint(p[0], ==, 0);
>  			break;
>  		} else if (code == AVC_CTYPE_INTERIM) {
> -			g_assert_cmpint(params[0], ==, 0);
> +			g_assert_cmpint(p[0], ==, 0);
>  			return true;
>  		}
> -		g_assert_cmpint(params[0], ==, 1);
> +		g_assert_cmpint(p[0], ==, 1);
>  		break;
>  	}

Applied.

-- 
BR
Szymon Janc

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] android/avrcp-lib: Use void pointer in register_notification
  2015-04-16 11:24 ` Szymon Janc
@ 2015-04-16 14:42   ` Luiz Augusto von Dentz
  2015-04-16 14:47     ` Szymon Janc
  0 siblings, 1 reply; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2015-04-16 14:42 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth

Hi Szymon,

On Thu, Apr 16, 2015 at 2:24 PM, Szymon Janc <szymon.janc@tieto.com> wrote:
> On Tuesday 14 of April 2015 15:48:35 Szymon Janc wrote:
>> In this callback params vary depending on code. Passing those as void*
>> allows to avoid extra memcpy that would be otherwise needed to
>> avoid warnings due to increased alignment when casting.
>> ---
>>  android/avrcp-lib.h | 2 +-
>>  android/avrcp.c     | 5 +++--
>>  unit/test-avrcp.c   | 9 +++++----
>>  3 files changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
>> index add7739..6554b12 100644
>> --- a/android/avrcp-lib.h
>> +++ b/android/avrcp-lib.h
>> @@ -224,7 +224,7 @@ struct avrcp_control_cfm {
>>                                       char **text, void *user_data);
>>       bool (*register_notification) (struct avrcp *session, int err,
>>                                       uint8_t code, uint8_t event,
>> -                                     uint8_t *params, void *user_data);
>> +                                     void *params, void *user_data);
>>       void (*set_volume) (struct avrcp *session, int err, uint8_t volume,
>>                                       void *user_data);
>>       void (*set_addressed) (struct avrcp *session, int err,
>> diff --git a/android/avrcp.c b/android/avrcp.c
>> index bcb4228..8c3e25a 100644
>> --- a/android/avrcp.c
>> +++ b/android/avrcp.c
>> @@ -748,11 +748,12 @@ static const struct avrcp_control_ind control_ind = {
>>
>>  static bool handle_register_notification_rsp(struct avrcp *session, int
>> err, uint8_t code, uint8_t event,
>> -                                             uint8_t *params,
>> +                                             void *params,
>>                                               void *user_data)
>>  {
>>       struct avrcp_device *dev = user_data;
>>       struct hal_ev_avrcp_volume_changed ev;
>> +     uint8_t *volume = params;
>>
>>       if (err < 0) {
>>               error("AVRCP: %s", strerror(-err));
>> @@ -766,7 +767,7 @@ static bool handle_register_notification_rsp(struct
>> avrcp *session, int err, return false;
>>
>>       ev.type = code;
>> -     ev.volume = params[0] & 0x7f;
>> +     ev.volume = volume[0];

With the above change we would not ignore the most significant bit.

>>       ipc_send_notif(hal_ipc, HAL_SERVICE_ID_AVRCP,
>>                                       HAL_EV_AVRCP_VOLUME_CHANGED,
>> diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
>> index a610ad5..01307e6 100644
>> --- a/unit/test-avrcp.c
>> +++ b/unit/test-avrcp.c
>> @@ -826,22 +826,23 @@ static void set_volume_rsp(struct avrcp *session, int
>> err, uint8_t volume,
>>
>>  static bool register_notification_rsp(struct avrcp *session, int err,
>>                                       uint8_t code, uint8_t event,
>> -                                     uint8_t *params, void *user_data)
>> +                                     void *params, void *user_data)
>>  {
>>       struct context *context = user_data;
>> +     uint8_t *p = params;
>>
>>       g_assert_cmpint(err, ==, 0);
>>
>>       switch (event) {
>>       case AVRCP_EVENT_VOLUME_CHANGED:
>>               if (g_str_equal(context->data->test_name, "/TP/VLH/BV-03-C")) {
>> -                     g_assert_cmpint(params[0], ==, 0);
>> +                     g_assert_cmpint(p[0], ==, 0);
>>                       break;
>>               } else if (code == AVC_CTYPE_INTERIM) {
>> -                     g_assert_cmpint(params[0], ==, 0);
>> +                     g_assert_cmpint(p[0], ==, 0);
>>                       return true;
>>               }
>> -             g_assert_cmpint(params[0], ==, 1);
>> +             g_assert_cmpint(p[0], ==, 1);
>>               break;
>>       }
>
> Applied.
>
> --
> BR
> Szymon Janc
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] android/avrcp-lib: Use void pointer in register_notification
  2015-04-16 14:42   ` Luiz Augusto von Dentz
@ 2015-04-16 14:47     ` Szymon Janc
  0 siblings, 0 replies; 4+ messages in thread
From: Szymon Janc @ 2015-04-16 14:47 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hi Luiz,

On Thursday 16 of April 2015 17:42:58 Luiz Augusto von Dentz wrote:
> Hi Szymon,
> 
> On Thu, Apr 16, 2015 at 2:24 PM, Szymon Janc <szymon.janc@tieto.com> wrote:
> > On Tuesday 14 of April 2015 15:48:35 Szymon Janc wrote:
> >> In this callback params vary depending on code. Passing those as void*
> >> allows to avoid extra memcpy that would be otherwise needed to
> >> avoid warnings due to increased alignment when casting.
> >> ---
> >> 
> >>  android/avrcp-lib.h | 2 +-
> >>  android/avrcp.c     | 5 +++--
> >>  unit/test-avrcp.c   | 9 +++++----
> >>  3 files changed, 9 insertions(+), 7 deletions(-)
> >> 
> >> diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
> >> index add7739..6554b12 100644
> >> --- a/android/avrcp-lib.h
> >> +++ b/android/avrcp-lib.h
> >> @@ -224,7 +224,7 @@ struct avrcp_control_cfm {
> >> 
> >>                                       char **text, void *user_data);
> >>       
> >>       bool (*register_notification) (struct avrcp *session, int err,
> >>       
> >>                                       uint8_t code, uint8_t event,
> >> 
> >> -                                     uint8_t *params, void *user_data);
> >> +                                     void *params, void *user_data);
> >> 
> >>       void (*set_volume) (struct avrcp *session, int err, uint8_t volume,
> >>       
> >>                                       void *user_data);
> >>       
> >>       void (*set_addressed) (struct avrcp *session, int err,
> >> 
> >> diff --git a/android/avrcp.c b/android/avrcp.c
> >> index bcb4228..8c3e25a 100644
> >> --- a/android/avrcp.c
> >> +++ b/android/avrcp.c
> >> @@ -748,11 +748,12 @@ static const struct avrcp_control_ind control_ind =
> >> {
> >> 
> >>  static bool handle_register_notification_rsp(struct avrcp *session, int
> >> 
> >> err, uint8_t code, uint8_t event,
> >> -                                             uint8_t *params,
> >> +                                             void *params,
> >> 
> >>                                               void *user_data)
> >>  
> >>  {
> >>  
> >>       struct avrcp_device *dev = user_data;
> >>       struct hal_ev_avrcp_volume_changed ev;
> >> 
> >> +     uint8_t *volume = params;
> >> 
> >>       if (err < 0) {
> >>       
> >>               error("AVRCP: %s", strerror(-err));
> >> 
> >> @@ -766,7 +767,7 @@ static bool handle_register_notification_rsp(struct
> >> avrcp *session, int err, return false;
> >> 
> >>       ev.type = code;
> >> 
> >> -     ev.volume = params[0] & 0x7f;
> >> +     ev.volume = volume[0];
> 
> With the above change we would not ignore the most significant bit.

This is already done by avrcp-lib. I just didn't bother to fix this in 
separate patch since line was changed anyway.

> 
> >>       ipc_send_notif(hal_ipc, HAL_SERVICE_ID_AVRCP,
> >>       
> >>                                       HAL_EV_AVRCP_VOLUME_CHANGED,
> >> 
> >> diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
> >> index a610ad5..01307e6 100644
> >> --- a/unit/test-avrcp.c
> >> +++ b/unit/test-avrcp.c
> >> @@ -826,22 +826,23 @@ static void set_volume_rsp(struct avrcp *session,
> >> int
> >> err, uint8_t volume,
> >> 
> >>  static bool register_notification_rsp(struct avrcp *session, int err,
> >>  
> >>                                       uint8_t code, uint8_t event,
> >> 
> >> -                                     uint8_t *params, void *user_data)
> >> +                                     void *params, void *user_data)
> >> 
> >>  {
> >>  
> >>       struct context *context = user_data;
> >> 
> >> +     uint8_t *p = params;
> >> 
> >>       g_assert_cmpint(err, ==, 0);
> >>       
> >>       switch (event) {
> >>       
> >>       case AVRCP_EVENT_VOLUME_CHANGED:
> >>               if (g_str_equal(context->data->test_name,
> >>               "/TP/VLH/BV-03-C")) {
> >> 
> >> -                     g_assert_cmpint(params[0], ==, 0);
> >> +                     g_assert_cmpint(p[0], ==, 0);
> >> 
> >>                       break;
> >>               
> >>               } else if (code == AVC_CTYPE_INTERIM) {
> >> 
> >> -                     g_assert_cmpint(params[0], ==, 0);
> >> +                     g_assert_cmpint(p[0], ==, 0);
> >> 
> >>                       return true;
> >>               
> >>               }
> >> 
> >> -             g_assert_cmpint(params[0], ==, 1);
> >> +             g_assert_cmpint(p[0], ==, 1);
> >> 
> >>               break;
> >>       
> >>       }
> > 
> > Applied.
> > 
> > --
> > BR
> > Szymon Janc
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth"
> > in the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
BR
Szymon Janc

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-04-16 14:47 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-14 13:48 [PATCH] android/avrcp-lib: Use void pointer in register_notification Szymon Janc
2015-04-16 11:24 ` Szymon Janc
2015-04-16 14:42   ` Luiz Augusto von Dentz
2015-04-16 14:47     ` Szymon Janc

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.