soc.2008.masterpassword: fe5e808c: Fixed many bugs and crashes in the keyri...

scrouaf at soc.pidgin.im scrouaf at soc.pidgin.im
Wed Aug 13 18:01:23 EDT 2008


-----------------------------------------------------------------
Revision: fe5e808c89691530839b026867f360123164bf06
Ancestor: b9aa916708d0fae7074d02a177500dafd5fe4e39
Author: scrouaf at soc.pidgin.im
Date: 2008-08-13T21:32:51
Branch: im.pidgin.soc.2008.masterpassword
URL: http://d.pidgin.im/viewmtn/revision/info/fe5e808c89691530839b026867f360123164bf06

Modified files:
        libpurple/account.c libpurple/account.h libpurple/keyring.c
        libpurple/keyring.h libpurple/plugins/keyrings/Makefile.am
        libpurple/plugins/keyrings/gnomekeyring.c
        libpurple/plugins/keyrings/internalkeyring.c
        pidgin/gtkprefs.c

ChangeLog: 

Fixed many bugs and crashes in the keyring subsystem.
Fixed linking error in Makefiles, but still some work there (not sure
how it will compile or load if gnome keyring is not installed).
However, the code does not seem to crash, and saves what's to be saved,
deletes it when it has to, and loads it properly.
Still a few cosmetic things to do (cleanup, notify, debug), and change
all accesses to keyrings to make them async. Trannie Carter's code will
be most helpfull on that.
There are also a few mem leaks I'm aware of, and will clean up.
This is the first actually working version of the code.

-------------- next part --------------
============================================================
--- libpurple/account.c	1a94bf3b6d2a29e838255e3fd3c420c9abcb68c2
+++ libpurple/account.c	5c5fb0c8a39b673058e891dc542f00e4b2593f08
@@ -44,7 +44,7 @@
 
 /**
  * TODO :
- *  - grab Trannie's code for asynch connection
+ *  - grab Trannie's code for async connection
  */
 
 
