linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH BlueZ 1/5] gdbus/watch: Fix crash when g_dbus_remove_watch is called from connect callback
@ 2013-09-04 19:04 Luiz Augusto von Dentz
  2013-09-04 19:04 ` [PATCH BlueZ 2/5] gdbus/watch: Fix aborting when removing D-Bus filter Luiz Augusto von Dentz
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2013-09-04 19:04 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

 at 0x40570C: update_service (watch.c:601)
 by 0x40584B: service_reply (watch.c:627)
 by 0x3B0700C511: ??? (in /usr/lib64/libdbus-1.so.3.7.4)
 by 0x3B0700F740: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.7.4)
 by 0x405167: message_dispatch (mainloop.c:76)
 by 0x3B03C48962: ??? (in /usr/lib64/libglib-2.0.so.0.3600.3)
 by 0x3B03C47E05: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3600.3)
 by 0x3B03C48157: ??? (in /usr/lib64/libglib-2.0.so.0.3600.3)
 by 0x3B03C48559: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3600.3)
Address 0x4c58a30 is 32 bytes inside a block of size 56 free'd
 at 0x4A074C4: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
 by 0x3B03C4D9AE: g_free (in /usr/lib64/libglib-2.0.so.0.3600.3)
 by 0x406102: filter_data_remove_callback (watch.c:378)
 by 0x405FC0: g_dbus_remove_watch (watch.c:798)
 by 0x40A22B: g_dbus_client_unref (client.c:1227)
 by 0x40570B: update_service (watch.c:599)
 by 0x40584B: service_reply (watch.c:627)
---
 gdbus/watch.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/gdbus/watch.c b/gdbus/watch.c
index 9e4f994..d334580 100644
--- a/gdbus/watch.c
+++ b/gdbus/watch.c
@@ -593,12 +593,16 @@ static gboolean update_service(void *user_data)
 {
 	struct service_data *data = user_data;
 	struct filter_callback *cb = data->callback;
+	DBusConnection *conn;
 
 	update_name_cache(data->name, data->owner);
+	conn = dbus_connection_ref(data->conn);
+	service_data_free(cb->data);
+
 	if (cb->conn_func)
-		cb->conn_func(data->conn, cb->user_data);
+		cb->conn_func(conn, cb->user_data);
 
-	service_data_free(data);
+	dbus_connection_unref(conn);
 
 	return FALSE;
 }
-- 
1.8.3.1


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

* [PATCH BlueZ 2/5] gdbus/watch: Fix aborting when removing D-Bus filter
  2013-09-04 19:04 [PATCH BlueZ 1/5] gdbus/watch: Fix crash when g_dbus_remove_watch is called from connect callback Luiz Augusto von Dentz
@ 2013-09-04 19:04 ` Luiz Augusto von Dentz
  2013-09-04 19:04 ` [PATCH BlueZ 3/5] gdbus/client: Use g_dbus_add_service_watch to track services Luiz Augusto von Dentz
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2013-09-04 19:04 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

In case of filter_data having a watch to a service name it may call
dbus_connection_remove_filter twice causing libdbus to abort:

process 24723: Attempt to remove filter function 0x4063e0 user data (nil), but no such filter has been added

To fix this the code will now only attempt to call
dbus_connection_remove_filter once in filter_data_free which is the
counterpart of filter_data_get where dbus_connection_add_filter is called.
---
 gdbus/watch.c | 25 ++++++++-----------------
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/gdbus/watch.c b/gdbus/watch.c
index d334580..a918535 100644
--- a/gdbus/watch.c
+++ b/gdbus/watch.c
@@ -281,6 +281,11 @@ static void filter_data_free(struct filter_data *data)
 {
 	GSList *l;
 
+	/* Remove filter if there are no listeners left for the connection */
+	if (filter_data_find(data->connection) == NULL)
+		dbus_connection_remove_filter(data->connection, message_filter,
+									NULL);
+
 	for (l = data->callbacks; l != NULL; l = l->next)
 		g_free(l->data);
 
@@ -360,8 +365,6 @@ static void service_data_free(struct service_data *data)
 static gboolean filter_data_remove_callback(struct filter_data *data,
 						struct filter_callback *cb)
 {
-	DBusConnection *connection;
-
 	data->callbacks = g_slist_remove(data->callbacks, cb);
 	data->processed = g_slist_remove(data->processed, cb);
 
@@ -385,16 +388,8 @@ static gboolean filter_data_remove_callback(struct filter_data *data,
 	if (data->registered && !remove_match(data))
 		return FALSE;
 
-	connection = dbus_connection_ref(data->connection);
 	listeners = g_slist_remove(listeners, data);
-
-	/* Remove filter if there are no listeners left for the connection */
-	if (filter_data_find(connection) == NULL)
-		dbus_connection_remove_filter(connection, message_filter,
-						NULL);
-
 	filter_data_free(data);
-	dbus_connection_unref(connection);
 
 	return TRUE;
 }
@@ -563,6 +558,9 @@ static DBusHandlerResult message_filter(DBusConnection *connection,
 								current);
 	}
 
+	if (delete_listener == NULL)
+		return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+
 	for (current = delete_listener; current != NULL;
 					current = delete_listener->next) {
 		GSList *l = current->data;
@@ -581,11 +579,6 @@ static DBusHandlerResult message_filter(DBusConnection *connection,
 
 	g_slist_free(delete_listener);
 
-	/* Remove filter if there are no listeners left for the connection */
-	if (filter_data_find(connection) == NULL)
-		dbus_connection_remove_filter(connection, message_filter,
-						NULL);
-
 	return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 }
 
@@ -814,6 +807,4 @@ void g_dbus_remove_all_watches(DBusConnection *connection)
 		listeners = g_slist_remove(listeners, data);
 		filter_data_call_and_free(data);
 	}
-
-	dbus_connection_remove_filter(connection, message_filter, NULL);
 }
-- 
1.8.3.1


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

* [PATCH BlueZ 3/5] gdbus/client: Use g_dbus_add_service_watch to track services
  2013-09-04 19:04 [PATCH BlueZ 1/5] gdbus/watch: Fix crash when g_dbus_remove_watch is called from connect callback Luiz Augusto von Dentz
  2013-09-04 19:04 ` [PATCH BlueZ 2/5] gdbus/watch: Fix aborting when removing D-Bus filter Luiz Augusto von Dentz
