[PATCH 1/2] core: Replace calls to g_queue_free_full function

[PATCH 1/2] core: Replace calls to g_queue_free_full function

[Giovanni Gherdovich]

The function g_queue_free_full is available only from GLib 2.32.
If BlueZ has to build against GLib 2.28, as stated in the configure.ac,
this patch replaces the calls to g_queue_free_full in the "core" BlueZ module
with its body, taken from the sources of GLib 2.32.
---
 src/adapter.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index e71cea8..73c9e58 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1697,7 +1697,8 @@ static void adapter_free(gpointer user_data)
 	if (adapter->auth_idle_id)
 		g_source_remove(adapter->auth_idle_id);
 
-	g_queue_free_full(adapter->auths, g_free);
+	g_queue_foreach(adapter->auths, (GFunc)g_free, NULL);
+	g_queue_free(adapter->auths);
 
 	sdp_list_free(adapter->services, NULL);
 
-- 
1.7.4.1

Re: [PATCH 1/2] core: Replace calls to g_queue_free_full function

[Marcel Holtmann]

Hi Giovanni,

> The function g_queue_free_full is available only from GLib 2.32.
> If BlueZ has to build against GLib 2.28, as stated in the configure.ac,
> this patch replaces the calls to g_queue_free_full in the "core" BlueZ module
> with its body, taken from the sources of GLib 2.32.
> ---
>  src/adapter.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/src/adapter.c b/src/adapter.c
> index e71cea8..73c9e58 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -1697,7 +1697,8 @@ static void adapter_free(gpointer user_data)
>  	if (adapter->auth_idle_id)
>  		g_source_remove(adapter->auth_idle_id);
>  
> -	g_queue_free_full(adapter->auths, g_free);
> +	g_queue_foreach(adapter->auths, (GFunc)g_free, NULL);

I do not like the usage of (GFunc) casting here. Please add a function
that properly frees this and has the right signature.

> +	g_queue_free(adapter->auths);
>  
>  	sdp_list_free(adapter->services, NULL);
>  

Regards

Marcel

[PATCH 1/2] core: Replace calls to g_queue_free_full function

[Giovanni Gherdovich]

The function g_queue_free_full is available only from GLib 2.32.
If BlueZ has to build against GLib 2.28, as stated in the configure.ac,
this patch replaces the calls to g_queue_free_full in the "core" BlueZ module
with its body, taken from the sources of GLib 2.32.
---
 src/adapter.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index e71cea8..f7fc00e 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1688,6 +1688,15 @@ int btd_adapter_stop(struct btd_adapter *adapter)
 	return 0;
 }
 
+static void g_free_wrapper(gpointer mem, gpointer dummy)
+{
+	/* 
+	 * Wrapper around GLib's g_free to match the signature
+	 * required for the second argument of g_queue_foreach.
+	 */
+	g_free(mem);
+}
+
 static void adapter_free(gpointer user_data)
 {
 	struct btd_adapter *adapter = user_data;
@@ -1697,7 +1706,8 @@ static void adapter_free(gpointer user_data)
 	if (adapter->auth_idle_id)
 		g_source_remove(adapter->auth_idle_id);
 
-	g_queue_free_full(adapter->auths, g_free);
+	g_queue_foreach(adapter->auths, g_free_wrapper, NULL);
+	g_queue_free(adapter->auths);
 
 	sdp_list_free(adapter->services, NULL);
 
-- 
1.7.4.1

Re: [PATCH 1/2] core: Replace calls to g_queue_free_full function

[Marcel Holtmann]

Hi Giovanni,

> The function g_queue_free_full is available only from GLib 2.32.
> If BlueZ has to build against GLib 2.28, as stated in the configure.ac,
> this patch replaces the calls to g_queue_free_full in the "core" BlueZ module
> with its body, taken from the sources of GLib 2.32.
> ---
>  src/adapter.c |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)
> 
> diff --git a/src/adapter.c b/src/adapter.c
> index e71cea8..f7fc00e 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -1688,6 +1688,15 @@ int btd_adapter_stop(struct btd_adapter *adapter)
>  	return 0;
>  }
>  
> +static void g_free_wrapper(gpointer mem, gpointer dummy)
> +{
> +	/* 
> +	 * Wrapper around GLib's g_free to match the signature
> +	 * required for the second argument of g_queue_foreach.
> +	 */
> +	g_free(mem);
> +}
> +

I would have done this:

	static void free_service_auth(gpointer data, gpointer user_data)
	{
		struct service_auth *auth = data;

		g_free(auth);
	}

