flimflam: revise multi-profile support to pin objects to a profile

src/profile.c

Index: src/profile.c
diff --git a/src/profile.c b/src/profile.c
index 30fb5cec27866ff5844c5f24e2094c3664dc31f3..6934b6cac569b40a87ff4d4a15da9f42d2a583ed 100644
--- a/src/profile.c
+++ b/src/profile.c
@@ -39,6 +39,7 @@
 
 struct connman_profile {
 	struct connman_storage_ident ident;
+	guint changed_timeout;
 	char *path;
 	char *name;
 	connman_bool_t offlinemode;
@@ -56,6 +57,38 @@ static struct connman_profile *default_profile = NULL;
 
 static DBusConnection *connection = NULL;
 
+/*
+ * Loading/Saving objects.
+ *
+ * Service objects go to the profile they are pinned too (typically

[Paul Stewart, 2011/03/30 00:56:18]

s/too/to/

+ * the active profile at the time they were created but this can be
+ * changed, e.g. from private -> global).
+ *
+ * Device and ipconfig objects go in the global profile (if any).
+ * This ensures that enable/disable state is maintained between
+ * users (and reboots); or possibly discarded (e.g. for testing).
+ *
+ * Likewise global state like offline mode is stored in the global
+ * profile (see above).
+ */
+
+/*
+ * Return the active profile; it's on the top of the stack.
+ */
+static inline struct connman_profile *active_profile(void)
+{
+	return cur_profile >= 0 ? profile_stack[cur_profile] : NULL;
+}
+
+/*
+ * Return the global profile; it's top-most non-user profile.
+ */
+static struct connman_profile *global_profile(void)
+{
+	/* TODO(sleffler) cheat for now */
+	return default_profile != NULL ? default_profile : NULL;
+}
+
 static int ident_equal(const struct connman_storage_ident *a,
 		       const struct connman_storage_ident *b)
 {
@@ -86,63 +119,72 @@ static void profiles_changed(void)
 
 connman_bool_t __connman_profile_get_offlinemode(void)
 {
-	return (default_profile == NULL) ? FALSE : default_profile->offlinemode;
+	struct connman_profile *profile = global_profile();
+	return (profile == NULL) ? FALSE : profile->offlinemode;
 }
 
 int __connman_profile_set_offlinemode(connman_bool_t offlinemode)
 {
-	_DBG_PROFILE("offlinemode %d", offlinemode);
+	struct connman_profile *profile = global_profile();
+	int ret;
+
+	_DBG_PROFILE("offlinemode %d profile %p", offlinemode, profile);
 
-	/* TODO(sleffler) disallow if no default profile? */
-	if (default_profile != NULL) {
+	/* NB: always succeeds (ATM) */
+	ret = __connman_device_set_offlinemode(offlinemode);
+	if (ret != 0)
+		return ret;
+
+	/* TODO(sleffler) sallow even if no global profile? */
+	if (profile != NULL) {
 		/*
 		 * OfflineMode is only saved to the default profile;
 		 * this ensures it is preserved across user changes.
 		 */
-		if (default_profile->offlinemode == offlinemode)
+		if (profile->offlinemode == offlinemode)
 			return -EALREADY;
 
-		default_profile->offlinemode = offlinemode;
+		profile->offlinemode = offlinemode;
 
 		connman_dbus_send_property_changed_variant(
-		    default_profile->path,
+		    profile->path,
 		    CONNMAN_PROFILE_INTERFACE, "OfflineMode",
 		    DBUS_TYPE_BOOLEAN, &offlinemode);
-	}
-	return __connman_device_set_offlinemode(offlinemode);
-}
 
-/*
- * Return the active profile; it's on the top of the stack.
- */
-static inline struct connman_profile *active_profile(void)
-{
-	return cur_profile >= 0 ? profile_stack[cur_profile] : NULL;
+		__connman_storage_save_profile(profile);
+	}
+	return 0;
 }
 
-/*
- * Load/Save objects.  When loading we walk the stack of profiles
- * until we find a successful load.  Saving always happens to the
- * top-most/active profile.
- */
-
 static inline int load_continue(int err)
 {
-	/* NB: ENXIO for no file, ESRCH for no group/key */
+	/* NB: ENXIO for no file, ESRCH for no entry */
 	return (err == -ENXIO || err == -ESRCH);
 }
 
 int __connman_profile_load_service(struct connman_service *service)
 {
-	int i, err;
+	struct connman_profile *profile =
+	    __connman_service_get_profile(service);
+	int err, i;
 
-	_DBG_PROFILE("service %p", service);
+	_DBG_PROFILE("service %p profile %p", service, profile);
 
+	if (profile != NULL)
+		return __connman_storage_load_service(service, &profile->ident);
+	/*
+	 * Not bound to a profile yet, search the stack for an
+	 * entry and if found bind the profile to the service.
+	 */
 	err = 0;
 	for (i = cur_profile; i >= 0; i--) {
-		const struct connman_profile *profile = profile_stack[i];
-
+		profile = profile_stack[i];
 		err = __connman_storage_load_service(service, &profile->ident);
+		if (err == 0) {
+			_DBG_PROFILE("bind to profile %p", profile);
+			__connman_service_set_profile(service, profile);
+			return 0;
+		}
 		if (!load_continue(err))
 			break;
 	}
@@ -151,34 +193,34 @@ int __connman_profile_load_service(struct connman_service *service)
 
 int __connman_profile_save_service(struct connman_service *service)
 {
-	struct connman_profile *profile = active_profile();
+	struct connman_profile *profile =
+	    __connman_service_get_profile(service);
 
 	_DBG_PROFILE("service %p profile %p", service, profile);
 
+	if (profile == NULL) {
+		/* not bound yet, bind to the active profile */
+		profile = active_profile();
+		_DBG_PROFILE("bind to profile %p", profile);
+		__connman_service_set_profile(service, profile);
+	}
 	return (profile == NULL) ? 0 :
 	    __connman_storage_save_service(service, &profile->ident);
 }
 
 int __connman_profile_load_device(struct connman_device *device)
 {
-	int i, err;
+	struct connman_profile *profile = global_profile();
 
-	_DBG_PROFILE("device %p", device);
-
-	err = 0;
-	for (i = cur_profile; i >= 0; i--) {
-		const struct connman_profile *profile = profile_stack[i];
+	_DBG_PROFILE("device %p profile %p", device, profile);
 
-		err = __connman_storage_load_device(device, &profile->ident);
-		if (!load_continue(err))
-			break;
-	}
-	return err;
+	return (profile == NULL) ? 0 :
+	    __connman_storage_load_device(device, &profile->ident);
 }
 
 int __connman_profile_save_device(struct connman_device *device)
 {
-	struct connman_profile *profile = active_profile();
+	struct connman_profile *profile = global_profile();
 
 	_DBG_PROFILE("device %p profile %p", device, profile);
 
@@ -188,25 +230,17 @@ int __connman_profile_save_device(struct connman_device *device)
 
 int __connman_profile_load_ipconfig(struct connman_ipconfig *ipconfig)
 {
-	int i, err;
+	struct connman_profile *profile = global_profile();
 
-	_DBG_PROFILE("ipconfig %p", ipconfig);
-
-	err = 0;
-	for (i = cur_profile; i >= 0; i--) {
-		const struct connman_profile *profile = profile_stack[i];
+	_DBG_PROFILE("ipconfig %p profile %p", ipconfig, profile);
 
-		err = __connman_storage_load_ipconfig(ipconfig,
-		    &profile->ident);
-		if (!load_continue(err))
-			break;
-	}
-	return err;
+	return (profile == NULL) ? 0:
+	    __connman_storage_load_ipconfig(ipconfig, &profile->ident);
 }
 
 int __connman_profile_save_ipconfig(const struct connman_ipconfig *ipconfig)
 {
-	struct connman_profile *profile = active_profile();
+	struct connman_profile *profile = global_profile();
 
 	_DBG_PROFILE("ipconfig %p profile %p", ipconfig, profile);
 
@@ -236,29 +270,29 @@ int __connman_profile_append_hidden_ssids(GSList **hidden_ssids,
 }
 
 /*
- * Save the default profile if registered.
+ * Return the object path for the specified profile.
  */
-int __connman_profile_save_default(void)
+const char *__connman_profile_get_path(const struct connman_profile *profile)
 {
-	if (default_profile != NULL)
-		__connman_storage_save_profile(default_profile);
-	return 0;
+	return profile != NULL ? profile->path : NULL;
 }
 
 /*
- * Save the active profile (if any).
+ * Return the profile given an object path.
  */
-int __connman_profile_save_active(void)
+struct connman_profile *__connman_profile_lookup_profile(const char *path)
 {
-	struct connman_profile *profile = active_profile();
-	return (profile == NULL) ? -EINVAL :
-	    __connman_storage_save_profile(profile);
+	return g_hash_table_lookup(profile_hash, path);
 }
 
 /*
- * Return the identifier for the active profile or NULL
- * if there is none.
+ * Return the active profile or NULL
  */
+struct connman_profile *__connman_profile_active_profile(void)
+{
+	return active_profile();
+}
+
 const struct connman_storage_ident *__connman_profile_active_ident(void)
 {
 	struct connman_profile *profile = active_profile();
@@ -275,13 +309,38 @@ const char *__connman_profile_active_path(void)
 	return profile != NULL ? profile->path : NULL;
 }
 
-static guint changed_timeout = 0;
+/*
+ * Delete an entry in the specified profile.
+ */
+int __connman_profile_delete_entry(struct connman_profile *profile,
+    const char *group)
+{
+	GKeyFile *keyfile;
+	gboolean status;
+
+	_DBG_PROFILE("profile %p group %s", profile, group);
+
+	keyfile = __connman_storage_open(&profile->ident);
+	if (keyfile == NULL) {
+		_DBG_PROFILE("cannot open key file");
+		return -EINVAL;
+	}
+
+	status = g_key_file_remove_group(keyfile, group, NULL);
+	__connman_storage_close(&profile->ident, keyfile, status);
+
+	if (status == FALSE) {
+		_DBG_PROFILE("cannot remove %s", group);
+		return -ENXIO;
+	}
+	return 0;
+}
 
-static void clear_timeout(void)
+static void clear_timeout(struct connman_profile *profile)
 {
-	if (changed_timeout > 0) {
-		g_source_remove(changed_timeout);
-		changed_timeout = 0;
+	if (profile->changed_timeout > 0) {
+		g_source_remove(profile->changed_timeout);
+		profile->changed_timeout = 0;
 	}
 }
 
@@ -291,9 +350,9 @@ static void append_services(DBusMessageIter *iter, void *arg)
 }
 static gboolean services_changed(gpointer user_data)
 {
-	struct connman_profile *profile = active_profile();
+	struct connman_profile *profile = user_data;
 
-	changed_timeout = 0;
+	profile->changed_timeout = 0;
 
 	if (profile != NULL) {
 		connman_dbus_send_property_changed_array(profile->path,
@@ -308,22 +367,27 @@ static gboolean services_changed(gpointer user_data)
 }
 
 /*
- * Handle changes to the profile.  Generate PropertyChanged signals
+ * Handle changes to a profile.  Generate PropertyChanged signals
  * on Manager.Services and Profile.Services for the currently active
  * profile.  To minimize overhead requests may be coalesced using a
  * 1 second delay on the signals.
  */
-void __connman_profile_changed(gboolean delayed)
+void __connman_profile_changed(struct connman_profile *profile,
+    gboolean delayed)
 {
-	_DBG_PROFILE("delayed %d changed_timeout %d", delayed, changed_timeout);
+	if (profile == NULL)
+		return;
 
-	clear_timeout();
+	_DBG_PROFILE("profile %p delayed %d changed_timeout %d",
+	    profile, delayed, profile->changed_timeout);
+
+	clear_timeout(profile);
 
 	if (delayed == TRUE) {
-		changed_timeout = g_timeout_add_seconds(1, services_changed,
-		    NULL);
+		profile->changed_timeout = g_timeout_add_seconds(1,
+		    services_changed, profile);
 	} else
-		services_changed(NULL);
+		services_changed(profile);
 }
 
 int __connman_profile_add_device(struct connman_device *device)
@@ -461,7 +525,7 @@ static DBusMessage *get_properties(DBusConnection *conn,
 		connman_dbus_dict_append_variant(&dict, "Name",
 		    DBUS_TYPE_STRING, &profile->name);
 
-	if (profile == default_profile)
+	if (profile == global_profile())
 		connman_dbus_dict_append_variant(&dict, "OfflineMode",
 		    DBUS_TYPE_BOOLEAN, &profile->offlinemode);
 
@@ -650,8 +714,7 @@ static DBusMessage *delete_entry(DBusConnection *conn,
 	DBusMessageIter iter;
 	const char *identifier;
 	struct connman_service *service;
-	GKeyFile *keyfile;
-	gboolean status;
+	int err;
 
 	_DBG_PROFILE("profile %s:%s", profile->ident.user,
 	    profile->ident.ident);
@@ -672,19 +735,9 @@ static DBusMessage *delete_entry(DBusConnection *conn,
 	}
 
 	/* Remove directly from profile */
-	keyfile = __connman_storage_open(&profile->ident);
-	if (keyfile == NULL) {
-		_DBG_PROFILE("cannot open key file");
-		return __connman_error_invalid_arguments(msg);
-	}
-
-	status = g_key_file_remove_group(keyfile, identifier, NULL);
-	__connman_storage_close(&profile->ident, keyfile, status);
-
-	if (status == FALSE) {
-		_DBG_PROFILE("cannot remove %s", identifier);
-		return __connman_error_not_found(msg);
-	}
+	err = __connman_profile_delete_entry(profile, identifier);
+	if (err)
+		return __connman_error_failed(msg, -err);
 
 	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
 }
@@ -731,6 +784,8 @@ static void unregister_profile(gpointer data)
 	g_dbus_unregister_interface(connection, profile->path,
 						CONNMAN_PROFILE_INTERFACE);
 
+	clear_timeout(profile);
+
 	if (profile == default_profile)
 		default_profile = NULL;
 
@@ -950,8 +1005,7 @@ int __connman_profile_push(const char *ident, const char *name,
 
 	profiles_changed();
 
-	/* NB: this is a noop if we are online (or trying to get online) */
-	__connman_service_auto_connect_any();
+	__connman_notifier_profile_push(profile);
 
 	return 0;
 }
@@ -998,15 +1052,20 @@ int __connman_profile_pop(const char *ident)
 		free_ident(&sid);
 	}
 
-	__connman_service_invalidate_profile(&profile->ident);
+	/*
+	 * Pop the stack, invalidate all objects holding references
+	 * to the profile, then remove it from the in-memory table.
+	 * We do the reclaim after notification in case anyone holding
+	 * a reference tries to use it.
+	 */
+	cur_profile--;
+
+	__connman_notifier_profile_pop(profile);
 
 	g_hash_table_remove(profile_hash, profile->path);
-	cur_profile--;
 
 	profiles_changed();
 
-	/* NB: no need to kick auto-connect; it will happen due to invalidate */
-
 	return 0;
 }
 
@@ -1222,8 +1281,6 @@ void __connman_profile_cleanup(void)
 	g_hash_table_destroy(profile_hash);
 	profile_hash = NULL;
 
-	clear_timeout();		/* cancel any pending timer */
-
 	connman_storage_unregister(&profile_storage);
 
 	dbus_connection_unref(connection);