@ 2013-09-04 19:04 ` Luiz Augusto von Dentz
  2013-09-04 19:04 ` [PATCH BlueZ 4/5] gdbus/client: Use g_dbus_add_signal_watch to track signals Luiz Augusto von Dentz
  2013-09-04 19:04 ` [PATCH BlueZ 5/5] gdbus/client: Use g_dbus_add_properties_watch to track properties Luiz Augusto von Dentz
  3 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2013-09-04 19:04 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This make the handling much simpler and avoids duplicates of the same
match rule.
---
 gdbus/client.c | 173 +++++++++++++--------------------------------------------
 1 file changed, 38 insertions(+), 135 deletions(-)

diff --git a/gdbus/client.c b/gdbus/client.c
index 8ebfaad..7a14d7e 100644
--- a/gdbus/client.c
+++ b/gdbus/client.c
@@ -36,8 +36,8 @@ struct GDBusClient {
 	int ref_count;
 	DBusConnection *dbus_conn;
 	char *service_name;
-	char *unique_name;
 	char *base_path;
+	guint watch;
 	GPtrArray *match_rules;
 	DBusPendingCall *pending_call;
 	DBusPendingCall *get_objects_call;
@@ -1051,74 +1051,36 @@ static void get_managed_objects(GDBusClient *client)
 	dbus_message_unref(msg);
 }
 
-static void get_name_owner_reply(DBusPendingCall *call, void *user_data)
+static void service_connect(DBusConnection *conn, void *user_data)
 {
 	GDBusClient *client = user_data;
-	DBusMessage *reply = dbus_pending_call_steal_reply(call);
-	DBusError error;
-	const char *name;
 
 	g_dbus_client_ref(client);
 
-	dbus_error_init(&error);
-
-	if (dbus_set_error_from_message(&error, reply) == TRUE) {
-		dbus_error_free(&error);
-		goto done;
-	}
-
-	if (dbus_message_get_args(reply, NULL, DBUS_TYPE_STRING, &name,
-						DBUS_TYPE_INVALID) == FALSE)
-		goto done;
-
-	if (client->unique_name == NULL) {
-		client->unique_name = g_strdup(name);
-
-		if (client->connect_func)
-			client->connect_func(client->dbus_conn,
-							client->connect_data);
-
-		get_managed_objects(client);
-	}
-
-done:
-	dbus_message_unref(reply);
+	if (client->connect_func)
+		client->connect_func(conn, client->connect_data);
 
-	dbus_pending_call_unref(client->pending_call);
-	client->pending_call = NULL;
+	get_managed_objects(client);
 
 	g_dbus_client_unref(client);
 }
 
-static void get_name_owner(GDBusClient *client, const char *name)
+static void service_disconnect(DBusConnection *conn, void *user_data)
 {
-	DBusMessage *msg;
-
-	msg = dbus_message_new_method_call(DBUS_SERVICE_DBUS, DBUS_PATH_DBUS,
-					DBUS_INTERFACE_DBUS, "GetNameOwner");
-	if (msg == NULL)
-		return;
-
-	dbus_message_append_args(msg, DBUS_TYPE_STRING, &name,
-						DBUS_TYPE_INVALID);
-
-	if (g_dbus_send_message_with_reply(client->dbus_conn, msg,
-					&client->pending_call, -1) == FALSE) {
-		dbus_message_unref(msg);
-		return;
-	}
+	GDBusClient *client = user_data;
 
-	dbus_pending_call_set_notify(client->pending_call,
-					get_name_owner_reply, client, NULL);
+	g_list_free_full(client->proxy_list, proxy_free);
+	client->proxy_list = NULL;
 
-	dbus_message_unref(msg);
+	if (client->disconn_func)
+		client->disconn_func(conn, client->disconn_data);
 }
 
 static DBusHandlerResult message_filter(DBusConnection *connection,
 					DBusMessage *message, void *user_data)
 {
 	GDBusClient *client = user_data;
-	const char *sender;
+	const char *sender, *path, *interface, *member;
 
 	if (dbus_message_get_type(message) != DBUS_MESSAGE_TYPE_SIGNAL)
 		return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
@@ -1127,98 +1089,41 @@ static DBusHandlerResult message_filter(DBusConnection *connection,
 	if (sender == NULL)
 		return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 
-	if (g_str_equal(sender, DBUS_SERVICE_DBUS) == TRUE) {
-		const char *interface, *member;
-		const char *name, *old, *new;
+	path = dbus_message_get_path(message);
+	interface = dbus_message_get_interface(message);
+	member = dbus_message_get_member(message);
 
-		interface = dbus_message_get_interface(message);
-
-		if (g_str_equal(interface, DBUS_INTERFACE_DBUS) == FALSE)
-			return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
-
-		member = dbus_message_get_member(message);
-
-		if (g_str_equal(member, "NameOwnerChanged") == FALSE)
+	if (g_str_equal(path, "/") == TRUE) {
+		if (g_str_equal(interface, DBUS_INTERFACE_DBUS
+						".ObjectManager") == FALSE)
 			return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 
-		if (dbus_message_get_args(message, NULL,
-						DBUS_TYPE_STRING, &name,
-						DBUS_TYPE_STRING, &old,
-						DBUS_TYPE_STRING, &new,
-						DBUS_TYPE_INVALID) == FALSE)
+		if (g_str_equal(member, "InterfacesAdded") == TRUE) {
+			interfaces_added(client, message);
 			return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+		}
 
-		if (g_str_equal(name, client->service_name) == FALSE)
+		if (g_str_equal(member, "InterfacesRemoved") == TRUE) {
+			interfaces_removed(client, message);
 			return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
-
-		if (*new == '\0' && client->unique_name != NULL &&
-				g_str_equal(old, client->unique_name) == TRUE) {
-
-			g_list_free_full(client->proxy_list, proxy_free);
-			client->proxy_list = NULL;
-
-			if (client->disconn_func)
-				client->disconn_func(client->dbus_conn,
-							client->disconn_data);
-
-			g_free(client->unique_name);
-			client->unique_name = NULL;
-		} else if (*old == '\0' && client->unique_name == NULL) {
-			client->unique_name = g_strdup(new);
-
-			if (client->connect_func)
-				client->connect_func(client->dbus_conn,
-							client->connect_data);
-
-			get_managed_objects(client);
 		}
 
 		return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 	}
 
-	if (client->unique_name == NULL)
+	if (g_str_has_prefix(path, client->base_path) == FALSE)
 		return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 
-	if (g_str_equal(sender, client->unique_name) == TRUE) {
-		const char *path, *interface, *member;
-
-		path = dbus_message_get_path(message);
-		interface = dbus_message_get_interface(message);
-		member = dbus_message_get_member(message);
-
-		if (g_str_equal(path, "/") == TRUE) {
-			if (g_str_equal(interface, DBUS_INTERFACE_DBUS
-						".ObjectManager") == FALSE)
-				return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
-
-			if (g_str_equal(member, "InterfacesAdded") == TRUE) {
-				interfaces_added(client, message);
-				return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
-			}
-
-			if (g_str_equal(member, "InterfacesRemoved") == TRUE) {
-				interfaces_removed(client, message);
-				return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
-			}
+	if (g_str_equal(interface, DBUS_INTERFACE_PROPERTIES) == TRUE) {
+		if (g_str_equal(member, "PropertiesChanged") == TRUE)
+			properties_changed(client, path, message);
 
-			return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
-		}
-
-		if (g_str_has_prefix(path, client->base_path) == FALSE)
-			return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
-
-		if (g_str_equal(interface, DBUS_INTERFACE_PROPERTIES) == TRUE) {
-			if (g_str_equal(member, "PropertiesChanged") == TRUE)
-				properties_changed(client, path, message);
-
-			return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
-		}
-
-		if (client->signal_func)
-			client->signal_func(client->dbus_conn,
-					message, client->signal_data);
+		return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 	}
 
+	if (client->signal_func)
+		client->signal_func(connection, message, client->signal_data);
+
 	return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 }
 
@@ -1245,16 +1150,13 @@ GDBusClient *g_dbus_client_new(DBusConnection *connection,
 	client->service_name = g_strdup(service);
 	client->base_path = g_strdup(path);
 
-	get_name_owner(client, client->service_name);
-
-	client->match_rules = g_ptr_array_sized_new(4);
+	client->match_rules = g_ptr_array_sized_new(3);
 	g_ptr_array_set_free_func(client->match_rules, g_free);
 
-	g_ptr_array_add(client->match_rules, g_strdup_printf("type='signal',"
-				"sender='%s',path='%s',interface='%s',"
-				"member='NameOwnerChanged',arg0='%s'",
-				DBUS_SERVICE_DBUS, DBUS_PATH_DBUS,
-				DBUS_INTERFACE_DBUS, client->service_name));
+	client->watch = g_dbus_add_service_watch(connection, service,
+						service_connect,
+						service_disconnect,
+						client, NULL);
 	g_ptr_array_add(client->match_rules, g_strdup_printf("type='signal',"
 				"sender='%s',"
 				"path='/',interface='%s.ObjectManager',"
@@ -1322,10 +1224,11 @@ void g_dbus_client_unref(GDBusClient *client)
 	if (client->disconn_func)
 		client->disconn_func(client->dbus_conn, client->disconn_data);
 
+	g_dbus_remove_watch(client->dbus_conn, client->watch);
+
 	dbus_connection_unref(client->dbus_conn);
 
 	g_free(client->service_name);
-	g_free(client->unique_name);
 	g_free(client->base_path);
 
 	g_free(client);
-- 
1.8.3.1


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

* [PATCH BlueZ 4/5] gdbus/client: Use g_dbus_add_signal_watch to track signals
  2013-09-04 19:04 [PATCH BlueZ 1/5] gdbus/watch: Fix crash when g_dbus_remove_watch is called from connect callback Luiz Augusto von Dentz
  2013-09-04 19:04 ` [PATCH BlueZ 2/5] gdbus/watch: Fix aborting when removing D-Bus filter Luiz Augusto von Dentz
  2013-09-04 19:04 ` [PATCH BlueZ 3/5] gdbus/client: Use g_dbus_add_service_watch to track services Luiz Augusto von Dentz
@ 2013-09-04 19:04 ` Luiz Augusto von Dentz
  2013-09-04 19:04 ` [PATCH BlueZ 5/5] gdbus/client: Use g_dbus_add_properties_watch to track properties Luiz Augusto von Dentz
  3 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2013-09-04 19:04 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This make the handling much simpler and avoids duplicates of the same
match rule.
---
 gdbus/client.c | 73 +++++++++++++++++++++++++++++-----------------------------
 1 file changed, 37 insertions(+), 36 deletions(-)

diff --git a/gdbus/client.c b/gdbus/client.c
index 7a14d7e..f1e6946 100644
--- a/gdbus/client.c
+++ b/gdbus/client.c
@@ -25,6 +25,7 @@
 #include <config.h>
 #endif
 
+#include <stdio.h>
 #include <glib.h>
 #include <dbus/dbus.h>
 
@@ -32,12 +33,18 @@
 
 #define METHOD_CALL_TIMEOUT (300 * 1000)
 
+#ifndef DBUS_INTERFACE_OBJECT
+#define DBUS_INTERFACE_OBJECT_MANAGER DBUS_INTERFACE_DBUS ".ObjectManager"
+#endif
+
 struct GDBusClient {
 	int ref_count;
 	DBusConnection *dbus_conn;
 	char *service_name;
 	char *base_path;
 	guint watch;
+	guint added_watch;
+	guint removed_watch;
 	GPtrArray *match_rules;
 	DBusPendingCall *pending_call;
 	DBusPendingCall *get_objects_call;
@@ -908,16 +915,18 @@ static void parse_interfaces(GDBusClient *client, const char *path,
 	}
 }
 
-static void interfaces_added(GDBusClient *client, DBusMessage *msg)
+static gboolean interfaces_added(DBusConnection *conn, DBusMessage *msg,
+							void *user_data)
 {
+	GDBusClient *client = user_data;
 	DBusMessageIter iter;
 	const char *path;
 
 	if (dbus_message_iter_init(msg, &iter) == FALSE)
-		return;
+		return TRUE;
 
 	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_OBJECT_PATH)
-		return;
+		return TRUE;
 
 	dbus_message_iter_get_basic(&iter, &path);
 	dbus_message_iter_next(&iter);
@@ -927,24 +936,28 @@ static void interfaces_added(GDBusClient *client, DBusMessage *msg)
 	parse_interfaces(client, path, &iter);
 
 	g_dbus_client_unref(client);
+
+	return TRUE;
 }
 
-static void interfaces_removed(GDBusClient *client, DBusMessage *msg)
+static gboolean interfaces_removed(DBusConnection *conn, DBusMessage *msg,
+							void *user_data)
 {
+	GDBusClient *client = user_data;
 	DBusMessageIter iter, entry;
 	const char *path;
 
 	if (dbus_message_iter_init(msg, &iter) == FALSE)
-		return;
+		return TRUE;
 
 	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_OBJECT_PATH)
-		return;
+		return TRUE;
 
 	dbus_message_iter_get_basic(&iter, &path);
 	dbus_message_iter_next(&iter);
 
 	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_ARRAY)
-		return;
+		return TRUE;
 
 	dbus_message_iter_recurse(&iter, &entry);
 
@@ -959,6 +972,8 @@ static void interfaces_removed(GDBusClient *client, DBusMessage *msg)
 	}
 
 	g_dbus_client_unref(client);
+
+	return TRUE;
 }
 
 static void parse_managed_objects(GDBusClient *client, DBusMessage *msg)
@@ -1093,24 +1108,6 @@ static DBusHandlerResult message_filter(DBusConnection *connection,
 	interface = dbus_message_get_interface(message);
 	member = dbus_message_get_member(message);
 
-	if (g_str_equal(path, "/") == TRUE) {
-		if (g_str_equal(interface, DBUS_INTERFACE_DBUS
-						".ObjectManager") == FALSE)
-			return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
-
-		if (g_str_equal(member, "InterfacesAdded") == TRUE) {
-			interfaces_added(client, message);
-			return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
-		}
-
-		if (g_str_equal(member, "InterfacesRemoved") == TRUE) {
-			interfaces_removed(client, message);
-			return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
-		}
-
-		return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
-	}
-
 	if (g_str_has_prefix(path, client->base_path) == FALSE)
 		return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 
@@ -1150,23 +1147,25 @@ GDBusClient *g_dbus_client_new(DBusConnection *connection,
 	client->service_name = g_strdup(service);
 	client->base_path = g_strdup(path);
 
-	client->match_rules = g_ptr_array_sized_new(3);
+	client->match_rules = g_ptr_array_sized_new(1);
 	g_ptr_array_set_free_func(client->match_rules, g_free);
 
 	client->watch = g_dbus_add_service_watch(connection, service,
 						service_connect,
 						service_disconnect,
 						client, NULL);
-	g_ptr_array_add(client->match_rules, g_strdup_printf("type='signal',"
-				"sender='%s',"
-				"path='/',interface='%s.ObjectManager',"
-				"member='InterfacesAdded'",
-				client->service_name, DBUS_INTERFACE_DBUS));
-	g_ptr_array_add(client->match_rules, g_strdup_printf("type='signal',"
-				"sender='%s',"
-				"path='/',interface='%s.ObjectManager',"
-				"member='InterfacesRemoved'",
-				client->service_name, DBUS_INTERFACE_DBUS));
+	client->added_watch = g_dbus_add_signal_watch(connection, service,
+						"/",
+						DBUS_INTERFACE_OBJECT_MANAGER,
+						"InterfacesAdded",
+						interfaces_added,
+						client, NULL);
+	client->removed_watch = g_dbus_add_signal_watch(connection, service,
+						"/",
+						DBUS_INTERFACE_OBJECT_MANAGER,
+						"InterfacesRemoved",
+						interfaces_removed,
+						client, NULL);
 	g_ptr_array_add(client->match_rules, g_strdup_printf("type='signal',"
 				"sender='%s',path_namespace='%s'",
 				client->service_name, client->base_path));
@@ -1225,6 +1224,8 @@ void g_dbus_client_unref(GDBusClient *client)
 		client->disconn_func(client->dbus_conn, client->disconn_data);
 
 	g_dbus_remove_watch(client->dbus_conn, client->watch);
+	g_dbus_remove_watch(client->dbus_conn, client->added_watch);
+	g_dbus_remove_watch(client->dbus_conn, client->removed_watch);
 
 	dbus_connection_unref(client->dbus_conn);
 
-- 
1.8.3.1


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

* [PATCH BlueZ 5/5] gdbus/client: Use g_dbus_add_properties_watch to track properties
  2013-09-04 19:04 [PATCH BlueZ 1/5] gdbus/watch: Fix crash when g_dbus_remove_watch is called from connect callback Luiz Augusto von Dentz
                   ` (2 preceding siblings ...)
  2013-09-04 19:04 ` [PATCH BlueZ 4/5] gdbus/client: Use g_dbus_add_signal_watch to track signals Luiz Augusto von Dentz
@ 2013-09-04 19:04 ` Luiz Augusto von Dentz
  3 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2013-09-04 19:04 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This make the handling much simpler and avoids duplicates of the same
