[PATCH] dbus: aggregate PropertiesChanged signals

Marcel Holtmann marcel at holtmann.org
Mon Jan 4 04:59:47 EST 2010


From: Witold Sowa <witold.sowa at gmail.com>

Instead of sending PropertiesChanged signals for each changed
property separately, mark properties as changed and send aggregated
PropertiesChanged signals for each interface in each object.
Agregated PropertoesChanged signal is sent
- for all object after responding on DBus call
- for specified object after manual call to
calling wpa_dbus_flush_object_changed_properties function
- for each object separately after short timeout (currently 5ms)
  which starts when first property in object is being marked changed
---
 wpa_supplicant/dbus/dbus_new.c              |   52 +++---
 wpa_supplicant/dbus/dbus_new.h              |    4 -
 wpa_supplicant/dbus/dbus_new_handlers_wps.c |   11 +-
 wpa_supplicant/dbus/dbus_new_helpers.c      |  262 ++++++++++++++++++++++-----
 wpa_supplicant/dbus/dbus_new_helpers.h      |   25 ++-
 5 files changed, 260 insertions(+), 94 deletions(-)

diff --git a/wpa_supplicant/dbus/dbus_new.c b/wpa_supplicant/dbus/dbus_new.c
index e2f0248..b13ed9d 100644
--- a/wpa_supplicant/dbus/dbus_new.c
+++ b/wpa_supplicant/dbus/dbus_new.c
@@ -474,17 +474,13 @@ void wpas_dbus_signal_network_enabled_changed(struct wpa_supplicant *wpa_s,
 					      struct wpa_ssid *ssid)
 {
 
-	struct network_handler_args args = { wpa_s, ssid };
 	char path[WPAS_DBUS_OBJECT_PATH_MAX];
 	os_snprintf(path, WPAS_DBUS_OBJECT_PATH_MAX,
 		    "%s/" WPAS_DBUS_NEW_NETWORKS_PART "/%d",
 		    wpa_s->dbus_new_path, ssid->id);
 
-	wpa_dbus_signal_property_changed(wpa_s->global->dbus,
-					 (WPADBusPropertyAccessor)
-					 wpas_dbus_getter_enabled, &args,
-					 path, WPAS_DBUS_NEW_IFACE_NETWORK,
-					 "Enabled");
+	wpa_dbus_mark_property_changed(wpa_s->global->dbus, path,
+				       WPAS_DBUS_NEW_IFACE_NETWORK, "Enabled");
 }
 
 
@@ -763,9 +759,9 @@ void wpas_dbus_signal_prop_changed(struct wpa_supplicant *wpa_s,
 		return;
 	}
 
-	wpa_dbus_signal_property_changed(wpa_s->global->dbus,
-					 getter, wpa_s, wpa_s->dbus_new_path,
-					 WPAS_DBUS_NEW_IFACE_INTERFACE, prop);
+	wpa_dbus_mark_property_changed(wpa_s->global->dbus,
+				       wpa_s->dbus_new_path,
+				       WPAS_DBUS_NEW_IFACE_INTERFACE, prop);
 }
 
 
