[PATCH 1/2] audio: Replace g_hash_table_contains() with g_hash_table_lookup()

[PATCH 1/2] audio: Replace g_hash_table_contains() with g_hash_table_lookup()

[Jaganath Kanakkassery]

g_hash_table_contains() is supported only from GLib 2.32. If BlueZ has to
build against GLib 2.28 this patch replaces g_hash_table_contains() to
g_hash_table_lookup()
---
 profiles/audio/player.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/profiles/audio/player.c b/profiles/audio/player.c
index 8748893..4198bdb 100644
--- a/profiles/audio/player.c
+++ b/profiles/audio/player.c
@@ -172,8 +172,13 @@ static gboolean get_status(const GDBusPropertyTable *property,
 static gboolean setting_exists(const GDBusPropertyTable *property, void *data)
 {
 	struct media_player *mp = data;
+	const char *value;
+
+	value = g_hash_table_lookup(mp->settings, property->name);
+	if (value == NULL)
+		return FALSE;
 
-	return g_hash_table_contains(mp->settings, property->name);
+	return TRUE;
 }
 
 static gboolean get_setting(const GDBusPropertyTable *property,
-- 
1.7.9.5

[PATCH 2/2] shared: Fix build break

[Jaganath Kanakkassery]

This patch fixes the below build error

src/shared/mgmt.c: In function ‘mgmt_cancel_index’:
src/shared/mgmt.c:559:30: error: cast to pointer from integer of
different size [-Werror=int-to-pointer-cast]
cc1: all warnings being treated as errors
make[1]: *** [src/shared/bluetoothd-mgmt.o] Error 1
make: *** [all] Error 2
---
 src/shared/mgmt.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/shared/mgmt.c b/src/shared/mgmt.c
index 2297caf..c832a88 100644
--- a/src/shared/mgmt.c
+++ b/src/shared/mgmt.c
@@ -553,10 +553,12 @@ bool mgmt_cancel(struct mgmt *mgmt, unsigned int id)
 
 bool mgmt_cancel_index(struct mgmt *mgmt, uint16_t index)
 {
+	guint id = index;
+
 	if (!mgmt)
 		return false;
 
-	return cancel_request(mgmt, GUINT_TO_POINTER(index),
+	return cancel_request(mgmt, GUINT_TO_POINTER(id),
 						compare_request_index);
 }
 
-- 
1.7.9.5

Re: [PATCH 1/2] audio: Replace g_hash_table_contains() with g_hash_table_lookup()

[Ludek Finstrle]

Hello,

Mon, Jan 07, 2013 at 06:03:52PM +0530, Jaganath Kanakkassery napsal(a):
> g_hash_table_contains() is supported only from GLib 2.32. If BlueZ has to
> build against GLib 2.28 this patch replaces g_hash_table_contains() to
> g_hash_table_lookup()
> ---
>  profiles/audio/player.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/profiles/audio/player.c b/profiles/audio/player.c
> index 8748893..4198bdb 100644
> --- a/profiles/audio/player.c
> +++ b/profiles/audio/player.c
> @@ -172,8 +172,13 @@ static gboolean get_status(const GDBusPropertyTable *property,
>  static gboolean setting_exists(const GDBusPropertyTable *property, void *data)
>  {
>  	struct media_player *mp = data;
> +	const char *value;
> +
> +	value = g_hash_table_lookup(mp->settings, property->name);
> +	if (value == NULL)
> +		return FALSE;
>  
> -	return g_hash_table_contains(mp->settings, property->name);
> +	return TRUE;
>  }

Doesn't

return g_hash_table_lookup(mp->settings, property->name) != NULL;

do the same? Maybe it's againist some code style but looks better
than several lines.

Luf

Re: [PATCH 1/2] audio: Replace g_hash_table_contains() with g_hash_table_lookup()

[Jaganath Kanakkassery]

Hi Ludek,

--------------------------------------------------
From: "Ludek Finstrle" <luf@pzkagis.cz>
Sent: Monday, January 07, 2013 6:42 PM
To: "Jaganath Kanakkassery" <jaganath.k@samsung.com>
Cc: <linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH 1/2] audio: Replace g_hash_table_contains() with 
g_hash_table_lookup()

> Hello,
>
> Mon, Jan 07, 2013 at 06:03:52PM +0530, Jaganath Kanakkassery napsal(a):
>> g_hash_table_contains() is supported only from GLib 2.32. If BlueZ has to
>> build against GLib 2.28 this patch replaces g_hash_table_contains() to
>> g_hash_table_lookup()
>> ---
>>  profiles/audio/player.c |    7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/profiles/audio/player.c b/profiles/audio/player.c
>> index 8748893..4198bdb 100644
>> --- a/profiles/audio/player.c
>> +++ b/profiles/audio/player.c
>> @@ -172,8 +172,13 @@ static gboolean get_status(const GDBusPropertyTable 
>> *property,
>>  static gboolean setting_exists(const GDBusPropertyTable *property, void 
>> *data)
>>  {
>>  struct media_player *mp = data;
>> + const char *value;
>> +
>> + value = g_hash_table_lookup(mp->settings, property->name);
>> + if (value == NULL)
>> + return FALSE;
>>
>> - return g_hash_table_contains(mp->settings, property->name);
>> + return TRUE;
>>  }
>
> Doesn't
>
> return g_hash_table_lookup(mp->settings, property->name) != NULL;
>
> do the same? Maybe it's againist some code style but looks better
> than several lines.

I think
return g_hash_table_lookup(mp->settings, property->name) ? TRUE : FALSE;
would be better?

Thanks,
Jaganath 

Re: [PATCH 1/2] audio: Replace g_hash_table_contains() with g_hash_table_lookup()

[Luiz Augusto von Dentz]

Hi Jaganath,

On Mon, Jan 7, 2013 at 3:52 PM, Jaganath Kanakkassery
<jaganath.k@samsung.com> wrote:
> Hi Ludek,
>
> --------------------------------------------------
> From: "Ludek Finstrle" <luf@pzkagis.cz>
> Sent: Monday, January 07, 2013 6:42 PM
> To: "Jaganath Kanakkassery" <jaganath.k@samsung.com>
> Cc: <linux-bluetooth@vger.kernel.org>
> Subject: Re: [PATCH 1/2] audio: Replace g_hash_table_contains() with
> g_hash_table_lookup()
>
>
>> Hello,
>>
>> Mon, Jan 07, 2013 at 06:03:52PM +0530, Jaganath Kanakkassery napsal(a):
>>>
>>> g_hash_table_contains() is supported only from GLib 2.32. If BlueZ has to
>>> build against GLib 2.28 this patch replaces g_hash_table_contains() to
>>> g_hash_table_lookup()
>>> ---
>>>  profiles/audio/player.c |    7 ++++++-
>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/profiles/audio/player.c b/profiles/audio/player.c
>>> index 8748893..4198bdb 100644
>>> --- a/profiles/audio/player.c
>>> +++ b/profiles/audio/player.c
>>> @@ -172,8 +172,13 @@ static gboolean get_status(const GDBusPropertyTable
>>> *property,
>>>  static gboolean setting_exists(const GDBusPropertyTable *property, void
>>> *data)
>>>  {
>>>  struct media_player *mp = data;
>>> + const char *value;
>>> +
>>> + value = g_hash_table_lookup(mp->settings, property->name);
>>> + if (value == NULL)
>>> + return FALSE;
>>>
>>> - return g_hash_table_contains(mp->settings, property->name);
>>> + return TRUE;
>>>  }
>>
>>
>> Doesn't
>>
>> return g_hash_table_lookup(mp->settings, property->name) != NULL;
>>
>> do the same? Maybe it's againist some code style but looks better
>> than several lines.
>
>
> I think
> return g_hash_table_lookup(mp->settings, property->name) ? TRUE : FALSE;
> would be better?

Shorter form is  != NULL, so lets go with it, still haven't figure out
why we didn't bring back glib-compat for such functions, when we
finally upgrade it will take much more time to revert this one by one.


--
Luiz Augusto von Dentz

Re: [PATCH 2/2] shared: Fix build break

[Marcel Holtmann]

Hi Jaganath,

> This patch fixes the below build error
> 
> src/shared/mgmt.c: In function ‘mgmt_cancel_index’:
> src/shared/mgmt.c:559:30: error: cast to pointer from integer of
> different size [-Werror=int-to-pointer-cast]
> cc1: all warnings being treated as errors
> make[1]: *** [src/shared/bluetoothd-mgmt.o] Error 1
> make: *** [all] Error 2
> ---
>  src/shared/mgmt.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/shared/mgmt.c b/src/shared/mgmt.c
> index 2297caf..c832a88 100644
> --- a/src/shared/mgmt.c
> +++ b/src/shared/mgmt.c
> @@ -553,10 +553,12 @@ bool mgmt_cancel(struct mgmt *mgmt, unsigned int id)
>  
>  bool mgmt_cancel_index(struct mgmt *mgmt, uint16_t index)
>  {
> +	guint id = index;
> +

I changed this into unsigned int instead when applying the patch.

Regards

Marcel

Re: [PATCH 1/2] audio: Replace g_hash_table_contains() with g_hash_table_lookup()

[Marcel Holtmann]

Hi Jaganath,

> g_hash_table_contains() is supported only from GLib 2.32. If BlueZ has to
> build against GLib 2.28 this patch replaces g_hash_table_contains() to
> g_hash_table_lookup()
> ---
>  profiles/audio/player.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/profiles/audio/player.c b/profiles/audio/player.c
> index 8748893..4198bdb 100644
> --- a/profiles/audio/player.c
> +++ b/profiles/audio/player.c
> @@ -172,8 +172,13 @@ static gboolean get_status(const GDBusPropertyTable *property,
>  static gboolean setting_exists(const GDBusPropertyTable *property, void *data)
>  {
>  	struct media_player *mp = data;
> +	const char *value;
> +
> +	value = g_hash_table_lookup(mp->settings, property->name);

	return value ? TRUE : FALSE;

Regards

Marcel

Re: [PATCH 1/2] audio: Replace g_hash_table_contains() with g_hash_table_lookup()

[Marcel Holtmann]

Hi Luiz,

<snip>

> > I think
> > return g_hash_table_lookup(mp->settings, property->name) ? TRUE : FALSE;
> > would be better?
> 
> Shorter form is  != NULL, so lets go with it, still haven't figure out
> why we didn't bring back glib-compat for such functions, when we
> finally upgrade it will take much more time to revert this one by one.

we could bring glib-compat back, but right now I do not want to
introduce more autoconf magic to do so.

I would require the latest GLib version (or at least a bit newer one) if
they wouldn't just start introducing new dependencies all the time.

Regards

Marcel