match rule.
---
 gdbus/client.c | 135 ++++++++++++++++++++++++---------------------------------
 1 file changed, 56 insertions(+), 79 deletions(-)

diff --git a/gdbus/client.c b/gdbus/client.c
index f1e6946..bd0eadc 100644
--- a/gdbus/client.c
+++ b/gdbus/client.c
@@ -67,7 +67,7 @@ struct GDBusProxy {
 	char *obj_path;
 	char *interface;
 	GHashTable *prop_list;
-	char *match_rule;
+	guint watch;
 	GDBusPropertyFunction prop_func;
 	void *prop_data;
 	GDBusProxyFunction removed_func;
@@ -358,6 +358,52 @@ static GDBusProxy *proxy_lookup(GDBusClient *client, const char *path,
 	return NULL;
 }
 
+static gboolean properties_changed(DBusConnection *conn, DBusMessage *msg,
+							void *user_data)
+{
+	GDBusProxy *proxy = user_data;
+	GDBusClient *client = proxy->client;
+	DBusMessageIter iter, entry;
+	const char *interface;
+
+	if (dbus_message_iter_init(msg, &iter) == FALSE)
+		return TRUE;
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+		return TRUE;
+
+	dbus_message_iter_get_basic(&iter, &interface);
+	dbus_message_iter_next(&iter);
+
+	update_properties(proxy, &iter, TRUE);
+
+	dbus_message_iter_next(&iter);
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_ARRAY)
+		return TRUE;
+
+	dbus_message_iter_recurse(&iter, &entry);
+
+	while (dbus_message_iter_get_arg_type(&entry) == DBUS_TYPE_STRING) {
+		const char *name;
+
+		dbus_message_iter_get_basic(&entry, &name);
+
+		g_hash_table_remove(proxy->prop_list, name);
+
+		if (proxy->prop_func)
+			proxy->prop_func(proxy, name, NULL, proxy->prop_data);
+
+		if (client->property_changed)
+			client->property_changed(proxy, name, NULL,
+							client->user_data);
+
+		dbus_message_iter_next(&entry);
+	}
+
+	return TRUE;
+}
+
 static GDBusProxy *proxy_new(GDBusClient *client, const char *path,
 						const char *interface)
 {
@@ -373,14 +419,12 @@ static GDBusProxy *proxy_new(GDBusClient *client, const char *path,
 
 	proxy->prop_list = g_hash_table_new_full(g_str_hash, g_str_equal,
 							NULL, prop_entry_free);
-
-	proxy->match_rule = g_strdup_printf("type='signal',"
-				"sender='%s',path='%s',interface='%s',"
-				"member='PropertiesChanged',arg0='%s'",
-				client->service_name, proxy->obj_path,
-				DBUS_INTERFACE_PROPERTIES, proxy->interface);
-
-	modify_match(client->dbus_conn, "AddMatch", proxy->match_rule);
+	proxy->watch = g_dbus_add_properties_watch(client->dbus_conn,
+							client->service_name,
+							proxy->obj_path,
+							proxy->interface,
+							properties_changed,
+							proxy, NULL);
 
 	return g_dbus_proxy_ref(proxy);
 }
@@ -395,11 +439,7 @@ static void proxy_free(gpointer data)
 		if (client->proxy_removed)
 			client->proxy_removed(proxy, client->user_data);
 
-		modify_match(client->dbus_conn, "RemoveMatch",
-							proxy->match_rule);
-
-		g_free(proxy->match_rule);
-		proxy->match_rule = NULL;
+		g_dbus_remove_watch(client->dbus_conn, proxy->watch);
 
 		g_hash_table_remove_all(proxy->prop_list);
 
@@ -800,64 +840,6 @@ static void refresh_properties(GDBusClient *client)
         }
 }
 