>  static void adapter_free(gpointer user_data)
>  {
>  	struct btd_adapter *adapter = user_data;
> @@ -1697,7 +1706,8 @@ static void adapter_free(gpointer user_data)
>  	if (adapter->auth_idle_id)
>  		g_source_remove(adapter->auth_idle_id);
>  
> -	g_queue_free_full(adapter->auths, g_free);
> +	g_queue_foreach(adapter->auths, g_free_wrapper, NULL);
> +	g_queue_free(adapter->auths);
>  
>  	sdp_list_free(adapter->services, NULL);
>  

Regards

Marcel

Re: [PATCH 1/2] core: Replace calls to g_queue_free_full function

[Giovanni Gherdovich]

Hi Marcel, Luiz, Vinicius, Anderson,

2013/1/2 Marcel Holtmann <marcel@holtmann.org>:
> [...]
>
> I would have done this:
>
>         static void free_service_auth(gpointer data, gpointer user_data)
>         {
>                 struct service_auth *auth = data;
>
>                 g_free(auth);
>         }


> change pending_destroy() to this:
>
>         static void pending_destroy(gpointer data, gpointer user_data)
>         {
>                 ...
>         }
>
> And then fix the callers to add an extra NULL.

I am about to send the modified patch.

I also had to revert another GLib function, g_slist_free_full,
to g_slist_foreach + g_slist_free, since having changed the signature
of pending_destroy, i didn't suit any more the second argument of
g_slist_free_full.
I hope that's not a big deal.


2013/1/2 Luiz Augusto von Dentz <luiz.dentz@gmail.com>:
> [...]
>
> There is something off here, in the past we did have an implementation
> of g_slist_free_full to overcome this dependency problem but it was
> removed in this commit:
>
> commit 84156dadb25ec0973752d34d84fc9ffb3c720988
> Author: Marcel Holtmann <marcel@holtmann.org>
> Date:   Mon Apr 16 18:22:24 2012 +0200
>
>     build: Remove glib-compat.h support
>

yes, here a link to the gitweb
http://git.kernel.org/?p=bluetooth/bluez.git;a=commitdiff;h=84156dadb25ec0973752d34d84fc9ffb3c720988;hp=1a79248e6be548696715f3bb2ddcf01630cf2c69

2013/1/2 Luiz Augusto von Dentz <luiz.dentz@gmail.com>:
> Hi Vinicius,
>
> On Wed, Jan 2, 2013 at 10:23 PM, Vinicius Gomes
>> [...]
>> The problem now is g_queue_free_full() not the g_slist_free_full().
>
> Right, but it is quite the same situation  [...]

It is exactly the same situation.
In the commit where the GLib team added g_queue_free_full,
they mentioned that they wanted to align the API for GQueue
to what they had for GSList and GList.

2013/1/3 Luiz Augusto von Dentz <luiz.dentz@gmail.com>:
> [...]
>
> Anyway regardless if we do update or not, I don't see why
> g_queue_free_full is different than g_slist_free_full, so instead of
> converting everything to g_queue_foreach + g_queue_free why we don't
> bring back glib-compat and do this in one place as we did for
> g_slist_free_full?

If you bring back that code, you bring back the evil cast of

void (*) (void *)

to

void (*) (void *, void *)

which I am trying to avoid.

Regards,
Giovanni

[PATCH 1/2] core: Replace calls to g_queue_free_full function

[Giovanni Gherdovich]

The function g_queue_free_full is available only from GLib 2.32.
If BlueZ has to build against GLib 2.28, as stated in the configure.ac,
this patch replaces the calls to g_queue_free_full in the "core" BlueZ module
with its body, taken from the sources of GLib 2.32.
---
 src/adapter.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index e71cea8..ca94fa9 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1688,6 +1688,13 @@ int btd_adapter_stop(struct btd_adapter *adapter)
 	return 0;
 }
 
+static void free_service_auth(gpointer data, gpointer user_data)
+{
+	struct service_auth *auth = data;
+
+	g_free(auth);
+}
+
 static void adapter_free(gpointer user_data)
 {
 	struct btd_adapter *adapter = user_data;
@@ -1697,7 +1704,8 @@ static void adapter_free(gpointer user_data)
 	if (adapter->auth_idle_id)
 		g_source_remove(adapter->auth_idle_id);
 
-	g_queue_free_full(adapter->auths, g_free);
+	g_queue_foreach(adapter->auths, free_service_auth, NULL);
+	g_queue_free(adapter->auths);
 
 	sdp_list_free(adapter->services, NULL);
 
-- 
1.7.4.1

Re: [PATCH 1/2] core: Replace calls to g_queue_free_full function

[Marcel Holtmann]

Hi Giovanni,

> The function g_queue_free_full is available only from GLib 2.32.
> If BlueZ has to build against GLib 2.28, as stated in the configure.ac,
> this patch replaces the calls to g_queue_free_full in the "core" BlueZ module
> with its body, taken from the sources of GLib 2.32.
> ---
>  src/adapter.c |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)

patch has been applied.

Regards

Marcel