Return-Path: MIME-Version: 1.0 In-Reply-To: References: <1349114021-19067-1-git-send-email-luiz.dentz@gmail.com> <1349114021-19067-2-git-send-email-luiz.dentz@gmail.com> From: Lucas De Marchi Date: Tue, 2 Oct 2012 07:49:00 -0300 Message-ID: Subject: Re: [PATCH BlueZ 02/10] gdbus: Remove connection from g_dbus_remove_watch To: Luiz Augusto von Dentz Cc: linux-bluetooth@vger.kernel.org Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-bluetooth-owner@vger.kernel.org List-ID: On Tue, Oct 2, 2012 at 5:05 AM, Luiz Augusto von Dentz wrote: > Hi Lucas, > > On Tue, Oct 2, 2012 at 7:23 AM, Lucas De Marchi > wrote: >> On Mon, Oct 1, 2012 at 2:53 PM, Luiz Augusto von Dentz >> wrote: >>> From: Luiz Augusto von Dentz >>> >>> The connection is not really needed since the list of listeners is >>> global not per connection, besides it is more convenient this way as >>> only the id is needed. >>> --- >>> gdbus/gdbus.h | 2 +- >>> gdbus/watch.c | 4 ++-- >>> 2 files changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h >>> index 0a8a27c..3bd8986 100644 >>> --- a/gdbus/gdbus.h >>> +++ b/gdbus/gdbus.h >>> @@ -217,7 +217,7 @@ guint g_dbus_add_signal_watch(DBusConnection *connection, >>> const char *interface, const char *member, >>> GDBusSignalFunction function, void *user_data, >>> GDBusDestroyFunction destroy); >>> -gboolean g_dbus_remove_watch(DBusConnection *connection, guint tag); >>> +gboolean g_dbus_remove_watch(guint id); >>> void g_dbus_remove_all_watches(DBusConnection *connection); >>> >>> #ifdef __cplusplus >>> diff --git a/gdbus/watch.c b/gdbus/watch.c >>> index 07feb61..00cedae 100644 >>> --- a/gdbus/watch.c >>> +++ b/gdbus/watch.c >>> @@ -285,7 +285,7 @@ static void filter_data_free(struct filter_data *data) >>> g_free(l->data); >>> >>> g_slist_free(data->callbacks); >>> - g_dbus_remove_watch(data->connection, data->name_watch); >>> + g_dbus_remove_watch(data->name_watch); >>> g_free(data->name); >>> g_free(data->owner); >>> g_free(data->path); >>> @@ -752,7 +752,7 @@ guint g_dbus_add_signal_watch(DBusConnection *connection, >>> return cb->id; >>> } >>> >>> -gboolean g_dbus_remove_watch(DBusConnection *connection, guint id) >>> +gboolean g_dbus_remove_watch(guint id) >>> { >>> struct filter_data *data; >>> struct filter_callback *cb; >>> -- >> >> This will lead to a broken build which is not nice to bisect. Any >> other option besides adding with another name, converting everybody >> and then renaming? >> >> The only other option I can think of is adding an ifdef and an option >> to build-sys... this would avoid the final renaming. > > I guess this time it worth having the break of bisect in favor of a > more convenient API, if you look at what the code is doing the > connection is useless and normally we end up having to call the core > to figure out a connection that is not used for anything. Ahn? did you read what I proposed as an option? I proposed splitting the patch in 4 as opposed to 2 as you did. 0001) WARNING, whitespace error below diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h index 0a8a27c..d254562 100644 --- a/gdbus/gdbus.h +++ b/gdbus/gdbus.h @@ -217,7 +217,12 @@ guint g_dbus_add_signal_watch(DBusConnection *connection, const char *interface, const char *member, GDBusSignalFunction function, void *user_data, GDBusDestroyFunction destroy); -gboolean g_dbus_remove_watch(DBusConnection *connection, guint tag); +#ifdef _GDBUS_WITH_NEW_REMOVE_WATCH +gboolean g_dbus_remove_watch(guint tag); +#else +gboolean g_dbus_remove_watch(DBusConnection *connection, guint tag); +#endif + void g_dbus_remove_all_watches(DBusConnection *connection); #ifdef __cplusplus 0002) Convert everybody adding the definition on Makefile.am 0003) Remove the define from Makefile.am 0004) Remove the old code from gdbus/ Lucas De Marchi