-static void properties_changed(GDBusClient *client, const char *path,
-							DBusMessage *msg)
-{
-	GDBusProxy *proxy = NULL;
-	DBusMessageIter iter, entry;
-	const char *interface;
-	GList *list;
-
-	if (dbus_message_iter_init(msg, &iter) == FALSE)
-		return;
-
-	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
-		return;
-
-	dbus_message_iter_get_basic(&iter, &interface);
-	dbus_message_iter_next(&iter);
-
-	for (list = g_list_first(client->proxy_list); list;
-						list = g_list_next(list)) {
-		GDBusProxy *data = list->data;
-
-		if (g_str_equal(data->interface, interface) == TRUE &&
-				g_str_equal(data->obj_path, path) == TRUE) {
-			proxy = data;
-			break;
-		}
-	}
-
-	if (proxy == NULL)
-		return;
-
-	update_properties(proxy, &iter, TRUE);
-
-	dbus_message_iter_next(&iter);
-
-	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_ARRAY)
-		return;
-
-	dbus_message_iter_recurse(&iter, &entry);
-
-	while (dbus_message_iter_get_arg_type(&entry) == DBUS_TYPE_STRING) {
-		const char *name;
-
-		dbus_message_iter_get_basic(&entry, &name);
-
-		g_hash_table_remove(proxy->prop_list, name);
-
-		if (proxy->prop_func)
-			proxy->prop_func(proxy, name, NULL, proxy->prop_data);
-
-		if (client->property_changed)
-			client->property_changed(proxy, name, NULL,
-							client->user_data);
-
-		dbus_message_iter_next(&entry);
-	}
-}
-
 static void parse_properties(GDBusClient *client, const char *path,
 				const char *interface, DBusMessageIter *iter)
 {
@@ -1095,7 +1077,7 @@ static DBusHandlerResult message_filter(DBusConnection *connection,
 					DBusMessage *message, void *user_data)
 {
 	GDBusClient *client = user_data;
-	const char *sender, *path, *interface, *member;
+	const char *sender, *path, *interface;
 
 	if (dbus_message_get_type(message) != DBUS_MESSAGE_TYPE_SIGNAL)
 		return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
@@ -1106,17 +1088,12 @@ static DBusHandlerResult message_filter(DBusConnection *connection,
 
 	path = dbus_message_get_path(message);
 	interface = dbus_message_get_interface(message);
-	member = dbus_message_get_member(message);
 
 	if (g_str_has_prefix(path, client->base_path) == FALSE)
 		return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 
-	if (g_str_equal(interface, DBUS_INTERFACE_PROPERTIES) == TRUE) {
-		if (g_str_equal(member, "PropertiesChanged") == TRUE)
-			properties_changed(client, path, message);
-
+	if (g_str_equal(interface, DBUS_INTERFACE_PROPERTIES) == TRUE)
 		return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
-	}
 
 	if (client->signal_func)
 		client->signal_func(connection, message, client->signal_data);
-- 
1.8.3.1


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

end of thread, other threads:[~2013-09-04 19:04 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-04 19:04 [PATCH BlueZ 1/5] gdbus/watch: Fix crash when g_dbus_remove_watch is called from connect callback Luiz Augusto von Dentz
2013-09-04 19:04 ` [PATCH BlueZ 2/5] gdbus/watch: Fix aborting when removing D-Bus filter Luiz Augusto von Dentz
2013-09-04 19:04 ` [PATCH BlueZ 3/5] gdbus/client: Use g_dbus_add_service_watch to track services Luiz Augusto von Dentz
2013-09-04 19:04 ` [PATCH BlueZ 4/5] gdbus/client: Use g_dbus_add_signal_watch to track signals Luiz Augusto von Dentz
2013-09-04 19:04 ` [PATCH BlueZ 5/5] gdbus/client: Use g_dbus_add_properties_watch to track properties Luiz Augusto von Dentz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).