Re: [PATCH] Fix signal watch when a service name is given

[Luiz Augusto von Dentz <luiz.dentz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 270 bytes --]

Hi,

On Tue, Feb 16, 2010 at 4:59 PM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi,
>
> This should fix g_dbus_add_signal_watch when a service like org.bluez is given.


Updating since the last one was broken.


-- 
Luiz Augusto von Dentz
Computer Engineer

[-- Attachment #2: 0002-Fix-signal-watch-when-a-service-name-is-given.patch --]
[-- Type: text/x-patch, Size: 9334 bytes --]

From 00d9fe7a749130d51f7d70cb0469005ef7445441 Mon Sep 17 00:00:00 2001
From: Luiz Augusto Von Dentz <luiz.dentz-von@nokia.com>
Date: Tue, 16 Feb 2010 17:11:27 +0200
Subject: [PATCH 2/2] Fix signal watch when a service name is given

The bus name should be resolved when adding a watch by service name since
messages do always come with sender set to owner's bus name, also it
should listen to owner updates since it can change without invalidating
the watch.
---
 gdbus/watch.c |  139 ++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 117 insertions(+), 22 deletions(-)

diff --git a/gdbus/watch.c b/gdbus/watch.c
index 1d479fa..864b4a3 100644
--- a/gdbus/watch.c
+++ b/gdbus/watch.c
@@ -55,19 +55,22 @@ struct filter_callback {
 struct filter_data {
 	DBusConnection *connection;
 	DBusHandleMessageFunction handle_func;
-	char *sender;
+	char *name;
+	char *owner;
 	char *path;
 	char *interface;
 	char *member;
 	char *argument;
 	GSList *callbacks;
 	GSList *processed;
+	guint name_watch;
 	gboolean lock;
 	gboolean registered;
 };
 
 static struct filter_data *filter_data_find(DBusConnection *connection,
-							const char *sender,
+							const char *name,
+							const char *owner,
 							const char *path,
 							const char *interface,
 							const char *member,
@@ -82,8 +85,12 @@ static struct filter_data *filter_data_find(DBusConnection *connection,
 		if (connection != data->connection)
 			continue;
 
-		if (sender && data->sender &&
-				g_str_equal(sender, data->sender) == FALSE)
+		if (name && data->name &&
+				g_str_equal(name, data->name) == FALSE)
+			continue;
+
+		if (owner && data->owner &&
+				g_str_equal(owner, data->owner) == FALSE)
 			continue;
 
 		if (path && data->path &&
@@ -110,13 +117,15 @@ static struct filter_data *filter_data_find(DBusConnection *connection,
 
 static void format_rule(struct filter_data *data, char *rule, size_t size)
 {
+	const char *sender;
 	int offset;
 
 	offset = snprintf(rule, size, "type='signal'");
+	sender = data->name ? : data->owner;
 
-	if (data->sender)
+	if (sender)
 		offset += snprintf(rule + offset, size - offset,
-				",sender='%s'", data->sender);
+				",sender='%s'", sender);
 	if (data->path)
 		offset += snprintf(rule + offset, size - offset,
 				",path='%s'", data->path);
@@ -183,8 +192,10 @@ static struct filter_data *filter_data_get(DBusConnection *connection,
 					const char *argument)
 {
 	struct filter_data *data;
+	const char *name = NULL, *owner = NULL;
 
-	if (!filter_data_find(connection, NULL, NULL, NULL, NULL, NULL)) {
+	if (!filter_data_find(connection, NULL, NULL, NULL, NULL, NULL,
+				NULL)) {
 		if (!dbus_connection_add_filter(connection,
 					message_filter, NULL, NULL)) {
 			error("dbus_connection_add_filter() failed");
@@ -192,15 +203,25 @@ static struct filter_data *filter_data_get(DBusConnection *connection,
 		}
 	}
 
-	data = filter_data_find(connection, sender, path, interface, member,
-					argument);
+	if (sender == NULL)
+		goto proceed;
+
+	if (sender[0] == ':')
+		owner = sender;
+	else
+		name = sender;
+
+proceed:
+	data = filter_data_find(connection, name, owner, path, interface,
+					member, argument);
 	if (data)
 		return data;
 
 	data = g_new0(struct filter_data, 1);
 
 	data->connection = dbus_connection_ref(connection);
-	data->sender = g_strdup(sender);
+	data->name = name ? g_strdup(name) : NULL;
+	data->owner = owner ? g_strdup(owner) : NULL;
 	data->path = g_strdup(path);
 	data->interface = g_strdup(interface);
 	data->member = g_strdup(member);
@@ -244,7 +265,9 @@ static void filter_data_free(struct filter_data *data)
 		g_free(l->data);
 
 	g_slist_free(data->callbacks);
-	g_free(data->sender);
+	g_dbus_remove_watch(data->connection, data->name_watch);
+	g_free(data->name);
+	g_free(data->owner);
 	g_free(data->path);
 	g_free(data->interface);
 	g_free(data->member);
@@ -322,7 +345,8 @@ static gboolean filter_data_remove_callback(struct filter_data *data,
 	filter_data_free(data);
 
 	/* Remove filter if there are no listeners left for the connection */
-	data = filter_data_find(connection, NULL, NULL, NULL, NULL, NULL);
+	data = filter_data_find(connection, NULL, NULL, NULL, NULL, NULL,
+					NULL);
 	if (!data)
 		dbus_connection_remove_filter(connection, message_filter,
 						NULL);
@@ -359,6 +383,37 @@ static DBusHandlerResult signal_filter(DBusConnection *connection,
 	return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 }
 
+static void update_name_cache(const char *name, const char *owner)
+{
+	GSList *l;
+
+	for (l = listeners; l != NULL; l = l->next) {
+		struct filter_data *data = l->data;
+
+		if (g_strcmp0(data->name, name) != 0)
+			continue;
+
+		g_free(data->owner);
+		data->owner = g_strdup(owner);
+	}
+}
+
+static const char *check_name_cache(const char *name)
+{
+	GSList *l;
+
+	for (l = listeners; l != NULL; l = l->next) {
+		struct filter_data *data = l->data;
+
+		if (g_strcmp0(data->name, name) != 0)
+			continue;
+
+		return data->owner;
+	}
+
+	return NULL;
+}
+
 static DBusHandlerResult service_filter(DBusConnection *connection,
 					DBusMessage *message, void *user_data)
 {
@@ -375,6 +430,8 @@ static DBusHandlerResult service_filter(DBusConnection *connection,
 		return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 	}
 
+	update_name_cache(name, new);
+
 	while (data->callbacks) {
 		cb = data->callbacks->data;
 
@@ -421,7 +478,9 @@ static DBusHandlerResult message_filter(DBusConnection *connection,
 	member = dbus_message_get_member(message);
 	dbus_message_get_args(message, NULL, DBUS_TYPE_STRING, &arg, DBUS_TYPE_INVALID);
 
-	data = filter_data_find(connection, sender, path, iface, member, arg);
+	/* Sender is always bus name */
+	data = filter_data_find(connection, NULL, sender, path, iface, member,
+					arg);
 	if (!data) {
 		error("Got %s.%s signal which has no listeners", iface, member);
 		return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
@@ -446,7 +505,8 @@ static DBusHandlerResult message_filter(DBusConnection *connection,
 	filter_data_free(data);
 
 	/* Remove filter if there no listener left for the connection */
-	data = filter_data_find(connection, NULL, NULL, NULL, NULL, NULL);
+	data = filter_data_find(connection, NULL, NULL, NULL, NULL, NULL,
+					NULL);
 	if (!data)
 		dbus_connection_remove_filter(connection, message_filter,
 						NULL);
@@ -456,16 +516,37 @@ static DBusHandlerResult message_filter(DBusConnection *connection,
 
 struct service_data {
 	DBusConnection *conn;
+	char *name;
+	const char *owner;
 	GDBusWatchFunction conn_func;
 	void *user_data;
 };
 
+static void service_data_free(struct service_data *data)
+{
+	dbus_connection_unref(data->conn);
+	g_free(data->name);
+	g_free(data);
+}
+
+static gboolean update_service(void *user_data)
+{
+	struct service_data *data = user_data;
+
+	update_name_cache(data->name, data->owner);
+	if (data->conn_func)
+		data->conn_func(data->conn, data->user_data);
+
+	service_data_free(data);
+
+	return FALSE;
+}
+
 static void service_reply(DBusPendingCall *call, void *user_data)
 {
 	struct service_data *data = user_data;
 	DBusMessage *reply;
 	DBusError error;
-	dbus_bool_t has_owner;
 
 	reply = dbus_pending_call_steal_reply(call);
 	if (reply == NULL)
@@ -474,22 +555,23 @@ static void service_reply(DBusPendingCall *call, void *user_data)
 	dbus_error_init(&error);
 
 	if (dbus_message_get_args(reply, &error,
-					DBUS_TYPE_BOOLEAN, &has_owner,
+					DBUS_TYPE_STRING, &data->owner,
 						DBUS_TYPE_INVALID) == FALSE) {
 		if (dbus_error_is_set(&error) == TRUE) {
 			error("%s", error.message);
 			dbus_error_free(&error);
 		} else {
-			error("Wrong arguments for NameHasOwner reply");
+			error("Wrong arguments for GetNameOwner reply");
 		}
 		goto done;
 	}
 
-	if (has_owner && data->conn_func)
-		data->conn_func(data->conn, data->user_data);
+	update_service(data);
 
+	return;
 done:
 	dbus_message_unref(reply);
+	service_data_free(data);
 }
 
 static void check_service(DBusConnection *connection, const char *name,
@@ -505,12 +587,19 @@ static void check_service(DBusConnection *connection, const char *name,
 		return;
 	}
 
-	data->conn = connection;
+	data->conn = dbus_connection_ref(connection);
+	data->name = g_strdup(name);
 	data->conn_func = connect;
 	data->user_data = user_data;
 
+	data->owner = check_name_cache(name);
+	if (data->owner != NULL) {
+		g_idle_add(update_service, data);
+		return;
+	}
+
 	message = dbus_message_new_method_call(DBUS_SERVICE_DBUS,
-			DBUS_PATH_DBUS, DBUS_INTERFACE_DBUS, "NameHasOwner");
+			DBUS_PATH_DBUS, DBUS_INTERFACE_DBUS, "GetNameOwner");
 	if (message == NULL) {
 		error("Can't allocate new message");
 		g_free(data);
@@ -596,6 +685,11 @@ guint g_dbus_add_signal_watch(DBusConnection *connection,
 	if (!cb)
 		return 0;
 
+	if (data->name != NULL && data->name_watch == 0)
+		data->name_watch = g_dbus_add_service_watch(connection,
+							data->name, NULL,
+							NULL, NULL, NULL);
+
 	return cb->id;
 }
 
@@ -625,7 +719,8 @@ void g_dbus_remove_all_watches(DBusConnection *connection)
 {
 	struct filter_data *data;
 
-	while ((data = filter_data_find(connection, NULL, NULL, NULL, NULL, NULL))) {
+	while ((data = filter_data_find(connection, NULL, NULL, NULL, NULL,
+					NULL, NULL))) {
 		listeners = g_slist_remove(listeners, data);
 		filter_data_call_and_free(data);
 	}
-- 
1.6.3.3