@@ -381,7 +381,7 @@ account_to_xmlnode(PurpleAccount *accoun
 	char *data;
 	PurplePresence *presence;
 	PurpleProxyInfo *proxy_info;
-	GError * error;
+	GError * error = NULL;
 	GDestroyNotify destroy;
 
 	node = xmlnode_new("account");
@@ -394,18 +394,22 @@ account_to_xmlnode(PurpleAccount *accoun
 
 	if (purple_account_get_remember_password(account))
 	{
-		purple_debug_info("accounts", "Exporting password.\n");
+		purple_debug_info("accounts", "Exporting password for account %s.\n",
+			purple_account_get_username(account));
 		purple_keyring_export_password(account, &keyring_id, 
 			&mode, &data, &error, &destroy);
 
 		if (error != NULL) {
 
-			/* Output debug info */
+			purple_debug_info("accounts",
+				"failed to export password for account %s : %s.\n",
+				purple_account_get_username(account),
+				error->message);
 
 		} else {
 
 			child = xmlnode_new_child(node, "password");
-			xmlnode_set_attrib(child, "keyringid", keyring_id);
+			xmlnode_set_attrib(child, "keyring_id", keyring_id);
 			xmlnode_set_attrib(child, "mode", mode);
 			xmlnode_insert_data(child, data, -1);
 
@@ -487,6 +491,8 @@ sync_accounts(void)
 		return;
 	}
 
+	purple_debug_info("account", "Syncing accounts.\n");
+
 	node = accounts_to_xmlnode();
 	data = xmlnode_to_formatted_str(node, NULL);
 	purple_util_write_data_to_file("accounts.xml", data, -1);
@@ -804,7 +810,7 @@ parse_account(xmlnode *node)
 	const char *keyring_id = NULL;
 	const char *mode = NULL;
 	char *data = NULL;
-	gboolean result;
+	gboolean result = FALSE;
 	GError * error = NULL;
 
 	child = xmlnode_get_child(node, "protocol");
@@ -906,6 +912,7 @@ parse_account(xmlnode *node)
 	}
 
 	/* Read the password */
+
 	child = xmlnode_get_child(node, "password");
 	if (child != NULL)
 	{
@@ -918,12 +925,14 @@ parse_account(xmlnode *node)
 		if (result == TRUE) {
 			purple_debug_info("accounts", "password imported successfully.\n");
 			purple_account_set_remember_password(ret, TRUE);
-			g_free(keyring_id);
-			g_free(mode);
-			g_free(data);
+			/*
+			g_free(keyring_id);	TODO :
+			g_free(mode);		This was commented becaus eof a double free.
+			g_free(data); 		I should figure out which one causes the bug to avoid leaks
+			*/
 		} else {
 			purple_debug_info("accounts", "failed to imported password.\n");
-			// FIXME handle error
+			/* TODO handle error */
 		}
 	}
 
@@ -1610,6 +1619,38 @@ purple_account_set_password(PurpleAccoun
 	schedule_accounts_save();
 }
 
+void 
+purple_account_set_password_async(PurpleAccount * account, 
+				  gchar * password,
+				  GDestroyNotify destroypassword,
+				  PurpleKeyringSaveCallback cb,
+				  gpointer data)
+{
+	/**
+	 * This is so we can force an account sync by calling
+	 * it with account == NULL.
+	 */
+	if(account != NULL) {
+
+		if (purple_account_get_remember_password(account) == FALSE) {
+
+			account->password = g_strdup(password);
+			purple_debug_info("account",
+				"Password for %s set, not sent to keyring.\n",
+				purple_account_get_username(account));
+
+			if (cb != NULL)
+				cb(account, NULL, data);
+
+		} else {
+
+			purple_keyring_set_password_async(account, password,
+				destroypassword, cb, data);
+
+		}
+	}
+	schedule_accounts_save();
+}
 void
 purple_account_set_alias(PurpleAccount *account, const char *alias)
 {
@@ -1677,6 +1718,10 @@ purple_account_set_connection(PurpleAcco
 	account->gc = gc;
 }
 
+/**
+ * FIXME :
+ *  This should add/remove the password to/from the keyring
+ */
 void
 purple_account_set_remember_password(PurpleAccount *account, gboolean value)
 {
============================================================
--- libpurple/account.h	78a50d84e465cd484a06ddd56de6abe839742278
+++ libpurple/account.h	09a9d48cd711d1fc47497ed0b3e71372caa84073
@@ -331,6 +331,22 @@ void purple_account_set_password(PurpleA
 void purple_account_set_password(PurpleAccount *account, const char *password);
 
 /**
+ * Set a password to be remembered.
+ * This should be renamed purple_account_set_password() when getting
+ * to 3.0. This calls the keyring function and syncs the accounts.xml
+ * @param account The account for which the password is to be saved.
+ * @param password The password to save.
+ * @param destroypassword A function called to free the password. Can be NULL.
+ * @param cb A callback for once the password is saved.
+ * @param data A pointer to be passed to the callback.
+ */
+void purple_account_set_password_async(PurpleAccount * account, 
+				  gchar * password,
+				  GDestroyNotify destroypassword,
+				  PurpleKeyringSaveCallback cb,
+				  gpointer data);
+
+/**
  * Sets the account's alias.
  *
  * @param account The account.
============================================================
--- libpurple/keyring.c	fa918cf8a827cdf18864523c77e017ad84984aab
+++ libpurple/keyring.c	1b687ba433c6da254a9d3890ceb016fef6f75907
@@ -1,8 +1,6 @@
 /**
  * @file keyring.c Keyring plugin API
- * @todo
- *  - purple_keyring_()
- *  - loading : find a way to fallback
+ * @ingroup core
  */
 
 /* purple
@@ -27,13 +25,22 @@
  */
 
 #include <glib.h>
-#include <string.h>>
+#include <string.h>
 #include "account.h"
 #include "keyring.h"
 #include "signals.h"
 #include "core.h"
 #include "debug.h"
 
+typedef struct _PurpleKeyringChangeTracker PurpleKeyringChangeTracker;
+
+static void purple_keyring_pref_cb(const char *, PurplePrefType, gconstpointer, gpointer);
+static PurpleKeyring * purple_keyring_find_keyring_by_id(char * id);
+static void purple_keyring_drop_passwords(const PurpleKeyring * keyring);
+static void purple_keyring_set_inuse_check_error_cb(const PurpleAccount *,GError *,gpointer);
+static void purple_keyring_set_inuse_got_pw_cb(PurpleAccount *, gchar *, GError *, gpointer);
+
+
 /******************************************/
 /** @name PurpleKeyring                   */
 /******************************************/
@@ -56,6 +63,18 @@ struct _PurpleKeyring
 	gpointer r3;	/* RESERVED */
 };
 
+struct _PurpleKeyringChangeTracker
+{
+	GError * error;			/* could probably be dropped */
+	PurpleKeyringSetInUseCallback cb;
+	gpointer data;
+	const PurpleKeyring * new;
+	const PurpleKeyring * old;	/* we are done when : finished == TRUE && read_outstanding == 0 */
+	int read_outstanding;
+	gboolean finished;
+	gboolean abort;
+	gboolean force;
+};
 
 /* Constructor */
 PurpleKeyring * 
@@ -268,8 +287,8 @@ static char * purple_keyring_to_use;
 static GList * purple_keyring_keyrings;		/* list of available keyrings   */
 static const PurpleKeyring * purple_keyring_inuse;	/* keyring being used	        */
 static char * purple_keyring_to_use;
+static guint purple_keyring_pref_cb_id;
 
-
 void 
 purple_keyring_init()
 {
@@ -296,15 +315,10 @@ purple_keyring_init()
 		purple_value_new(PURPLE_TYPE_BOXED, "PurpleKeyring *")); /* a pointer to the keyring */
 
 	/* see what keyring we want to use */
-
-	purple_prefs_add_string("/purple/keyring/active", "txt");
-	purple_prefs_connect_callback(NULL, "/purple/keyring/active",
-				purple_keyring_pref_cb, NULL);
-
-
 	touse = purple_prefs_get_string("/purple/keyring/active");
 
 	if (touse == NULL) {
+		purple_prefs_add_none("/purple/keyring");
 		purple_prefs_add_string("/purple/keyring/active", FALLBACK_KEYRING);
 		purple_keyring_to_use = g_strdup(FALLBACK_KEYRING);
 
@@ -313,8 +327,8 @@ purple_keyring_init()
 		purple_keyring_to_use = g_strdup(touse);
 	}
 
-	purple_prefs_connect_callback(NULL, "/purple/keyring/active",
-				purple_keyring_pref_cb, NULL);
+	purple_keyring_pref_cb_id = purple_prefs_connect_callback(NULL, 
+		"/purple/keyring/active", purple_keyring_pref_cb, NULL);
 
 	purple_debug_info("keyring", "purple_keyring_init() done, selected keyring is : %s.\n",
 		purple_keyring_to_use);
@@ -372,27 +386,16 @@ purple_keyring_drop_passwords(const Purp
 
 	g_return_if_fail(save != NULL);
 
-	for (cur = purple_accounts_get_all(); 
+	for (cur = purple_accounts_get_all();
 	     cur != NULL;
 	     cur = cur->next)
-		save(cur->data, "", NULL, NULL, NULL);
+		save(cur->data, NULL, NULL, NULL, NULL);
 
 	return;
 }
 
-struct _PurpleKeyringChangeTracker
-{
-	GError * error;			// could probably be dropped
-	PurpleKeyringSetInUseCallback cb;
-	gpointer data;
-	const PurpleKeyring * new;
-	const PurpleKeyring * old;		// we are done when : finished == TRUE && read_outstanding == 0
-	int read_outstanding;
-	gboolean finished;
-	gboolean abort;
-	gboolean force;
-};
 
+
 static void
 purple_keyring_set_inuse_check_error_cb(const PurpleAccount * account,
 					GError * error,
@@ -403,7 +406,7 @@ purple_keyring_set_inuse_check_error_cb(
 	PurpleKeyringClose close;
 	struct _PurpleKeyringChangeTracker * tracker;
 
-	tracker = (struct _PurpleKeyringChangeTracker *)data;
+	tracker = (PurpleKeyringChangeTracker *)data;
 
 	g_return_if_fail(tracker->abort == FALSE);
 
@@ -419,31 +422,39 @@ purple_keyring_set_inuse_check_error_cb(
 		switch(error->code) {
 
 			case ERR_NOCAP :
-				g_debug("Keyring could not save password for account %s : %s", name, error->message);
+				purple_debug_info("keyring",
+					"Keyring could not save password for account %s : %s\n",
+					name, error->message);
 				break;
 
 			case ERR_NOPASSWD :
-				g_debug("No password found while changing keyring for account %s : %s",
-					 name, error->message);
+				purple_debug_info("keyring",
+					"No password found while changing keyring for account %s : %s\n",
+					name, error->message);
 				break;
 
 			case ERR_NOCHANNEL :
-				g_debug("Failed to communicate with backend while changing keyring for account %s : %s Aborting changes.",
-					 name, error->message);
+				purple_debug_info("keyring",
+					"Failed to communicate with backend while changing keyring for account %s : %s Aborting changes.\n",
+					name, error->message);
 				tracker->abort = TRUE;
 				break;
 
 			default :
-				g_debug("Unknown error while changing keyring for account %s : %s", name, error->message);
+				purple_debug_info("keyring",
+					"Unknown error while changing keyring for account %s : %s\n",
+					name, error->message);
 				break;
 		}
 	}
-
 	/* if this was the last one */
 	if (tracker->finished == TRUE && tracker->read_outstanding == 0) {
 	
 		if (tracker->abort == TRUE && tracker->force == FALSE) {
-
+			/**
+			 * TODO :
+			 *  - create faulty keyring to test this code.
+			 */
 			if (tracker->cb != NULL)
 				tracker->cb(tracker->old, FALSE, tracker->error, tracker->data);
 
@@ -453,30 +464,60 @@ purple_keyring_set_inuse_check_error_cb(
 			if (close != NULL)
 				close(NULL);
 
+			purple_debug_info("keyring",
+				"Failed to change keyring, aborting");
+
+			/* FIXME : this should maybe be in a callback
+			 *  cancel the prefs change 
+			 */
+			purple_prefs_disconnect_callback(purple_keyring_pref_cb_id);
+			purple_prefs_set_string("/purple/keyring/active",
+				purple_keyring_get_id(tracker->old));
+			purple_keyring_pref_cb_id = purple_prefs_connect_callback(NULL, 
+				"/purple/keyring/active", purple_keyring_pref_cb, NULL);
+
+
 		} else {
 			close = purple_keyring_get_close_keyring(tracker->old);
-			close(&error);
+			if (close != NULL)
+				close(&error);
 
-			tracker->cb(tracker->new, TRUE, error, tracker->data);
+			purple_keyring_inuse = tracker->new;
+			purple_keyring_drop_passwords(tracker->old);
+
+			purple_debug_info("keyring",
+				"Successfully changed keyring.\n");
+
+			if (tracker->cb != NULL)
+				tracker->cb(tracker->new, TRUE, error, tracker->data);
 		}
 
 		g_free(tracker);
 	}
+	/**
+	 * This is kind of hackish. It will schedule an account save,
+	 * and then return because account == NULL.
+	 * Another way to do this would be to expose the
+	 * schedule_accounts_save() function, but other such functions
+	 * are not exposed. So these was done for consistency.
+	 */
+	purple_account_set_password_async(NULL, NULL, NULL, NULL, NULL);
+
 	return;
 }
 
 
 static void
-purple_keyring_set_inuse_got_pw_cb(const PurpleAccount * account, 
+purple_keyring_set_inuse_got_pw_cb(PurpleAccount * account, 
 				  gchar * password, 
 				  GError * error, 
 				  gpointer data)
 {
 	const PurpleKeyring * new;
 	PurpleKeyringSave save;
-	struct _PurpleKeyringChangeTracker * tracker;
+	PurpleKeyringChangeTracker * tracker;
 
-	tracker = (struct _PurpleKeyringChangeTracker *)data;
+	tracker = (PurpleKeyringChangeTracker *)data;
 	new = tracker->new;
 
 	g_return_if_fail(tracker->abort == FALSE);
@@ -498,6 +539,7 @@ purple_keyring_set_inuse_got_pw_cb(const
 
 	} else {
 
+
 		save = purple_keyring_get_save_password(new);
 
 		if (save != NULL) {
@@ -532,11 +574,14 @@ purple_keyring_set_inuse(const PurpleKey
 	const PurpleKeyring * oldkeyring;
 	PurpleKeyringRead read = NULL;
 	PurpleKeyringClose close;
-	struct _PurpleKeyringChangeTracker * tracker;
+	PurpleKeyringChangeTracker * tracker;
 	GError * error = NULL; 
 
-	purple_debug_info("keyring", "Attempting to set new keyring : %s.\n",
-		newkeyring->id);
+	if (newkeyring != NULL)
+		purple_debug_info("keyring", "Attempting to set new keyring : %s.\n",
+			newkeyring->id);
+	else
+		purple_debug_info("keyring", "Attempting to set new keyring : NULL.\n");
 
 	oldkeyring = purple_keyring_get_inuse();
 
@@ -544,9 +589,11 @@ purple_keyring_set_inuse(const PurpleKey
 		read = purple_keyring_get_read_password(oldkeyring);
 
 		if (read == NULL) {
+			/*
 			error = g_error_new(ERR_PIDGINKEYRING , ERR_NOCAP,
 				"Existing keyring cannot read passwords");
-			g_debug("Existing keyring cannot read passwords");
+			*/
+			purple_debug_info("keyring", "Existing keyring cannot read passwords");
 
 			/* at this point, we know the keyring won't let us
 			 * read passwords, so there no point in copying them.
@@ -562,7 +609,7 @@ purple_keyring_set_inuse(const PurpleKey
 				close(NULL);	/* we can't do much about errors at this point*/
 
 		} else {
-			tracker = g_malloc(sizeof(struct _PurpleKeyringChangeTracker));
+			tracker = g_malloc(sizeof(PurpleKeyringChangeTracker));
 			oldkeyring = purple_keyring_get_inuse();
 
 			tracker->cb = cb;
@@ -602,7 +649,7 @@ purple_keyring_set_inuse(const PurpleKey
 
 
 
-void 
+static void 
 purple_keyring_pref_cb(const char *pref,
 		       PurplePrefType type,
 		       gconstpointer id,
@@ -617,7 +664,7 @@ purple_keyring_pref_cb(const char *pref,
 	new = purple_keyring_get_keyring_by_id(id);
 	g_return_if_fail(new != NULL);
 
-	purple_keyring_set_inuse(new, FALSE, /* XXX */NULL, data);	/* FIXME This should have a callback that can cancel the action */
+	purple_keyring_set_inuse(new, FALSE, NULL, data);	/* FIXME Maybe this should have a callback that can cancel the action */
 
 	return;
 }
@@ -627,7 +674,7 @@ purple_keyring_get_options()
 {
 	const GList * keyrings;
 	PurpleKeyring * keyring;
-	static GList * list = NULL;
+	GList * list = NULL;
 
 	for (keyrings = purple_keyring_get_keyrings();
 	     keyrings != NULL;
@@ -636,6 +683,8 @@ purple_keyring_get_options()
 		keyring = keyrings->data;
 		list = g_list_append(list, keyring->name);
 		list = g_list_append(list, keyring->id);
+		purple_debug_info("keyring", "adding pair : %s, %s.\n",
+			keyring->name, keyring->id);
 	}
 
 	return list;
@@ -707,23 +756,34 @@ purple_keyring_unregister(PurpleKeyring 
 	PurpleKeyring * fallback;
 	const char * keyring_id;
 
+	g_return_if_fail(keyring != NULL);
+	
+	purple_debug_info("keyring", 
+		"Unregistering keyring %s",
+		purple_keyring_get_id(keyring));
+
 	core = purple_get_core();
 	keyring_id = purple_keyring_get_id(keyring);
 	purple_signal_emit(core, "keyring-unregister", keyring_id, keyring);
 
 	inuse = purple_keyring_get_inuse();
+	fallback = purple_keyring_find_keyring_by_id(FALLBACK_KEYRING);
 
 	if (inuse == keyring) {
-		fallback = purple_keyring_find_keyring_by_id(FALLBACK_KEYRING);
+		if (inuse != fallback) {
+			/* TODO : check result ? */
+			purple_keyring_set_inuse(fallback, TRUE, NULL, NULL);
 
-		/* this is problematic. If it fails, we won't detect it */
-		purple_keyring_set_inuse(fallback, TRUE, NULL, NULL);
+		} else {
+			fallback = NULL;
+			purple_keyring_set_inuse(NULL, TRUE, NULL, NULL);
+		}
 	}
 
 	purple_keyring_keyrings = g_list_remove(purple_keyring_keyrings,
 		keyring);
 
-	purple_debug_info("keyring", "Keyring %s unregistered", keyring->id);
+	//purple_debug_info("keyring", "Keyring %s unregistered", keyring->id);
 }
 
 
@@ -747,8 +807,10 @@ purple_keyring_import_password(PurpleAcc
 	PurpleKeyringImportPassword import;
 	const char * realid;
 
-	purple_debug_info("keyring", "Importing password for account %s (%s).\n",
-		purple_account_get_username(account), purple_account_get_protocol_id(account));
+	purple_debug_info("keyring", "Importing password for account %s (%s) to keyring %s.\n",
+		purple_account_get_username(account),
+		purple_account_get_protocol_id(account),
+		keyringid);
 
 	inuse = purple_keyring_get_inuse();
 
@@ -768,11 +830,13 @@ purple_keyring_import_password(PurpleAcc
 	 *  - or the configured keyring is the fallback, compatible one.
 	 */
 	if ((keyringid != NULL && g_strcmp0(realid, keyringid) != 0) ||
-	    g_strcmp0(FALLBACK_KEYRING, realid)) {
+	    (keyringid == NULL && g_strcmp0(FALLBACK_KEYRING, realid))) {
+
 		*error = g_error_new(ERR_PIDGINKEYRING , ERR_INVALID,
 			"Specified keyring id does not match the configured one.");
 		purple_debug_info("keyring",
-			"Specified keyring id does not match the configured one. Data will be lost.");
+			"Specified keyring id does not match the configured one (%s vs. %s). Data will be lost.\n",
+			keyringid, realid);
 		return FALSE;
 	}
 
@@ -798,23 +862,29 @@ purple_keyring_export_password(PurpleAcc
 	const PurpleKeyring * inuse;
 	PurpleKeyringExportPassword export;
 
-	purple_debug_info("keyring", "exporting password.\n");
-
 	inuse = purple_keyring_get_inuse();
 
 	if (inuse == NULL) {
 		*error = g_error_new(ERR_PIDGINKEYRING , ERR_NOKEYRING,
-			"No keyring configured, cannot import password info");
-		g_debug("No keyring configured, cannot import password info");
+			"No keyring configured, cannot export password info");
+		purple_debug_info("keyring",
+			"No keyring configured, cannot export password info");
 		return FALSE;
 	}
 
 	*keyringid = purple_keyring_get_id(inuse);
 
+	purple_debug_info("keyring",
+		"Exporting password for account %s (%s) from keyring %s.\n",
+		purple_account_get_username(account),
+		purple_account_get_protocol_id(account),
+		*keyringid);
+
 	if (*keyringid == NULL) {
 		*error = g_error_new(ERR_PIDGINKEYRING , ERR_INVALID,
 			"Plugin does not have a keyring id");
-		g_debug("Plugin does not have a keyring id");
+		purple_debug_info("keyring",
+			"Configured keyring does not have a keyring id, cannot export password");
 		return FALSE;
 	}
 
@@ -823,7 +893,8 @@ purple_keyring_export_password(PurpleAcc
 	if (export == NULL) {
 		*error = g_error_new(ERR_PIDGINKEYRING , ERR_NOCAP,
 			"Keyring cannot export password info.");
-		g_debug("Keyring cannot export password info. This might be normal");
+		purple_debug_info("keyring",
+			"Keyring cannot export password info. This might be normal");
 		return FALSE;
 	}
 
============================================================
--- libpurple/keyring.h	e32f85339ae46262e4b6d77c9a78219a2641ada1
+++ libpurple/keyring.h	2a60b386ea7e439a55a4543bf9ca533d2bbc09a2
@@ -4,7 +4,6 @@
  *
  * @todo 
  *  - Offer a way to prompt the user for a password or for a password change.
- *  - write accessors and types for sync access.
  */
 
 /* purple
@@ -216,26 +215,28 @@ typedef gboolean (*PurpleKeyringExportPa
 /***************************************/
 /*@{*/
 
-PurpleKeyring *
-purple_keyring_get_keyring_by_id(const char * id);
-GList *
-purple_keyring_get_options(void);
-void 
-purple_keyring_pref_cb(const char *pref,
-		       PurplePrefType type,
-		       gconstpointer name,
-		       gpointer data);
 /**
+ * Find a keyring from it's keyring id.
+ * @param id The id for the keyring.
+ * @return The keyring, or NULL if not found.
+ */
+PurpleKeyring * purple_keyring_get_keyring_by_id(const char * id);
+
+/**
+ * Get a list of id/name pairs (for preferences)
+ * @return The list.
+ */
+GList * purple_keyring_get_options(void);
+
+/**
  * Prepare stuff at startup.
  */
-void 
-purple_keyring_init(void);
+void purple_keyring_init(void);
 
 /**
  * Do some cleanup.
  */
-void
-purple_keyring_uninit(void);
+void purple_keyring_uninit(void);
 
 /**
  * Get the keyring list. Used by the UI.
============================================================
--- libpurple/plugins/keyrings/Makefile.am	dcd3320d138b0cbb58e2b1404a526afc08cd884f
+++ libpurple/plugins/keyrings/Makefile.am	d73056f2569849c5c5ee37dd18d2a8fb571fea95
@@ -7,6 +7,7 @@ GNOME_KEYRING_CFLAGS = -I/usr/include/gn
 internalkeyring_la_LDFLAGS  = -module -avoid-version
 
 GNOME_KEYRING_CFLAGS = -I/usr/include/gnome-keyring-1
+GNOME_KEYRING_LIBS   = -lgnome-keyring
 
 GKRSOURCES = gnomekeyring.c
 IKSOURCES = internalkeyring.c
============================================================
--- libpurple/plugins/keyrings/gnomekeyring.c	6dc26ae46caa529ce83ca3d66f329a1babdbeb62
+++ libpurple/plugins/keyrings/gnomekeyring.c	fc7c4c3ae33380eba0a5a5c885b872497fb631c9
@@ -191,6 +191,12 @@ gkp_save(PurpleAccount * account,
 	storage->user_data = data;
 
 	if(password != NULL) {
+
+		purple_debug_info("Gnome keyring plugin",
+			"Updating password for account %s (%s).\n",
+			purple_account_get_username(account),
+			purple_account_get_protocol_id(account));
+
 		gnome_keyring_store_password(GNOME_KEYRING_NETWORK_PASSWORD,
 					     NULL, 	/*default keyring */
 					     g_strdup_printf("pidgin-%s", purple_account_get_username(account)),
@@ -207,6 +213,11 @@ gkp_save(PurpleAccount * account,
 
 	} else {	/* password == NULL, delete password. */
 
+		purple_debug_info("Gnome keyring plugin",
+			"Forgetting password for account %s (%s).\n",
+			purple_account_get_username(account),
+			purple_account_get_protocol_id(account));
+
 		gnome_keyring_delete_password(GNOME_KEYRING_NETWORK_PASSWORD,
 					      gkp_save_continue,
 					      storage,
@@ -232,6 +243,10 @@ gkp_save_continue(GnomeKeyringResult res
 		switch(result)
 		{
 			case GNOME_KEYRING_RESULT_NO_MATCH :
+				purple_debug_info("Gnome keyring plugin",
+					"Could not update password for %s (%s) : not found.\n",
+					purple_account_get_username(account),
+					purple_account_get_protocol_id(account));
 				error = g_error_new(ERR_GNOMEKEYRINGPLUGIN, 
 					ERR_NOPASSWD, "Could not update password for %s : not found",
 					purple_account_get_username(account));
@@ -242,6 +257,10 @@ gkp_save_continue(GnomeKeyringResult res
 
 			case GNOME_KEYRING_RESULT_NO_KEYRING_DAEMON :
 			case GNOME_KEYRING_RESULT_IO_ERROR :
+				purple_debug_info("Gnome keyring plugin",
+					"Failed to communicate with gnome keyring (account : %s (%s)).\n",
+					purple_account_get_username(account),
+					purple_account_get_protocol_id(account));
 				error = g_error_new(ERR_GNOMEKEYRINGPLUGIN, 
 					ERR_NOCHANNEL, "Failed to communicate with gnome keyring (account : %s).",
 					purple_account_get_username(account));
@@ -251,6 +270,10 @@ gkp_save_continue(GnomeKeyringResult res
 				return;
 
 			default :
+				purple_debug_info("Gnome keyring plugin",
+					"Unknown error (account : %s (%s)).\n",
+					purple_account_get_username(account),
+					purple_account_get_protocol_id(account));
 				error = g_error_new(ERR_GNOMEKEYRINGPLUGIN, 
 					ERR_NOCHANNEL, "Unknown error (account : %s).",
 					purple_account_get_username(account));
@@ -309,7 +332,7 @@ gkp_close(GError ** error)
 static void
 gkp_close(GError ** error)
 {
-	gkp_uninit();
+	return;
 }
 
 static gboolean
@@ -318,6 +341,8 @@ gkp_import_password(PurpleAccount * acco
 		    char * data,
 		    GError ** error)
 {
+	purple_debug_info("Gnome Keyring plugin",
+		"Importing password.\n");
 	return TRUE;
 }
 
@@ -328,9 +353,11 @@ gkp_export_password(PurpleAccount * acco
 				 GError ** error,
 				 GDestroyNotify * destroy)
 {
+	purple_debug_info("Gnome Keyring plugin",
+		"Exporting password.\n");
 	*data = NULL;
 	*mode = NULL;
-	destroy = NULL;
+	*destroy = NULL;
 
 	return TRUE;
 }
@@ -341,6 +368,8 @@ gkp_init()
 static gboolean
 gkp_init()
 {
+	purple_debug_info("gnome-keyring-plugin", "init.\n");
+
 	if (gnome_keyring_is_available()) {
 
 		keyring_handler = purple_keyring_new();
@@ -371,6 +400,9 @@ gkp_uninit()
 static void
 gkp_uninit()
 {
+	purple_debug_info("gnome-keyring-plugin", "uninit.\n");
+	gkp_close(NULL);
+	purple_keyring_unregister(keyring_handler);
 	purple_keyring_free(keyring_handler);
 	keyring_handler = NULL;
 }
============================================================
--- libpurple/plugins/keyrings/internalkeyring.c	d6f5ea559eb568dac11ec0b224ad0ba744e5d7d1
+++ libpurple/plugins/keyrings/internalkeyring.c	59f457c7cdfcae457e24b449ff93a07eb520e95f
@@ -62,17 +62,22 @@
 	g_hash_table_lookup (internal_keyring_passwords, account)
 #define SET_PASSWORD(account, password) \
 	g_hash_table_replace(internal_keyring_passwords, account, password)
+#define ACTIVATE()\
+	if (internal_keyring_is_active == FALSE)\
+		internal_keyring_open();	
+	
 
-
-GHashTable * internal_keyring_passwords = NULL;
+static GHashTable * internal_keyring_passwords = NULL;
 static PurpleKeyring * keyring_handler = NULL;
+static gboolean internal_keyring_is_active = FALSE;
 
 /* a few prototypes : */
-static void 		internal_keyring_read(const PurpleAccount *, PurpleKeyringReadCallback, gpointer);
-static void 		internal_keyring_save(const PurpleAccount *, gchar *, GDestroyNotify, PurpleKeyringSaveCallback, gpointer);
+static void 		internal_keyring_read(PurpleAccount *, PurpleKeyringReadCallback, gpointer);
+static void 		internal_keyring_save(PurpleAccount *, gchar *, GDestroyNotify, PurpleKeyringSaveCallback, gpointer);
 static const char * 	internal_keyring_read_sync(const PurpleAccount *);
 static void 		internal_keyring_save_sync(PurpleAccount *, const gchar *);
 static void		internal_keyring_close(GError **);
+static void		internal_keyring_open(void);
 static gboolean		internal_keyring_import_password(PurpleAccount *, char *, char *, GError **);
 static gboolean 	internal_keyring_export_password(PurpleAccount *, const char **, char **, GError **, GDestroyNotify *);
 static void		internal_keyring_init(void);
@@ -85,28 +90,37 @@ static void 
 /*     Keyring interface                       */
 /***********************************************/
 static void 
-internal_keyring_read(const PurpleAccount * account,
+internal_keyring_read(PurpleAccount * account,
 		      PurpleKeyringReadCallback cb,
 		      gpointer data)
 {
 	char * password;
 	GError * error;
 
+	ACTIVATE();
+
+	purple_debug_info("Internal Keyring",
+		"Reading password for account %s (%s).\n",
+		purple_account_get_username(account),
+		purple_account_get_protocol_id(account));
+
 	password = GET_PASSWORD(account);
 
 	if (password != NULL) {
-		cb(account, password, NULL, data);
+		if(cb != NULL)
+			cb(account, password, NULL, data);
 	} else {
 		error = g_error_new(ERR_PIDGINKEYRING, 
 			ERR_NOPASSWD, "password not found");
-		cb(account, NULL, error, data);
+		if(cb != NULL)
+			cb(account, NULL, error, data);
 		g_error_free(error);
 	}
 	return;
 }
 
 static void
-internal_keyring_save(const PurpleAccount * account,
+internal_keyring_save(PurpleAccount * account,
 		      gchar * password,
 		      GDestroyNotify destroy,
 		      PurpleKeyringSaveCallback cb,
@@ -114,17 +128,29 @@ internal_keyring_save(const PurpleAccoun
 {
 	gchar * copy;
 
+	ACTIVATE();
+
 	if (password == NULL) {
 		g_hash_table_remove(internal_keyring_passwords, account);
+		purple_debug_info("Internal Keyring",
+			"Deleted password for account %s (%s).\n",
+			purple_account_get_username(account),
+			purple_account_get_protocol_id(account));
 	} else {
 		copy = g_strdup(password);
 		SET_PASSWORD((void *)account, copy);	/* cast prevents warning because account is const */
+		purple_debug_info("Internal Keyring",
+			"Updated password for account %s (%s).\n",
+			purple_account_get_username(account),
+			purple_account_get_protocol_id(account));
+
 	}
 
 	if(destroy != NULL)
 		destroy(password);
 
-	cb(account, NULL, data);
+	if (cb != NULL)
+		cb(account, NULL, data);
 	return;
 }
 
@@ -132,7 +158,13 @@ internal_keyring_read_sync(const PurpleA
 static const char * 
 internal_keyring_read_sync(const PurpleAccount * account)
 {
-	purple_debug_info("keyring", "password was read\n");
+	ACTIVATE();
+
+	purple_debug_info("Internal Keyring (sync)", 
+		"Password for %s (%s) was read.\n",
+		purple_account_get_username(account),
+		purple_account_get_protocol_id(account));
+
 	return GET_PASSWORD(account);
 }
 
@@ -142,22 +174,43 @@ internal_keyring_save_sync(PurpleAccount
 {
 	gchar * copy;
 
+	ACTIVATE();
+
 	if (password == NULL) {
 		g_hash_table_remove(internal_keyring_passwords, account);
+		purple_debug_info("Internal Keyring (sync)", 
+			"Password for %s (%s) was deleted.\n",
+			purple_account_get_username(account),
+			purple_account_get_protocol_id(account));
 	} else {
 		copy = g_strdup(password);
 		SET_PASSWORD(account, copy);
+		purple_debug_info("Internal Keyring (sync)", 
+			"Password for %s (%s) was set.\n",
+			purple_account_get_username(account),
+			purple_account_get_protocol_id(account));
 	}
-	purple_debug_info("keyring", "password was set\n");
+
 	return;
 }
 
 static void
 internal_keyring_close(GError ** error)
 {
-	internal_keyring_uninit();
+	internal_keyring_is_active = FALSE;
+
+	g_hash_table_destroy(internal_keyring_passwords);
+	internal_keyring_passwords = NULL;
 }
 
+static void
+internal_keyring_open()
+{
+	internal_keyring_passwords = g_hash_table_new_full(g_direct_hash,
+		g_direct_equal, NULL, g_free);
+	internal_keyring_is_active = TRUE;
+}
+
 static gboolean
 internal_keyring_import_password(PurpleAccount * account, 
 				 char * mode,
@@ -166,6 +219,11 @@ internal_keyring_import_password(PurpleA
 {
 	gchar * copy;
 
+	ACTIVATE();
+
+	purple_debug_info("Internal keyring",
+		"Importing password");
+
 	if (account != NULL && 
 	    data != NULL &&
 	    (mode == NULL || g_strcmp0(mode, "cleartext") == 0)) {
@@ -180,6 +238,8 @@ internal_keyring_import_password(PurpleA
 		return FALSE;
 
 	}
+
+	return TRUE;
 }
 
 static gboolean 
@@ -191,6 +251,11 @@ internal_keyring_export_password(PurpleA
 {
 	gchar * password;
 
+	ACTIVATE();
+
+	purple_debug_info("Internal keyring",
+		"Exporting password");
+
 	password = GET_PASSWORD(account);
 
 	if (password == NULL) {
@@ -223,18 +288,14 @@ internal_keyring_init()
 	purple_keyring_set_export_password(keyring_handler, internal_keyring_export_password);
 
 	purple_keyring_register(keyring_handler);
-
-	internal_keyring_passwords = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, g_free);
 }
 
 static void
 internal_keyring_uninit()
 {
-	purple_keyring_free(keyring_handler);
-	keyring_handler = NULL;
+	internal_keyring_close(NULL);
+	purple_keyring_unregister(keyring_handler);
 
-	g_hash_table_destroy(internal_keyring_passwords);
-	internal_keyring_passwords = NULL;
 }
 
 
@@ -254,6 +315,10 @@ internal_keyring_unload(PurplePlugin *pl
 internal_keyring_unload(PurplePlugin *plugin)
 {
 	internal_keyring_uninit();
+
+	purple_keyring_free(keyring_handler);
+	keyring_handler = NULL;
+
 	return TRUE;
 }
 
============================================================
--- pidgin/gtkprefs.c	6bbd4830d17cc896eeba0ea1e521a7f8ed8ca03a
+++ pidgin/gtkprefs.c	cf2f94bbb38876801c5be326e4d8bf987b901fd0
@@ -1643,7 +1643,7 @@ keyring_page(void)
 	GList *names;
 
 
-	purple_debug_info("prefs", "drawing keyring prefs page");
+	purple_debug_info("prefs", "drawing keyring prefs page.\n");
 	ret = gtk_vbox_new(FALSE, PIDGIN_HIG_CAT_SPACE);
 	gtk_container_set_border_width (GTK_CONTAINER (ret), PIDGIN_HIG_BORDER);
 


More information about the Commits mailing list