@@ -777,12 +773,9 @@ void wpas_dbus_signal_prop_changed(struct wpa_supplicant *wpa_s,
  */
 void wpas_dbus_signal_debug_level_changed(struct wpa_global *global)
 {
-	wpa_dbus_signal_property_changed(global->dbus,
-					 (WPADBusPropertyAccessor)
-					 wpas_dbus_getter_debug_level,
-					 global, WPAS_DBUS_NEW_PATH,
-					 WPAS_DBUS_NEW_INTERFACE,
-					 "DebugLevel");
+	wpa_dbus_mark_property_changed(global->dbus, WPAS_DBUS_NEW_PATH,
+				       WPAS_DBUS_NEW_INTERFACE,
+				       "DebugLevel");
 }
 
 
@@ -794,12 +787,9 @@ void wpas_dbus_signal_debug_level_changed(struct wpa_global *global)
  */
 void wpas_dbus_signal_debug_timestamp_changed(struct wpa_global *global)
 {
-	wpa_dbus_signal_property_changed(global->dbus,
-					 (WPADBusPropertyAccessor)
-					 wpas_dbus_getter_debug_timestamp,
-					 global, WPAS_DBUS_NEW_PATH,
-					 WPAS_DBUS_NEW_INTERFACE,
-					 "DebugTimestamp");
+	wpa_dbus_mark_property_changed(global->dbus, WPAS_DBUS_NEW_PATH,
+				       WPAS_DBUS_NEW_INTERFACE,
+				       "DebugTimestamp");
 }
 
 
@@ -811,12 +801,9 @@ void wpas_dbus_signal_debug_timestamp_changed(struct wpa_global *global)
  */
 void wpas_dbus_signal_debug_show_keys_changed(struct wpa_global *global)
 {
-	wpa_dbus_signal_property_changed(global->dbus,
-					 (WPADBusPropertyAccessor)
-					 wpas_dbus_getter_debug_show_keys,
-					 global, WPAS_DBUS_NEW_PATH,
-					 WPAS_DBUS_NEW_INTERFACE,
-					 "DebugShowKeys");
+	wpa_dbus_mark_property_changed(global->dbus, WPAS_DBUS_NEW_PATH,
+				       WPAS_DBUS_NEW_INTERFACE,
+				       "DebugShowKeys");
 }
 
 
@@ -827,11 +814,22 @@ static void wpas_dbus_register(struct wpa_dbus_object_desc *obj_desc,
 			       const struct wpa_dbus_property_desc *properties,
 			       const struct wpa_dbus_signal_desc *signals)
 {
+	int n;
+
 	obj_desc->user_data = priv;
 	obj_desc->user_data_free_func = priv_free;
 	obj_desc->methods = methods;
 	obj_desc->properties = properties;
 	obj_desc->signals = signals;
+
+	for (n = 0; properties && properties->dbus_property ;properties++);
+		n++;
+
+	obj_desc->prop_changed_flags = os_zalloc(n * sizeof(u8));
+	if (!obj_desc->prop_changed_flags)
+		wpa_printf(MSG_DEBUG, "dbus: %s: can't register handlers",
+			   __func__);
+
 }
 
 
diff --git a/wpa_supplicant/dbus/dbus_new.h b/wpa_supplicant/dbus/dbus_new.h
index 896e8f0..c7a56e8 100644
--- a/wpa_supplicant/dbus/dbus_new.h
+++ b/wpa_supplicant/dbus/dbus_new.h
@@ -33,10 +33,6 @@ enum wpas_dbus_prop {
 
 #define WPAS_DBUS_OBJECT_PATH_MAX 150
 
-#define WPAS_DBUS_NEW_SERVICE		"fi.w1.wpa_supplicant1"
-#define WPAS_DBUS_NEW_PATH		"/fi/w1/wpa_supplicant1"
-#define WPAS_DBUS_NEW_INTERFACE		"fi.w1.wpa_supplicant1"
-
 #define WPAS_DBUS_NEW_PATH_INTERFACES	WPAS_DBUS_NEW_PATH "/Interfaces"
 #define WPAS_DBUS_NEW_IFACE_INTERFACE	WPAS_DBUS_NEW_INTERFACE ".Interface"
 #define WPAS_DBUS_NEW_IFACE_WPS WPAS_DBUS_NEW_IFACE_INTERFACE ".WPS"
diff --git a/wpa_supplicant/dbus/dbus_new_handlers_wps.c b/wpa_supplicant/dbus/dbus_new_handlers_wps.c
index 0eeb552..dc44a59 100644
--- a/wpa_supplicant/dbus/dbus_new_handlers_wps.c
+++ b/wpa_supplicant/dbus/dbus_new_handlers_wps.c
@@ -322,13 +322,10 @@ DBusMessage * wpas_dbus_setter_process_credentials(
 	wpa_s->conf->wps_cred_processing = (process_credentials ? 2 : 1);
 
 	if ((wpa_s->conf->wps_cred_processing != 1) != old_pc)
-		wpa_dbus_signal_property_changed(
-			wpa_s->global->dbus,
-			(WPADBusPropertyAccessor)
-			wpas_dbus_getter_process_credentials,
-			wpa_s, wpa_s->dbus_new_path,
-			WPAS_DBUS_NEW_IFACE_WPS,
-			"ProcessCredentials");
+		wpa_dbus_mark_property_changed(wpa_s->global->dbus,
+					       wpa_s->dbus_new_path,
+					       WPAS_DBUS_NEW_IFACE_WPS,
+					       "ProcessCredentials");
 
 	return NULL;
 }
diff --git a/wpa_supplicant/dbus/dbus_new_helpers.c b/wpa_supplicant/dbus/dbus_new_helpers.c
index 03630a5..caa3833 100644
--- a/wpa_supplicant/dbus/dbus_new_helpers.c
+++ b/wpa_supplicant/dbus/dbus_new_helpers.c
@@ -19,6 +19,7 @@
 #include "dbus_common.h"
 #include "dbus_common_i.h"
 #include "dbus_new_helpers.h"
+#include "eloop.h"
 
 
 /**
@@ -435,6 +436,9 @@ static DBusHandlerResult message_handler(DBusConnection *connection,
 			dbus_connection_send(connection, reply, NULL);
 		dbus_message_unref(reply);
 	}
+
+	wpa_dbus_flush_all_changed_properties(connection);
+
 	return DBUS_HANDLER_RESULT_HANDLED;
 }
 
@@ -456,6 +460,9 @@ void free_dbus_object_desc(struct wpa_dbus_object_desc *obj_dsc)
 	if (obj_dsc->user_data_free_func)
 		obj_dsc->user_data_free_func(obj_dsc->user_data);
 
+	os_free(obj_dsc->path);
+	os_free(obj_dsc->prop_changed_flags);
+
 	os_free(obj_dsc);
 }
 
@@ -489,6 +496,7 @@ int wpa_dbus_ctrl_iface_init(struct wpas_dbus_priv *iface,
 	};
 
 	obj_desc->connection = iface->con;
+	obj_desc->path = os_strdup(dbus_path);
 
 	/* Register the message handler for the global dbus interface */
 	if (!dbus_connection_register_object_path(iface->con,
@@ -556,6 +564,7 @@ int wpa_dbus_register_object_per_iface(
 
 	con = ctrl_iface->con;
 	obj_desc->connection = con;
+	obj_desc->path = os_strdup(path);
 
 	/* Register the message handler for the interface functions */
 	if (!dbus_connection_register_object_path(con, path, &vtable,
@@ -569,6 +578,9 @@ int wpa_dbus_register_object_per_iface(
 }
 
 
+static void flush_object_timeout_handler(void *eloop_ctx, void *timeout_ctx);
+
+
 /**
  * wpa_dbus_unregister_object_per_iface - Unregisters DBus object
  * @ctrl_iface: Pointer to dbus private data
@@ -580,7 +592,11 @@ int wpa_dbus_register_object_per_iface(
 int wpa_dbus_unregister_object_per_iface(
 	struct wpas_dbus_priv *ctrl_iface, const char *path)
 {
+
 	DBusConnection *con = ctrl_iface->con;
+
+	eloop_cancel_timeout(flush_object_timeout_handler, con, (void*) path);
+
 	if (!dbus_connection_unregister_object_path(con, path))
 		return -1;
 
@@ -588,77 +604,85 @@ int wpa_dbus_unregister_object_per_iface(
 }
 
 
-/**
- * wpas_dbus_signal_network_added - Send a property changed signal
- * @iface: dbus priv struct
- * @property_getter: propperty getter used to fetch new property value
- * @getter_arg: argument passed to property getter
- * @path: path to object which property has changed
- * @interface_name: signal and property interface
- * @property_name: name of property which has changed
- *
- * Notify listeners about changing value of some property. Signal
- * contains property name and its value fetched using given property
- * getter.
- */
-void wpa_dbus_signal_property_changed(struct wpas_dbus_priv *iface,
-				      WPADBusPropertyAccessor property_getter,
-				      void *getter_arg,
-				      const char *path,
-				      const char *interface_name,
-				      const char *property_name)
+static void put_changed_properties(const struct wpa_dbus_object_desc *obj_dsc,
+				   const char *interface,
+				   DBusMessageIter *dict_iter)
 {
+	DBusMessage *getter_reply;
+	DBusMessageIter prop_iter, entry_iter;
+	const struct wpa_dbus_property_desc *dsc;
+	int i;
 
-	DBusConnection *connection;
-	DBusMessage *msg, *getter_reply;
-	DBusMessageIter prop_iter, signal_iter, dict_iter, entry_iter;
+	for (dsc = obj_dsc->properties, i = 0; dsc && dsc->dbus_property;
+	     dsc++, i++) {
 
-	if (!iface)
-		return;
-	connection = iface->con;
+		if (!obj_dsc->prop_changed_flags[i])
+			continue;
+		else if (os_strcmp(dsc->dbus_interface, interface))
+			continue;
+		else
+			obj_dsc->prop_changed_flags[i] = 0;
 
-	if (!property_getter || !path || !interface_name || !property_name) {
-		wpa_printf(MSG_ERROR, "dbus: %s: A parameter not specified",
-			   __func__);
-		return;
-	}
+		getter_reply = dsc->getter(NULL, obj_dsc->user_data);
+		if (!getter_reply ||
+		    dbus_message_get_type(getter_reply) ==
+			    DBUS_MESSAGE_TYPE_ERROR) {
+			wpa_printf(MSG_ERROR, "dbus: %s: Cannot get new value of "
+				   "property %s", __func__, dsc->dbus_property);
+			continue;
+		}
 
-	getter_reply = property_getter(NULL, getter_arg);
-	if (!getter_reply ||
-	    dbus_message_get_type(getter_reply) == DBUS_MESSAGE_TYPE_ERROR) {
-		wpa_printf(MSG_ERROR, "dbus: %s: Cannot get new value of "
-			   "property %s", __func__, property_name);
-		return;
+		if (!dbus_message_iter_init(getter_reply, &prop_iter) ||
+		    !dbus_message_iter_open_container(dict_iter,
+						      DBUS_TYPE_DICT_ENTRY,
+						      NULL, &entry_iter) ||
+		    !dbus_message_iter_append_basic(&entry_iter,
+						    DBUS_TYPE_STRING,
+						    &dsc->dbus_property))
+			goto err;
+
+		recursive_iter_copy(&prop_iter, &entry_iter);
+
+		if (!dbus_message_iter_close_container(dict_iter, &entry_iter))
+			goto err;
+
+		dbus_message_unref(getter_reply);
 	}
 
-	msg = dbus_message_new_signal(path, interface_name,
+	return;
+err:
+	wpa_printf(MSG_ERROR, "dbus: %s: Cannot construct signal", __func__);
+}
+
+
+static void send_prop_changed_signal(DBusConnection *con,
+					    const char *path,
+					    const char *interface,
+					    const struct wpa_dbus_object_desc *obj_dsc)
+{
+	DBusMessage *msg;
+	DBusMessageIter signal_iter, dict_iter;
+
+	msg = dbus_message_new_signal(path, interface,
 				      "PropertiesChanged");
 	if (msg == NULL) {
-		dbus_message_unref(getter_reply);
 		return;
 	}
 
-	dbus_message_iter_init(getter_reply, &prop_iter);
 	dbus_message_iter_init_append(msg, &signal_iter);
 
 	if (!dbus_message_iter_open_container(&signal_iter, DBUS_TYPE_ARRAY,
-					      "{sv}", &dict_iter) ||
-	    !dbus_message_iter_open_container(&dict_iter, DBUS_TYPE_DICT_ENTRY,
-					      NULL, &entry_iter) ||
-	    !dbus_message_iter_append_basic(&entry_iter, DBUS_TYPE_STRING,
-					    &property_name))
+					      "{sv}", &dict_iter))
 		goto err;
 
-	recursive_iter_copy(&prop_iter, &entry_iter);
+	put_changed_properties(obj_dsc, interface, &dict_iter);
 
-	if (!dbus_message_iter_close_container(&dict_iter, &entry_iter) ||
-	    !dbus_message_iter_close_container(&signal_iter, &dict_iter))
+	if (!dbus_message_iter_close_container(&signal_iter, &dict_iter))
 		goto err;
 
-	dbus_connection_send(connection, msg, NULL);
+	dbus_connection_send(con, msg, NULL);
 
 out:
-	dbus_message_unref(getter_reply);
 	dbus_message_unref(msg);
 	return;
 
@@ -668,6 +692,146 @@ err:
 	goto out;
 }
 
+static void flush_object_timeout_handler(void *eloop_ctx, void *timeout_ctx)
+{
+	wpa_printf(MSG_DEBUG, "dbus: %s: timeout. sending "
+			      "changed properties of object %s",__func__,
+			      (char *) timeout_ctx);
+	wpa_dbus_flush_object_changed_properties(eloop_ctx, timeout_ctx);
+}
+
+static void recursive_flush_changed_properties(DBusConnection *con,
+					      const char *path)
+{
+	char **objects = NULL;
+	char *subobj_path = NULL;
+	int i;
+
+	wpa_dbus_flush_object_changed_properties(con, path);
+
+	subobj_path = os_malloc(WPAS_DBUS_OBJECT_PATH_MAX);
+	if(!subobj_path ||!dbus_connection_list_registered(con,
+							   path, &objects))
+		goto out;
+
+	for(i = 0; objects[i]; i++) {
+		snprintf(subobj_path, WPAS_DBUS_OBJECT_PATH_MAX,
+			 "%s/%s", path, objects[i]);
+		recursive_flush_changed_properties(con, subobj_path);
+	}
+
+out:
+	dbus_free_string_array(objects);
+	os_free(subobj_path);
+}
+
+/**
+ * wpa_dbus_flush_all_changed_properties - Send all PropertiesChanged signals
+ * @con: DBus connection
+ *
+ * Traverses through all registered objects and sends PropertiesChanged for
+ * each properties.
+ */
+void wpa_dbus_flush_all_changed_properties(DBusConnection *con)
+{
+	recursive_flush_changed_properties(con, WPAS_DBUS_NEW_PATH);
+}
+
+
+/**
+ * wpa_dbus_flush_object_changed_properties - Send PropertiesChanged for object
+ * @con: DBus connection
+ * @path: path to a DBus object for which PropertiesChanged will be sent.
+ *
+ * Iterates over all properties registered with object and for each interface
+ * containing properties marked as changed sends a PropertiesChanged signal
+ * containing names and new values of properties that has changed.
+ * You need to call this function after wpa_dbus_mark_property_changed
+ * if you want to send PropertiesChanged signal immediately (i.e. without
+ * waiting timeout to expire). PropertiesChanged signal for an object is sent
+ * automatically short time after first marking  property as changed. All
+ * PropertiesChanged signals are sent automatically after responding on DBus
+ * message so if you marked a property changed as a result of DBus call
+ * (e.g. param setter), you usually don't need to call this function.
+ */
+void wpa_dbus_flush_object_changed_properties(DBusConnection *con,
+					      const char *path)
+{
+	struct wpa_dbus_object_desc *obj_desc = NULL;
+	const struct wpa_dbus_property_desc *dsc;
+	int i;
+
+	eloop_cancel_timeout(flush_object_timeout_handler, con, (void *) path);
+
+	dbus_connection_get_object_path_data(con, path, (void **) &obj_desc);
+
+	if (!obj_desc)
+		return;
+
+	dsc = obj_desc->properties;
+	for (dsc = obj_desc->properties, i = 0; dsc && dsc->dbus_property;
+	     dsc++, i++)
+		if (obj_desc->prop_changed_flags[i])
+			send_prop_changed_signal(con, path,
+						 dsc->dbus_interface, obj_desc);
+}
+
+
+#define WPA_DBUS_SEND_PROP_CHANGED_TIMEOUT 5
+
+
+/**
+ * wpa_dbus_mark_property_changed - Mark a property as changed and
+ * @iface: dbus priv struct
+ * @path: path to DBus object which property has changed
+ * @interface: interface containing changed property
+ * @property: property name which has changed
+ *
+ * Iterates over all properties registered with object and marks the
+ * one given in parameters as changed. All parameters registered for
+ * an object within single interface will be aggregated together and
+ * sent in one PropertiesChanged signal when function
+ * wpa_dbus_flush_object_changed_propertis will be called.
+ */
+void wpa_dbus_mark_property_changed(struct wpas_dbus_priv *iface,
+				char *path, const char *interface,
+				const char *property)
+{
+	struct wpa_dbus_object_desc *obj_desc = NULL;
+	const struct wpa_dbus_property_desc *dsc;
+	int i = 0;
+
+	dbus_connection_get_object_path_data(iface->con, path,
+					     (void **) &obj_desc);
+	if (!obj_desc) {
+		wpa_printf(MSG_ERROR, "dbus: wpa_dbus_property_changed: "
+			   "could not obtain object's private data: %s", path);
+		return;
+	}
+
+	for (dsc = obj_desc->properties; dsc && dsc->dbus_property; dsc++, i++)
+		if (!strcmp(property, dsc->dbus_property) &&
+		    !strcmp(interface, dsc->dbus_interface)) {
+			obj_desc->prop_changed_flags[i] = 1;
+			break;
+		}
+
+	if (!dsc || !dsc->dbus_property) {
+		wpa_printf(MSG_ERROR, "dbus: wpa_dbus_property_changed: "
+			   "no property %s in object %s", property, path);
+		return;
+	}
+
+	if(!eloop_is_timeout_registered(flush_object_timeout_handler,
+					iface->con,
+					obj_desc->path)) {
+		eloop_register_timeout(0, WPA_DBUS_SEND_PROP_CHANGED_TIMEOUT,
+				       flush_object_timeout_handler,
+				       iface->con,
+				       obj_desc->path);
+	}
+}
+
 
 /**
  * wpa_dbus_get_object_properties - Put object's properties into dictionary
diff --git a/wpa_supplicant/dbus/dbus_new_helpers.h b/wpa_supplicant/dbus/dbus_new_helpers.h
index f34fdc3..4ec2a1f 100644
--- a/wpa_supplicant/dbus/dbus_new_helpers.h
+++ b/wpa_supplicant/dbus/dbus_new_helpers.h
@@ -18,6 +18,10 @@
 
 #include <dbus/dbus.h>
 
+#define WPAS_DBUS_NEW_SERVICE		"fi.w1.wpa_supplicant1"
+#define WPAS_DBUS_NEW_PATH		"/fi/w1/wpa_supplicant1"
+#define WPAS_DBUS_NEW_INTERFACE	"fi.w1.wpa_supplicant1"
+
 typedef DBusMessage * (* WPADBusMethodHandler)(DBusMessage *message,
 					       void *user_data);
 typedef void (* WPADBusArgumentFreeFunction)(void *handler_arg);
@@ -27,12 +31,16 @@ typedef DBusMessage * (* WPADBusPropertyAccessor)(DBusMessage *message,
 
 struct wpa_dbus_object_desc {
 	DBusConnection *connection;
+	char *path;
 
 	/* list of methods, properties and signals registered with object */
 	const struct wpa_dbus_method_desc *methods;
 	const struct wpa_dbus_signal_desc *signals;
 	const struct wpa_dbus_property_desc *properties;
 
+	/* property changed flags */
+	u8 *prop_changed_flags;
+
 	/* argument for method handlers and properties
 	 * getter and setter functions */
 	void *user_data;
@@ -123,17 +131,20 @@ int wpa_dbus_unregister_object_per_iface(
 	struct wpas_dbus_priv *ctrl_iface,
 	const char *path);
 
-void wpa_dbus_signal_property_changed(struct wpas_dbus_priv *iface,
-				      WPADBusPropertyAccessor property_getter,
-				      void *getter_arg,
-				      const char *path,
-				      const char *interface_name,
-				      const char *property_name);
-
 void wpa_dbus_get_object_properties(struct wpas_dbus_priv *iface,
 				    const char *path, const char *interface,
 				    DBusMessageIter *dict_iter);
 
+
+void wpa_dbus_flush_all_changed_properties(DBusConnection *con);
+
+void wpa_dbus_flush_object_changed_properties(DBusConnection *con,
+					      const char *path);
+
+void wpa_dbus_mark_property_changed(struct wpas_dbus_priv *iface,
+				char *path, const char *interface,
+				const char *property);
+
 DBusMessage * wpa_dbus_introspect(DBusMessage *message,
 				  struct wpa_dbus_object_desc *obj_dsc);
 
-- 
1.6.5.2



More information about the HostAP mailing list